From 3c993457b75433c2bdc88fa29240db9b604f01f2 Mon Sep 17 00:00:00 2001 From: Charlie Chen <34498985+ws4charlie@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:21:55 -0600 Subject: [PATCH] fix: fix Bitcoin incorrect 'origin' in zContext caused by revertAddress option (#3192) * fix incorrect zContext origin caused by the replacement of sender address on BTC inbound revert * add changelog entry * fix unit test failure * check btc sender address in the CCTX struct --- changelog.md | 1 + ...d_deposit_and_call_revert_other_address.go | 4 ++ testutil/sample/crosschain.go | 16 +++++++ x/crosschain/types/cctx.go | 14 +++++- x/crosschain/types/cctx_test.go | 35 ++++++++++++++ x/crosschain/types/revert_options.go | 14 ++++++ x/crosschain/types/revert_options_test.go | 46 ++++++++++++++++++- zetaclient/chains/bitcoin/observer/event.go | 12 ++--- .../chains/bitcoin/observer/event_test.go | 6 ++- 9 files changed, 138 insertions(+), 10 deletions(-) diff --git a/changelog.md b/changelog.md index 6992b9a706..4e145b2114 100644 --- a/changelog.md +++ b/changelog.md @@ -35,6 +35,7 @@ * [3155](https://github.com/zeta-chain/node/pull/3155) - fix potential panic in the Bitcoin inscription parsing * [3162](https://github.com/zeta-chain/node/pull/3162) - skip depositor fee calculation if transaction does not involve TSS address * [3179](https://github.com/zeta-chain/node/pull/3179) - support inbound trackers for v2 cctx +* [3192](https://github.com/zeta-chain/node/pull/3192) - fix incorrect zContext origin caused by the replacement of 'sender' with 'revertAddress' ## v21.0.0 diff --git a/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go index 8ecb3b0d5f..19e9893b95 100644 --- a/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go @@ -45,6 +45,10 @@ func TestBitcoinStdMemoDepositAndCallRevertOtherAddress(r *runner.E2ERunner, arg // Now we want to make sure revert TX is completed. cctx := utils.WaitCctxRevertedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient) + // Make sure inbound sender and revert address are correct + assert.Equal(r, cctx.InboundParams.Sender, r.BTCDeployerAddress.EncodeAddress()) + assert.Equal(r, cctx.GetCurrentOutboundParam().Receiver, revertAddress) + // Check revert tx receiver address and amount receiver, value := r.QueryOutboundReceiverAndAmount(cctx.OutboundParams[1].Hash) assert.Equal(r, revertAddress, receiver) diff --git a/testutil/sample/crosschain.go b/testutil/sample/crosschain.go index 25733e1fef..4ac29ec697 100644 --- a/testutil/sample/crosschain.go +++ b/testutil/sample/crosschain.go @@ -217,6 +217,22 @@ func CrossChainTx(t *testing.T, index string) *types.CrossChainTx { } } +func CrossChainTxV2(t *testing.T, index string) *types.CrossChainTx { + r := newRandFromStringSeed(t, index) + + return &types.CrossChainTx{ + Creator: AccAddress(), + Index: GetCctxIndexFromString(index), + ZetaFees: math.NewUint(uint64(r.Int63())), + RelayedMessage: StringRandom(r, 32), + CctxStatus: Status(t, index), + InboundParams: InboundParams(r), + OutboundParams: []*types.OutboundParams{OutboundParams(r), OutboundParams(r)}, + ProtocolContractVersion: types.ProtocolContractVersion_V2, + RevertOptions: types.NewEmptyRevertOptions(), + } +} + // CustomCctxsInBlockRange create 1 cctx per block in block range [lowBlock, highBlock] (inclusive) func CustomCctxsInBlockRange( t *testing.T, diff --git a/x/crosschain/types/cctx.go b/x/crosschain/types/cctx.go index 2b84b5f26c..b2354a79e1 100644 --- a/x/crosschain/types/cctx.go +++ b/x/crosschain/types/cctx.go @@ -10,6 +10,7 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/zeta-chain/node/pkg/chains" observertypes "github.com/zeta-chain/node/x/observer/types" ) @@ -114,10 +115,21 @@ func (m *CrossChainTx) AddRevertOutbound(gasLimit uint64) error { return fmt.Errorf("cannot revert before trying to process an outbound tx") } + // in protocol contract V1, developers can specify a revert address for Bitcoin chains + // TODO: remove this V1 logic after switching Bitcoin to V2 architecture + // https://github.com/zeta-chain/node/issues/2711 + revertReceiver := m.InboundParams.Sender + if m.ProtocolContractVersion == ProtocolContractVersion_V1 && + chains.IsBitcoinChain(m.InboundParams.SenderChainId, []chains.Chain{}) { + revertAddress, valid := m.RevertOptions.GetBTCRevertAddress(m.InboundParams.SenderChainId) + if valid { + revertReceiver = revertAddress + } + } + // in protocol contract V2, developers can specify a specific address to receive the revert // if not specified, the sender address is used // note: this option is current only support for EVM type chains - revertReceiver := m.InboundParams.Sender if m.ProtocolContractVersion == ProtocolContractVersion_V2 { revertAddress, valid := m.RevertOptions.GetEVMRevertAddress() if valid { diff --git a/x/crosschain/types/cctx_test.go b/x/crosschain/types/cctx_test.go index f49768f8a0..1369e2dee0 100644 --- a/x/crosschain/types/cctx_test.go +++ b/x/crosschain/types/cctx_test.go @@ -4,8 +4,10 @@ import ( "math/rand" "testing" + "github.com/btcsuite/btcd/chaincfg" "github.com/stretchr/testify/require" + "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/x/crosschain/types" ) @@ -134,6 +136,39 @@ func Test_SetRevertOutboundValues(t *testing.T) { require.Equal(t, types.TxFinalizationStatus_Executed, cctx.OutboundParams[0].TxFinalizationStatus) }) + t.Run("successfully set BTC revert address V1", func(t *testing.T) { + cctx := sample.CrossChainTx(t, "test") + cctx.InboundParams.SenderChainId = chains.BitcoinTestnet.ChainId + cctx.OutboundParams = cctx.OutboundParams[:1] + cctx.RevertOptions.RevertAddress = sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params) + + err := cctx.AddRevertOutbound(100) + require.NoError(t, err) + require.Len(t, cctx.OutboundParams, 2) + require.Equal(t, cctx.GetCurrentOutboundParam().Receiver, cctx.RevertOptions.RevertAddress) + require.Equal(t, cctx.GetCurrentOutboundParam().ReceiverChainId, cctx.InboundParams.SenderChainId) + require.Equal(t, cctx.GetCurrentOutboundParam().Amount, cctx.OutboundParams[0].Amount) + require.Equal(t, cctx.GetCurrentOutboundParam().CallOptions.GasLimit, uint64(100)) + require.Equal(t, cctx.GetCurrentOutboundParam().TssPubkey, cctx.OutboundParams[0].TssPubkey) + require.Equal(t, types.TxFinalizationStatus_Executed, cctx.OutboundParams[0].TxFinalizationStatus) + }) + + t.Run("successfully set EVM revert address V2", func(t *testing.T) { + cctx := sample.CrossChainTxV2(t, "test") + cctx.OutboundParams = cctx.OutboundParams[:1] + cctx.RevertOptions.RevertAddress = sample.EthAddress().Hex() + + err := cctx.AddRevertOutbound(100) + require.NoError(t, err) + require.Len(t, cctx.OutboundParams, 2) + require.Equal(t, cctx.GetCurrentOutboundParam().Receiver, cctx.RevertOptions.RevertAddress) + require.Equal(t, cctx.GetCurrentOutboundParam().ReceiverChainId, cctx.InboundParams.SenderChainId) + require.Equal(t, cctx.GetCurrentOutboundParam().Amount, cctx.OutboundParams[0].Amount) + require.Equal(t, cctx.GetCurrentOutboundParam().CallOptions.GasLimit, uint64(100)) + require.Equal(t, cctx.GetCurrentOutboundParam().TssPubkey, cctx.OutboundParams[0].TssPubkey) + require.Equal(t, types.TxFinalizationStatus_Executed, cctx.OutboundParams[0].TxFinalizationStatus) + }) + t.Run("failed to set revert outbound values if revert outbound already exists", func(t *testing.T) { cctx := sample.CrossChainTx(t, "test") err := cctx.AddRevertOutbound(100) diff --git a/x/crosschain/types/revert_options.go b/x/crosschain/types/revert_options.go index 6cdb3040bf..4893b19ad3 100644 --- a/x/crosschain/types/revert_options.go +++ b/x/crosschain/types/revert_options.go @@ -6,6 +6,7 @@ import ( "github.com/zeta-chain/protocol-contracts/v2/pkg/gatewayevm.sol" "github.com/zeta-chain/protocol-contracts/v2/pkg/gatewayzevm.sol" + "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/pkg/crypto" ) @@ -75,6 +76,19 @@ func (r RevertOptions) GetEVMRevertAddress() (ethcommon.Address, bool) { return addr, !crypto.IsEmptyAddress(addr) } +// GetBTCRevertAddress validates and returns the BTC revert address +func (r RevertOptions) GetBTCRevertAddress(chainID int64) (string, bool) { + btcAddress, err := chains.DecodeBtcAddress(r.RevertAddress, chainID) + if err != nil { + return "", false + } + if !chains.IsBtcAddressSupported(btcAddress) { + return "", false + } + + return btcAddress.EncodeAddress(), true +} + // GetEVMAbortAddress returns the EVM abort address // if the abort address is not a valid address, it returns false func (r RevertOptions) GetEVMAbortAddress() (ethcommon.Address, bool) { diff --git a/x/crosschain/types/revert_options_test.go b/x/crosschain/types/revert_options_test.go index 46981855ea..c91927dd86 100644 --- a/x/crosschain/types/revert_options_test.go +++ b/x/crosschain/types/revert_options_test.go @@ -1,11 +1,14 @@ package types_test import ( + "testing" + + "github.com/btcsuite/btcd/chaincfg" "github.com/stretchr/testify/require" + "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/pkg/constant" "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/x/crosschain/types" - "testing" ) func TestRevertOptions_GetEVMRevertAddress(t *testing.T) { @@ -44,6 +47,47 @@ func TestRevertOptions_GetEVMRevertAddress(t *testing.T) { }) } +func TestRevertOptions_GetBTCRevertAddress(t *testing.T) { + t.Run("valid Bitcoin revert address", func(t *testing.T) { + addr := sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params) + actualAddr, valid := types.RevertOptions{ + RevertAddress: addr, + }.GetBTCRevertAddress(chains.BitcoinTestnet.ChainId) + + require.True(t, valid) + require.Equal(t, addr, actualAddr) + }) + + t.Run("invalid Bitcoin revert address", func(t *testing.T) { + actualAddr, valid := types.RevertOptions{ + // it's a regnet address, not testnet + RevertAddress: "bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", + }.GetBTCRevertAddress(chains.BitcoinTestnet.ChainId) + + require.False(t, valid) + require.Empty(t, actualAddr) + }) + + t.Run("empty revert address", func(t *testing.T) { + actualAddr, valid := types.RevertOptions{ + RevertAddress: "", + }.GetBTCRevertAddress(chains.BitcoinTestnet.ChainId) + + require.False(t, valid) + require.Empty(t, actualAddr) + }) + + t.Run("unsupported Bitcoin revert address", func(t *testing.T) { + actualAddr, valid := types.RevertOptions{ + // address not supported + RevertAddress: "035e4ae279bd416b5da724972c9061ec6298dac020d1e3ca3f06eae715135cdbec", + }.GetBTCRevertAddress(chains.BitcoinTestnet.ChainId) + + require.False(t, valid) + require.Empty(t, actualAddr) + }) +} + func TestRevertOptions_GetEVMAbortAddress(t *testing.T) { t.Run("valid abort address", func(t *testing.T) { addr := sample.EthAddress() diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index 69657d29f1..7cd581a1cc 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -221,11 +221,10 @@ func (ob *Observer) NewInboundVoteFromStdMemo( event *BTCInboundEvent, amountSats *big.Int, ) *crosschaintypes.MsgVoteInbound { - // replace 'sender' with 'revertAddress' if specified in the memo, so that - // zetacore will refund to the address specified by the user in the revert options. - sender := event.FromAddress - if event.MemoStd.RevertOptions.RevertAddress != "" { - sender = event.MemoStd.RevertOptions.RevertAddress + // inject the 'revertAddress' specified in the memo, so that + // zetacore will create a revert outbound that points to the custom revert address. + revertOptions := crosschaintypes.RevertOptions{ + RevertAddress: event.MemoStd.RevertOptions.RevertAddress, } // make a legacy message so that zetacore can process it as V1 @@ -234,7 +233,7 @@ func (ob *Observer) NewInboundVoteFromStdMemo( return crosschaintypes.NewMsgVoteInbound( ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), - sender, + event.FromAddress, ob.Chain().ChainId, event.FromAddress, event.ToAddress, @@ -249,5 +248,6 @@ func (ob *Observer) NewInboundVoteFromStdMemo( 0, crosschaintypes.ProtocolContractVersion_V1, false, // not relevant for v1 + crosschaintypes.WithRevertOptions(revertOptions), ) } diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index 5ed8e9b103..80566b55cd 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -423,7 +423,7 @@ func Test_NewInboundVoteFromStdMemo(t *testing.T) { // expected vote memoBytesExpected := append(event.MemoStd.Receiver.Bytes(), event.MemoStd.Payload...) expectedVote := crosschaintypes.MsgVoteInbound{ - Sender: revertOptions.RevertAddress, // should be overridden by revert address + Sender: event.FromAddress, SenderChainId: chain.ChainId, TxOrigin: event.FromAddress, Receiver: event.ToAddress, @@ -437,7 +437,9 @@ func Test_NewInboundVoteFromStdMemo(t *testing.T) { }, CoinType: coin.CoinType_Gas, ProtocolContractVersion: crosschaintypes.ProtocolContractVersion_V1, - RevertOptions: crosschaintypes.NewEmptyRevertOptions(), // ignored by V1 + RevertOptions: crosschaintypes.RevertOptions{ + RevertAddress: revertOptions.RevertAddress, // should be overridden by revert address + }, } // create new inbound vote V1 with standard memo