Skip to content

Commit

Permalink
fix: CLI: adjust TermMax for extend-claim used by a different client (f…
Browse files Browse the repository at this point in the history 
…ilecoin-project#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 <[email protected]>
  • Loading branch information
@LexLuthr @ribasushi
2 people authored and ribasushi committed May 16, 2024
1 parent d478f50 commit 6fb5d41
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 74 deletions.
185 changes: 120 additions & 65 deletions cli/filplus.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,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",
Expand Down Expand Up @@ -967,6 +971,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 {
Expand Down Expand Up @@ -1070,7 +1079,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
}
Expand Down Expand Up @@ -1123,7 +1132,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 {
Expand All @@ -1142,7 +1151,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
Expand All @@ -1163,17 +1172,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,
Expand Down Expand Up @@ -1205,17 +1220,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,
Expand All @@ -1236,17 +1257,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,
Expand All @@ -1259,22 +1286,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]

oclaimMsg := &types.Message{
To: verifreg.Address,
From: wallet,
Method: verifreg.Methods.ExtendClaimTerms,
Params: params,
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)
}

msgs = append(msgs, oclaimMsg)
}

if len(newClaims) > 0 {
Expand All @@ -1293,32 +1327,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 {
Expand Down Expand Up @@ -1354,7 +1362,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
Expand Down
10 changes: 7 additions & 3 deletions documentation/en/cli-lotus.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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
```

Expand Down
14 changes: 8 additions & 6 deletions itests/direct_data_onboard_verified_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand All @@ -873,12 +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" event
// New TermMax should be more than 5 years
require.Greater(t, int(newclaim.TermMax), verifregtypes13.MaximumVerifiedAllocationTerm)
}

0 comments on commit 6fb5d41

Please sign in to comment.