[rewarding] move claim address check to LoadProto() and SanityCheck() #4304
Conversation
|
|
#4277 added a new field |
| if err != nil { | ||
| log.L().Debug("Invalid claim to address", zap.Error(err)) | ||
| return p.settleUserAction(ctx, sm, uint64(iotextypes.ReceiptStatus_Failure), si, nil) | ||
| } |
There was a problem hiding this comment.
this address check can (and should) be done before Handle() in the action's LoadProto() and SanityCheck(), this is consistent with other actions like candidate register
dustinxie
changed the title
dustinxie
force-pushed
the
enter
branch
4 times, most recently
from
6da2e90
to
041292a
Compare
| if claimTo == "" { | ||
| claimTo = protocol.MustGetActionCtx(ctx).Caller.String() | ||
| addr := protocol.MustGetActionCtx(ctx).Caller | ||
| if protocol.MustGetFeatureCtx(ctx).AddClaimRewardAddress && act.Address() != nil { |
There was a problem hiding this comment.
shall we reject an action if !protocol.MustGetFeatureCtx(ctx).AddClaimRewardAddress && act.Address() != nil?
action/claimreward.go
Outdated
| @@ -217,8 +232,16 @@ func NewClaimFromRewardingFundFromABIBinary(data []byte) (*ClaimFromRewardingFun | |||
| if ac.data, ok = paramsMap["data"].([]byte); !ok { | |||
| return nil, errDecodeFailure | |||
| } | |||
| if ac.address, ok = paramsMap["address"].(string); !ok { | |||
| var s string | |||
| if s, ok = paramsMap["address"].(string); !ok { | |||
There was a problem hiding this comment.
should keep backword compatibility, allowing without address param
There was a problem hiding this comment.
add a 2nd commit to keep both v1 and v2 ABI
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, |
There was a problem hiding this comment.
need to keep the current (v1) ABI for backward compatibility
| "4d26d0736ebd9e69bd5994f3730b05a2d48c810b3bb54818be65d02004cf4ff4", | ||
| "04830579b50e01602c2015c24e72fbc48bca1cca1e601b119ca73abe2e0b5bd61fcb7874567e091030d6b644f927445d80e00b3f9ca0c566c21c30615e94c343da", | ||
| "8d38efe45794d7fceea10b2262c23c12245959db", | ||
| }, |
There was a problem hiding this comment.
this is test with v1 ABI, fixed and restored
the original #4277 shouldn't have broken this
action/claimreward.go
Outdated
| return nil, err | ||
| } | ||
| return append(_claimRewardingMethodV1.ID, data...), nil | ||
| } | ||
| var addr string | ||
| if c.address != nil { |
action/claimreward.go
Outdated
| var ( | ||
| act *ClaimFromRewardingFund | ||
| err error | ||
| ) | ||
| if act, err = newClaimFromRewardingFundFromABIv2(data); err == nil { | ||
| return act, nil | ||
| } | ||
| if err != errWrongMethodSig { | ||
| // data is v2-encoded, but got error when decoding | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
if len(data) >= 4 {
switch {
case bytes.Equal(_claimRewardingMethodV2.ID[:], data[:4]):
....
case bytes.Equal(_claimRewardingMethodV1.ID[:], data[:4]):
....
}
}
return nil, errDecodeFailure
Description
#4277 added a
claimTofield to receive claim reward, so other EOA can help claim reward on another's behalf.move the address check from inside
protocol.Handleto the action'sLoadProto()andSanityCheck(), which is consistent with other action's behaviorFixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: