Skip to content

Commit

Permalink
fix: fix Bitcoin incorrect 'origin' in zContext caused by revertAddre…
Browse files Browse the repository at this point in the history
…ss 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
  • Loading branch information
ws4charlie authored Nov 21, 2024
1 parent 495235e commit 4927f90
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 10 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions testutil/sample/crosschain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 13 additions & 1 deletion x/crosschain/types/cctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 35 additions & 0 deletions x/crosschain/types/cctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions x/crosschain/types/revert_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 45 additions & 1 deletion x/crosschain/types/revert_options_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions zetaclient/chains/bitcoin/observer/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -249,5 +248,6 @@ func (ob *Observer) NewInboundVoteFromStdMemo(
0,
crosschaintypes.ProtocolContractVersion_V1,
false, // not relevant for v1
crosschaintypes.WithRevertOptions(revertOptions),
)
}
6 changes: 4 additions & 2 deletions zetaclient/chains/bitcoin/observer/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down

0 comments on commit 4927f90

Please sign in to comment.