diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index c8d92fce28..0efc57bed1 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -250,6 +250,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) { e2etests.TestERC20DepositRestrictedName, } zetaTests := []string{ + e2etests.TestZetaDepositInvalidAddressName, e2etests.TestZetaWithdrawName, e2etests.TestMessagePassingExternalChainsName, e2etests.TestMessagePassingRevertFailExternalChainsName, diff --git a/e2e/e2etests/e2etests.go b/e2e/e2etests/e2etests.go index 5b810276db..2d60a1a309 100644 --- a/e2e/e2etests/e2etests.go +++ b/e2e/e2etests/e2etests.go @@ -10,11 +10,12 @@ const ( ZETA tests Test transfer of ZETA asset across chains */ - TestZetaDepositName = "zeta_deposit" - TestZetaDepositNewAddressName = "zeta_deposit_new_address" - TestZetaDepositRestrictedName = "zeta_deposit_restricted" - TestZetaWithdrawName = "zeta_withdraw" - TestZetaWithdrawBTCRevertName = "zeta_withdraw_btc_revert" // #nosec G101 - not a hardcoded password + TestZetaDepositName = "zeta_deposit" + TestZetaDepositInvalidAddressName = "zeta_deposit_invalid_address" + TestZetaDepositNewAddressName = "zeta_deposit_new_address" + TestZetaDepositRestrictedName = "zeta_deposit_restricted" + TestZetaWithdrawName = "zeta_withdraw" + TestZetaWithdrawBTCRevertName = "zeta_withdraw_btc_revert" // #nosec G101 - not a hardcoded password /* Message passing tests @@ -163,6 +164,14 @@ var AllE2ETests = []runner.E2ETest{ }, TestZetaDeposit, ), + runner.NewE2ETest( + TestZetaDepositInvalidAddressName, + "deposit ZETA from Ethereum to ZEVM but use invalid address", + []runner.ArgDefinition{ + {Description: "amount in azeta", DefaultValue: "10000000000000000000"}, + }, + TestZetaDepositToInvalidAddress, + ), runner.NewE2ETest( TestZetaDepositNewAddressName, "deposit ZETA from Ethereum to a new ZEVM address which does not exist yet", diff --git a/e2e/e2etests/test_migrate_chain_support.go b/e2e/e2etests/test_migrate_chain_support.go index b5222a81d7..00e45e0177 100644 --- a/e2e/e2etests/test_migrate_chain_support.go +++ b/e2e/e2etests/test_migrate_chain_support.go @@ -38,7 +38,7 @@ func TestMigrateChainSupport(r *runner.E2ERunner, _ []string) { // deposit most of the ZETA supply on ZetaChain zetaAmount := big.NewInt(1e18) zetaAmount = zetaAmount.Mul(zetaAmount, big.NewInt(20_000_000_000)) // 20B Zeta - r.DepositZetaWithAmount(r.EVMAddress(), zetaAmount) + r.DepositZetaWithAmount(r.EVMAddress().Bytes(), zetaAmount) // do an ethers withdraw on the previous chain (0.01eth) for some interaction TestEtherWithdraw(r, []string{"10000000000000000"}) diff --git a/e2e/e2etests/test_zeta_deposit.go b/e2e/e2etests/test_zeta_deposit.go index 00b672ee61..ba9b363ad6 100644 --- a/e2e/e2etests/test_zeta_deposit.go +++ b/e2e/e2etests/test_zeta_deposit.go @@ -1,7 +1,11 @@ package e2etests import ( + "fmt" + "math/big" + "github.com/stretchr/testify/require" + "github.com/zeta-chain/zetacore/x/crosschain/types" "github.com/zeta-chain/zetacore/e2e/runner" "github.com/zeta-chain/zetacore/e2e/utils" @@ -13,9 +17,27 @@ func TestZetaDeposit(r *runner.E2ERunner, args []string) { // parse deposit amount amount := parseBigInt(r, args[0]) - hash := r.DepositZetaWithAmount(r.EVMAddress(), amount) + hash := r.DepositZetaWithAmount(r.EVMAddress().Bytes(), amount) + + // wait for the cctx to be mined + cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, hash.Hex(), r.CctxClient, r.Logger, r.CctxTimeout) + r.Logger.CCTX(*cctx, "deposit") +} + +func TestZetaDepositToInvalidAddress(r *runner.E2ERunner, args []string) { + require.Len(r, args, 1) + + amount, ok := big.NewInt(0).SetString(args[0], 10) + require.True(r, ok, "Invalid amount specified for TestZetaDeposit.") + + hash := r.DepositZetaWithAmount([]byte("invalid"), amount) + + r.Logger.Print(fmt.Sprintf("Deposit to invalid address tx hass: %s", hash.Hex())) // wait for the cctx to be mined cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, hash.Hex(), r.CctxClient, r.Logger, r.CctxTimeout) r.Logger.CCTX(*cctx, "deposit") + r.Logger.Print(fmt.Sprintf("CCTX: %v", cctx.Index)) + utils.RequireCCTXStatus(r, cctx, types.CctxStatus_Reverted, "deposit with invalid address") + utils.ContainsStringInCCTXStatusMessage(r, cctx, types.ErrInvalidReceiverAddress.Error(), "deposit with invalid address") } diff --git a/e2e/e2etests/test_zeta_deposit_new_address.go b/e2e/e2etests/test_zeta_deposit_new_address.go index 79a1360db9..164277cc9c 100644 --- a/e2e/e2etests/test_zeta_deposit_new_address.go +++ b/e2e/e2etests/test_zeta_deposit_new_address.go @@ -15,7 +15,7 @@ func TestZetaDepositNewAddress(r *runner.E2ERunner, args []string) { amount := parseBigInt(r, args[0]) newAddress := sample.EthAddress() - hash := r.DepositZetaWithAmount(newAddress, amount) + hash := r.DepositZetaWithAmount(newAddress.Bytes(), amount) // wait for the cctx to be mined cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, hash.Hex(), r.CctxClient, r.Logger, r.CctxTimeout) diff --git a/e2e/e2etests/test_zeta_deposit_restricted_address.go b/e2e/e2etests/test_zeta_deposit_restricted_address.go index d760ccafbd..76af85e5af 100644 --- a/e2e/e2etests/test_zeta_deposit_restricted_address.go +++ b/e2e/e2etests/test_zeta_deposit_restricted_address.go @@ -15,5 +15,5 @@ func TestZetaDepositRestricted(r *runner.E2ERunner, args []string) { amount := parseBigInt(r, args[0]) // Deposit amount to restricted address - r.DepositZetaWithAmount(ethcommon.HexToAddress(testutils.RestrictedEVMAddressTest), amount) + r.DepositZetaWithAmount(ethcommon.HexToAddress(testutils.RestrictedEVMAddressTest).Bytes(), amount) } diff --git a/e2e/runner/zeta.go b/e2e/runner/zeta.go index fec82f459a..a7c8e96a50 100644 --- a/e2e/runner/zeta.go +++ b/e2e/runner/zeta.go @@ -95,11 +95,11 @@ func (r *E2ERunner) DepositZeta() ethcommon.Hash { amount := big.NewInt(1e18) amount = amount.Mul(amount, big.NewInt(100)) // 100 Zeta - return r.DepositZetaWithAmount(r.EVMAddress(), amount) + return r.DepositZetaWithAmount(r.EVMAddress().Bytes(), amount) } // DepositZetaWithAmount deposits ZETA on ZetaChain from the ZETA smart contract on EVM with the specified amount -func (r *E2ERunner) DepositZetaWithAmount(to ethcommon.Address, amount *big.Int) ethcommon.Hash { +func (r *E2ERunner) DepositZetaWithAmount(to []byte, amount *big.Int) ethcommon.Hash { tx, err := r.ZetaEth.Approve(r.EVMAuth, r.ConnectorEthAddr, amount) require.NoError(r, err) @@ -117,7 +117,7 @@ func (r *E2ERunner) DepositZetaWithAmount(to ethcommon.Address, amount *big.Int) // TODO: allow user to specify destination chain id // https://github.com/zeta-chain/node-private/issues/41 DestinationChainId: zetaChainID, - DestinationAddress: to.Bytes(), + DestinationAddress: to, DestinationGasLimit: big.NewInt(250_000), Message: nil, ZetaValueAndGas: amount, diff --git a/e2e/utils/require.go b/e2e/utils/require.go index 7471bf9b4e..512b18028c 100644 --- a/e2e/utils/require.go +++ b/e2e/utils/require.go @@ -22,6 +22,17 @@ func RequireCCTXStatus( require.Equal(t, expected, cctx.CctxStatus.Status, msg+errSuffix(msgAndArgs...)) } +func ContainsStringInCCTXStatusMessage( + t require.TestingT, + cctx *crosschaintypes.CrossChainTx, + expected string, + msgAndArgs ...any) { + msg := fmt.Sprintf("cctx status message does not contain %s cctx index %s", expected, cctx.Index) + + require.NotNil(t, cctx.CctxStatus) + require.Contains(t, cctx.CctxStatus.StatusMessage, expected, msg+errSuffix(msgAndArgs...)) +} + // RequireTxSuccessful checks if the receipt status is successful. // Currently, it accepts eth receipt, but we can make this more generic by using type assertion. func RequireTxSuccessful(t require.TestingT, receipt *ethtypes.Receipt, msgAndArgs ...any) { diff --git a/pkg/address/errors.go b/pkg/address/errors.go new file mode 100644 index 0000000000..6bb082bf9f --- /dev/null +++ b/pkg/address/errors.go @@ -0,0 +1,35 @@ +package address + +import ( + "fmt" +) + +// InvalidAddressError represents an error for an invalid address. +type InvalidAddressError struct { + Address string + Msg string +} + +func (e *InvalidAddressError) Error() string { + return fmt.Sprintf("Invalid address: %s, %s", e.Address, e.Msg) +} + +// InvalidChainError represents an error for an invalid chain ID. +type InvalidChainError struct { + ChainID int64 + Msg string +} + +func (e *InvalidChainError) Error() string { + return fmt.Sprintf("Invalid chain ID: %d, %s", e.ChainID, e.Msg) +} + +// InvalidNetworkError represents an error for an invalid network. +type InvalidNetworkError struct { + Network string + Msg string +} + +func (e *InvalidNetworkError) Error() string { + return fmt.Sprintf("Invalid network: %s, %s", e.Network, e.Msg) +} diff --git a/pkg/address/validate_address.go b/pkg/address/validate_address.go new file mode 100644 index 0000000000..d721cecf63 --- /dev/null +++ b/pkg/address/validate_address.go @@ -0,0 +1,64 @@ +package address + +import ( + "fmt" + + ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/zeta-chain/zetacore/pkg/chains" +) + +// ValidateAddressForChain validates the address for the chain +// NOTE: since these checks are currently not used, we don't provide additional chains for simplicity +// https://github.com/zeta-chain/node/issues/2234 +// https://github.com/zeta-chain/node/issues/2385 +// NOTE: We should eventually not using these hard-coded checks at all for same reasons as above + +// TODO : Use this function to validate Sender and Receiver address for CCTX +// https://github.com/zeta-chain/node/issues/2697 +func ValidateAddressForChain(address string, chainID int64, additionalChains []chains.Chain) error { + chain, found := chains.GetChainFromChainID(chainID, additionalChains) + if !found { + return &InvalidChainError{ChainID: chainID, Msg: "chain not supported"} + } + switch chain.Network { + case chains.Network_eth: + return ValidateEVMAddress(address) + case chains.Network_zeta: + return nil + case chains.Network_btc: + return ValidateBTCAddress(address, chainID) + case chains.Network_polygon: + return ValidateEVMAddress(address) + case chains.Network_bsc: + return ValidateEVMAddress(address) + case chains.Network_optimism: + return nil + case chains.Network_base: + return nil + case chains.Network_solana: + return nil + default: + return &InvalidNetworkError{Network: chain.Network.String(), Msg: "network not supported"} + } +} + +func ValidateEVMAddress(address string) error { + if !ethcommon.IsHexAddress(address) { + return &InvalidAddressError{Address: address, Msg: "not a valid hex address"} + } + if ethcommon.HexToAddress(address) == (ethcommon.Address{}) { + return &InvalidAddressError{Address: address, Msg: "empty address"} + } + return nil +} + +func ValidateBTCAddress(address string, chainID int64) error { + addr, err := chains.DecodeBtcAddress(address, chainID) + if err != nil { + return &InvalidAddressError{Address: address, Msg: fmt.Sprintf("failed to decode address: %s chainId %d", err, chainID)} + } + if !chains.IsBtcAddressSupported(addr) { + return &InvalidAddressError{Address: address, Msg: "address not supported"} + } + return nil +} diff --git a/pkg/address/validate_address_test.go b/pkg/address/validate_address_test.go new file mode 100644 index 0000000000..9fff6114aa --- /dev/null +++ b/pkg/address/validate_address_test.go @@ -0,0 +1,151 @@ +package address_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/zeta-chain/zetacore/pkg/address" + "github.com/zeta-chain/zetacore/pkg/chains" + "github.com/zeta-chain/zetacore/testutil/sample" +) + +func appendChains(chainLists ...[]chains.Chain) []chains.Chain { + var combined []chains.Chain + for _, chains := range chainLists { + combined = append(combined, chains...) + } + return combined +} + +func TestValidateAddressForChain(t *testing.T) { + additionalChains := []chains.Chain{} + evmChains := []chains.Chain{ + // Ethereum + chains.Ethereum, + chains.Sepolia, + chains.Goerli, + chains.GoerliLocalnet, + // Polygon + chains.Polygon, + chains.Amoy, + chains.Mumbai, + // BSC + chains.BscMainnet, + chains.BscTestnet, + } + zetaChains := []chains.Chain{ + chains.ZetaChainMainnet, + chains.ZetaChainTestnet, + chains.ZetaChainDevnet, + chains.ZetaChainPrivnet, + } + baseChains := []chains.Chain{ + chains.BaseMainnet, + chains.BaseSepolia, + } + + solanaChains := []chains.Chain{ + chains.SolanaMainnet, + chains.SolanaDevnet, + chains.SolanaLocalnet, + } + + opChains := []chains.Chain{ + chains.OptimismMainnet, + chains.OptimismSepolia, + } + + bitcoinChains := []chains.Chain{ + chains.BitcoinMainnet, + chains.BitcoinTestnet, + chains.BitcoinRegtest, + } + + // Append all chains + allChains := appendChains(evmChains, zetaChains, baseChains, solanaChains, opChains, bitcoinChains) + require.ElementsMatch(t, allChains, chains.DefaultChainsList()) + + //test for invalid chain + require.ErrorContains(t, address.ValidateAddressForChain("0x123", 123, additionalChains), "chain not supported") + + // test for evm chain chains + for _, chain := range evmChains { + require.Error(t, address.ValidateAddressForChain("0x123", chain.ChainId, additionalChains)) + require.Error(t, address.ValidateAddressForChain("", chain.ChainId, additionalChains)) + require.Error(t, address.ValidateAddressForChain("%%%%", chain.ChainId, additionalChains)) + require.NoError( + t, + address.ValidateAddressForChain(sample.EthAddress().Hex(), chain.ChainId, additionalChains), + ) + sample.EthAddress() + } + + // test for zeta chain + for _, chain := range zetaChains { + require.NoError(t, address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.EthAddress().Hex(), chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.SolanaAddress(t), chain.ChainId, additionalChains)) + } + // test for base chain + for _, chain := range baseChains { + require.NoError(t, address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.EthAddress().Hex(), chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.SolanaAddress(t), chain.ChainId, additionalChains)) + } + + // test for solana chain + for _, chain := range solanaChains { + require.NoError(t, address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.EthAddress().Hex(), chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.SolanaAddress(t), chain.ChainId, additionalChains)) + } + + // test for optimism chain + for _, chain := range opChains { + require.NoError(t, address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain("bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.EthAddress().Hex(), chain.ChainId, additionalChains)) + require.NoError(t, address.ValidateAddressForChain(sample.SolanaAddress(t), chain.ChainId, additionalChains)) + } + + // test for btc chain + require.NoError( + t, + address.ValidateAddressForChain( + "bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", + chains.BitcoinMainnet.ChainId, + additionalChains, + ), + ) + require.NoError( + t, + address.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chains.BitcoinMainnet.ChainId, additionalChains), + ) + require.NoError( + t, + address.ValidateAddressForChain("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", chains.BitcoinMainnet.ChainId, additionalChains), + ) + require.NoError( + t, + address.ValidateAddressForChain("bc1qysd4sp9q8my59ul9wsf5rvs9p387hf8vfwatzu", chains.BitcoinMainnet.ChainId, additionalChains), + ) + require.NoError( + t, + address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chains.BitcoinRegtest.ChainId, additionalChains), + ) + + // Invalid if the chain params are incorrect + require.Error( + t, + address.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chains.BitcoinMainnet.ChainId, additionalChains), + ) + // Invalid if address string is invalid + require.Error(t, address.ValidateAddressForChain("", chains.BitcoinRegtest.ChainId, additionalChains)) +} diff --git a/x/crosschain/keeper/cctx_gateway_zevm.go b/x/crosschain/keeper/cctx_gateway_zevm.go index d9061be54a..861ed80a89 100644 --- a/x/crosschain/keeper/cctx_gateway_zevm.go +++ b/x/crosschain/keeper/cctx_gateway_zevm.go @@ -2,7 +2,6 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/zeta-chain/zetacore/x/crosschain/types" ) diff --git a/x/crosschain/keeper/evm_deposit.go b/x/crosschain/keeper/evm_deposit.go index 2b46cedcb6..d9d12712bd 100644 --- a/x/crosschain/keeper/evm_deposit.go +++ b/x/crosschain/keeper/evm_deposit.go @@ -24,7 +24,10 @@ const InCCTXIndexKey = "inCctxIndex" // returns (isContractReverted, err) // (true, non-nil) means CallEVM() reverted func (k Keeper) HandleEVMDeposit(ctx sdk.Context, cctx *types.CrossChainTx) (bool, error) { - to := ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver) + to, err := cctx.GetValidReceiverAddress() + if err != nil { + return true, err + } sender := ethcommon.HexToAddress(cctx.InboundParams.Sender) var ethTxHash ethcommon.Hash inboundAmount := cctx.GetInboundParams().Amount.BigInt() diff --git a/x/crosschain/keeper/evm_deposit_test.go b/x/crosschain/keeper/evm_deposit_test.go index 18bc103bc3..fa41749c49 100644 --- a/x/crosschain/keeper/evm_deposit_test.go +++ b/x/crosschain/keeper/evm_deposit_test.go @@ -51,6 +51,82 @@ func TestMsgServer_HandleEVMDeposit(t *testing.T) { fungibleMock.AssertExpectations(t) }) + t.Run("cannot process Zeta deposit if the receiver address is invalid zevm address for cointype zeta", func(t *testing.T) { + k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseFungibleMock: true, + }) + + receiver := "invalid" + amount := big.NewInt(42) + sender := sample.EthAddress() + senderChainId := int64(0) + + // call HandleEVMDeposit + cctx := sample.CrossChainTx(t, "foo") + cctx.GetCurrentOutboundParam().Receiver = receiver + cctx.GetInboundParams().Amount = math.NewUintFromBigInt(amount) + cctx.GetInboundParams().CoinType = coin.CoinType_Zeta + cctx.GetInboundParams().SenderChainId = senderChainId + cctx.InboundParams.Sender = sender.String() + reverted, err := k.HandleEVMDeposit( + ctx, + cctx, + ) + require.ErrorIs(t, err, types.ErrInvalidReceiverAddress) + require.True(t, reverted) + }) + + t.Run("cannot process Zeta deposit if the receiver address is invalid zevm address for cointype gas", func(t *testing.T) { + k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseFungibleMock: true, + }) + + receiver := sample.EthAddress() + amount := big.NewInt(42) + sender := sample.EthAddress() + senderChainId := int64(0) + + // call HandleEVMDeposit + cctx := sample.CrossChainTx(t, "foo") + cctx.GetCurrentOutboundParam().Receiver = receiver.Hex() + cctx.GetInboundParams().Amount = math.NewUintFromBigInt(amount) + cctx.GetInboundParams().CoinType = coin.CoinType_Gas + cctx.GetInboundParams().SenderChainId = senderChainId + cctx.RelayedMessage = "0xXXXX" + cctx.InboundParams.Sender = sender.String() + reverted, err := k.HandleEVMDeposit( + ctx, + cctx, + ) + require.ErrorIs(t, err, types.ErrInvalidReceiverAddress) + require.True(t, reverted) + }) + + t.Run("cannot process Zeta deposit if the receiver address is invalid zevm address for cointype erc20", func(t *testing.T) { + k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ + UseFungibleMock: true, + }) + + receiver := "invalid" + amount := big.NewInt(42) + sender := sample.EthAddress() + senderChainId := int64(0) + + // call HandleEVMDeposit + cctx := sample.CrossChainTx(t, "foo") + cctx.GetCurrentOutboundParam().Receiver = receiver + cctx.GetInboundParams().Amount = math.NewUintFromBigInt(amount) + cctx.GetInboundParams().CoinType = coin.CoinType_ERC20 + cctx.GetInboundParams().SenderChainId = senderChainId + cctx.InboundParams.Sender = sender.String() + reverted, err := k.HandleEVMDeposit( + ctx, + cctx, + ) + require.ErrorIs(t, err, types.ErrInvalidReceiverAddress) + require.True(t, reverted) + }) + t.Run("should return error with non-reverted if deposit Zeta fails", func(t *testing.T) { k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{ UseFungibleMock: true, @@ -522,7 +598,7 @@ func TestMsgServer_HandleEVMDeposit(t *testing.T) { ctx, cctx, ) - require.ErrorIs(t, err, types.ErrUnableToParseAddress) + require.ErrorIs(t, err, types.ErrInvalidReceiverAddress) }) t.Run("should deposit into address if address is parsed", func(t *testing.T) { diff --git a/x/crosschain/types/cctx.go b/x/crosschain/types/cctx.go index d7a77e259c..9f626b760e 100644 --- a/x/crosschain/types/cctx.go +++ b/x/crosschain/types/cctx.go @@ -9,7 +9,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ethcommon "github.com/ethereum/go-ethereum/common" - + "github.com/zeta-chain/zetacore/pkg/address" + "github.com/zeta-chain/zetacore/pkg/chains" + "github.com/zeta-chain/zetacore/pkg/coin" observertypes "github.com/zeta-chain/zetacore/x/observer/types" ) @@ -203,6 +205,42 @@ func (m CrossChainTx) SetOutboundBallotIndex(index string) { } // GetCctxIndexFromBytes returns the CCTX index from a byte array. +func (m CrossChainTx) GetValidReceiverAddress() (ethcommon.Address, error) { + inboundCoinType := m.InboundParams.CoinType + switch inboundCoinType { + // For coin type ZETA or ERC20 the receiver is added to the receiver field of the inbound vote so we can use it directly + case coin.CoinType_Zeta, coin.CoinType_ERC20: + { + err := address.ValidateEVMAddress(m.GetCurrentOutboundParam().Receiver) + if err != nil { + return ethcommon.Address{}, cosmoserrors.Wrap(ErrInvalidReceiverAddress, err.Error()) + } + return ethcommon.HexToAddress(m.GetCurrentOutboundParam().Receiver), nil + } + // For coin type gas we create the message by composing the receiver and the call data so we need to parse the address from the RelayedMessage + // The receiver in this case is set to me the sender of the message + // For cases where this is not the case we should try and use the receiver field of the inbound vote + case coin.CoinType_Gas: + { + to := m.GetCurrentOutboundParam().Receiver + parsedAddress, _, err := chains.ParseAddressAndData(m.RelayedMessage) + if err != nil { + return ethcommon.Address{}, cosmoserrors.Wrap(ErrInvalidReceiverAddress, err.Error()) + } + if parsedAddress != (ethcommon.Address{}) { + to = parsedAddress.String() + } + err = address.ValidateEVMAddress(to) + if err != nil { + return ethcommon.Address{}, cosmoserrors.Wrap(ErrInvalidReceiverAddress, err.Error()) + } + return ethcommon.HexToAddress(to), nil + } + default: + return ethcommon.Address{}, fmt.Errorf("invalid coin type %s", inboundCoinType) + } +} + func GetCctxIndexFromBytes(sendHash [32]byte) string { return fmt.Sprintf("0x%s", hex.EncodeToString(sendHash[:])) } diff --git a/x/crosschain/types/errors.go b/x/crosschain/types/errors.go index 40182fe313..d5ddd886d8 100644 --- a/x/crosschain/types/errors.go +++ b/x/crosschain/types/errors.go @@ -55,6 +55,8 @@ var ( 1156, "migration tx from an old tss address detected", ) + + ErrInvalidReceiverAddress = errorsmod.Register(ModuleName, 1160, "invalid receiver address") ErrValidatingInbound = errorsmod.Register(ModuleName, 1157, "unable to validate inbound") ErrInvalidGasLimit = errorsmod.Register(ModuleName, 1158, "invalid gas limit") ErrUnableToSetOutboundInfo = errorsmod.Register(ModuleName, 1159, "unable to set outbound info") diff --git a/x/crosschain/types/message_whitelist_erc20.go b/x/crosschain/types/message_whitelist_erc20.go index a9a396e05f..c1691657a4 100644 --- a/x/crosschain/types/message_whitelist_erc20.go +++ b/x/crosschain/types/message_whitelist_erc20.go @@ -4,7 +4,7 @@ import ( cosmoserrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/zeta-chain/zetacore/pkg/address" "github.com/zeta-chain/zetacore/x/fungible/types" ) @@ -54,8 +54,9 @@ func (msg *MsgWhitelistERC20) ValidateBasic() error { return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } // check if the system contract address is valid - if ethcommon.HexToAddress(msg.Erc20Address) == (ethcommon.Address{}) { - return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid ERC20 contract address (%s)", msg.Erc20Address) + err = address.ValidateEVMAddress(msg.Erc20Address) + if err != nil { + return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid erc20 address (%s): %s", msg.Erc20Address, err) } if msg.Decimals > 128 { return cosmoserrors.Wrapf(types.ErrInvalidDecimals, "invalid decimals (%d)", msg.Decimals) diff --git a/x/crosschain/types/validate.go b/x/crosschain/types/validate.go index 1f1dd3814d..a479aa648a 100644 --- a/x/crosschain/types/validate.go +++ b/x/crosschain/types/validate.go @@ -5,7 +5,6 @@ import ( "regexp" "cosmossdk.io/errors" - ethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/zeta-chain/zetacore/pkg/chains" @@ -46,33 +45,3 @@ func ValidateHashForChain(hash string, chainID int64) error { } return fmt.Errorf("invalid chain id %d", chainID) } - -// ValidateAddressForChain validates the address for the chain -// NOTE: since these checks are currently not used, we don't provide additional chains for simplicity -// TODO: use authorityKeeper.GetChainInfo to provide additional chains -// https://github.com/zeta-chain/node/issues/2234 -// https://github.com/zeta-chain/node/issues/2385 -// NOTE: We should eventually not using these hard-coded checks at all for same reasons as above -func ValidateAddressForChain(address string, chainID int64) error { - // we do not validate the address for zeta chain as the address field can be btc or eth address - if chains.IsZetaChain(chainID, []chains.Chain{}) { - return nil - } - if chains.IsEthereumChain(chainID, []chains.Chain{}) { - if !ethcommon.IsHexAddress(address) { - return fmt.Errorf("invalid address %s , chain %d", address, chainID) - } - return nil - } - if chains.IsBitcoinChain(chainID, []chains.Chain{}) { - addr, err := chains.DecodeBtcAddress(address, chainID) - if err != nil { - return fmt.Errorf("invalid address %s , chain %d: %s", address, chainID, err) - } - if !chains.IsBtcAddressSupported(addr) { - return fmt.Errorf("unsupported address %s", address) - } - return nil - } - return fmt.Errorf("invalid chain id %d", chainID) -} diff --git a/x/crosschain/types/validate_test.go b/x/crosschain/types/validate_test.go index 0ab3e20885..f31b2cd88b 100644 --- a/x/crosschain/types/validate_test.go +++ b/x/crosschain/types/validate_test.go @@ -10,57 +10,6 @@ import ( "github.com/zeta-chain/zetacore/x/crosschain/types" ) -func TestValidateAddressForChain(t *testing.T) { - // test for eth chain - require.Error(t, types.ValidateAddressForChain("0x123", chains.Goerli.ChainId)) - require.Error(t, types.ValidateAddressForChain("", chains.Goerli.ChainId)) - require.Error(t, types.ValidateAddressForChain("%%%%", chains.Goerli.ChainId)) - require.NoError( - t, - types.ValidateAddressForChain("0x792c127Fa3AC1D52F904056Baf1D9257391e7D78", chains.Goerli.ChainId), - ) - - // test for btc chain - require.NoError( - t, - types.ValidateAddressForChain( - "bc1p4scddlkkuw9486579autxumxmkvuphm5pz4jvf7f6pdh50p2uzqstawjt9", - chains.BitcoinMainnet.ChainId, - ), - ) - require.NoError( - t, - types.ValidateAddressForChain("327z4GyFM8Y8DiYfasGKQWhRK4MvyMSEgE", chains.BitcoinMainnet.ChainId), - ) - require.NoError( - t, - types.ValidateAddressForChain("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", chains.BitcoinMainnet.ChainId), - ) - require.Error( - t, - types.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chains.BitcoinMainnet.ChainId), - ) - require.Error(t, types.ValidateAddressForChain("", chains.BitcoinRegtest.ChainId)) - require.NoError( - t, - types.ValidateAddressForChain("bc1qysd4sp9q8my59ul9wsf5rvs9p387hf8vfwatzu", chains.BitcoinMainnet.ChainId), - ) - require.NoError( - t, - types.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chains.BitcoinRegtest.ChainId), - ) - - // test for zeta chain - require.NoError( - t, - types.ValidateAddressForChain("bcrt1qs758ursh4q9z627kt3pp5yysm78ddny6txaqgw", chains.ZetaChainMainnet.ChainId), - ) - require.NoError( - t, - types.ValidateAddressForChain("0x792c127Fa3AC1D52F904056Baf1D9257391e7D78", chains.ZetaChainMainnet.ChainId), - ) -} - func TestValidateCCTXIndex(t *testing.T) { require.NoError(t, types.ValidateCCTXIndex("0x84bd5c9922b63c52d8a9ca686e0a57ff978150b71be0583514d01c27aa341910")) require.NoError(t, types.ValidateCCTXIndex(sample.ZetaIndex(t))) diff --git a/x/fungible/types/message_update_system_contract.go b/x/fungible/types/message_update_system_contract.go index c613847bf6..b34d53feec 100644 --- a/x/fungible/types/message_update_system_contract.go +++ b/x/fungible/types/message_update_system_contract.go @@ -4,7 +4,7 @@ import ( cosmoserrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/zeta-chain/zetacore/pkg/address" ) const TypeMsgUpdateSystemContract = "update_system_contract" @@ -45,12 +45,9 @@ func (msg *MsgUpdateSystemContract) ValidateBasic() error { return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } // check if the system contract address is valid - if ethcommon.HexToAddress(msg.NewSystemContractAddress) == (ethcommon.Address{}) { - return cosmoserrors.Wrapf( - sdkerrors.ErrInvalidAddress, - "invalid system contract address (%s)", - msg.NewSystemContractAddress, - ) + err = address.ValidateEVMAddress(msg.NewSystemContractAddress) + if err != nil { + return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid system contract address (%s): %s", msg.NewSystemContractAddress, err) } return nil