From 525eedcf4f222ef1ee76047e758c35b4479a8066 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 14 Feb 2024 11:19:46 -0500 Subject: [PATCH 1/9] add error check for legacy tx --- common/address.go | 3 +++ common/address_test.go | 5 +++++ x/crosschain/keeper/evm_hooks.go | 21 ++++++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/common/address.go b/common/address.go index 77b4234fed..b3a432ea8c 100644 --- a/common/address.go +++ b/common/address.go @@ -72,6 +72,9 @@ func DecodeBtcAddress(inputAddress string, chainID int64) (address btcutil.Addre return nil, fmt.Errorf("chain params not found") } address, err = btcutil.DecodeAddress(inputAddress, chainParams) + if err != nil { + return nil, fmt.Errorf("decode address failed: %s", err.Error()) + } ok := address.IsForNet(chainParams) if !ok { return nil, fmt.Errorf("address is not for network %s", chainParams.Name) diff --git a/common/address_test.go b/common/address_test.go index af386c3bdc..732ba34cd6 100644 --- a/common/address_test.go +++ b/common/address_test.go @@ -34,6 +34,11 @@ func TestDecodeBtcAddress(t *testing.T) { _, err := DecodeBtcAddress("tb1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18332) require.ErrorContains(t, err, "runtime error: invalid memory address or nil pointer dereference") }) + t.Run("legacy address", func(t *testing.T) { + _, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", 18332) + require.ErrorContains(t, err, "decode address failed") + + }) t.Run("valid address", func(t *testing.T) { _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18444) require.NoError(t, err) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 5b896473a8..8623e0269d 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -72,6 +72,15 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr return err } } + // We were unable to parse the ZRC20 withdrawal event + if err != nil && eventWithdrawal == nil { + ctx.Logger().Error("Error parsing ZRC20 withdrawal event: %s", err.Error()) + } + // We were able to parse the ZRC20 withdrawal event, but we were unable to process it as the information was incorrect + if err != nil && eventWithdrawal != nil { + ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", + eventWithdrawal.From.Hex(), string(eventWithdrawal.To), eventWithdrawal.Value.String(), eventWithdrawal.Gasfee.String(), eventWithdrawal.ProtocolFlatFee.String(), err.Error())) + } eZeta, err := ParseZetaSentEvent(*log, connectorZEVMAddr) if err == nil { if err := k.ProcessZetaSentEvent(ctx, eZeta, emittingContract, txOrigin); err != nil { @@ -257,23 +266,25 @@ func (k Keeper) ParseZRC20WithdrawalEvent(ctx sdk.Context, log ethtypes.Log) (*z if err != nil { return nil, err } - + // The event was parsed; that means the user has deposited tokens to the contract. + // We still need to check if the information entered by the user is correct. + // If the information is not correct, the outbound cctx will not be created. coin, found := k.fungibleKeeper.GetForeignCoins(ctx, event.Raw.Address.Hex()) if !found { - return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: cannot find foreign coin with contract address %s", event.Raw.Address.Hex()) + return event, fmt.Errorf("ParseZRC20WithdrawalEvent: cannot find foreign coin with contract address %s", event.Raw.Address.Hex()) } chainID := coin.ForeignChainId if common.IsBitcoinChain(chainID) { if event.Value.Cmp(big.NewInt(0)) <= 0 { - return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid amount %s", event.Value.String()) + return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid amount %s", event.Value.String()) } addr, err := common.DecodeBtcAddress(string(event.To), chainID) if err != nil { - return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s: %s", event.To, err) + return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s: %s", event.To, err) } _, ok := addr.(*btcutil.AddressWitnessPubKeyHash) if !ok { - return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To) + return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To) } } return event, nil From 066bac3c4643167d3c9e17f968650bb23f2b660c Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 14 Feb 2024 11:20:35 -0500 Subject: [PATCH 2/9] add error check for legacy tx --- x/crosschain/keeper/evm_hooks.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 8623e0269d..9a2413c240 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -72,10 +72,6 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr return err } } - // We were unable to parse the ZRC20 withdrawal event - if err != nil && eventWithdrawal == nil { - ctx.Logger().Error("Error parsing ZRC20 withdrawal event: %s", err.Error()) - } // We were able to parse the ZRC20 withdrawal event, but we were unable to process it as the information was incorrect if err != nil && eventWithdrawal != nil { ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", From 53bf6a688dc8cfcd1268d811fb631f65998e4a1f Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 14 Feb 2024 18:12:53 -0500 Subject: [PATCH 3/9] add unit tests --- changelog.md | 1 + common/address_test.go | 34 +++++-- x/crosschain/keeper/evm_hooks.go | 16 ++- x/crosschain/keeper/evm_hooks_test.go | 140 ++++++++++++++++++++++++++ x/fungible/keeper/evm.go | 1 - 5 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 x/crosschain/keeper/evm_hooks_test.go diff --git a/changelog.md b/changelog.md index b0f89948f6..1f1806fb0a 100644 --- a/changelog.md +++ b/changelog.md @@ -33,6 +33,7 @@ * [1733](https://github.com/zeta-chain/node/pull/1733) - remove the unnecessary 2x multiplier in the convertGasToZeta RPC * [1721](https://github.com/zeta-chain/node/issues/1721) - zetaclient should provide bitcoin_chain_id when querying TSS address * [1744](https://github.com/zeta-chain/node/pull/1744) - added cmd to encrypt tss keyshare file, allowing empty tss password for backward compatibility. +* [1758](https://github.com/zeta-chain/node/pull/1758) - add error check for BTC decode function ### Tests diff --git a/common/address_test.go b/common/address_test.go index 732ba34cd6..d3a7d979e5 100644 --- a/common/address_test.go +++ b/common/address_test.go @@ -23,25 +23,41 @@ func TestAddress(t *testing.T) { func TestDecodeBtcAddress(t *testing.T) { t.Run("invalid string", func(t *testing.T) { - _, err := DecodeBtcAddress("�U�ڷ���i߭����꿚�l", 18332) + _, err := DecodeBtcAddress("�U�ڷ���i߭����꿚�l", BtcTestNetChain().ChainId) require.ErrorContains(t, err, "runtime error: index out of range") }) t.Run("invalid chain", func(t *testing.T) { _, err := DecodeBtcAddress("14CEjTd5ci3228J45GdnGeUKLSSeCWUQxK", 0) - require.ErrorContains(t, err, "is not a Bitcoin chain") + require.ErrorContains(t, err, "is not a bitcoin chain") }) - t.Run("nil pointer dereference", func(t *testing.T) { - _, err := DecodeBtcAddress("tb1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18332) - require.ErrorContains(t, err, "runtime error: invalid memory address or nil pointer dereference") + t.Run("invalid checksum", func(t *testing.T) { + _, err := DecodeBtcAddress("tb1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcTestNetChain().ChainId) + require.ErrorContains(t, err, "invalid checksum") }) - t.Run("legacy address", func(t *testing.T) { - _, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", 18332) + t.Run("valid legacy main-net address address incorrect params TestNet", func(t *testing.T) { + _, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcTestNetChain().ChainId) require.ErrorContains(t, err, "decode address failed") + }) + t.Run("valid legacy main-net address address incorrect params RegTestNet", func(t *testing.T) { + _, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcRegtestChain().ChainId) + require.ErrorContains(t, err, "decode address failed") + }) + t.Run("valid legacy main-net address address correct params", func(t *testing.T) { + _, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcMainnetChain().ChainId) + require.NoError(t, err) }) - t.Run("valid address", func(t *testing.T) { - _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18444) + t.Run("valid legacy testnet address with correct params", func(t *testing.T) { + _, err := DecodeBtcAddress("n2TCLD16i8SNjwPCcgGBkTEeG6CQAcYTN1", BtcTestNetChain().ChainId) require.NoError(t, err) }) + t.Run("non legacy valid address with incorrect params", func(t *testing.T) { + _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcMainnetChain().ChainId) + require.ErrorContains(t, err, "address is not for network main-net") + }) + t.Run("non legacy valid address with correct params", func(t *testing.T) { + _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcTestNetChain().ChainId) + require.NoError(t, err) + }) } diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 9a2413c240..176b074f70 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -72,17 +72,25 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr return err } } - // We were able to parse the ZRC20 withdrawal event, but we were unable to process it as the information was incorrect + // We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect + // This means that there are some funds locked in the contract which cannot have a outbound , this can be a candidate for a refund + // TODO : Consider returning error or auto refunding the funds to the user if err != nil && eventWithdrawal != nil { ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", eventWithdrawal.From.Hex(), string(eventWithdrawal.To), eventWithdrawal.Value.String(), eventWithdrawal.Gasfee.String(), eventWithdrawal.ProtocolFlatFee.String(), err.Error())) } + eZeta, err := ParseZetaSentEvent(*log, connectorZEVMAddr) if err == nil { if err := k.ProcessZetaSentEvent(ctx, eZeta, emittingContract, txOrigin); err != nil { return err } } + // We were able to parse the ZetaSent event. However, we were unable to process it as the information was incorrect + if err != nil && eZeta != nil { + ctx.Logger().Error(fmt.Sprintf("Error processing Zeta Sent event , from Address: %s ,to : %s,value %s, err %s", + eZeta.Raw.Address.Hex(), string(eZeta.DestinationAddress), eZeta.ZetaValueAndGas.String(), err.Error())) + } } return nil } @@ -93,7 +101,7 @@ func (k Keeper) ProcessZRC20WithdrawalEvent(ctx sdk.Context, event *zrc20.ZRC20W if !k.zetaObserverKeeper.IsInboundEnabled(ctx) { return types.ErrNotEnoughPermissions } - ctx.Logger().Info("ZRC20 withdrawal to %s amount %d\n", hex.EncodeToString(event.To), event.Value) + ctx.Logger().Info(fmt.Sprintf("ZRC20 withdrawal to %s amount %d", hex.EncodeToString(event.To), event.Value)) tss, found := k.zetaObserverKeeper.GetTSS(ctx) if !found { return errorsmod.Wrap(types.ErrCannotFindTSSKeys, "ProcessZRC20WithdrawalEvent: cannot be processed without TSS keys") @@ -142,7 +150,6 @@ func (k Keeper) ProcessZRC20WithdrawalEvent(ctx sdk.Context, event *zrc20.ZRC20W // Get gas price and amount gasprice, found := k.GetGasPrice(ctx, receiverChain.ChainId) if !found { - fmt.Printf("gasprice not found for %s\n", receiverChain) return fmt.Errorf("gasprice not found for %s", receiverChain) } cctx.GetCurrentOutTxParam().OutboundTxGasPrice = fmt.Sprintf("%d", gasprice.Prices[gasprice.MedianIndex]) @@ -282,6 +289,7 @@ func (k Keeper) ParseZRC20WithdrawalEvent(ctx sdk.Context, log ethtypes.Log) (*z if !ok { return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To) } + } return event, nil } @@ -300,7 +308,7 @@ func ParseZetaSentEvent(log ethtypes.Log, connectorZEVM ethcommon.Address) (*con } if event.Raw.Address != connectorZEVM { - return nil, fmt.Errorf("ParseZetaSentEvent: event address %s does not match connectorZEVM %s", event.Raw.Address.Hex(), connectorZEVM.Hex()) + return event, fmt.Errorf("ParseZetaSentEvent: event address %s does not match connectorZEVM %s", event.Raw.Address.Hex(), connectorZEVM.Hex()) } return event, nil } diff --git a/x/crosschain/keeper/evm_hooks_test.go b/x/crosschain/keeper/evm_hooks_test.go new file mode 100644 index 0000000000..32f310fc72 --- /dev/null +++ b/x/crosschain/keeper/evm_hooks_test.go @@ -0,0 +1,140 @@ +package keeper_test + +import ( + "encoding/json" + "testing" + + ethtypes "github.com/ethereum/go-ethereum/core/types" + "github.com/stretchr/testify/require" + zetacommon "github.com/zeta-chain/zetacore/common" + testkeeper "github.com/zeta-chain/zetacore/testutil/keeper" + fungibletypes "github.com/zeta-chain/zetacore/x/fungible/types" +) + +func TestTransactionReceipt(t *testing.T) { + t.Run("should not process transaction for legacy address with correct chain params", func(t *testing.T) { + k, ctx, _, zk := testkeeper.CrosschainKeeper(t) + _ = k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName) + + // load receipt + receipt := ethtypes.Receipt{} + err := json.Unmarshal([]byte(GetSampleBlockLegacyToAddress()), &receipt) + require.NoError(t, err) + + // set foreign coins for the contract + zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{ + Zrc20ContractAddress: receipt.Logs[0].Address.String(), + ForeignChainId: zetacommon.BtcMainnetChain().ChainId, + }) + // logs 1, 2, 3 are not valid + // log 4 is valid , but the to address is not valid + for i, log := range receipt.Logs { + eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) + if i < 3 { + require.Error(t, err) + require.Nil(t, eventWithdrawal) + continue + } + require.ErrorContains(t, err, "not P2WPKH address") + require.NotNil(t, eventWithdrawal) + require.Equal(t, "1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", string(eventWithdrawal.To)) + } + }) + t.Run("should not process transaction for legacy mainnet address with Testnet params ,", func(t *testing.T) { + k, ctx, _, zk := testkeeper.CrosschainKeeper(t) + _ = k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName) + + // load receipt + receipt := ethtypes.Receipt{} + err := json.Unmarshal([]byte(GetSampleBlockLegacyToAddress()), &receipt) + require.NoError(t, err) + + // set foreign coins for the contract + zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{ + Zrc20ContractAddress: receipt.Logs[0].Address.String(), + ForeignChainId: zetacommon.BtcTestNetChain().ChainId, + }) + // logs 1, 2, 3 are not valid + // log 4 is valid , but the to address is not valid + for i, log := range receipt.Logs { + eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) + if i < 3 { + require.Error(t, err) + require.Nil(t, eventWithdrawal) + continue + } + require.ErrorContains(t, err, "decode address failed") + require.NotNil(t, eventWithdrawal) + require.Equal(t, "1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", string(eventWithdrawal.To)) + } + }) + + t.Run("should not process transaction for legacy mainnet address with RegTest params ,", func(t *testing.T) { + k, ctx, _, zk := testkeeper.CrosschainKeeper(t) + _ = k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName) + + // load receipt + receipt := ethtypes.Receipt{} + err := json.Unmarshal([]byte(GetSampleBlockLegacyToAddress()), &receipt) + require.NoError(t, err) + + // set foreign coins for the contract + zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{ + Zrc20ContractAddress: receipt.Logs[0].Address.String(), + ForeignChainId: zetacommon.BtcRegtestChain().ChainId, + }) + // logs 1, 2, 3 are not valid + // log 4 is valid , but the to address is not valid + for i, log := range receipt.Logs { + eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) + if i < 3 { + require.Error(t, err) + require.Nil(t, eventWithdrawal) + continue + } + require.ErrorContains(t, err, "decode address failed") + require.NotNil(t, eventWithdrawal) + require.Equal(t, "1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", string(eventWithdrawal.To)) + } + }) + + t.Run("should process valid tx", func(t *testing.T) { + k, ctx, _, zk := testkeeper.CrosschainKeeper(t) + _ = k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName) + + // load receipt + receipt := ethtypes.Receipt{} + err := json.Unmarshal([]byte(GetValidSampleBlock()), &receipt) + require.NoError(t, err) + + // set foreign coins for the contract + zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{ + Zrc20ContractAddress: receipt.Logs[0].Address.String(), + ForeignChainId: zetacommon.BtcMainnetChain().ChainId, + }) + // logs 1, 2, 3 are not valid + // log 4 is valid + for i, log := range receipt.Logs { + eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) + if i < 3 { + require.Error(t, err) + require.Nil(t, eventWithdrawal) + continue + } + + require.NoError(t, err) + require.NotNil(t, eventWithdrawal) + require.Equal(t, "bc1qysd4sp9q8my59ul9wsf5rvs9p387hf8vfwatzu", string(eventWithdrawal.To)) + } + }) +} + +// receiver is 1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3 +func GetSampleBlockLegacyToAddress() string { + return "{\n \"type\": \"0x2\",\n \"root\": \"0x\",\n \"status\": \"0x1\",\n \"cumulativeGasUsed\": \"0x4e7a38\",\n \"logsBloom\": \"0x00000000000000000000010000020000000000000000000000000000000000020000000100000000000000000000000080000000000000000000000400200000200000000002000000000008000000000000000000000000000000000000000000000000020000000000000000800800000040000000000000000010000000000000000000000000000000000000000000000000000004000000000000000000020000000000000000000000000000000000000000000000000000000000010000000002000000000000000000000000000000000000000000000000000020000010000000000000000001000000000000000000040200000000000000000000\",\n \"logs\": [\n {\n \"address\": \"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\n \"topics\": [\n \"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",\n \"0x000000000000000000000000313e74f7755afbae4f90e02ca49f8f09ff934a37\",\n \"0x000000000000000000000000735b14bb79463307aacbed86daf3322b1e6226ab\"\n ],\n \"data\": \"0x0000000000000000000000000000000000000000000000000000000000003790\",\n \"blockNumber\": \"0x1a2ad3\",\n \"transactionHash\": \"0x81126c18c7ca7d1fb7ded6644a87802e91bf52154ee4af7a5b379354e24fb6e0\",\n \"transactionIndex\": \"0x10\",\n \"blockHash\": \"0x5cb338544f64a226f4bfccb7a8d977f861c13ad73f7dd4317b66b00dd95de51c\",\n \"logIndex\": \"0x46\",\n \"removed\": false\n },\n {\n \"address\": \"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\n \"topics\": [\n \"0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925\",\n \"0x000000000000000000000000313e74f7755afbae4f90e02ca49f8f09ff934a37\",\n \"0x00000000000000000000000013a0c5930c028511dc02665e7285134b6d11a5f4\"\n ],\n \"data\": \"0x00000000000000000000000000000000000000000000000000000000006a1217\",\n \"blockNumber\": \"0x1a2ad3\",\n \"transactionHash\": \"0x81126c18c7ca7d1fb7ded6644a87802e91bf52154ee4af7a5b379354e24fb6e0\",\n \"transactionIndex\": \"0x10\",\n \"blockHash\": \"0x5cb338544f64a226f4bfccb7a8d977f861c13ad73f7dd4317b66b00dd95de51c\",\n \"logIndex\": \"0x47\",\n \"removed\": false\n },\n {\n \"address\": \"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\n \"topics\": [\n \"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",\n \"0x000000000000000000000000313e74f7755afbae4f90e02ca49f8f09ff934a37\",\n \"0x0000000000000000000000000000000000000000000000000000000000000000\"\n ],\n \"data\": \"0x00000000000000000000000000000000000000000000000000000000006a0c70\",\n \"blockNumber\": \"0x1a2ad3\",\n \"transactionHash\": \"0x81126c18c7ca7d1fb7ded6644a87802e91bf52154ee4af7a5b379354e24fb6e0\",\n \"transactionIndex\": \"0x10\",\n \"blockHash\": \"0x5cb338544f64a226f4bfccb7a8d977f861c13ad73f7dd4317b66b00dd95de51c\",\n \"logIndex\": \"0x48\",\n \"removed\": false\n },\n {\n \"address\": \"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\n \"topics\": [\n \"0x9ffbffc04a397460ee1dbe8c9503e098090567d6b7f4b3c02a8617d800b6d955\",\n \"0x000000000000000000000000313e74f7755afbae4f90e02ca49f8f09ff934a37\"\n ],\n \"data\": \"0x000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000006a0c700000000000000000000000000000000000000000000000000000000000003790000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000223145595676584c7573437874567545776f59765752794e35455a5458775056766f33000000000000000000000000000000000000000000000000000000000000\",\n \"blockNumber\": \"0x1a2ad3\",\n \"transactionHash\": \"0x81126c18c7ca7d1fb7ded6644a87802e91bf52154ee4af7a5b379354e24fb6e0\",\n \"transactionIndex\": \"0x10\",\n \"blockHash\": \"0x5cb338544f64a226f4bfccb7a8d977f861c13ad73f7dd4317b66b00dd95de51c\",\n \"logIndex\": \"0x49\",\n \"removed\": false\n }\n ],\n \"transactionHash\": \"0x81126c18c7ca7d1fb7ded6644a87802e91bf52154ee4af7a5b379354e24fb6e0\",\n \"contractAddress\": \"0x0000000000000000000000000000000000000000\",\n \"gasUsed\": \"0x12521\",\n \"blockHash\": \"0x5cb338544f64a226f4bfccb7a8d977f861c13ad73f7dd4317b66b00dd95de51c\",\n \"blockNumber\": \"0x1a2ad3\",\n \"transactionIndex\": \"0x10\"\n}\n" +} + +// receiver is bc1qysd4sp9q8my59ul9wsf5rvs9p387hf8vfwatzu +func GetValidSampleBlock() string { + return "{\"type\":\"0x2\",\"root\":\"0x\",\"status\":\"0x1\",\"cumulativeGasUsed\":\"0x1f25ed\",\"logsBloom\":\"0x00000000000000000000000000020000000000000000000000000000000000020000000100000000000000000040000080000000000000000000000400200000200000000002000000000008000000000000000000000000000000000000000000000000020000000000000000800800000000000000000000000010000000000000000000000000000000000000000000000000000004000000000000000000020000000001000000000000000000000000000000000000000000000000010000000002000000000000000010000000000000000000000000000000000020000010000000000000000000000000000000000000040200000000000000000000\",\"logs\":[{\"address\":\"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",\"0x00000000000000000000000033ead83db0d0c682b05ead61e8d8f481bb1b4933\",\"0x000000000000000000000000735b14bb79463307aacbed86daf3322b1e6226ab\"],\"data\":\"0x0000000000000000000000000000000000000000000000000000000000003d84\",\"blockNumber\":\"0x1a00f3\",\"transactionHash\":\"0x9aaefece38fd2bd87077038a63fffb7c84cc8dd1ed01de134a8504a1f9a410c3\",\"transactionIndex\":\"0x8\",\"blockHash\":\"0x9517356f0b3877990590421266f02a4ff349b7476010ee34dd5f0dfc85c2684f\",\"logIndex\":\"0x28\",\"removed\":false},{\"address\":\"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\"topics\":[\"0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925\",\"0x00000000000000000000000033ead83db0d0c682b05ead61e8d8f481bb1b4933\",\"0x00000000000000000000000013a0c5930c028511dc02665e7285134b6d11a5f4\"],\"data\":\"0x0000000000000000000000000000000000000000000000000000000000978c98\",\"blockNumber\":\"0x1a00f3\",\"transactionHash\":\"0x9aaefece38fd2bd87077038a63fffb7c84cc8dd1ed01de134a8504a1f9a410c3\",\"transactionIndex\":\"0x8\",\"blockHash\":\"0x9517356f0b3877990590421266f02a4ff349b7476010ee34dd5f0dfc85c2684f\",\"logIndex\":\"0x29\",\"removed\":false},{\"address\":\"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",\"0x00000000000000000000000033ead83db0d0c682b05ead61e8d8f481bb1b4933\",\"0x0000000000000000000000000000000000000000000000000000000000000000\"],\"data\":\"0x0000000000000000000000000000000000000000000000000000000000003039\",\"blockNumber\":\"0x1a00f3\",\"transactionHash\":\"0x9aaefece38fd2bd87077038a63fffb7c84cc8dd1ed01de134a8504a1f9a410c3\",\"transactionIndex\":\"0x8\",\"blockHash\":\"0x9517356f0b3877990590421266f02a4ff349b7476010ee34dd5f0dfc85c2684f\",\"logIndex\":\"0x2a\",\"removed\":false},{\"address\":\"0x13a0c5930c028511dc02665e7285134b6d11a5f4\",\"topics\":[\"0x9ffbffc04a397460ee1dbe8c9503e098090567d6b7f4b3c02a8617d800b6d955\",\"0x00000000000000000000000033ead83db0d0c682b05ead61e8d8f481bb1b4933\"],\"data\":\"0x000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000030390000000000000000000000000000000000000000000000000000000000003d840000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002a626331717973643473703971386d793539756c3977736635727673397033383768663876667761747a7500000000000000000000000000000000000000000000\",\"blockNumber\":\"0x1a00f3\",\"transactionHash\":\"0x9aaefece38fd2bd87077038a63fffb7c84cc8dd1ed01de134a8504a1f9a410c3\",\"transactionIndex\":\"0x8\",\"blockHash\":\"0x9517356f0b3877990590421266f02a4ff349b7476010ee34dd5f0dfc85c2684f\",\"logIndex\":\"0x2b\",\"removed\":false}],\"transactionHash\":\"0x9aaefece38fd2bd87077038a63fffb7c84cc8dd1ed01de134a8504a1f9a410c3\",\"contractAddress\":\"0x0000000000000000000000000000000000000000\",\"gasUsed\":\"0x12575\",\"blockHash\":\"0x9517356f0b3877990590421266f02a4ff349b7476010ee34dd5f0dfc85c2684f\",\"blockNumber\":\"0x1a00f3\",\"transactionIndex\":\"0x8\"}\n" +} diff --git a/x/fungible/keeper/evm.go b/x/fungible/keeper/evm.go index 0555c0cb95..53fe3c9859 100644 --- a/x/fungible/keeper/evm.go +++ b/x/fungible/keeper/evm.go @@ -583,7 +583,6 @@ func (k Keeper) CallEVM( if ok { errMes = fmt.Sprintf("%s, reason: %v", errMes, revertErr.ErrorData()) } - return resp, cosmoserrors.Wrapf(err, errMes) } return resp, nil From e571315410970eec37057da440609566b031366f Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 14 Feb 2024 18:28:46 -0500 Subject: [PATCH 4/9] fix unit tests --- common/address_test.go | 4 ++-- common/chain.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/address_test.go b/common/address_test.go index d3a7d979e5..d39945935c 100644 --- a/common/address_test.go +++ b/common/address_test.go @@ -54,10 +54,10 @@ func TestDecodeBtcAddress(t *testing.T) { t.Run("non legacy valid address with incorrect params", func(t *testing.T) { _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcMainnetChain().ChainId) - require.ErrorContains(t, err, "address is not for network main-net") + require.ErrorContains(t, err, "address is not for network mainnet") }) t.Run("non legacy valid address with correct params", func(t *testing.T) { - _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcTestNetChain().ChainId) + _, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcRegtestChain().ChainId) require.NoError(t, err) }) } diff --git a/common/chain.go b/common/chain.go index d048a251b1..5254f150f6 100644 --- a/common/chain.go +++ b/common/chain.go @@ -191,7 +191,7 @@ func GetBTCChainParams(chainID int64) (*chaincfg.Params, error) { case 8332: return &chaincfg.MainNetParams, nil default: - return nil, fmt.Errorf("error chainID %d is not a Bitcoin chain", chainID) + return nil, fmt.Errorf("error chainID %d is not a bitcoin chain", chainID) } } @@ -204,7 +204,7 @@ func GetBTCChainIDFromChainParams(params *chaincfg.Params) (int64, error) { case chaincfg.MainNetParams.Name: return 8332, nil default: - return 0, fmt.Errorf("error chain %s is not a Bitcoin chain", params.Name) + return 0, fmt.Errorf("error chain %s is not a bitcoin chain", params.Name) } } From 3133a5ec70339ebb091dfa0f075e5c2061a21292 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 14 Feb 2024 18:35:17 -0500 Subject: [PATCH 5/9] add todo for unit tests --- x/crosschain/keeper/evm_hooks.go | 1 + x/crosschain/keeper/evm_hooks_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 176b074f70..8e63a0ddb1 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -55,6 +55,7 @@ func (k Keeper) PostTxProcessing( // ProcessLogs post-processes logs emitted by a zEVM contract; if the log contains Withdrawal event // from registered ZRC20 contract, new CCTX will be created to trigger and track outbound // transaction. +// TODO : https://github.com/zeta-chain/node/issues/1759 func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContract ethcommon.Address, txOrigin string) error { system, found := k.fungibleKeeper.GetSystemContract(ctx) if !found { diff --git a/x/crosschain/keeper/evm_hooks_test.go b/x/crosschain/keeper/evm_hooks_test.go index 32f310fc72..6a56d39ed8 100644 --- a/x/crosschain/keeper/evm_hooks_test.go +++ b/x/crosschain/keeper/evm_hooks_test.go @@ -55,7 +55,7 @@ func TestTransactionReceipt(t *testing.T) { ForeignChainId: zetacommon.BtcTestNetChain().ChainId, }) // logs 1, 2, 3 are not valid - // log 4 is valid , but the to address is not valid + // log 4 is valid , but the `to` address is not valid for i, log := range receipt.Logs { eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) if i < 3 { @@ -84,7 +84,7 @@ func TestTransactionReceipt(t *testing.T) { ForeignChainId: zetacommon.BtcRegtestChain().ChainId, }) // logs 1, 2, 3 are not valid - // log 4 is valid , but the to address is not valid + // log 4 is valid , but the `to` address is not valid for i, log := range receipt.Logs { eventWithdrawal, err := k.ParseZRC20WithdrawalEvent(ctx, *log) if i < 3 { From e192f0db1d33fdf82c1ea7ed15f27d94506e9f23 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 15 Feb 2024 01:13:16 -0500 Subject: [PATCH 6/9] add comments --- x/crosschain/keeper/evm_hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 8e63a0ddb1..ec52189a46 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -74,7 +74,7 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr } } // We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect - // This means that there are some funds locked in the contract which cannot have a outbound , this can be a candidate for a refund + // This means that there are some funds locked in the contract which cannot have an outbound , this can be a candidate for a refund // TODO : Consider returning error or auto refunding the funds to the user if err != nil && eventWithdrawal != nil { ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", From c13db31c9b7fcfe482c5b1fa80f3882c07c0fd4b Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 15 Feb 2024 11:41:56 -0500 Subject: [PATCH 7/9] Update x/crosschain/keeper/evm_hooks.go Co-authored-by: Lucas Bertrand --- x/crosschain/keeper/evm_hooks.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index ec52189a46..625a506e64 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -55,7 +55,8 @@ func (k Keeper) PostTxProcessing( // ProcessLogs post-processes logs emitted by a zEVM contract; if the log contains Withdrawal event // from registered ZRC20 contract, new CCTX will be created to trigger and track outbound // transaction. -// TODO : https://github.com/zeta-chain/node/issues/1759 +// TODO: implement unit tests +// https://github.com/zeta-chain/node/issues/1759 func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContract ethcommon.Address, txOrigin string) error { system, found := k.fungibleKeeper.GetSystemContract(ctx) if !found { From 5e5e25dc7bdcf78eee2a5ea5fe09160251a0188e Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 15 Feb 2024 11:44:47 -0500 Subject: [PATCH 8/9] change error checks --- x/crosschain/keeper/evm_hooks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 625a506e64..02ed057752 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -77,7 +77,7 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr // We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect // This means that there are some funds locked in the contract which cannot have an outbound , this can be a candidate for a refund // TODO : Consider returning error or auto refunding the funds to the user - if err != nil && eventWithdrawal != nil { + if eventWithdrawal != nil { ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", eventWithdrawal.From.Hex(), string(eventWithdrawal.To), eventWithdrawal.Value.String(), eventWithdrawal.Gasfee.String(), eventWithdrawal.ProtocolFlatFee.String(), err.Error())) } @@ -89,7 +89,7 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr } } // We were able to parse the ZetaSent event. However, we were unable to process it as the information was incorrect - if err != nil && eZeta != nil { + if eZeta != nil { ctx.Logger().Error(fmt.Sprintf("Error processing Zeta Sent event , from Address: %s ,to : %s,value %s, err %s", eZeta.Raw.Address.Hex(), string(eZeta.DestinationAddress), eZeta.ZetaValueAndGas.String(), err.Error())) } From aaea061766d4b155b32783fcb417ae9388328e95 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 15 Feb 2024 14:12:33 -0500 Subject: [PATCH 9/9] resolve comments --- common/address.go | 2 +- x/crosschain/keeper/evm_hooks.go | 16 +++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/common/address.go b/common/address.go index b3a432ea8c..c79a4c5c9f 100644 --- a/common/address.go +++ b/common/address.go @@ -73,7 +73,7 @@ func DecodeBtcAddress(inputAddress string, chainID int64) (address btcutil.Addre } address, err = btcutil.DecodeAddress(inputAddress, chainParams) if err != nil { - return nil, fmt.Errorf("decode address failed: %s", err.Error()) + return nil, fmt.Errorf("decode address failed: %s , for input address %s", err.Error(), inputAddress) } ok := address.IsForNet(chainParams) if !ok { diff --git a/x/crosschain/keeper/evm_hooks.go b/x/crosschain/keeper/evm_hooks.go index 02ed057752..6294a3ade4 100644 --- a/x/crosschain/keeper/evm_hooks.go +++ b/x/crosschain/keeper/evm_hooks.go @@ -73,11 +73,10 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr if err := k.ProcessZRC20WithdrawalEvent(ctx, eventWithdrawal, emittingContract, txOrigin); err != nil { return err } - } - // We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect - // This means that there are some funds locked in the contract which cannot have an outbound , this can be a candidate for a refund - // TODO : Consider returning error or auto refunding the funds to the user - if eventWithdrawal != nil { + // We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect + // This means that there are some funds locked in the contract which cannot have an outbound , this can be a candidate for a refund + // TODO : Consider returning error or auto refunding the funds to the user + } else if eventWithdrawal != nil { ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s", eventWithdrawal.From.Hex(), string(eventWithdrawal.To), eventWithdrawal.Value.String(), eventWithdrawal.Gasfee.String(), eventWithdrawal.ProtocolFlatFee.String(), err.Error())) } @@ -88,11 +87,6 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr return err } } - // We were able to parse the ZetaSent event. However, we were unable to process it as the information was incorrect - if eZeta != nil { - ctx.Logger().Error(fmt.Sprintf("Error processing Zeta Sent event , from Address: %s ,to : %s,value %s, err %s", - eZeta.Raw.Address.Hex(), string(eZeta.DestinationAddress), eZeta.ZetaValueAndGas.String(), err.Error())) - } } return nil } @@ -310,7 +304,7 @@ func ParseZetaSentEvent(log ethtypes.Log, connectorZEVM ethcommon.Address) (*con } if event.Raw.Address != connectorZEVM { - return event, fmt.Errorf("ParseZetaSentEvent: event address %s does not match connectorZEVM %s", event.Raw.Address.Hex(), connectorZEVM.Hex()) + return nil, fmt.Errorf("ParseZetaSentEvent: event address %s does not match connectorZEVM %s", event.Raw.Address.Hex(), connectorZEVM.Hex()) } return event, nil }