fix: CLI: adjust TermMax for extend-claim used by a different client (#11764)

* fix Datacap TermMax

* fix itest

* add batching

* make batch size variable

* docsgen-cli

* reduce batch size, fix batch dc

* apply suggestion from review

* put back TODO

---------

Co-authored-by: LexLuthr <lexluthr@protocol.ai>

cli/filplus.go

@@ -934,7 +934,11 @@ var filplusSignRemoveDataCapProposal = &cli.Command{
 
 var filplusExtendClaimCmd = &cli.Command{
 	Name:  "extend-claim",
-	Usage: "extend claim expiration (TermMax)",
+	Usage: "extends claim expiration (TermMax)",
+	UsageText: `Extends claim expiration (TermMax).
+If the client is original client then claim can be extended to maximum 5 years and no Datacap is required.
+If the client id different then claim can be extended up to maximum 5 years from now and Datacap is required.
+`,
 	Flags: []cli.Flag{
 		&cli.Int64Flag{
 			Name:    "term-max",
@@ -966,6 +970,11 @@ var filplusExtendClaimCmd = &cli.Command{
 			Usage: "number of block confirmations to wait for",
 			Value: int(build.MessageConfidence),
 		},
+		&cli.IntFlag{
+			Name:  "batch-size",
+			Usage: "number of extend requests per batch. If set incorrectly, this will lead to out of gas error",
+			Value: 500,
+		},
 	},
 	ArgsUsage: "<claim1> <claim2> ... or <miner1=claim1> <miner2=claims2> ...",
 	Action: func(cctx *cli.Context) error {
@@ -1069,7 +1078,7 @@ var filplusExtendClaimCmd = &cli.Command{
 			}
 		}
 
-		msgs, err := CreateExtendClaimMsg(ctx, api, claimMap, miners, clientAddr, abi.ChainEpoch(tmax), all, cctx.Bool("assume-yes"))
+		msgs, err := CreateExtendClaimMsg(ctx, api, claimMap, miners, clientAddr, abi.ChainEpoch(tmax), all, cctx.Bool("assume-yes"), cctx.Int("batch-size"))
 		if err != nil {
 			return err
 		}
@@ -1122,7 +1131,7 @@ type ProvInfo struct {
 // 6. Extend all claims for multiple miner IDs with different client address (2 messages)
 // 7. Extend specified claims for a miner ID with different client address (2 messages)
 // 8. Extend specific claims for specific miner ID with different client address (2 messages)
-func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifregtypes13.ClaimId]ProvInfo, miners []string, wallet address.Address, tmax abi.ChainEpoch, all, assumeYes bool) ([]*types.Message, error) {
+func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifregtypes13.ClaimId]ProvInfo, miners []string, wallet address.Address, tmax abi.ChainEpoch, all, assumeYes bool, batchSize int) ([]*types.Message, error) {
 
 	ac, err := api.StateLookupID(ctx, wallet, types.EmptyTSK)
 	if err != nil {
@@ -1141,7 +1150,7 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 	}
 
 	var terms []verifregtypes13.ClaimTerm
-	var newClaims []verifregtypes13.ClaimExtensionRequest
+	newClaims := make(map[verifregtypes13.ClaimExtensionRequest]big.Int)
 	rDataCap := big.NewInt(0)
 
 	// If --all is set
@@ -1162,17 +1171,23 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 			for claimID, claim := range claims {
 				claimID := claimID
 				claim := claim
-				if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
+				// If the client is not the original client - burn datacap
-					// If client is not same - needs to burn datacap
 				if claim.Client != wid {
-						newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{
+					// The new duration should be greater than the original deal duration and claim should not already be expired
+					if head.Height()+tmax-claim.TermStart > claim.TermMax-claim.TermStart && claim.TermStart+claim.TermMax > head.Height() {
+						req := verifregtypes13.ClaimExtensionRequest{
 							Claim:    verifregtypes13.ClaimId(claimID),
 							Provider: abi.ActorID(mid),
-							TermMax:  tmax,
+							TermMax:  head.Height() + tmax - claim.TermStart,
-						})
+						}
+						newClaims[req] = big.NewInt(int64(claim.Size))
 						rDataCap.Add(big.NewInt(int64(claim.Size)).Int, rDataCap.Int)
+					}
+					// If new duration shorter than the original duration then do nothing
 					continue
 				}
+				// For original client, compare duration(TermMax) and claim should not already be expired
+				if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
 					terms = append(terms, verifregtypes13.ClaimTerm{
 						ClaimId:  verifregtypes13.ClaimId(claimID),
 						TermMax:  tmax,
@@ -1204,17 +1219,23 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 			if !ok {
 				return nil, xerrors.Errorf("claim %d not found for provider %s", claimID, miners[0])
 			}
-			if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
+			// If the client is not the original client - burn datacap
-				// If client is not same - needs to burn datacap
 			if claim.Client != wid {
-					newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{
+				// The new duration should be greater than the original deal duration and claim should not already be expired
+				if head.Height()+tmax-claim.TermStart > claim.TermMax-claim.TermStart && claim.TermStart+claim.TermMax > head.Height() {
+					req := verifregtypes13.ClaimExtensionRequest{
 						Claim:    claimID,
 						Provider: abi.ActorID(mid),
-						TermMax:  tmax,
+						TermMax:  head.Height() + tmax - claim.TermStart,
-					})
+					}
+					newClaims[req] = big.NewInt(int64(claim.Size))
 					rDataCap.Add(big.NewInt(int64(claim.Size)).Int, rDataCap.Int)
+				}
+				// If new duration shorter than the original duration then do nothing
 				continue
 			}
+			// For original client, compare duration(TermMax) and claim should not already be expired
+			if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
 				terms = append(terms, verifregtypes13.ClaimTerm{
 					ClaimId:  claimID,
 					TermMax:  tmax,
@@ -1235,17 +1256,23 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 			if claim == nil {
 				return nil, xerrors.Errorf("claim %d not found in the actor state", claimID)
 			}
-			if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
+			// If the client is not the original client - burn datacap
-				// If client is not same - needs to burn datacap
 			if claim.Client != wid {
-					newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{
+				// The new duration should be greater than the original deal duration and claim should not already be expired
+				if head.Height()+tmax-claim.TermStart > claim.TermMax-claim.TermStart && claim.TermStart+claim.TermMax > head.Height() {
+					req := verifregtypes13.ClaimExtensionRequest{
 						Claim:    claimID,
 						Provider: prov.ID,
-						TermMax:  tmax,
+						TermMax:  head.Height() + tmax - claim.TermStart,
-					})
+					}
+					newClaims[req] = big.NewInt(int64(claim.Size))
 					rDataCap.Add(big.NewInt(int64(claim.Size)).Int, rDataCap.Int)
+				}
+				// If new duration shorter than the original duration then do nothing
 				continue
 			}
+			// For original client, compare duration(TermMax) and claim should not already be expired
+			if claim.TermMax < tmax && claim.TermStart+claim.TermMax > head.Height() {
 				terms = append(terms, verifregtypes13.ClaimTerm{
 					ClaimId:  claimID,
 					TermMax:  tmax,
@@ -1258,23 +1285,30 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 	var msgs []*types.Message
 
 	if len(terms) > 0 {
-		params, err := actors.SerializeParams(&verifregtypes13.ExtendClaimTermsParams{
+		// Batch in 500 to avoid running out of gas
-			Terms: terms,
+		for i := 0; i < len(terms); i += batchSize {
-		})
+			batchEnd := i + batchSize
+			if batchEnd > len(terms) {
+				batchEnd = len(terms)
+			}
 
+			batch := terms[i:batchEnd]
+
+			params, err := actors.SerializeParams(&verifregtypes13.ExtendClaimTermsParams{
+				Terms: batch,
+			})
 			if err != nil {
 				return nil, xerrors.Errorf("failed to searialise the parameters: %s", err)
 			}
-
 			oclaimMsg := &types.Message{
 				To:     verifreg.Address,
 				From:   wallet,
 				Method: verifreg.Methods.ExtendClaimTerms,
 				Params: params,
 			}
-
 			msgs = append(msgs, oclaimMsg)
 		}
+	}
 
 	if len(newClaims) > 0 {
 		// Get datacap balance
@@ -1292,32 +1326,6 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 			return nil, xerrors.Errorf("requested datacap %s is greater then the available datacap %s", rDataCap, aDataCap)
 		}
 
-		ncparams, err := actors.SerializeParams(&verifregtypes13.AllocationRequests{
-			Extensions: newClaims,
-		})
-
-		if err != nil {
-			return nil, xerrors.Errorf("failed to searialise the parameters: %s", err)
-		}
-
-		transferParams, err := actors.SerializeParams(&datacap2.TransferParams{
-			To:           builtin.VerifiedRegistryActorAddr,
-			Amount:       big.Mul(rDataCap, builtin.TokenPrecision),
-			OperatorData: ncparams,
-		})
-
-		if err != nil {
-			return nil, xerrors.Errorf("failed to serialize transfer parameters: %s", err)
-		}
-
-		nclaimMsg := &types.Message{
-			To:     builtin.DatacapActorAddr,
-			From:   wallet,
-			Method: datacap.Methods.TransferExported,
-			Params: transferParams,
-			Value:  big.Zero(),
-		}
-
 		if !assumeYes {
 			out := fmt.Sprintf("Some of the specified allocation have a different client address and will require %d Datacap to extend. Proceed? Yes [Y/y] / No [N/n], Ctrl+C (^C) to exit", rDataCap.Int)
 			validate := func(input string) error {
@@ -1353,8 +1361,55 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre
 			}
 		}
 
+		// Create a map of just keys, so we can easily batch based on the numeric keys
+		keys := make([]verifregtypes13.ClaimExtensionRequest, 0, len(newClaims))
+		for k := range newClaims {
+			keys = append(keys, k)
+		}
+
+		// Batch in 500 to avoid running out of gas
+		for i := 0; i < len(keys); i += batchSize {
+			batchEnd := i + batchSize
+			if batchEnd > len(newClaims) {
+				batchEnd = len(newClaims)
+			}
+
+			batch := keys[i:batchEnd]
+
+			// Calculate Datacap for this batch
+			dcap := big.NewInt(0)
+			for _, k := range batch {
+				dc := newClaims[k]
+				dcap.Add(dcap.Int, dc.Int)
+			}
+
+			ncparams, err := actors.SerializeParams(&verifregtypes13.AllocationRequests{
+				Extensions: batch,
+			})
+			if err != nil {
+				return nil, xerrors.Errorf("failed to searialise the parameters: %s", err)
+			}
+
+			transferParams, err := actors.SerializeParams(&datacap2.TransferParams{
+				To:           builtin.VerifiedRegistryActorAddr,
+				Amount:       big.Mul(dcap, builtin.TokenPrecision),
+				OperatorData: ncparams,
+			})
+
+			if err != nil {
+				return nil, xerrors.Errorf("failed to serialize transfer parameters: %s", err)
+			}
+
+			nclaimMsg := &types.Message{
+				To:     builtin.DatacapActorAddr,
+				From:   wallet,
+				Method: datacap.Methods.TransferExported,
+				Params: transferParams,
+				Value:  big.Zero(),
+			}
 			msgs = append(msgs, nclaimMsg)
 		}
+	}
 
 	return msgs, nil
 }

documentation/en/cli-lotus.md

@@ -1192,7 +1192,7 @@ COMMANDS:
    list-claims                    List claims available in verified registry actor or made by provider if specified
    remove-expired-allocations     remove expired allocations (if no allocations are specified all eligible allocations are removed)
    remove-expired-claims          remove expired claims (if no claims are specified all eligible claims are removed)
-   extend-claim                   extend claim expiration (TermMax)
+   extend-claim                   extends claim expiration (TermMax)
    help, h                        Shows a list of commands or help for one command
 
 OPTIONS:
@@ -1329,10 +1329,13 @@ OPTIONS:
 ### lotus filplus extend-claim
 ```
 NAME:
-   lotus filplus extend-claim - extend claim expiration (TermMax)
+   lotus filplus extend-claim - extends claim expiration (TermMax)
 
 USAGE:
-   lotus filplus extend-claim [command options] <claim1> <claim2> ... or <miner1=claim1> <miner2=claims2> ...
+   Extends claim expiration (TermMax).
+   If the client is original client then claim can be extended to maximum 5 years and no Datacap is required.
+   If the client id different then claim can be extended up to maximum 5 years from now and Datacap is required.
+
 
 OPTIONS:
    --term-max value, --tmax value                                                                               The maximum period for which a provider can earn quality-adjusted power for the piece (epochs). Default is 5 years. (default: 5256000)
@@ -1341,6 +1344,7 @@ OPTIONS:
    --miner value, -m value, --provider value, -p value [ --miner value, -m value, --provider value, -p value ]  storage provider address[es]
    --assume-yes, -y, --yes                                                                                      automatic yes to prompts; assume 'yes' as answer to all prompts and run non-interactively (default: false)
    --confidence value                                                                                           number of block confirmations to wait for (default: 5)
+   --batch-size value                                                                                           number of extend requests per batch. If set incorrectly, this will lead to out of gas error (default: 500)
    --help, -h                                                                                                   show help
 ```
 

itests/direct_data_onboard_verified_test.go

@@ -840,12 +840,11 @@ func TestVerifiedDDOExtendClaim(t *testing.T) {
 	pcm[verifregtypes13.ClaimId(allocationId)] = prov
 
 	// Extend claim with same client
-	msgs, err := cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, verifiedClientAddr1, (builtin.EpochsInYear*3)+3000, false, true)
+	msgs, err := cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, verifiedClientAddr1, (builtin.EpochsInYear*3)+3000, false, true, 100)
 	require.NoError(t, err)
 	require.NotNil(t, msgs)
 	require.Len(t, msgs, 1)
 
-	// MpoolBatchPushMessage method will take care of gas estimation and funds check
 	smsg, err := client.MpoolPushMessage(ctx, msgs[0], nil)
 	require.NoError(t, err)
 
@@ -859,11 +858,11 @@ func TestVerifiedDDOExtendClaim(t *testing.T) {
 	require.EqualValues(t, newclaim.TermMax-oldclaim.TermMax, 3000)
 
 	// Extend claim with non-verified client | should fail
-	_, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, unverifiedClient.Address, verifregtypes13.MaximumVerifiedAllocationTerm, false, true)
+	_, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, unverifiedClient.Address, verifregtypes13.MaximumVerifiedAllocationTerm, false, true, 100)
 	require.ErrorContains(t, err, "does not have any datacap")
 
 	// Extend all claim with verified client
-	msgs, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, nil, []string{miner.ActorAddr.String()}, verifiedClientAddr2, verifregtypes13.MaximumVerifiedAllocationTerm, true, true)
+	msgs, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, nil, []string{miner.ActorAddr.String()}, verifiedClientAddr2, verifregtypes13.MaximumVerifiedAllocationTerm, true, true, 100)
 	require.NoError(t, err)
 	require.Len(t, msgs, 1)
 	smsg, err = client.MpoolPushMessage(ctx, msgs[0], nil)
@@ -873,14 +872,15 @@ func TestVerifiedDDOExtendClaim(t *testing.T) {
 	require.True(t, wait.Receipt.ExitCode.IsSuccess())
 
 	// Extend all claims with lower TermMax
-	msgs, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, verifiedClientAddr2, builtin.EpochsInYear*4, false, true)
+	msgs, err = cli.CreateExtendClaimMsg(ctx, client.FullNode, pcm, []string{}, verifiedClientAddr2, builtin.EpochsInYear*4, false, true, 100)
 	require.NoError(t, err)
 	require.Nil(t, msgs)
 
 	newclaim, err = client.StateGetClaim(ctx, miner.ActorAddr, verifreg.ClaimId(allocationId), types.EmptyTSK)
 	require.NoError(t, err)
 	require.NotNil(t, newclaim)
-	require.EqualValues(t, newclaim.TermMax, verifregtypes13.MaximumVerifiedAllocationTerm)
 
-	// TODO: check "claim-updated" message
+	// TODO: check "claim-updated" event
+	// New TermMax should be more than 5 years
+	require.Greater(t, int(newclaim.TermMax), verifregtypes13.MaximumVerifiedAllocationTerm)
 }