From a19085e81e0e1b8f7e9df25a7a3a9147e532eaac Mon Sep 17 00:00:00 2001 From: Francisco de Borja Aranda Castillejo Date: Tue, 19 Nov 2024 18:24:23 +0100 Subject: [PATCH] add more checks to zrc20 is valid --- precompiles/bank/method_deposit.go | 2 +- precompiles/bank/method_withdraw.go | 2 +- precompiles/staking/method_distribute.go | 2 +- .../staking/method_get_validators_test.go | 31 +++++++++++++++++++ precompiles/types/coin.go | 20 ++++++++++-- precompiles/types/coin_test.go | 17 +++++----- 6 files changed, 61 insertions(+), 13 deletions(-) diff --git a/precompiles/bank/method_deposit.go b/precompiles/bank/method_deposit.go index 43ba2a492f..48edc76ac5 100644 --- a/precompiles/bank/method_deposit.go +++ b/precompiles/bank/method_deposit.go @@ -80,7 +80,7 @@ func (c *Contract) deposit( // this way we map ZRC20 addresses to cosmos denoms "zevm/0x12345". // - Mint coins to the fungible module. // - Send coins from fungible to the caller. - coinSet, err := precompiletypes.CreateCoinSet(zrc20Addr, amount) + coinSet, err := precompiletypes.CreateZRC20CoinSet(zrc20Addr, amount) if err != nil { return nil, err } diff --git a/precompiles/bank/method_withdraw.go b/precompiles/bank/method_withdraw.go index 24f83bf5e4..642e113047 100644 --- a/precompiles/bank/method_withdraw.go +++ b/precompiles/bank/method_withdraw.go @@ -79,7 +79,7 @@ func (c *Contract) withdraw( } } - coinSet, err := precompiletypes.CreateCoinSet(zrc20Addr, amount) + coinSet, err := precompiletypes.CreateZRC20CoinSet(zrc20Addr, amount) if err != nil { return nil, err } diff --git a/precompiles/staking/method_distribute.go b/precompiles/staking/method_distribute.go index c173517aeb..7a285d20af 100644 --- a/precompiles/staking/method_distribute.go +++ b/precompiles/staking/method_distribute.go @@ -42,7 +42,7 @@ func (c *Contract) distribute( } // Create the coinSet in advance, if this step fails do not lock ZRC20. - coinSet, err := precompiletypes.CreateCoinSet(zrc20Addr, amount) + coinSet, err := precompiletypes.CreateZRC20CoinSet(zrc20Addr, amount) if err != nil { return nil, err } diff --git a/precompiles/staking/method_get_validators_test.go b/precompiles/staking/method_get_validators_test.go index 6fd3957c96..068973ca6a 100644 --- a/precompiles/staking/method_get_validators_test.go +++ b/precompiles/staking/method_get_validators_test.go @@ -5,6 +5,7 @@ import ( "testing" "cosmossdk.io/math" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" precompiletypes "github.com/zeta-chain/node/precompiles/types" "github.com/zeta-chain/node/testutil/sample" @@ -44,6 +45,36 @@ func Test_GetValidators(t *testing.T) { require.Len(t, list, 0) }) + t.Run("should return an empty list for an invalid staker", func(t *testing.T) { + /* ARRANGE */ + s := newTestSuite(t) + + // Create validator. + validator := sample.Validator(t, rand.New(rand.NewSource(42))) + s.sdkKeepers.StakingKeeper.SetValidator(s.ctx, validator) + + /* ACT */ + // Call getRewards. + getValidatorsMethod := s.stkContractABI.Methods[GetValidatorsMethodName] + + s.mockVMContract.Input = packInputArgs( + t, + getValidatorsMethod, + []interface{}{common.Address{}}..., + ) + + bytes, err := s.stkContract.Run(s.mockEVM, s.mockVMContract, false) + require.NoError(t, err) + + res, err := getValidatorsMethod.Outputs.Unpack(bytes) + require.NoError(t, err) + require.NotEmpty(t, res) + + list, ok := res[0].([]string) + require.True(t, ok) + require.Len(t, list, 0) + }) + t.Run("should return staker's validator list", func(t *testing.T) { /* ARRANGE */ s := newTestSuite(t) diff --git a/precompiles/types/coin.go b/precompiles/types/coin.go index f8f41eb766..b2f04c2eed 100644 --- a/precompiles/types/coin.go +++ b/precompiles/types/coin.go @@ -17,13 +17,20 @@ func ZRC20ToCosmosDenom(ZRC20Address common.Address) string { return config.ZRC20DenomPrefix + ZRC20Address.String() } -func CreateCoinSet(zrc20address common.Address, amount *big.Int) (sdk.Coins, error) { +func CreateZRC20CoinSet(zrc20address common.Address, amount *big.Int) (sdk.Coins, error) { defer func() { if r := recover(); r != nil { return } }() + if (zrc20address == common.Address{}) { + return nil, &ErrInvalidAddr{ + Got: zrc20address.String(), + Reason: "empty address", + } + } + denom := ZRC20ToCosmosDenom(zrc20address) coin := sdk.NewCoin(denom, math.NewIntFromBigInt(amount)) @@ -53,5 +60,14 @@ func CreateCoinSet(zrc20address common.Address, amount *big.Int) (sdk.Coins, err // CoinIsZRC20 checks if a given coin is a ZRC20 coin based on its denomination. func CoinIsZRC20(denom string) bool { - return strings.HasPrefix(denom, config.ZRC20DenomPrefix) + // Fail fast if the prefix is not set. + if !strings.HasPrefix(denom, config.ZRC20DenomPrefix) { + return false + } + + // Prefix is correctly set, extract the zrc20 address. + zrc20Addr := strings.TrimPrefix(denom, config.ZRC20DenomPrefix) + + // Return true only if address is not empty and is a valid hex address. + return common.HexToAddress(zrc20Addr) != common.Address{} && common.IsHexAddress(zrc20Addr) } diff --git a/precompiles/types/coin_test.go b/precompiles/types/coin_test.go index 9566662dfe..c8e0e4c841 100644 --- a/precompiles/types/coin_test.go +++ b/precompiles/types/coin_test.go @@ -20,7 +20,7 @@ func Test_createCoinSet(t *testing.T) { tokenDenom := ZRC20ToCosmosDenom(tokenAddr) amount := big.NewInt(100) - coinSet, err := CreateCoinSet(tokenAddr, amount) + coinSet, err := CreateZRC20CoinSet(tokenAddr, amount) require.NoError(t, err, "createCoinSet should not return an error") require.NotNil(t, coinSet, "coinSet should not be nil") @@ -34,18 +34,19 @@ func Test_CoinIsZRC20(t *testing.T) { denom string expected bool }{ - {"zrc20/0x0123456789abcdef", true}, - {"zrc20/0xabcdef0123456789", true}, - {"zrc200xabcdef", false}, - {"foo/0x0123456789", false}, + {"", false}, // Empty string. + {"zrc20/", false}, // Missing address. + {"zrc20/0x514910771af9ca656af840dff83e8264ecf986ca", true}, // Valid ZRC20 address. + {"zrc20/0xCa14007Eff0dB1f8135f4C25B34De49AB0d42766", true}, // Valid ZRC20 address. + {"zrc200xabcdef", false}, // Malformed prefix. + {"foo/0x0123456789", false}, // Invalid prefix. + {"ZRC20/0x0123456789abcdef", false}, // Invalid prefix. } for _, tt := range test { t.Run(tt.denom, func(t *testing.T) { result := CoinIsZRC20(tt.denom) - if result != tt.expected { - t.Errorf("got %v, want %v", result, tt.expected) - } + require.Equal(t, tt.expected, result, "got %v, want %v", result, tt.expected) }) } }