From f001407535720ebb2d0faad68c22cd97c8d81b80 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Tue, 18 Jun 2024 20:42:03 -0700 Subject: [PATCH] [rewarding] keep both v1 and v2 claimRewardingInterfaceABI for backward compatibility (#4304) --- action/builder_test.go | 14 ++--- action/claimreward.go | 84 ++++++++++++++++++++------- action/claimreward.s | 0 action/claimreward_test.go | 83 +++++++++++--------------- action/protocol/rewarding/protocol.go | 8 ++- action/rlp_tx_test.go | 34 +++++------ 6 files changed, 127 insertions(+), 96 deletions(-) delete mode 100644 action/claimreward.s diff --git a/action/builder_test.go b/action/builder_test.go index c7ef2a4d47..bdbb92171f 100644 --- a/action/builder_test.go +++ b/action/builder_test.go @@ -84,9 +84,9 @@ func TestBuildRewardingAction(t *testing.T) { }) t.Run("Success", func(t *testing.T) { - method := _claimRewardingMethod + method := _claimRewardingMethodV1 t.Run("ClaimRewarding", func(t *testing.T) { - inputs := MustNoErrorV(method.Inputs.Pack(big.NewInt(101), []byte("any"), "")) + inputs := MustNoErrorV(method.Inputs.Pack(big.NewInt(101), []byte("any"))) elp, err := eb.BuildRewardingAction(types.NewTx(&types.LegacyTx{ Nonce: 1, GasPrice: big.NewInt(10004), @@ -100,13 +100,12 @@ func TestBuildRewardingAction(t *testing.T) { r.Equal(big.NewInt(10004), elp.GasPrice()) r.Equal(uint64(10000), elp.GasLimit()) r.Equal(big.NewInt(101), elp.Action().(*ClaimFromRewardingFund).Amount()) - }) t.Run("Debug", func(t *testing.T) { eb := &EnvelopeBuilder{} eb.SetChainID(4689) - inputs := MustNoErrorV(method.Inputs.Pack(big.NewInt(100), []byte("any"), "")) + inputs := MustNoErrorV(method.Inputs.Pack(big.NewInt(100), []byte("any"))) to := common.HexToAddress("0xA576C141e5659137ddDa4223d209d4744b2106BE") tx := types.NewTx(&types.LegacyTx{ Nonce: 0, @@ -141,8 +140,6 @@ func TestBuildRewardingAction(t *testing.T) { }) } -func ptr[V any](v V) *V { return &v } - func TestEthTxUtils(t *testing.T) { r := require.New(t) var ( @@ -166,10 +163,11 @@ func TestEthTxUtils(t *testing.T) { addr, err := address.FromHex("0xA576C141e5659137ddDa4223d209d4744b2106BE") r.NoError(err) - tx, _ := ptr((&ClaimFromRewardingFundBuilder{Builder: *builder}). + act := (&ClaimFromRewardingFundBuilder{Builder: *builder}). SetAddress(addr). SetData([]byte("any")). - SetAmount(big.NewInt(1)).Build()).ToEthTx(chainID) + SetAmount(big.NewInt(1)).Build() + tx, _ := act.ToEthTx(chainID) var ( signer1, _ = NewEthSigner(iotextypes.Encoding_ETHEREUM_EIP155, chainID) diff --git a/action/claimreward.go b/action/claimreward.go index c7541f6798..b7852b1e30 100644 --- a/action/claimreward.go +++ b/action/claimreward.go @@ -32,14 +32,32 @@ const _claimRewardingInterfaceABI = `[ "internalType": "uint8[]", "name": "data", "type": "uint8[]" + } + ], + "name": "claim", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" }, { "internalType": "string", "name": "address", "type": "string" + }, + { + "internalType": "uint8[]", + "name": "data", + "type": "uint8[]" } ], - "name": "claim", + "name": "claimFor", "outputs": [], "stateMutability": "nonpayable", "type": "function" @@ -52,8 +70,10 @@ var ( // ClaimFromRewardingFundGasPerByte represents the claimFromRewardingFund payload gas per uint ClaimFromRewardingFundGasPerByte = uint64(100) - _claimRewardingMethod abi.Method - _ EthCompatibleAction = (*ClaimFromRewardingFund)(nil) + _claimRewardingMethodV1 abi.Method + _claimRewardingMethodV2 abi.Method + _ EthCompatibleAction = (*ClaimFromRewardingFund)(nil) + errWrongMethodSig = errors.New("wrong method signature") ) func init() { @@ -62,10 +82,14 @@ func init() { panic(err) } var ok bool - _claimRewardingMethod, ok = claimRewardInterface.Methods["claim"] + _claimRewardingMethodV1, ok = claimRewardInterface.Methods["claim"] if !ok { panic("fail to load the claim method") } + _claimRewardingMethodV2, ok = claimRewardInterface.Methods["claimFor"] + if !ok { + panic("fail to load the claimTo method") + } } // ClaimFromRewardingFund is the action to claim reward from the rewarding fund @@ -185,15 +209,19 @@ func (b *ClaimFromRewardingFundBuilder) Build() ClaimFromRewardingFund { // encodeABIBinary encodes data in abi encoding func (c *ClaimFromRewardingFund) encodeABIBinary() ([]byte, error) { - var addr string - if c.address != nil { - addr = c.address.String() + if c.address == nil { + // this is v1 ABI before adding address field + data, err := _claimRewardingMethodV1.Inputs.Pack(c.Amount(), c.Data()) + if err != nil { + return nil, err + } + return append(_claimRewardingMethodV1.ID, data...), nil } - data, err := _claimRewardingMethod.Inputs.Pack(c.Amount(), c.Data(), addr) + data, err := _claimRewardingMethodV2.Inputs.Pack(c.Amount(), c.address.String(), c.Data()) if err != nil { return nil, err } - return append(_claimRewardingMethod.ID, data...), nil + return append(_claimRewardingMethodV2.ID, data...), nil } // ToEthTx converts action to eth-compatible tx @@ -214,29 +242,43 @@ func (c *ClaimFromRewardingFund) ToEthTx(_ uint32) (*types.Transaction, error) { // NewClaimFromRewardingFundFromABIBinary decodes data into action func NewClaimFromRewardingFundFromABIBinary(data []byte) (*ClaimFromRewardingFund, error) { + if len(data) <= 4 { + return nil, errDecodeFailure + } + var ( paramsMap = map[string]interface{}{} - ok bool + ok, isV2 bool ac ClaimFromRewardingFund ) - // sanity check - if len(data) <= 4 || !bytes.Equal(_claimRewardingMethod.ID[:], data[:4]) { - return nil, errDecodeFailure - } - if err := _claimRewardingMethod.Inputs.UnpackIntoMap(paramsMap, data[4:]); err != nil { - return nil, err + switch { + case bytes.Equal(_claimRewardingMethodV2.ID[:], data[:4]): + if err := _claimRewardingMethodV2.Inputs.UnpackIntoMap(paramsMap, data[4:]); err != nil { + return nil, err + } + isV2 = true + case bytes.Equal(_claimRewardingMethodV1.ID[:], data[:4]): + if err := _claimRewardingMethodV1.Inputs.UnpackIntoMap(paramsMap, data[4:]); err != nil { + return nil, err + } + default: + return nil, errWrongMethodSig } + if ac.amount, ok = paramsMap["amount"].(*big.Int); !ok { return nil, errDecodeFailure } if ac.data, ok = paramsMap["data"].([]byte); !ok { return nil, errDecodeFailure } - var s string - if s, ok = paramsMap["address"].(string); !ok { - return nil, errDecodeFailure - } - if len(s) > 0 { + if isV2 { + var s string + if s, ok = paramsMap["address"].(string); !ok { + return nil, errDecodeFailure + } + if len(s) == 0 { + return nil, errors.Wrap(errDecodeFailure, "address is empty") + } addr, err := address.FromString(s) if err != nil { return nil, err diff --git a/action/claimreward.s b/action/claimreward.s deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/action/claimreward_test.go b/action/claimreward_test.go index b893c3b569..261b882680 100644 --- a/action/claimreward_test.go +++ b/action/claimreward_test.go @@ -1,4 +1,4 @@ -package action_test +package action import ( "math/big" @@ -6,26 +6,18 @@ import ( _ "unsafe" "github.com/ethereum/go-ethereum/accounts/abi" - "github.com/ethereum/go-ethereum/common" "github.com/iotexproject/iotex-address/address" "github.com/iotexproject/iotex-proto/golang/iotextypes" "github.com/pkg/errors" "github.com/stretchr/testify/require" - "github.com/iotexproject/iotex-core/action" "github.com/iotexproject/iotex-core/pkg/util/assertions" ) -//go:linkname _claimRewardingMethod github.com/iotexproject/iotex-core/action._claimRewardingMethod -var _claimRewardingMethod abi.Method - -//go:linkname _rewardingProtocolEthAddr github.com/iotexproject/iotex-core/action._rewardingProtocolEthAddr -var _rewardingProtocolEthAddr common.Address - func TestClaimRewardIntrinsicGas(t *testing.T) { r := require.New(t) - builder := &action.ClaimFromRewardingFundBuilder{} + builder := &ClaimFromRewardingFundBuilder{} rc := builder.Build() gas, err := rc.IntrinsicGas() @@ -51,7 +43,7 @@ func TestClaimRewardIntrinsicGas(t *testing.T) { func TestClaimRewardSanityCheck(t *testing.T) { r := require.New(t) - builder := &action.ClaimFromRewardingFundBuilder{} + builder := &ClaimFromRewardingFundBuilder{} builder.SetAmount(big.NewInt(1)) rc := builder.Build() @@ -61,13 +53,13 @@ func TestClaimRewardSanityCheck(t *testing.T) { builder.SetAmount(big.NewInt(-1)) rc = builder.Build() err := rc.SanityCheck() - r.ErrorIs(err, action.ErrNegativeValue) + r.ErrorIs(err, ErrNegativeValue) } func TestClaimRewardCost(t *testing.T) { r := require.New(t) - builder := &action.ClaimFromRewardingFundBuilder{} + builder := &ClaimFromRewardingFundBuilder{} builder.SetGasPrice(big.NewInt(1000000000000)) rc := builder.Build() @@ -96,14 +88,14 @@ func TestClaimRewardCost(t *testing.T) { func TestClaimRewardToEthTx(t *testing.T) { r := require.New(t) - builder := &action.ClaimFromRewardingFundBuilder{} + builder := &ClaimFromRewardingFundBuilder{} builder.SetAmount(big.NewInt(101)) rc := builder.Build() tx, err := rc.ToEthTx(0) r.NoError(err) r.Equal(tx.To().String(), _rewardingProtocolEthAddr.String()) - r.Equal(tx.Data()[:4], _claimRewardingMethod.ID) + r.Equal(tx.Data()[:4], _claimRewardingMethodV1.ID) r.Equal(tx.Value().String(), "0") addr, err := address.FromHex("0xA576C141e5659137ddDa4223d209d4744b2106BE") @@ -115,7 +107,7 @@ func TestClaimRewardToEthTx(t *testing.T) { rc = builder.Build() tx, err = rc.ToEthTx(0) r.NoError(err) - r.Equal(tx.Data()[:4], _claimRewardingMethod.ID) + r.Equal(tx.Data()[:4], _claimRewardingMethodV2.ID) r.Equal(tx.Value().String(), "0") } @@ -134,13 +126,13 @@ func TestNewRewardingClaimFromABIBinary(t *testing.T) { Indexed: false, }, abi.Argument{ - Name: "data", - Type: assertions.MustNoErrorV(abi.NewType("uint8[]", "uint8[]", nil)), + Name: "address", + Type: assertions.MustNoErrorV(abi.NewType("string", "string", nil)), Indexed: false, }, abi.Argument{ - Name: "address", - Type: assertions.MustNoErrorV(abi.NewType("string", "string", nil)), + Name: "data", + Type: assertions.MustNoErrorV(abi.NewType("uint8[]", "uint8[]", nil)), Indexed: false, }, } @@ -148,48 +140,56 @@ func TestNewRewardingClaimFromABIBinary(t *testing.T) { ) t.Run("CheckMethodDefine", func(t *testing.T) { - method = abi.NewMethod("claim", "claim", abi.Function, "nonpayable", false, false, inputs, outputs) - r.Equal(method, _claimRewardingMethod) + method = abi.NewMethod("claimFor", "claimFor", abi.Function, "nonpayable", false, false, inputs, outputs) + r.Equal(method, _claimRewardingMethodV2) }) t.Run("InvalidMethodSignature", func(t *testing.T) { - input := assertions.MustNoErrorV(method.Inputs.Pack(amount, data, addr)) + input := assertions.MustNoErrorV(method.Inputs.Pack(amount, addr, data)) methodsig := []byte{'1', '2', '3', 4} // invalid calldata := append(methodsig, input...) - _, err := action.NewClaimFromRewardingFundFromABIBinary(calldata) - r.ErrorContains(err, "failed to decode") + _, err := NewClaimFromRewardingFundFromABIBinary(calldata) + r.Equal(errWrongMethodSig, err) }) t.Run("MissingSomeArgument", func(t *testing.T) { - _inputs := _claimRewardingMethod.Inputs + _inputs := _claimRewardingMethodV2.Inputs calldata := append( method.ID, - assertions.MustNoErrorV(inputs.Pack(amount, data, addr))..., + assertions.MustNoErrorV(inputs.Pack(amount, addr, data))..., ) for i := 0; i < len(_inputs); i++ { old := inputs[i].Name _inputs[i].Name = "any" - _, err := action.NewClaimFromRewardingFundFromABIBinary(calldata) - r.ErrorContains(err, "failed to decode") + _, err := NewClaimFromRewardingFundFromABIBinary(calldata) + r.Equal(errDecodeFailure, err) _inputs[i].Name = old } }) + t.Run("EmptyAddress", func(t *testing.T) { + calldata := append( + method.ID[:], + assertions.MustNoErrorV(method.Inputs.Pack(amount, "", data))...) + _, err := NewClaimFromRewardingFundFromABIBinary(calldata) + r.ErrorContains(err, "address is empty") + }) + t.Run("InvalidAddress", func(t *testing.T) { calldata := append( method.ID[:], - assertions.MustNoErrorV(method.Inputs.Pack(amount, data, "0x1231231232113"))...) - _, err := action.NewClaimFromRewardingFundFromABIBinary(calldata) + assertions.MustNoErrorV(method.Inputs.Pack(amount, "0x1231231232113", data))...) + _, err := NewClaimFromRewardingFundFromABIBinary(calldata) r.Equal(address.ErrInvalidAddr, errors.Cause(err)) }) t.Run("Success", func(t *testing.T) { calldata := append( method.ID[:], - assertions.MustNoErrorV(method.Inputs.Pack(amount, data, addr))...) - ret, err := action.NewClaimFromRewardingFundFromABIBinary(calldata) + assertions.MustNoErrorV(method.Inputs.Pack(amount, addr, data))...) + ret, err := NewClaimFromRewardingFundFromABIBinary(calldata) r.NoError(err) r.Equal(ret.Address().String(), addr) r.Equal(ret.Amount(), amount) @@ -197,22 +197,9 @@ func TestNewRewardingClaimFromABIBinary(t *testing.T) { }) } -/* - func TestClaimFromRewardingFund(t *testing.T) { - b := ClaimFromRewardingFundBuilder{} - s1 := b.SetAmount(big.NewInt(1)). - SetData([]byte{2}). - Build() - proto := s1.Proto() - s2 := ClaimFromRewardingFund{} - require.NoError(t, s2.LoadProto(proto)) - assert.Equal(t, s1.Amount(), s2.Amount()) - assert.Equal(t, s2.Data(), s2.Data()) - } -*/ func TestClaimFromRewardingFund(t *testing.T) { r := require.New(t) - c := &action.ClaimFromRewardingFund{} + c := &ClaimFromRewardingFund{} t.Run("InvalidAmountProtoValue", func(t *testing.T) { p := &iotextypes.ClaimFromRewardingFund{ @@ -268,7 +255,7 @@ func TestClaimFromRewardingFund(t *testing.T) { if i == 1 { addr, _ = address.FromString("io10a298zmzvrt4guq79a9f4x7qedj59y7ery84he") } - builder := &action.ClaimFromRewardingFundBuilder{} + builder := &ClaimFromRewardingFundBuilder{} builder.SetAmount(big.NewInt(205)) builder.SetData([]byte("abc")) builder.SetAddress(addr) diff --git a/action/protocol/rewarding/protocol.go b/action/protocol/rewarding/protocol.go index 958d03697f..c3390f4748 100644 --- a/action/protocol/rewarding/protocol.go +++ b/action/protocol/rewarding/protocol.go @@ -184,7 +184,7 @@ func createGrantRewardAction(rewardType int, height uint64) action.Envelope { // Validate validates a reward action func (p *Protocol) Validate(ctx context.Context, act action.Action, sr protocol.StateReader) error { - switch act.(type) { + switch act := act.(type) { case *action.GrantReward: actionCtx := protocol.MustGetActionCtx(ctx) if !address.Equal(protocol.MustGetBlockCtx(ctx).Producer, actionCtx.Caller) { @@ -193,6 +193,10 @@ func (p *Protocol) Validate(ctx context.Context, act action.Action, sr protocol. if actionCtx.GasPrice != nil && actionCtx.GasPrice.Cmp(big.NewInt(0)) != 0 || actionCtx.IntrinsicGas != 0 { return errors.New("invalid gas price or intrinsic gas for reward action") } + case *action.ClaimFromRewardingFund: + if !protocol.MustGetFeatureCtx(ctx).AddClaimRewardAddress && act.Address() != nil { + return errors.New("claim reward address not enabled yet") + } } return nil } @@ -216,7 +220,7 @@ func (p *Protocol) Handle( case *action.ClaimFromRewardingFund: si := sm.Snapshot() addr := protocol.MustGetActionCtx(ctx).Caller - if protocol.MustGetFeatureCtx(ctx).AddClaimRewardAddress && act.Address() != nil { + if act.Address() != nil { addr = act.Address() } rlog, err := p.Claim(ctx, sm, act.Amount(), addr) diff --git a/action/rlp_tx_test.go b/action/rlp_tx_test.go index 3dfad72e8e..6b44c9bfe6 100644 --- a/action/rlp_tx_test.go +++ b/action/rlp_tx_test.go @@ -299,21 +299,21 @@ var ( "04dc4c548c3a478278a6a09ffa8b5c4b384368e49654b35a6961ee8288fc889cdc39e9f8194e41abdbfac248ef9dc3f37b131a36ee2c052d974c21c1d2cd56730b", "1e14d5373e1af9cc77f0032ad2cd0fba8be5ea2e", }, - //{ - // "rewardingClaim", - // "f8c6806482520894a576c141e5659137ddda4223d209d4744b2106be80b8642df163ef0000000000000000000000000000000000000000000000000000000000000065000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000008224c6a03d62943431b8ca0e8ea0a9c79dc8f64df14c965ca5486da124804013778ed566a065956b185116b65cec0ec9bcf9539e924784a4b448190b0aa9d017f1719be921", - // 0, - // 21000, - // "100", - // "0", - // "0xA576C141e5659137ddDa4223d209d4744b2106BE", - // _evmNetworkID, - // iotextypes.Encoding_ETHEREUM_EIP155, - // 100, - // "4d26d0736ebd9e69bd5994f3730b05a2d48c810b3bb54818be65d02004cf4ff4", - // "04830579b50e01602c2015c24e72fbc48bca1cca1e601b119ca73abe2e0b5bd61fcb7874567e091030d6b644f927445d80e00b3f9ca0c566c21c30615e94c343da", - // "8d38efe45794d7fceea10b2262c23c12245959db", - //}, + { + "rewardingClaim", + "f8c6806482520894a576c141e5659137ddda4223d209d4744b2106be80b8642df163ef0000000000000000000000000000000000000000000000000000000000000065000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000008224c6a03d62943431b8ca0e8ea0a9c79dc8f64df14c965ca5486da124804013778ed566a065956b185116b65cec0ec9bcf9539e924784a4b448190b0aa9d017f1719be921", + 0, + 21000, + "100", + "0", + "0xA576C141e5659137ddDa4223d209d4744b2106BE", + _evmNetworkID, + iotextypes.Encoding_ETHEREUM_EIP155, + 100, + "4d26d0736ebd9e69bd5994f3730b05a2d48c810b3bb54818be65d02004cf4ff4", + "04830579b50e01602c2015c24e72fbc48bca1cca1e601b119ca73abe2e0b5bd61fcb7874567e091030d6b644f927445d80e00b3f9ca0c566c21c30615e94c343da", + "8d38efe45794d7fceea10b2262c23c12245959db", + }, { "rewardingDeposit", "f8c6016482520894a576c141e5659137ddda4223d209d4744b2106be80b86427852a6b0000000000000000000000000000000000000000000000000000000000000065000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000008224c6a013b7679dbabcb0f97b93942436f5072cca3c7fe43451a8fedcdf3c84c1344e1da02af4cc67594c0200b59f4e30ba149af15e546acbfc69fa31f14e8788ab063d85", @@ -778,8 +778,8 @@ func TestEthTxDecodeVerifyV2(t *testing.T) { txto: MustNoErrorV(address.FromBytes(address.RewardingProtocolAddrHash[:])).String(), txamount: big.NewInt(0), txdata: append( - _claimRewardingMethod.ID, - MustNoErrorV(_claimRewardingMethod.Inputs.Pack(amount, data, to))..., + _claimRewardingMethodV2.ID, + MustNoErrorV(_claimRewardingMethodV2.Inputs.Pack(amount, to, data))..., ), action: &ClaimFromRewardingFund{ AbstractAction: AbstractAction{