From 0e4677618fa97f4d6981b559592928b7360577ee Mon Sep 17 00:00:00 2001 From: LexLuthr <88259624+LexLuthr@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:18:28 +0400 Subject: [PATCH] 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 --- cli/filplus.go | 187 +++++++++++++------- documentation/en/cli-lotus.md | 10 +- itests/direct_data_onboard_verified_test.go | 14 +- 3 files changed, 135 insertions(+), 76 deletions(-) diff --git a/cli/filplus.go b/cli/filplus.go index fbb922a24..8c39c21e8 100644 --- a/cli/filplus.go +++ b/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: " ... or ...", 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 client is not same - needs to burn datacap - if claim.Client != wid { - newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{ + // If the client is not the original client - burn datacap + if claim.Client != wid { + // 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) - continue } + // 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 client is not same - needs to burn datacap - if claim.Client != wid { - newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{ + // If the client is not the original client - burn datacap + if claim.Client != wid { + // 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) - continue } + // 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 client is not same - needs to burn datacap - if claim.Client != wid { - newClaims = append(newClaims, verifregtypes13.ClaimExtensionRequest{ + // If the client is not the original client - burn datacap + if claim.Client != wid { + // 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) - continue } + // 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,22 +1285,29 @@ 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{ - Terms: terms, - }) + // Batch in 500 to avoid running out of gas + for i := 0; i < len(terms); i += batchSize { + batchEnd := i + batchSize + if batchEnd > len(terms) { + batchEnd = len(terms) + } - if err != nil { - return nil, xerrors.Errorf("failed to searialise the parameters: %s", err) + 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) } - - oclaimMsg := &types.Message{ - To: verifreg.Address, - From: wallet, - Method: verifreg.Methods.ExtendClaimTerms, - Params: params, - } - - msgs = append(msgs, oclaimMsg) } if len(newClaims) > 0 { @@ -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,7 +1361,54 @@ func CreateExtendClaimMsg(ctx context.Context, api api.FullNode, pcm map[verifre } } - msgs = append(msgs, nclaimMsg) + // 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 diff --git a/documentation/en/cli-lotus.md b/documentation/en/cli-lotus.md index ad04b68ec..36f1e1059 100644 --- a/documentation/en/cli-lotus.md +++ b/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] ... or ... + 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 ``` diff --git a/itests/direct_data_onboard_verified_test.go b/itests/direct_data_onboard_verified_test.go index 530b3cca6..7415570a3 100644 --- a/itests/direct_data_onboard_verified_test.go +++ b/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) }