Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix Bitcoin incorrect 'origin' in zContext caused by revertAddress option #3192

Merged
merged 6 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
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(),
}
}
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved

// 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)
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
if valid {
revertReceiver = revertAddress
}
}

ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
// 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) {
ws4charlie marked this conversation as resolved.
Show resolved Hide resolved
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),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use event.MemoStd.RevertOptions here instead of revertOptions which only contains RevertAddress and omitting CallOnRevert, AbortAddress, etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zetacored currently only takes the field of RevertAddress from RevertOptions struct in the V1 architecture. So zetaclientd screens out all noises except the RevertAddress before migration to V2 architecture. In V2, we'll open them up in both zetacored and zetaclientd.

)
}
Loading