From 53f1bad523e77035f8aa5d9a4f3a6cb9df4291e7 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Thu, 17 Oct 2024 17:04:14 -0500 Subject: [PATCH 01/10] initial commit of btc revert address --- cmd/zetae2e/local/local.go | 2 +- e2e/e2etests/e2etests.go | 31 ++- ...> test_bitcoin_deposit_and_call_revert.go} | 8 +- x/crosschain/types/message_vote_inbound.go | 7 + .../types/message_vote_inbound_test.go | 39 +++ zetaclient/chains/bitcoin/observer/inbound.go | 241 ++++++++++++++---- zetaclient/chains/evm/observer/v2_inbound.go | 4 +- zetaclient/zetacore/constant.go | 3 + 8 files changed, 268 insertions(+), 67 deletions(-) rename e2e/e2etests/{test_bitcoin_deposit_refund.go => test_bitcoin_deposit_and_call_revert.go} (84%) diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index de46edf450..c62b4eb5c0 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -300,7 +300,7 @@ func localE2ETest(cmd *cobra.Command, _ []string) { bitcoinTests := []string{ e2etests.TestBitcoinDepositName, e2etests.TestBitcoinDepositAndCallName, - e2etests.TestBitcoinDepositRefundName, + e2etests.TestBitcoinDepositAndCallRevertName, e2etests.TestBitcoinWithdrawSegWitName, e2etests.TestBitcoinWithdrawInvalidAddressName, e2etests.TestZetaWithdrawBTCRevertName, diff --git a/e2e/e2etests/e2etests.go b/e2e/e2etests/e2etests.go index fe6428385d..d737dc4ffa 100644 --- a/e2e/e2etests/e2etests.go +++ b/e2e/e2etests/e2etests.go @@ -72,18 +72,21 @@ const ( Bitcoin tests Test transfer of Bitcoin asset across chains */ - TestBitcoinDepositName = "bitcoin_deposit" - TestBitcoinDepositRefundName = "bitcoin_deposit_refund" - TestBitcoinDepositAndCallName = "bitcoin_deposit_and_call" - TestBitcoinWithdrawSegWitName = "bitcoin_withdraw_segwit" - TestBitcoinWithdrawTaprootName = "bitcoin_withdraw_taproot" - TestBitcoinWithdrawMultipleName = "bitcoin_withdraw_multiple" - TestBitcoinWithdrawLegacyName = "bitcoin_withdraw_legacy" - TestBitcoinWithdrawP2WSHName = "bitcoin_withdraw_p2wsh" - TestBitcoinWithdrawP2SHName = "bitcoin_withdraw_p2sh" - TestBitcoinWithdrawInvalidAddressName = "bitcoin_withdraw_invalid" - TestBitcoinWithdrawRestrictedName = "bitcoin_withdraw_restricted" - TestExtractBitcoinInscriptionMemoName = "bitcoin_memo_from_inscription" + TestBitcoinDepositName = "bitcoin_deposit" + TestBitcoinDepositAndCallName = "bitcoin_deposit_and_call" + TestBitcoinDepositAndCallRevertName = "bitcoin_deposit_and_call_revert" + TestStdBitcoinDepositName = "std_bitcoin_deposit" + TestStdBitcoinDepositAndCallName = "std_bitcoin_deposit_and_call" + TestStdBitcoinDepositAndCallRevertName = "std_bitcoin_deposit_and_call_revert" + TestBitcoinWithdrawSegWitName = "bitcoin_withdraw_segwit" + TestBitcoinWithdrawTaprootName = "bitcoin_withdraw_taproot" + TestBitcoinWithdrawMultipleName = "bitcoin_withdraw_multiple" + TestBitcoinWithdrawLegacyName = "bitcoin_withdraw_legacy" + TestBitcoinWithdrawP2WSHName = "bitcoin_withdraw_p2wsh" + TestBitcoinWithdrawP2SHName = "bitcoin_withdraw_p2sh" + TestBitcoinWithdrawInvalidAddressName = "bitcoin_withdraw_invalid" + TestBitcoinWithdrawRestrictedName = "bitcoin_withdraw_restricted" + TestExtractBitcoinInscriptionMemoName = "bitcoin_memo_from_inscription" /* Application tests @@ -485,11 +488,11 @@ var AllE2ETests = []runner.E2ETest{ TestBitcoinDepositAndCall, ), runner.NewE2ETest( - TestBitcoinDepositRefundName, + TestBitcoinDepositAndCallRevertName, "deposit Bitcoin into ZEVM; expect refund", []runner.ArgDefinition{ {Description: "amount in btc", DefaultValue: "0.1"}, }, - TestBitcoinDepositRefund, + TestBitcoinDepositAndCallRevert, ), runner.NewE2ETest( TestBitcoinWithdrawSegWitName, diff --git a/e2e/e2etests/test_bitcoin_deposit_refund.go b/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go similarity index 84% rename from e2e/e2etests/test_bitcoin_deposit_refund.go rename to e2e/e2etests/test_bitcoin_deposit_and_call_revert.go index b0189d4b69..25685e0396 100644 --- a/e2e/e2etests/test_bitcoin_deposit_refund.go +++ b/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go @@ -7,11 +7,12 @@ import ( "github.com/zeta-chain/node/e2e/runner" "github.com/zeta-chain/node/e2e/utils" + "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/x/crosschain/types" zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin" ) -func TestBitcoinDepositRefund(r *runner.E2ERunner, args []string) { +func TestBitcoinDepositAndCallRevert(r *runner.E2ERunner, args []string) { // ARRANGE // Given BTC address r.SetBtcAddress(r.Name, false) @@ -32,7 +33,10 @@ func TestBitcoinDepositRefund(r *runner.E2ERunner, args []string) { // ACT // Send BTC to TSS address with a dummy memo - txHash, err := r.SendToTSSFromDeployerWithMemo(amount, utxos, []byte("gibberish-memo")) + // zetacore should revert cctx if call is made on a non-existing address + nonExistReceiver := sample.EthAddress() + badMemo := append(nonExistReceiver.Bytes(), []byte("gibberish-memo")...) + txHash, err := r.SendToTSSFromDeployerWithMemo(amount, utxos, badMemo) require.NoError(r, err) require.NotEmpty(r, txHash) diff --git a/x/crosschain/types/message_vote_inbound.go b/x/crosschain/types/message_vote_inbound.go index c73c9a24d4..956d2769d1 100644 --- a/x/crosschain/types/message_vote_inbound.go +++ b/x/crosschain/types/message_vote_inbound.go @@ -22,6 +22,13 @@ const MaxMessageLength = 10240 // InboundVoteOption is a function that sets some option on the inbound vote message type InboundVoteOption func(*MsgVoteInbound) +// WithMemoRevertOptions sets the revert options for inbound vote message +func WithRevertOptions(revertOptions RevertOptions) InboundVoteOption { + return func(msg *MsgVoteInbound) { + msg.RevertOptions = revertOptions + } +} + // WithZEVMRevertOptions sets the revert options for the inbound vote message (ZEVM format) // the function convert the type from abigen to type defined in proto func WithZEVMRevertOptions(revertOptions gatewayzevm.RevertOptions) InboundVoteOption { diff --git a/x/crosschain/types/message_vote_inbound_test.go b/x/crosschain/types/message_vote_inbound_test.go index 77631f20fd..9c91b72895 100644 --- a/x/crosschain/types/message_vote_inbound_test.go +++ b/x/crosschain/types/message_vote_inbound_test.go @@ -42,6 +42,45 @@ func TestNewMsgVoteInbound(t *testing.T) { require.EqualValues(t, types.NewEmptyRevertOptions(), msg.RevertOptions) }) + t.Run("can set revert options", func(t *testing.T) { + revertAddress := sample.EthAddress() + abortAddress := sample.EthAddress() + revertMessage := sample.Bytes() + + msg := types.NewMsgVoteInbound( + sample.AccAddress(), + sample.AccAddress(), + 31, + sample.String(), + sample.String(), + 31, + math.NewUint(31), + sample.String(), + sample.String(), + 31, + 31, + coin.CoinType_Gas, + sample.String(), + 31, + types.ProtocolContractVersion_V2, + true, + types.WithRevertOptions(types.RevertOptions{ + RevertAddress: revertAddress.Hex(), + CallOnRevert: true, + AbortAddress: abortAddress.Hex(), + RevertMessage: revertMessage, + RevertGasLimit: math.NewUint(21000), + }), + ) + require.EqualValues(t, types.RevertOptions{ + RevertAddress: revertAddress.Hex(), + CallOnRevert: true, + AbortAddress: abortAddress.Hex(), + RevertMessage: revertMessage, + RevertGasLimit: math.NewUint(21000), + }, msg.RevertOptions) + }) + t.Run("can set ZEVM revert options", func(t *testing.T) { revertAddress := sample.EthAddress() abortAddress := sample.EthAddress() diff --git a/zetaclient/chains/bitcoin/observer/inbound.go b/zetaclient/chains/bitcoin/observer/inbound.go index 1461096763..25126d6053 100644 --- a/zetaclient/chains/bitcoin/observer/inbound.go +++ b/zetaclient/chains/bitcoin/observer/inbound.go @@ -1,6 +1,7 @@ package observer import ( + "bytes" "context" "encoding/hex" "fmt" @@ -8,13 +9,17 @@ import ( cosmosmath "cosmossdk.io/math" "github.com/btcsuite/btcd/btcjson" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" ethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" "github.com/rs/zerolog" + "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/pkg/coin" + "github.com/zeta-chain/node/pkg/constant" + "github.com/zeta-chain/node/pkg/crypto" "github.com/zeta-chain/node/pkg/memo" crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" @@ -22,6 +27,7 @@ import ( "github.com/zeta-chain/node/zetaclient/compliance" "github.com/zeta-chain/node/zetaclient/config" zctx "github.com/zeta-chain/node/zetaclient/context" + "github.com/zeta-chain/node/zetaclient/logs" "github.com/zeta-chain/node/zetaclient/types" "github.com/zeta-chain/node/zetaclient/zetacore" ) @@ -43,6 +49,9 @@ type BTCInboundEvent struct { // MemoBytes is the memo of inbound MemoBytes []byte + // MemoStd is the standard inbound memo if it can be decoded + MemoStd *memo.InboundMemo + // BlockNumber is the block number of the inbound BlockNumber uint64 @@ -50,6 +59,46 @@ type BTCInboundEvent struct { TxHash string } +// InboundProcessability is an enum representing the processability of an inbound +type InboundProcessability int + +const ( + // InboundProcessabilityGood represents a processable inbound + InboundProcessabilityGood InboundProcessability = iota + + // InboundProcessabilityDonation represents a donation inbound + InboundProcessabilityDonation + + // InboundProcessabilityComplianceViolation represents a compliance violation + InboundProcessabilityComplianceViolation +) + +// IsProcessable checks if the inbound event is processable +func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { + // compliance check on sender and receiver addresses + if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { + return InboundProcessabilityComplianceViolation + } + + // compliance check on receiver, revert/abort addresses in standard memo + if event.MemoStd != nil { + if config.ContainRestrictedAddress( + event.MemoStd.Receiver.Hex(), + event.MemoStd.RevertOptions.RevertAddress, + event.MemoStd.RevertOptions.AbortAddress, + ) { + return InboundProcessabilityComplianceViolation + } + } + + // donation check + if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { + return InboundProcessabilityDonation + } + + return InboundProcessabilityGood +} + // WatchInbound watches Bitcoin chain for inbounds on a ticker // It starts a ticker and run ObserveInbound // TODO(revamp): move all ticker related methods in the same file @@ -100,8 +149,6 @@ func (ob *Observer) WatchInbound(ctx context.Context) error { // ObserveInbound observes the Bitcoin chain for inbounds and post votes to zetacore // TODO(revamp): simplify this function into smaller functions func (ob *Observer) ObserveInbound(ctx context.Context) error { - zetaCoreClient := ob.ZetacoreClient() - // get and update latest block height currentBlock, err := ob.btcClient.GetBlockCount() if err != nil { @@ -166,22 +213,11 @@ func (ob *Observer) ObserveInbound(ctx context.Context) error { // post inbound vote message to zetacore for _, event := range events { - msg := ob.GetInboundVoteMessageFromBtcEvent(event) + msg := ob.GetInboundVoteFromBtcEvent(event) if msg != nil { - zetaHash, ballot, err := zetaCoreClient.PostVoteInbound( - ctx, - zetacore.PostVoteInboundGasLimit, - zetacore.PostVoteInboundExecutionGasLimit, - msg, - ) + _, err = ob.PostVoteInbound(ctx, msg, zetacore.PostVoteInboundExecutionGasLimit) if err != nil { - ob.logger.Inbound.Error(). - Err(err). - Msgf("observeInboundBTC: error posting to zetacore for tx %s", event.TxHash) - return err // we have to re-scan this block next time - } else if zetaHash != "" { - ob.logger.Inbound.Info().Msgf("observeInboundBTC: PostVoteInbound zeta tx hash: %s inbound %s ballot %s fee %v", - zetaHash, event.TxHash, ballot, event.DepositorFee) + return errors.Wrapf(err, "error PostVoteInbound") // we have to re-scan this block next time } } } @@ -319,7 +355,7 @@ func (ob *Observer) CheckReceiptForBtcTxHash(ctx context.Context, txHash string, return "", errors.New("no btc deposit event found") } - msg := ob.GetInboundVoteMessageFromBtcEvent(event) + msg := ob.GetInboundVoteFromBtcEvent(event) if msg == nil { return "", errors.New("no message built for btc sent to TSS") } @@ -383,52 +419,161 @@ func FilterAndParseIncomingTx( return events, nil } -// GetInboundVoteMessageFromBtcEvent converts a BTCInboundEvent to a MsgVoteInbound to enable voting on the inbound on zetacore -func (ob *Observer) GetInboundVoteMessageFromBtcEvent(inbound *BTCInboundEvent) *crosschaintypes.MsgVoteInbound { - ob.logger.Inbound.Debug().Msgf("Processing inbound: %s", inbound.TxHash) - amount := big.NewFloat(inbound.Value) - amount = amount.Mul(amount, big.NewFloat(1e8)) - amountInt, _ := amount.Int(nil) - message := hex.EncodeToString(inbound.MemoBytes) +// CheckEventProcessability checks if the inbound event is processable +func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { + // check if the event is processable + result := event.CheckProcessability() + switch result { + case InboundProcessabilityGood: + return true + case InboundProcessabilityDonation: + logFields := map[string]any{ + logs.FieldChain: ob.Chain().ChainId, + logs.FieldTx: event.TxHash, + } + ob.Logger().Inbound.Info().Fields(logFields).Msgf("thank you rich folk for your donation!") + return false + case InboundProcessabilityComplianceViolation: + compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance, + false, ob.Chain().ChainId, event.TxHash, event.FromAddress, event.ToAddress, "BTC") + return false + } + + return true +} + +// DecodeMemoBytes decodes the memo bytes as either standard or legacy memo +func (ob *Observer) DecodeMemoBytes(event *BTCInboundEvent) error { + var receiver ethcommon.Address + + // try to decode the standard memo as the preferred format + memoStd, err := memo.DecodeFromBytes(event.MemoBytes) + if err == nil { + event.MemoStd = memoStd + receiver = memoStd.Receiver + } else { + // fallback to legacy memo if standard memo decoding fails + parsedAddress, _, err := memo.DecodeLegacyMemoHex(hex.EncodeToString(event.MemoBytes)) + if err != nil { + return errors.Wrap(err, "invalid legacy memo") + } + receiver = parsedAddress + } + + // ensure the receiver is valid + if crypto.IsEmptyAddress(receiver) { + return errors.New("got empty receiver address from memo") + } + event.ToAddress = receiver.Hex() + + // ensure the revert address is valid and supported + if event.MemoStd != nil { + revertAddressStr := memoStd.RevertOptions.RevertAddress + revertAddressBtc, err := chains.DecodeBtcAddress(revertAddressStr, ob.Chain().ChainId) + if err != nil { + return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddressStr) + } + if !chains.IsBtcAddressSupported(revertAddressBtc) { + return fmt.Errorf("unsupported revert address in memo: %s", revertAddressStr) + } + } - // compliance check - // if the inbound contains restricted addresses, return nil - if ob.DoesInboundContainsRestrictedAddress(inbound) { + return nil +} + +// GetInboundVoteFromBtcEvent converts a BTCInboundEvent to a MsgVoteInbound to enable voting on the inbound on zetacore +func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosschaintypes.MsgVoteInbound { + // prepare logger fields + lf := map[string]any{ + logs.FieldModule: logs.ModNameInbound, + logs.FieldMethod: "GetInboundVoteMessageFromBtcEvent", + logs.FieldChain: ob.Chain().ChainId, + logs.FieldTx: event.TxHash, + } + + // decode memo from bytes + err := ob.DecodeMemoBytes(event) + if err != nil { + ob.Logger().Inbound.Info().Fields(lf).Msgf("invalid memo bytes: %s", hex.EncodeToString(event.MemoBytes)) + return nil + } + + // check if the event is processable + if !ob.CheckEventProcessability(event) { return nil } - return zetacore.GetInboundVoteMessage( - inbound.FromAddress, + // convert the amount to integer (satoshis) + amount := big.NewFloat(event.Value) + amount = amount.Mul(amount, big.NewFloat(btcutil.SatoshiPerBitcoin)) + amountInt, _ := amount.Int(nil) + + // create inbound vote message contract V1 for legacy memo + if event.MemoStd == nil { + return ob.NewInboundVoteV1(event, amountInt) + } + + // create inbound vote message for standard memo + return ob.NewInboundVoteMemoStd(event, amountInt) +} + +// NewInboundVoteV1 creates a MsgVoteInbound message for inbound that uses legacy memo +func (ob *Observer) NewInboundVoteV1(event *BTCInboundEvent, amountSats *big.Int) *crosschaintypes.MsgVoteInbound { + message := hex.EncodeToString(event.MemoBytes) + + return crosschaintypes.NewMsgVoteInbound( + ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), + event.FromAddress, ob.Chain().ChainId, - inbound.FromAddress, - inbound.FromAddress, + event.FromAddress, + event.ToAddress, ob.ZetacoreClient().Chain().ChainId, - cosmosmath.NewUintFromBigInt(amountInt), + cosmosmath.NewUintFromBigInt(amountSats), message, - inbound.TxHash, - inbound.BlockNumber, + event.TxHash, + event.BlockNumber, 0, coin.CoinType_Gas, "", - ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), 0, + crosschaintypes.ProtocolContractVersion_V1, + false, // not relevant for v1 ) } -// DoesInboundContainsRestrictedAddress returns true if the inbound contains restricted addresses -// TODO(revamp): move all compliance related functions in a specific file -func (ob *Observer) DoesInboundContainsRestrictedAddress(inTx *BTCInboundEvent) bool { - receiver := "" - parsedAddress, _, err := memo.DecodeLegacyMemoHex(hex.EncodeToString(inTx.MemoBytes)) - if err == nil && parsedAddress != (ethcommon.Address{}) { - receiver = parsedAddress.Hex() - } - if config.ContainRestrictedAddress(inTx.FromAddress, receiver) { - compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance, - false, ob.Chain().ChainId, inTx.TxHash, inTx.FromAddress, receiver, "BTC") - return true +// NewInboundVoteMemoStd creates a MsgVoteInbound message for inbound that uses standard memo +// TODO: rename this function as 'EventToInboundVoteV2' to use ProtocolContractVersion_V2 and enable more options +// https://github.com/zeta-chain/node/issues/2711 +func (ob *Observer) NewInboundVoteMemoStd(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 } - return false + + // make a legacy message so that zetacore can process it + msgBytes := append(event.MemoStd.Receiver.Bytes(), event.MemoStd.Payload...) + message := hex.EncodeToString(msgBytes) + + return crosschaintypes.NewMsgVoteInbound( + ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), + sender, + ob.Chain().ChainId, + event.FromAddress, + event.ToAddress, + ob.ZetacoreClient().Chain().ChainId, + cosmosmath.NewUintFromBigInt(amountSats), + message, + event.TxHash, + event.BlockNumber, + 0, + coin.CoinType_Gas, + "", + 0, + crosschaintypes.ProtocolContractVersion_V1, + false, // not relevant for v1 + ) } // GetBtcEvent returns a valid BTCInboundEvent or nil diff --git a/zetaclient/chains/evm/observer/v2_inbound.go b/zetaclient/chains/evm/observer/v2_inbound.go index 246dda603a..1ff98f2af9 100644 --- a/zetaclient/chains/evm/observer/v2_inbound.go +++ b/zetaclient/chains/evm/observer/v2_inbound.go @@ -186,7 +186,7 @@ func (ob *Observer) newDepositInboundVote(event *gatewayevm.GatewayEVMDeposited) hex.EncodeToString(event.Payload), event.Raw.TxHash.Hex(), event.Raw.BlockNumber, - 1_500_000, + zetacore.PostVoteInboundCallOptionsGasLimit, coinType, event.Asset.Hex(), event.Raw.Index, @@ -321,7 +321,7 @@ func (ob *Observer) newCallInboundVote(event *gatewayevm.GatewayEVMCalled) types hex.EncodeToString(event.Payload), event.Raw.TxHash.Hex(), event.Raw.BlockNumber, - 1_500_000, + zetacore.PostVoteInboundCallOptionsGasLimit, coin.CoinType_NoAssetCall, "", event.Raw.Index, diff --git a/zetaclient/zetacore/constant.go b/zetaclient/zetacore/constant.go index 1457dd0c58..ab13e741d0 100644 --- a/zetaclient/zetacore/constant.go +++ b/zetaclient/zetacore/constant.go @@ -24,6 +24,9 @@ const ( // PostVoteInboundMessagePassingExecutionGasLimit is the gas limit for voting on, and executing ,observed inbound tx related to message passing (coin_type == zeta) PostVoteInboundMessagePassingExecutionGasLimit = 4_000_000 + // PostVoteInboundCallOptionsGasLimit is the gas limit for inbound call options + PostVoteInboundCallOptionsGasLimit uint64 = 1_500_000 + // AddOutboundTrackerGasLimit is the gas limit for adding tx hash to out tx tracker AddOutboundTrackerGasLimit = 200_000 From cb30c66630b52b1b5e374f0048e1bb0c6659caab Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Mon, 21 Oct 2024 09:56:49 -0500 Subject: [PATCH 02/10] add e2e tests for bitcoin standard memo deposit --- cmd/zetae2e/local/local.go | 5 + e2e/e2etests/e2etests.go | 71 +++++++++++--- e2e/e2etests/test_bitcoin_deposit.go | 2 +- .../test_bitcoin_deposit_and_call_revert.go | 31 ++---- e2e/e2etests/test_bitcoin_donation.go | 44 +++++++++ e2e/e2etests/test_bitcoin_std_deposit.go | 59 +++++++++++ .../test_bitcoin_std_deposit_and_call.go | 57 +++++++++++ ...est_bitcoin_std_deposit_and_call_revert.go | 53 ++++++++++ ...d_deposit_and_call_revert_other_address.go | 62 ++++++++++++ e2e/e2etests/test_stress_btc_deposit.go | 2 +- e2e/runner/bitcoin.go | 68 ++++++++++--- e2e/utils/zetacore.go | 21 ++++ zetaclient/chains/bitcoin/observer/event.go | 76 +++++++++++++++ .../chains/bitcoin/observer/event_test.go | 97 +++++++++++++++++++ zetaclient/chains/bitcoin/observer/inbound.go | 96 +++--------------- .../chains/bitcoin/observer/inbound_test.go | 37 +++++++ zetaclient/chains/bitcoin/observer/witness.go | 2 +- zetaclient/chains/bitcoin/tx_script.go | 55 +++++++---- zetaclient/chains/bitcoin/tx_script_test.go | 90 +++++++++-------- 19 files changed, 733 insertions(+), 195 deletions(-) create mode 100644 e2e/e2etests/test_bitcoin_donation.go create mode 100644 e2e/e2etests/test_bitcoin_std_deposit.go create mode 100644 e2e/e2etests/test_bitcoin_std_deposit_and_call.go create mode 100644 e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go create mode 100644 e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go create mode 100644 zetaclient/chains/bitcoin/observer/event.go create mode 100644 zetaclient/chains/bitcoin/observer/event_test.go diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index c62b4eb5c0..9b11a9a3f2 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -301,6 +301,11 @@ func localE2ETest(cmd *cobra.Command, _ []string) { e2etests.TestBitcoinDepositName, e2etests.TestBitcoinDepositAndCallName, e2etests.TestBitcoinDepositAndCallRevertName, + e2etests.TestBitcoinDonationName, + e2etests.TestBitcoinStdMemoDepositName, + e2etests.TestBitcoinStdMemoDepositAndCallName, + e2etests.TestBitcoinStdMemoDepositAndCallRevertName, + e2etests.TestBitcoinStdMemoDepositAndCallRevertOtherAddressName, e2etests.TestBitcoinWithdrawSegWitName, e2etests.TestBitcoinWithdrawInvalidAddressName, e2etests.TestZetaWithdrawBTCRevertName, diff --git a/e2e/e2etests/e2etests.go b/e2e/e2etests/e2etests.go index d737dc4ffa..4a85ed7433 100644 --- a/e2e/e2etests/e2etests.go +++ b/e2e/e2etests/e2etests.go @@ -72,21 +72,23 @@ const ( Bitcoin tests Test transfer of Bitcoin asset across chains */ - TestBitcoinDepositName = "bitcoin_deposit" - TestBitcoinDepositAndCallName = "bitcoin_deposit_and_call" - TestBitcoinDepositAndCallRevertName = "bitcoin_deposit_and_call_revert" - TestStdBitcoinDepositName = "std_bitcoin_deposit" - TestStdBitcoinDepositAndCallName = "std_bitcoin_deposit_and_call" - TestStdBitcoinDepositAndCallRevertName = "std_bitcoin_deposit_and_call_revert" - TestBitcoinWithdrawSegWitName = "bitcoin_withdraw_segwit" - TestBitcoinWithdrawTaprootName = "bitcoin_withdraw_taproot" - TestBitcoinWithdrawMultipleName = "bitcoin_withdraw_multiple" - TestBitcoinWithdrawLegacyName = "bitcoin_withdraw_legacy" - TestBitcoinWithdrawP2WSHName = "bitcoin_withdraw_p2wsh" - TestBitcoinWithdrawP2SHName = "bitcoin_withdraw_p2sh" - TestBitcoinWithdrawInvalidAddressName = "bitcoin_withdraw_invalid" - TestBitcoinWithdrawRestrictedName = "bitcoin_withdraw_restricted" - TestExtractBitcoinInscriptionMemoName = "bitcoin_memo_from_inscription" + TestBitcoinDepositName = "bitcoin_deposit" + TestBitcoinDepositAndCallName = "bitcoin_deposit_and_call" + TestBitcoinDepositAndCallRevertName = "bitcoin_deposit_and_call_revert" + TestBitcoinDonationName = "bitcoin_donation" + TestBitcoinStdMemoDepositName = "bitcoin_std_memo_deposit" + TestBitcoinStdMemoDepositAndCallName = "bitcoin_std_memo_deposit_and_call" + TestBitcoinStdMemoDepositAndCallRevertName = "bitcoin_std_memo_deposit_and_call_revert" + TestBitcoinStdMemoDepositAndCallRevertOtherAddressName = "bitcoin_std_memo_deposit_and_call_revert_other_address" + TestBitcoinWithdrawSegWitName = "bitcoin_withdraw_segwit" + TestBitcoinWithdrawTaprootName = "bitcoin_withdraw_taproot" + TestBitcoinWithdrawMultipleName = "bitcoin_withdraw_multiple" + TestBitcoinWithdrawLegacyName = "bitcoin_withdraw_legacy" + TestBitcoinWithdrawP2WSHName = "bitcoin_withdraw_p2wsh" + TestBitcoinWithdrawP2SHName = "bitcoin_withdraw_p2sh" + TestBitcoinWithdrawInvalidAddressName = "bitcoin_withdraw_invalid" + TestBitcoinWithdrawRestrictedName = "bitcoin_withdraw_restricted" + TestExtractBitcoinInscriptionMemoName = "bitcoin_memo_from_inscription" /* Application tests @@ -494,6 +496,45 @@ var AllE2ETests = []runner.E2ETest{ }, TestBitcoinDepositAndCallRevert, ), + runner.NewE2ETest( + TestBitcoinDonationName, + "donate Bitcoin to TSS address", []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.001"}, + }, + TestBitcoinDonation, + ), + runner.NewE2ETest( + TestBitcoinStdMemoDepositName, + "deposit Bitcoin into ZEVM with standard memo", + []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.1"}, + }, + TestBitcoinStdMemoDeposit, + ), + runner.NewE2ETest( + TestBitcoinStdMemoDepositAndCallName, + "deposit Bitcoin into ZEVM and call a contract with standard memo", + []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.1"}, + }, + TestBitcoinStdMemoDepositAndCall, + ), + runner.NewE2ETest( + TestBitcoinStdMemoDepositAndCallRevertName, + "deposit Bitcoin into ZEVM and call a contract with standard memo; expect revert", + []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.1"}, + }, + TestBitcoinStdMemoDepositAndCallRevert, + ), + runner.NewE2ETest( + TestBitcoinStdMemoDepositAndCallRevertOtherAddressName, + "deposit Bitcoin into ZEVM and call a contract with standard memo; expect revert to other address", + []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.1"}, + }, + TestBitcoinStdMemoDepositAndCallRevertOtherAddress, + ), runner.NewE2ETest( TestBitcoinWithdrawSegWitName, "withdraw BTC from ZEVM to a SegWit address", diff --git a/e2e/e2etests/test_bitcoin_deposit.go b/e2e/e2etests/test_bitcoin_deposit.go index c9c6fdbf45..590a5c81d8 100644 --- a/e2e/e2etests/test_bitcoin_deposit.go +++ b/e2e/e2etests/test_bitcoin_deposit.go @@ -15,7 +15,7 @@ func TestBitcoinDeposit(r *runner.E2ERunner, args []string) { r.SetBtcAddress(r.Name, false) - txHash := r.DepositBTCWithAmount(depositAmount) + txHash := r.DepositBTCWithAmount(depositAmount, nil) // wait for the cctx to be mined cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout) diff --git a/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go b/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go index 25685e0396..eed10485bf 100644 --- a/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go +++ b/e2e/e2etests/test_bitcoin_deposit_and_call_revert.go @@ -1,14 +1,12 @@ package e2etests import ( - "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zeta-chain/node/e2e/runner" "github.com/zeta-chain/node/e2e/utils" "github.com/zeta-chain/node/testutil/sample" - "github.com/zeta-chain/node/x/crosschain/types" zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin" ) @@ -42,29 +40,12 @@ func TestBitcoinDepositAndCallRevert(r *runner.E2ERunner, args []string) { // ASSERT // Now we want to make sure refund TX is completed. - // Let's check that zetaclient issued a refund on BTC - searchForCrossChainWithBtcRefund := utils.Matches(func(tx types.CrossChainTx) bool { - return tx.GetCctxStatus().Status == types.CctxStatus_Reverted && - len(tx.OutboundParams) == 2 && - tx.OutboundParams[1].Hash != "" - }) + cctx := utils.WaitCctxRevertedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient) - cctxs := utils.WaitCctxByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient, searchForCrossChainWithBtcRefund) - require.Len(r, cctxs, 1) + // Check revert tx receiver address and amount + receiver, value := r.QueryOutboundReceiverAndAmount(cctx.OutboundParams[1].Hash) + assert.Equal(r, r.BTCDeployerAddress.EncodeAddress(), receiver) + assert.Positive(r, value) - // Pick btc tx hash from the cctx - btcTxHash, err := chainhash.NewHashFromStr(cctxs[0].OutboundParams[1].Hash) - require.NoError(r, err) - - // Query the BTC network to check the refund transaction - refundTx, err := r.BtcRPCClient.GetTransaction(btcTxHash) - require.NoError(r, err, refundTx) - - // Finally, check the refund transaction details - refundTxDetails := refundTx.Details[0] - assert.Equal(r, "receive", refundTxDetails.Category) - assert.Equal(r, r.BTCDeployerAddress.EncodeAddress(), refundTxDetails.Address) - assert.NotEmpty(r, refundTxDetails.Amount) - - r.Logger.Info("Sent %f BTC to TSS with invalid memo, got refund of %f BTC", amount, refundTxDetails.Amount) + r.Logger.Info("Sent %f BTC to TSS with invalid memo, got refund of %d satoshis", amount, value) } diff --git a/e2e/e2etests/test_bitcoin_donation.go b/e2e/e2etests/test_bitcoin_donation.go new file mode 100644 index 0000000000..54a3eefdaf --- /dev/null +++ b/e2e/e2etests/test_bitcoin_donation.go @@ -0,0 +1,44 @@ +package e2etests + +import ( + "time" + + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/node/e2e/runner" + "github.com/zeta-chain/node/pkg/constant" + crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" + zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin" +) + +func TestBitcoinDonation(r *runner.E2ERunner, args []string) { + // ARRANGE + // Given BTC address + r.SetBtcAddress(r.Name, false) + + // Given "Live" BTC network + stop := r.MineBlocksIfLocalBitcoin() + defer stop() + + // Given amount to send + require.Len(r, args, 1) + amount := parseFloat(r, args[0]) + amountTotal := amount + zetabitcoin.DefaultDepositorFee + + // Given a list of UTXOs + utxos, err := r.ListDeployerUTXOs() + require.NoError(r, err) + require.NotEmpty(r, utxos) + + // ACT + // Send BTC to TSS address with donation message + memo := []byte(constant.DonationMessage) + txHash, err := r.SendToTSSFromDeployerWithMemo(amountTotal, utxos, memo) + require.NoError(r, err) + + // ASSERT after 20 seconds + time.Sleep(time.Second * 20) + req := &crosschaintypes.QueryInboundHashToCctxDataRequest{InboundHash: txHash.String()} + _, err = r.CctxClient.InTxHashToCctxData(r.Ctx, req) + require.Error(r, err) +} diff --git a/e2e/e2etests/test_bitcoin_std_deposit.go b/e2e/e2etests/test_bitcoin_std_deposit.go new file mode 100644 index 0000000000..a98a34a3f1 --- /dev/null +++ b/e2e/e2etests/test_bitcoin_std_deposit.go @@ -0,0 +1,59 @@ +package e2etests + +import ( + "math/big" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/node/e2e/runner" + "github.com/zeta-chain/node/e2e/utils" + "github.com/zeta-chain/node/pkg/memo" + crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" + "github.com/zeta-chain/node/zetaclient/chains/bitcoin" +) + +func TestBitcoinStdMemoDeposit(r *runner.E2ERunner, args []string) { + // setup deployer BTC address + r.SetBtcAddress(r.Name, false) + + // parse amount to deposit + require.Len(r, args, 1) + amount := parseFloat(r, args[0]) + + // get ERC20 BTC balance before deposit + balanceBefore, err := r.BTCZRC20.BalanceOf(&bind.CallOpts{}, r.EVMAddress()) + require.NoError(r, err) + r.Logger.Info("runner balance of BTC before deposit: %d satoshis", balanceBefore) + + // create standard memo with receiver address + memo := &memo.InboundMemo{ + Header: memo.Header{ + Version: 0, + EncodingFmt: memo.EncodingFmtCompactShort, + OpCode: memo.OpCodeDeposit, + }, + FieldsV0: memo.FieldsV0{ + Receiver: r.EVMAddress(), // to deployer self + }, + } + + // deposit BTC with standard memo + txHash := r.DepositBTCWithAmount(amount, memo) + + // wait for the cctx to be mined + cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout) + r.Logger.CCTX(*cctx, "bitcoin_std_memo_deposit") + utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) + + // get ERC20 BTC balance after deposit + balanceAfter, err := r.BTCZRC20.BalanceOf(&bind.CallOpts{}, r.EVMAddress()) + require.NoError(r, err) + r.Logger.Info("runner balance of BTC after deposit: %d satoshis", balanceAfter) + + // the runner balance should be increased by the deposit amount + amountIncreased := new(big.Int).Sub(balanceAfter, balanceBefore) + amountSatoshis, err := bitcoin.GetSatoshis(amount) + require.NoError(r, err) + require.Equal(r, uint64(amountSatoshis), amountIncreased.Uint64()) +} diff --git a/e2e/e2etests/test_bitcoin_std_deposit_and_call.go b/e2e/e2etests/test_bitcoin_std_deposit_and_call.go new file mode 100644 index 0000000000..365bea556c --- /dev/null +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call.go @@ -0,0 +1,57 @@ +package e2etests + +import ( + "math/big" + + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/node/e2e/runner" + "github.com/zeta-chain/node/e2e/utils" + "github.com/zeta-chain/node/pkg/memo" + testcontract "github.com/zeta-chain/node/testutil/contracts" + crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" + zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin" +) + +func TestBitcoinStdMemoDepositAndCall(r *runner.E2ERunner, args []string) { + // setup deployer BTC address + r.SetBtcAddress(r.Name, false) + + // start mining blocks if local bitcoin + stop := r.MineBlocksIfLocalBitcoin() + defer stop() + + // parse amount to deposit + require.Len(r, args, 1) + amount := parseFloat(r, args[0]) + + // deploy an example contract in ZEVM + contractAddr, _, contract, err := testcontract.DeployExample(r.ZEVMAuth, r.ZEVMClient) + require.NoError(r, err) + + // create standard memo with [receiver, payload] + memo := &memo.InboundMemo{ + Header: memo.Header{ + Version: 0, + EncodingFmt: memo.EncodingFmtCompactShort, + OpCode: memo.OpCodeDepositAndCall, + }, + FieldsV0: memo.FieldsV0{ + Receiver: contractAddr, + Payload: []byte("hello satoshi"), + }, + } + + // deposit BTC with standard memo + txHash := r.DepositBTCWithAmount(amount, memo) + + // wait for the cctx to be mined + cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout) + r.Logger.CCTX(*cctx, "bitcoin_std_memo_deposit_and_call") + utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) + + // check if example contract has been called, 'bar' value should be set to amount + amoutSats, err := zetabitcoin.GetSatoshis(amount) + require.NoError(r, err) + utils.MustHaveCalledExampleContract(r, contract, big.NewInt(amoutSats)) +} diff --git a/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go new file mode 100644 index 0000000000..d346049802 --- /dev/null +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go @@ -0,0 +1,53 @@ +package e2etests + +import ( + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/node/e2e/runner" + "github.com/zeta-chain/node/e2e/utils" + "github.com/zeta-chain/node/pkg/memo" + "github.com/zeta-chain/node/testutil/sample" +) + +func TestBitcoinStdMemoDepositAndCallRevert(r *runner.E2ERunner, args []string) { + // ARRANGE + // Given BTC address + r.SetBtcAddress(r.Name, false) + + // Start mining blocks + stop := r.MineBlocksIfLocalBitcoin() + defer stop() + + // Parse amount to send + require.Len(r, args, 1) + amount := parseFloat(r, args[0]) + + // Create a memo to call non-existing contract + memo := &memo.InboundMemo{ + Header: memo.Header{ + Version: 0, + EncodingFmt: memo.EncodingFmtCompactShort, + OpCode: memo.OpCodeDepositAndCall, + }, + FieldsV0: memo.FieldsV0{ + Receiver: sample.EthAddress(), // non-existing contract + Payload: []byte("a payload"), + }, + } + + // ACT + // Deposit + txHash := r.DepositBTCWithAmount(amount, memo) + + // ASSERT + // Now we want to make sure revert TX is completed. + cctx := utils.WaitCctxRevertedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient) + + // Check revert tx receiver address and amount + receiver, value := r.QueryOutboundReceiverAndAmount(cctx.OutboundParams[1].Hash) + assert.Equal(r, r.BTCDeployerAddress.EncodeAddress(), receiver) + assert.Positive(r, value) + + r.Logger.Print("Sent %f BTC to TSS to call non-existing contract, got refund of %d satoshis", amount, value) +} 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 new file mode 100644 index 0000000000..d30f93409d --- /dev/null +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert_other_address.go @@ -0,0 +1,62 @@ +package e2etests + +import ( + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zeta-chain/node/e2e/runner" + "github.com/zeta-chain/node/e2e/utils" + "github.com/zeta-chain/node/pkg/memo" + "github.com/zeta-chain/node/testutil/sample" + "github.com/zeta-chain/node/x/crosschain/types" +) + +func TestBitcoinStdMemoDepositAndCallRevertOtherAddress(r *runner.E2ERunner, args []string) { + // ARRANGE + // Given BTC address + r.SetBtcAddress(r.Name, false) + + // Start mining blocks + stop := r.MineBlocksIfLocalBitcoin() + defer stop() + + // Parse amount to send + require.Len(r, args, 1) + amount := parseFloat(r, args[0]) + + // Create a memo to call non-existing contract + revertAddress := "bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2" + memo := &memo.InboundMemo{ + Header: memo.Header{ + Version: 0, + EncodingFmt: memo.EncodingFmtCompactShort, + OpCode: memo.OpCodeDepositAndCall, + }, + FieldsV0: memo.FieldsV0{ + Receiver: sample.EthAddress(), // non-existing contract + Payload: []byte("a payload"), + RevertOptions: types.RevertOptions{ + RevertAddress: revertAddress, + }, + }, + } + + // ACT + // Deposit + txHash := r.DepositBTCWithAmount(amount, memo) + + // ASSERT + // Now we want to make sure revert TX is completed. + cctx := utils.WaitCctxRevertedByInboundHash(r.Ctx, r, txHash.String(), r.CctxClient) + + // Check revert tx receiver address and amount + receiver, value := r.QueryOutboundReceiverAndAmount(cctx.OutboundParams[1].Hash) + assert.Equal(r, revertAddress, receiver) + assert.Positive(r, value) + + r.Logger.Print( + "Sent %f BTC to TSS to call non-existing contract, got refund of %d satoshis to other address", + amount, + value, + ) +} diff --git a/e2e/e2etests/test_stress_btc_deposit.go b/e2e/e2etests/test_stress_btc_deposit.go index 53caea09df..bedf004bdf 100644 --- a/e2e/e2etests/test_stress_btc_deposit.go +++ b/e2e/e2etests/test_stress_btc_deposit.go @@ -30,7 +30,7 @@ func TestStressBTCDeposit(r *runner.E2ERunner, args []string) { // send the deposits for i := 0; i < numDeposits; i++ { i := i - txHash := r.DepositBTCWithAmount(depositAmount) + txHash := r.DepositBTCWithAmount(depositAmount, nil) r.Logger.Print("index %d: starting deposit, tx hash: %s", i, txHash.String()) eg.Go(func() error { return monitorBTCDeposit(r, txHash, i, time.Now()) }) diff --git a/e2e/runner/bitcoin.go b/e2e/runner/bitcoin.go index d2c04fccf0..c822e63d0c 100644 --- a/e2e/runner/bitcoin.go +++ b/e2e/runner/bitcoin.go @@ -21,6 +21,7 @@ import ( "github.com/zeta-chain/node/e2e/utils" "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/pkg/constant" + "github.com/zeta-chain/node/pkg/memo" crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin" btcobserver "github.com/zeta-chain/node/zetaclient/chains/bitcoin/observer" @@ -76,10 +77,8 @@ func (r *E2ERunner) GetTop20UTXOsForTssAddress() ([]btcjson.ListUnspentResult, e return utxos, nil } -// DepositBTCWithAmount deposits BTC on ZetaChain with a specific amount -func (r *E2ERunner) DepositBTCWithAmount(amount float64) *chainhash.Hash { - r.Logger.Print("⏳ depositing BTC into ZEVM") - +// DepositBTCWithAmount deposits BTC into ZetaChain with a specific amount +func (r *E2ERunner) DepositBTCWithAmount(amount float64, memo *memo.InboundMemo) *chainhash.Hash { // list deployer utxos utxos, err := r.ListDeployerUTXOs() require.NoError(r, err) @@ -100,8 +99,16 @@ func (r *E2ERunner) DepositBTCWithAmount(amount float64) *chainhash.Hash { r.Logger.Info(" spendableUTXOs: %d", spendableUTXOs) r.Logger.Info("Now sending two txs to TSS address...") + // add depositor fee so that receiver gets the exact given 'amount' in ZetaChain amount += zetabitcoin.DefaultDepositorFee - txHash, err := r.SendToTSSFromDeployerToDeposit(amount, utxos) + + // deposit to TSS address + var txHash *chainhash.Hash + if memo != nil { + txHash, err = r.DepositBTCWithStandardMemo(amount, utxos, memo) + } else { + txHash, err = r.DepositBTCWithLegacyMemo(amount, utxos) + } require.NoError(r, err) r.Logger.Info("send BTC to TSS txHash: %s", txHash.String()) @@ -140,11 +147,11 @@ func (r *E2ERunner) DepositBTC() { // send two transactions to the TSS address amount1 := 1.1 + zetabitcoin.DefaultDepositorFee - _, err = r.SendToTSSFromDeployerToDeposit(amount1, utxos[:2]) + _, err = r.DepositBTCWithLegacyMemo(amount1, utxos[:2]) require.NoError(r, err) amount2 := 0.05 + zetabitcoin.DefaultDepositorFee - txHash2, err := r.SendToTSSFromDeployerToDeposit(amount2, utxos[2:4]) + txHash2, err := r.DepositBTCWithLegacyMemo(amount2, utxos[2:4]) require.NoError(r, err) // send a donation to the TSS address to compensate for the funds minted automatically during pool creation @@ -168,13 +175,33 @@ func (r *E2ERunner) DepositBTC() { require.Equal(r, 1, balance.Sign(), "balance should be positive") } -func (r *E2ERunner) SendToTSSFromDeployerToDeposit(amount float64, inputUTXOs []btcjson.ListUnspentResult) ( - *chainhash.Hash, - error, -) { +// DepositBTCWithLegacyMemo deposits BTC from the deployer address to the TSS using legacy memo +func (r *E2ERunner) DepositBTCWithLegacyMemo( + amount float64, + inputUTXOs []btcjson.ListUnspentResult, +) (*chainhash.Hash, error) { + r.Logger.Info("⏳ depositing BTC into ZEVM with legacy memo") + return r.SendToTSSFromDeployerWithMemo(amount, inputUTXOs, r.EVMAddress().Bytes()) } +// DepositBTCWithStandardMemo deposits BTC from the deployer address to the TSS using standard memo +func (r *E2ERunner) DepositBTCWithStandardMemo( + amount float64, + inputUTXOs []btcjson.ListUnspentResult, + memoStd *memo.InboundMemo, +) (*chainhash.Hash, error) { + r.Logger.Info("⏳ depositing BTC into ZEVM with standard memo") + + // encode memo to bytes + memoBytes, err := memoStd.EncodeToBytes() + require.NoError(r, err) + + r.Logger.Print("Standard memoBytes: %s, length: %d", hex.EncodeToString(memoBytes), len(memoBytes)) + + return r.SendToTSSFromDeployerWithMemo(amount, inputUTXOs, memoBytes) +} + func (r *E2ERunner) SendToTSSFromDeployerWithMemo( amount float64, inputUTXOs []btcjson.ListUnspentResult, @@ -366,6 +393,25 @@ func (r *E2ERunner) GenerateToAddressIfLocalBitcoin( return nil, nil } +// QueryOutboundReceiverAndAmount queries the outbound receiver and amount (in satoshis) from the given txid +func (r *E2ERunner) QueryOutboundReceiverAndAmount(txid string) (string, int64) { + txHash, err := chainhash.NewHashFromStr(txid) + require.NoError(r, err) + + // query outbound raw transaction + revertTx, err := r.BtcRPCClient.GetRawTransaction(txHash) + require.NoError(r, err, revertTx) + require.True(r, len(revertTx.MsgTx().TxOut) >= 2) + + // parse receiver address from pkScript + txOutput := revertTx.MsgTx().TxOut[1] + pkScript := txOutput.PkScript + receiver, err := zetabitcoin.DecodeScriptP2WPKH(hex.EncodeToString(pkScript), r.BitcoinParams) + require.NoError(r, err) + + return receiver, txOutput.Value +} + // MineBlocksIfLocalBitcoin mines blocks on the local BTC chain at a rate of 1 blocks every 5 seconds // and returns a channel that can be used to stop the mining // If the chain is not local, the function does nothing diff --git a/e2e/utils/zetacore.go b/e2e/utils/zetacore.go index 6d50be10da..186b135388 100644 --- a/e2e/utils/zetacore.go +++ b/e2e/utils/zetacore.go @@ -204,6 +204,27 @@ type waitConfig struct { matchFunction func(tx crosschaintypes.CrossChainTx) bool } +// WaitCctxRevertedByInboundHash waits until cctx is reverted by inbound hash. +func WaitCctxRevertedByInboundHash( + ctx context.Context, + t require.TestingT, + hash string, + c CCTXClient, +) crosschaintypes.CrossChainTx { + // criteria for reverted cctx + searchForCCTXReverted := Matches(func(tx crosschaintypes.CrossChainTx) bool { + return tx.GetCctxStatus().Status == crosschaintypes.CctxStatus_Reverted && + len(tx.OutboundParams) == 2 && + tx.OutboundParams[1].Hash != "" + }) + + // wait for cctx to be reverted + cctxs := WaitCctxByInboundHash(ctx, t, hash, c, searchForCCTXReverted) + require.Len(t, cctxs, 1) + + return cctxs[0] +} + // WaitCctxByInboundHash waits until cctx appears by inbound hash. func WaitCctxByInboundHash( ctx context.Context, diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go new file mode 100644 index 0000000000..b1988c7d9c --- /dev/null +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -0,0 +1,76 @@ +package observer + +import ( + "bytes" + + "github.com/zeta-chain/node/pkg/constant" + "github.com/zeta-chain/node/pkg/memo" + "github.com/zeta-chain/node/zetaclient/config" +) + +// InboundProcessability is an enum representing the processability of an inbound +type InboundProcessability int + +const ( + // InboundProcessabilityGood represents a processable inbound + InboundProcessabilityGood InboundProcessability = iota + + // InboundProcessabilityDonation represents a donation inbound + InboundProcessabilityDonation + + // InboundProcessabilityComplianceViolation represents a compliance violation + InboundProcessabilityComplianceViolation +) + +// BTCInboundEvent represents an incoming transaction event +type BTCInboundEvent struct { + // FromAddress is the first input address + FromAddress string + + // ToAddress is the TSS address + ToAddress string + + // Value is the amount of BTC + Value float64 + + // DepositorFee is the deposit fee + DepositorFee float64 + + // MemoBytes is the memo of inbound + MemoBytes []byte + + // MemoStd is the standard inbound memo if it can be decoded + MemoStd *memo.InboundMemo + + // BlockNumber is the block number of the inbound + BlockNumber uint64 + + // TxHash is the hash of the inbound + TxHash string +} + +// IsProcessable checks if the inbound event is processable +func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { + // compliance check on sender and receiver addresses + if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { + return InboundProcessabilityComplianceViolation + } + + // compliance check on receiver, revert/abort addresses in standard memo + if event.MemoStd != nil { + if config.ContainRestrictedAddress( + event.MemoStd.Receiver.Hex(), + event.MemoStd.RevertOptions.RevertAddress, + event.MemoStd.RevertOptions.AbortAddress, + ) { + return InboundProcessabilityComplianceViolation + } + } + + // donation check + if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { + return InboundProcessabilityDonation + } + + return InboundProcessabilityGood +} diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go new file mode 100644 index 0000000000..abe43f85b3 --- /dev/null +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -0,0 +1,97 @@ +package observer_test + +import ( + "testing" + + crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" + + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" + "github.com/zeta-chain/node/pkg/constant" + "github.com/zeta-chain/node/pkg/memo" + "github.com/zeta-chain/node/testutil/sample" + "github.com/zeta-chain/node/zetaclient/chains/bitcoin/observer" + "github.com/zeta-chain/node/zetaclient/config" + "github.com/zeta-chain/node/zetaclient/testutils" +) + +func Test_CheckProcessability(t *testing.T) { + // setup compliance config + cfg := config.Config{ + ComplianceConfig: config.ComplianceConfig{}, + } + + // add restricted address + restrictedBtcAddress := sample.RestrictedBtcAddressTest + restrictedEvmAddress := sample.RestrictedEVMAddressTest + cfg.ComplianceConfig.RestrictedAddresses = []string{restrictedBtcAddress, restrictedEvmAddress} + config.LoadComplianceConfig(cfg) + + // test cases + tests := []struct { + name string + event *observer.BTCInboundEvent + expected observer.InboundProcessability + }{ + { + name: "should return InboundProcessabilityGood for a processable inbound event", + event: &observer.BTCInboundEvent{ + FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu", + ToAddress: testutils.TSSAddressBTCAthens3, + }, + expected: observer.InboundProcessabilityGood, + }, + { + name: "should return InboundProcessabilityComplianceViolation for a restricted sender address", + event: &observer.BTCInboundEvent{ + FromAddress: restrictedBtcAddress, + ToAddress: testutils.TSSAddressBTCAthens3, + }, + expected: observer.InboundProcessabilityComplianceViolation, + }, + { + name: "should return InboundProcessabilityComplianceViolation for a restricted receiver address in standard memo", + event: &observer.BTCInboundEvent{ + FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu", + ToAddress: testutils.TSSAddressBTCAthens3, + MemoStd: &memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + Receiver: common.HexToAddress(restrictedEvmAddress), + }, + }, + }, + expected: observer.InboundProcessabilityComplianceViolation, + }, + { + name: "should return InboundProcessabilityComplianceViolation for a restricted revert address in standard memo", + event: &observer.BTCInboundEvent{ + FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu", + ToAddress: testutils.TSSAddressBTCAthens3, + MemoStd: &memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + RevertOptions: crosschaintypes.RevertOptions{ + RevertAddress: restrictedBtcAddress, + }, + }, + }, + }, + expected: observer.InboundProcessabilityComplianceViolation, + }, + { + name: "should return InboundProcessabilityDonation for a donation inbound event", + event: &observer.BTCInboundEvent{ + FromAddress: "tb1quhassyrlj43qar0mn0k5sufyp6mazmh2q85lr6ex8ehqfhxpzsksllwrsu", + ToAddress: testutils.TSSAddressBTCAthens3, + MemoBytes: []byte(constant.DonationMessage), + }, + expected: observer.InboundProcessabilityDonation, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.event.CheckProcessability() + require.Equal(t, tt.expected, result) + }) + } +} diff --git a/zetaclient/chains/bitcoin/observer/inbound.go b/zetaclient/chains/bitcoin/observer/inbound.go index 25126d6053..e76dc0a1e0 100644 --- a/zetaclient/chains/bitcoin/observer/inbound.go +++ b/zetaclient/chains/bitcoin/observer/inbound.go @@ -25,80 +25,12 @@ import ( "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/chains/interfaces" "github.com/zeta-chain/node/zetaclient/compliance" - "github.com/zeta-chain/node/zetaclient/config" zctx "github.com/zeta-chain/node/zetaclient/context" "github.com/zeta-chain/node/zetaclient/logs" "github.com/zeta-chain/node/zetaclient/types" "github.com/zeta-chain/node/zetaclient/zetacore" ) -// BTCInboundEvent represents an incoming transaction event -type BTCInboundEvent struct { - // FromAddress is the first input address - FromAddress string - - // ToAddress is the TSS address - ToAddress string - - // Value is the amount of BTC - Value float64 - - // DepositorFee is the deposit fee - DepositorFee float64 - - // MemoBytes is the memo of inbound - MemoBytes []byte - - // MemoStd is the standard inbound memo if it can be decoded - MemoStd *memo.InboundMemo - - // BlockNumber is the block number of the inbound - BlockNumber uint64 - - // TxHash is the hash of the inbound - TxHash string -} - -// InboundProcessability is an enum representing the processability of an inbound -type InboundProcessability int - -const ( - // InboundProcessabilityGood represents a processable inbound - InboundProcessabilityGood InboundProcessability = iota - - // InboundProcessabilityDonation represents a donation inbound - InboundProcessabilityDonation - - // InboundProcessabilityComplianceViolation represents a compliance violation - InboundProcessabilityComplianceViolation -) - -// IsProcessable checks if the inbound event is processable -func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { - // compliance check on sender and receiver addresses - if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { - return InboundProcessabilityComplianceViolation - } - - // compliance check on receiver, revert/abort addresses in standard memo - if event.MemoStd != nil { - if config.ContainRestrictedAddress( - event.MemoStd.Receiver.Hex(), - event.MemoStd.RevertOptions.RevertAddress, - event.MemoStd.RevertOptions.AbortAddress, - ) { - return InboundProcessabilityComplianceViolation - } - } - - // donation check - if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { - return InboundProcessabilityDonation - } - - return InboundProcessabilityGood -} - // WatchInbound watches Bitcoin chain for inbounds on a ticker // It starts a ticker and run ObserveInbound // TODO(revamp): move all ticker related methods in the same file @@ -442,11 +374,15 @@ func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { return true } -// DecodeMemoBytes decodes the memo bytes as either standard or legacy memo -func (ob *Observer) DecodeMemoBytes(event *BTCInboundEvent) error { - var receiver ethcommon.Address +// DecodeEventMemoBytes decodes the memo bytes as either standard or legacy memo +func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { + // skip decoding donation tx as it won't go through zetacore + if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { + return nil + } // try to decode the standard memo as the preferred format + var receiver ethcommon.Address memoStd, err := memo.DecodeFromBytes(event.MemoBytes) if err == nil { event.MemoStd = memoStd @@ -467,14 +403,14 @@ func (ob *Observer) DecodeMemoBytes(event *BTCInboundEvent) error { event.ToAddress = receiver.Hex() // ensure the revert address is valid and supported - if event.MemoStd != nil { - revertAddressStr := memoStd.RevertOptions.RevertAddress - revertAddressBtc, err := chains.DecodeBtcAddress(revertAddressStr, ob.Chain().ChainId) + revertAddress := memoStd.RevertOptions.RevertAddress + if event.MemoStd != nil && revertAddress != "" { + btcAddress, err := chains.DecodeBtcAddress(revertAddress, ob.Chain().ChainId) if err != nil { - return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddressStr) + return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) } - if !chains.IsBtcAddressSupported(revertAddressBtc) { - return fmt.Errorf("unsupported revert address in memo: %s", revertAddressStr) + if !chains.IsBtcAddressSupported(btcAddress) { + return fmt.Errorf("unsupported revert address in memo: %s", revertAddress) } } @@ -491,8 +427,8 @@ func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosscha logs.FieldTx: event.TxHash, } - // decode memo from bytes - err := ob.DecodeMemoBytes(event) + // decode event memo bytes + err := ob.DecodeEventMemoBytes(event) if err != nil { ob.Logger().Inbound.Info().Fields(lf).Msgf("invalid memo bytes: %s", hex.EncodeToString(event.MemoBytes)) return nil @@ -634,7 +570,7 @@ func GetBtcEventWithoutWitness( // 2nd vout must be a valid OP_RETURN memo vout1 := tx.Vout[1] - memo, found, err = bitcoin.DecodeOpReturnMemo(vout1.ScriptPubKey.Hex, tx.Txid) + memo, found, err = bitcoin.DecodeOpReturnMemo(vout1.ScriptPubKey.Hex) if err != nil { logger.Error().Err(err).Msgf("GetBtcEvent: error decoding OP_RETURN memo: %s", vout1.ScriptPubKey.Hex) return nil, nil diff --git a/zetaclient/chains/bitcoin/observer/inbound_test.go b/zetaclient/chains/bitcoin/observer/inbound_test.go index 8b01e222a1..293987196d 100644 --- a/zetaclient/chains/bitcoin/observer/inbound_test.go +++ b/zetaclient/chains/bitcoin/observer/inbound_test.go @@ -2,7 +2,9 @@ package observer_test import ( "bytes" + "encoding/binary" "encoding/hex" + "fmt" "math" "path" "strings" @@ -12,6 +14,7 @@ import ( "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/txscript" "github.com/pkg/errors" "github.com/rs/zerolog/log" "github.com/stretchr/testify/mock" @@ -189,6 +192,40 @@ func TestGetSenderAddressByVin(t *testing.T) { }) } +func Test_DecodeEventMemoBytes(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinRegtest + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + + buf := make([]byte, 2) + binary.LittleEndian.PutUint16(buf, uint16(256)) + + t.Run("should decode standard memo bytes", func(t *testing.T) { + // create test standard memo bytes + //memoHex := "5a010001283d810090edf4043e75247eaebce848806237fd" + memoHex := "5a01100757bcbefff7c37b24602e378ce938a1a1851513e40961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432" + + "5a01100757bcbefff7c37b24602e378ce938a1a1851513e40961207061796c6f61642c626372743171793970716d6b3270" + memo, _ := hex.DecodeString(memoHex) + nullData, err := txscript.NullDataScript(memo) + require.NoError(t, err) + fmt.Printf("nullData: %s\n", hex.EncodeToString(nullData)) + + memoBytes, err := hex.DecodeString(memoHex) + require.NoError(t, err) + + event := &observer.BTCInboundEvent{ + MemoBytes: memoBytes, + } + + // decode memo bytes + err = ob.DecodeEventMemoBytes(event) + require.NoError(t, err) + }) +} + func TestGetBtcEventWithoutWitness(t *testing.T) { // load archived inbound P2WPKH raw result // https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa diff --git a/zetaclient/chains/bitcoin/observer/witness.go b/zetaclient/chains/bitcoin/observer/witness.go index 86b22f95cf..22ce75719b 100644 --- a/zetaclient/chains/bitcoin/observer/witness.go +++ b/zetaclient/chains/bitcoin/observer/witness.go @@ -130,7 +130,7 @@ func tryExtractOpRet(tx btcjson.TxRawResult, logger zerolog.Logger) []byte { return nil } - memo, found, err := bitcoin.DecodeOpReturnMemo(tx.Vout[1].ScriptPubKey.Hex, tx.Txid) + memo, found, err := bitcoin.DecodeOpReturnMemo(tx.Vout[1].ScriptPubKey.Hex) if err != nil { logger.Error().Err(err).Msgf("tryExtractOpRet: error decoding OP_RETURN memo: %s", tx.Vout[1].ScriptPubKey.Hex) return nil diff --git a/zetaclient/chains/bitcoin/tx_script.go b/zetaclient/chains/bitcoin/tx_script.go index f5bc856d5d..1f2a896a7f 100644 --- a/zetaclient/chains/bitcoin/tx_script.go +++ b/zetaclient/chains/bitcoin/tx_script.go @@ -2,10 +2,8 @@ package bitcoin // #nosec G507 ripemd160 required for bitcoin address encoding import ( - "bytes" "encoding/hex" "fmt" - "strconv" "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" @@ -16,7 +14,6 @@ import ( "golang.org/x/crypto/ripemd160" "github.com/zeta-chain/node/pkg/chains" - "github.com/zeta-chain/node/pkg/constant" ) const ( @@ -169,27 +166,49 @@ func DecodeScriptP2PKH(scriptHex string, net *chaincfg.Params) (string, error) { // DecodeOpReturnMemo decodes memo from OP_RETURN script // returns (memo, found, error) -func DecodeOpReturnMemo(scriptHex string, txid string) ([]byte, bool, error) { - if len(scriptHex) >= 4 && scriptHex[:2] == "6a" { // OP_RETURN - memoSize, err := strconv.ParseInt(scriptHex[2:4], 16, 32) - if err != nil { - return nil, false, errors.Wrapf(err, "error decoding memo size: %s", scriptHex) +func DecodeOpReturnMemo(scriptHex string) ([]byte, bool, error) { + // decode hex script + scriptBytes, err := hex.DecodeString(scriptHex) + if err != nil { + return nil, false, errors.Wrapf(err, "error decoding script hex: %s", scriptHex) + } + + // skip non-OP_RETURN script + // OP_RETURN script has to be at least 2 bytes: [OP_RETURN + dataLen] + if len(scriptBytes) < 2 || scriptBytes[0] != txscript.OP_RETURN { + return nil, false, nil + } + + // extract appended data in the OP_RETURN script + var memoBytes []byte + var memoSize = scriptBytes[1] + switch { + case memoSize < txscript.OP_PUSHDATA1: + // memo size has to match the actual data + if int(memoSize) != (len(scriptBytes) - 2) { + return nil, false, fmt.Errorf("memo size mismatch: %d != %d", memoSize, (len(scriptBytes) - 2)) } - if int(memoSize) != (len(scriptHex)-4)/2 { - return nil, false, fmt.Errorf("memo size mismatch: %d != %d", memoSize, (len(scriptHex)-4)/2) + memoBytes = scriptBytes[2:] + case memoSize == txscript.OP_PUSHDATA1: + // when data size >= OP_PUSHDATA1 (76), Bitcoin uses 2 bytes to represent the length: [OP_PUSHDATA1 + dataLen] + // see: https://github.com/btcsuite/btcd/blob/master/txscript/scriptbuilder.go#L183 + if len(scriptBytes) < 3 { + return nil, false, fmt.Errorf("script too short: %s", scriptHex) } + memoSize = scriptBytes[2] - memoBytes, err := hex.DecodeString(scriptHex[4:]) - if err != nil { - return nil, false, errors.Wrapf(err, "error hex decoding memo: %s", scriptHex) - } - if bytes.Equal(memoBytes, []byte(constant.DonationMessage)) { - return nil, false, fmt.Errorf("donation tx: %s", txid) + // memo size has to match the actual data + if int(memoSize) != (len(scriptBytes) - 3) { + return nil, false, fmt.Errorf("memo size mismatch: %d != %d", memoSize, (len(scriptBytes) - 3)) } - return memoBytes, true, nil + memoBytes = scriptBytes[3:] + default: + // should never happen + // OP_RETURN script won't carry more than 80 bytes + return nil, false, fmt.Errorf("invalid OP_RETURN script: %s", scriptHex) } - return nil, false, nil + return memoBytes, true, nil } // DecodeScript decodes memo wrapped in an inscription like script in witness diff --git a/zetaclient/chains/bitcoin/tx_script_test.go b/zetaclient/chains/bitcoin/tx_script_test.go index cf54b4553f..1af2966223 100644 --- a/zetaclient/chains/bitcoin/tx_script_test.go +++ b/zetaclient/chains/bitcoin/tx_script_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "github.com/zeta-chain/node/pkg/chains" - "github.com/zeta-chain/node/pkg/constant" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/testutils" ) @@ -341,24 +340,26 @@ func TestDecodeOpReturnMemo(t *testing.T) { require.Equal(t, scriptHex, rawResult.Vout[1].ScriptPubKey.Hex) t.Run("should decode memo from OP_RETURN output", func(t *testing.T) { - memo, found, err := bitcoin.DecodeOpReturnMemo(rawResult.Vout[1].ScriptPubKey.Hex, txHash) + memo, found, err := bitcoin.DecodeOpReturnMemo(rawResult.Vout[1].ScriptPubKey.Hex) require.NoError(t, err) require.True(t, found) // [OP_RETURN, 0x14,<20-byte-hash>] require.Equal(t, scriptHex[4:], hex.EncodeToString(memo)) }) + t.Run("should return nil memo non-OP_RETURN output", func(t *testing.T) { // modify the OP_RETURN to OP_1 scriptInvalid := strings.Replace(scriptHex, "6a", "51", 1) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid, txHash) + memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid) require.NoError(t, err) require.False(t, found) require.Nil(t, memo) }) - t.Run("should return nil memo on invalid script", func(t *testing.T) { + + t.Run("should return nil memo on script less than 2 byte", func(t *testing.T) { // use known short script scriptInvalid := "00" - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid, txHash) + memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid) require.NoError(t, err) require.False(t, found) require.Nil(t, memo) @@ -366,45 +367,48 @@ func TestDecodeOpReturnMemo(t *testing.T) { } func TestDecodeOpReturnMemoErrors(t *testing.T) { - // https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa - txHash := "847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa" - scriptHex := "6a1467ed0bcc4e1256bc2ce87d22e190d63a120114bf" - - t.Run("should return error on invalid memo size", func(t *testing.T) { - // use invalid memo size - scriptInvalid := strings.Replace(scriptHex, "6a14", "6axy", 1) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid, txHash) - require.ErrorContains(t, err, "error decoding memo size") - require.False(t, found) - require.Nil(t, memo) - }) - - t.Run("should return error on memo size mismatch", func(t *testing.T) { - // use wrong memo size - scriptInvalid := strings.Replace(scriptHex, "6a14", "6a13", 1) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid, txHash) - require.ErrorContains(t, err, "memo size mismatch") - require.False(t, found) - require.Nil(t, memo) - }) - - t.Run("should return error on invalid hex", func(t *testing.T) { - // use invalid hex - scriptInvalid := strings.Replace(scriptHex, "6a1467", "6a14xy", 1) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid, txHash) - require.ErrorContains(t, err, "error hex decoding memo") - require.False(t, found) - require.Nil(t, memo) - }) + tests := []struct { + name string + scriptHex string + errMsg string + }{ + { + name: "should return error on invalid hex", + scriptHex: "6a14xy", + errMsg: "error decoding script hex", + }, + { + name: "should return error on memo size < 76 (OP_PUSHDATA1) mismatch", + scriptHex: "6a15" + // 20 bytes memo, but length is set to 21(0x15) + "67ed0bcc4e1256bc2ce87d22e190d63a120114bf", + errMsg: "memo size mismatch", + }, + { + name: "should return error when memo size >= 76 (OP_PUSHDATA1) but script is too short", + scriptHex: "6a4c", // 2 bytes only, requires at least 3 bytes + errMsg: "script too short", + }, + { + name: "should return error on memo size >= 76 (OP_PUSHDATA1) mismatch", + scriptHex: "6a4c4e" + // 79 bytes memo, but length is set to 78(0x4e) + "5a0110070a30d55c1031d30dab3b3d85f47b8f1d03df2d480961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432", + errMsg: "memo size mismatch", + }, + { + name: "should return error on invalid OP_RETURN", + scriptHex: "6a4d0001", // OP_PUSHDATA2, length is set to 256 (0x0001, little-endian) + errMsg: "invalid OP_RETURN script", + }, + } - t.Run("should return nil memo on donation tx", func(t *testing.T) { - // use donation sctipt "6a0a4920616d207269636821" - scriptDonation := "6a0a" + hex.EncodeToString([]byte(constant.DonationMessage)) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptDonation, txHash) - require.ErrorContains(t, err, "donation tx") - require.False(t, found) - require.Nil(t, memo) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + memo, found, err := bitcoin.DecodeOpReturnMemo(tt.scriptHex) + require.ErrorContains(t, err, tt.errMsg) + require.False(t, found) + require.Nil(t, memo) + }) + } } func TestDecodeSenderFromScript(t *testing.T) { From 68ff67d06caa903a50f1ae0ee1b2c219523e7616 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Tue, 22 Oct 2024 01:02:28 -0500 Subject: [PATCH 03/10] add more unit tests --- changelog.md | 1 + cmd/zetae2e/local/local.go | 86 ++--- e2e/runner/bitcoin.go | 2 - testutil/helpers.go | 10 + testutil/sample/crypto.go | 15 + zetaclient/chains/bitcoin/observer/event.go | 145 +++++++- .../chains/bitcoin/observer/event_test.go | 311 +++++++++++++++++- zetaclient/chains/bitcoin/observer/inbound.go | 133 -------- .../chains/bitcoin/observer/inbound_test.go | 104 +++--- 9 files changed, 582 insertions(+), 225 deletions(-) diff --git a/changelog.md b/changelog.md index 2dd8e75f9a..a447359fe6 100644 --- a/changelog.md +++ b/changelog.md @@ -20,6 +20,7 @@ * [2896](https://github.com/zeta-chain/node/pull/2896) - add TON inbound observation * [2987](https://github.com/zeta-chain/node/pull/2987) - add non-EVM standard inbound memo package * [2979](https://github.com/zeta-chain/node/pull/2979) - add fungible keeper ability to lock/unlock ZRC20 tokens +* [3025](https://github.com/zeta-chain/node/pull/3025) - standard memo for Bitcoin inbound ### Refactor diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index 9b11a9a3f2..922245e894 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -268,78 +268,78 @@ func localE2ETest(cmd *cobra.Command, _ []string) { if !skipRegular { // defines all tests, if light is enabled, only the most basic tests are run and advanced are skipped erc20Tests := []string{ - e2etests.TestERC20WithdrawName, - e2etests.TestMultipleERC20WithdrawsName, - e2etests.TestERC20DepositAndCallRefundName, - e2etests.TestZRC20SwapName, + // e2etests.TestERC20WithdrawName, + // e2etests.TestMultipleERC20WithdrawsName, + // e2etests.TestERC20DepositAndCallRefundName, + // e2etests.TestZRC20SwapName, } erc20AdvancedTests := []string{ - e2etests.TestERC20DepositRestrictedName, + //e2etests.TestERC20DepositRestrictedName, } zetaTests := []string{ - e2etests.TestZetaWithdrawName, - e2etests.TestMessagePassingExternalChainsName, - e2etests.TestMessagePassingRevertFailExternalChainsName, - e2etests.TestMessagePassingRevertSuccessExternalChainsName, + // e2etests.TestZetaWithdrawName, + // e2etests.TestMessagePassingExternalChainsName, + // e2etests.TestMessagePassingRevertFailExternalChainsName, + // e2etests.TestMessagePassingRevertSuccessExternalChainsName, } zetaAdvancedTests := []string{ - e2etests.TestZetaDepositRestrictedName, - e2etests.TestZetaDepositName, - e2etests.TestZetaDepositNewAddressName, + // e2etests.TestZetaDepositRestrictedName, + // e2etests.TestZetaDepositName, + // e2etests.TestZetaDepositNewAddressName, } zevmMPTests := []string{} zevmMPAdvancedTests := []string{ - e2etests.TestMessagePassingZEVMToEVMName, - e2etests.TestMessagePassingEVMtoZEVMName, - e2etests.TestMessagePassingEVMtoZEVMRevertName, - e2etests.TestMessagePassingZEVMtoEVMRevertName, - e2etests.TestMessagePassingZEVMtoEVMRevertFailName, - e2etests.TestMessagePassingEVMtoZEVMRevertFailName, + // e2etests.TestMessagePassingZEVMToEVMName, + // e2etests.TestMessagePassingEVMtoZEVMName, + // e2etests.TestMessagePassingEVMtoZEVMRevertName, + // e2etests.TestMessagePassingZEVMtoEVMRevertName, + // e2etests.TestMessagePassingZEVMtoEVMRevertFailName, + // e2etests.TestMessagePassingEVMtoZEVMRevertFailName, } bitcoinTests := []string{ - e2etests.TestBitcoinDepositName, - e2etests.TestBitcoinDepositAndCallName, + // e2etests.TestBitcoinDepositName, + // e2etests.TestBitcoinDepositAndCallName, e2etests.TestBitcoinDepositAndCallRevertName, e2etests.TestBitcoinDonationName, e2etests.TestBitcoinStdMemoDepositName, e2etests.TestBitcoinStdMemoDepositAndCallName, e2etests.TestBitcoinStdMemoDepositAndCallRevertName, e2etests.TestBitcoinStdMemoDepositAndCallRevertOtherAddressName, - e2etests.TestBitcoinWithdrawSegWitName, - e2etests.TestBitcoinWithdrawInvalidAddressName, - e2etests.TestZetaWithdrawBTCRevertName, - e2etests.TestCrosschainSwapName, + // e2etests.TestBitcoinWithdrawSegWitName, + // e2etests.TestBitcoinWithdrawInvalidAddressName, + // e2etests.TestZetaWithdrawBTCRevertName, + // e2etests.TestCrosschainSwapName, } bitcoinAdvancedTests := []string{ - e2etests.TestBitcoinWithdrawTaprootName, - e2etests.TestBitcoinWithdrawLegacyName, - e2etests.TestBitcoinWithdrawMultipleName, - e2etests.TestBitcoinWithdrawP2SHName, - e2etests.TestBitcoinWithdrawP2WSHName, - e2etests.TestBitcoinWithdrawRestrictedName, + // e2etests.TestBitcoinWithdrawTaprootName, + // e2etests.TestBitcoinWithdrawLegacyName, + // e2etests.TestBitcoinWithdrawMultipleName, + // e2etests.TestBitcoinWithdrawP2SHName, + // e2etests.TestBitcoinWithdrawP2WSHName, + // e2etests.TestBitcoinWithdrawRestrictedName, } ethereumTests := []string{ - e2etests.TestEtherWithdrawName, - e2etests.TestContextUpgradeName, - e2etests.TestEtherDepositAndCallName, - e2etests.TestEtherDepositAndCallRefundName, + // e2etests.TestEtherWithdrawName, + // e2etests.TestContextUpgradeName, + // e2etests.TestEtherDepositAndCallName, + // e2etests.TestEtherDepositAndCallRefundName, } ethereumAdvancedTests := []string{ - e2etests.TestEtherWithdrawRestrictedName, + //e2etests.TestEtherWithdrawRestrictedName, } precompiledContractTests := []string{} if !skipPrecompiles { precompiledContractTests = []string{ - e2etests.TestPrecompilesPrototypeName, - e2etests.TestPrecompilesPrototypeThroughContractName, - e2etests.TestPrecompilesStakingName, - // Disabled until further notice, check https://github.com/zeta-chain/node/issues/3005. - // e2etests.TestPrecompilesStakingThroughContractName, - e2etests.TestPrecompilesBankName, - e2etests.TestPrecompilesBankFailName, - e2etests.TestPrecompilesBankThroughContractName, + // e2etests.TestPrecompilesPrototypeName, + // e2etests.TestPrecompilesPrototypeThroughContractName, + // e2etests.TestPrecompilesStakingName, + // // Disabled until further notice, check https://github.com/zeta-chain/node/issues/3005. + // // e2etests.TestPrecompilesStakingThroughContractName, + // e2etests.TestPrecompilesBankName, + // e2etests.TestPrecompilesBankFailName, + // e2etests.TestPrecompilesBankThroughContractName, } } diff --git a/e2e/runner/bitcoin.go b/e2e/runner/bitcoin.go index c822e63d0c..21cd2cac38 100644 --- a/e2e/runner/bitcoin.go +++ b/e2e/runner/bitcoin.go @@ -197,8 +197,6 @@ func (r *E2ERunner) DepositBTCWithStandardMemo( memoBytes, err := memoStd.EncodeToBytes() require.NoError(r, err) - r.Logger.Print("Standard memoBytes: %s, length: %d", hex.EncodeToString(memoBytes), len(memoBytes)) - return r.SendToTSSFromDeployerWithMemo(amount, inputUTXOs, memoBytes) } diff --git a/testutil/helpers.go b/testutil/helpers.go index dc49a0b024..f39c4931fb 100644 --- a/testutil/helpers.go +++ b/testutil/helpers.go @@ -1,11 +1,14 @@ package testutil import ( + "encoding/hex" "fmt" "os" "strings" + "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const helpersFile = "testutil/helpers.go" @@ -36,3 +39,10 @@ func exit(err error) { os.Exit(1) } + +// HexToBytes convert hex string to bytes +func HexToBytes(t *testing.T, hexStr string) []byte { + bytes, err := hex.DecodeString(hexStr) + require.NoError(t, err) + return bytes +} diff --git a/testutil/sample/crypto.go b/testutil/sample/crypto.go index e14b64f967..144b7d8e68 100644 --- a/testutil/sample/crypto.go +++ b/testutil/sample/crypto.go @@ -7,6 +7,9 @@ import ( "strconv" "testing" + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/cometbft/cometbft/crypto/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" @@ -57,6 +60,18 @@ func EthAddress() ethcommon.Address { return ethcommon.BytesToAddress(sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()).Bytes()) } +// BtcAddressP2WPKH returns a sample btc P2WPKH address +func BtcAddressP2WPKH(t *testing.T, net *chaincfg.Params) string { + privateKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + pubKeyHash := btcutil.Hash160(privateKey.PubKey().SerializeCompressed()) + addr, err := btcutil.NewAddressWitnessPubKeyHash(pubKeyHash, net) + require.NoError(t, err) + + return addr.String() +} + // SolanaPrivateKey returns a sample solana private key func SolanaPrivateKey(t *testing.T) solana.PrivateKey { privKey, err := solana.NewRandomPrivateKey() diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index b1988c7d9c..2021c19632 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -2,10 +2,23 @@ package observer import ( "bytes" + "encoding/hex" + "fmt" + "math/big" + cosmosmath "cosmossdk.io/math" + ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/pkg/errors" + + "github.com/zeta-chain/node/pkg/chains" + "github.com/zeta-chain/node/pkg/coin" "github.com/zeta-chain/node/pkg/constant" + "github.com/zeta-chain/node/pkg/crypto" "github.com/zeta-chain/node/pkg/memo" + crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" + "github.com/zeta-chain/node/zetaclient/compliance" "github.com/zeta-chain/node/zetaclient/config" + "github.com/zeta-chain/node/zetaclient/logs" ) // InboundProcessability is an enum representing the processability of an inbound @@ -27,7 +40,7 @@ type BTCInboundEvent struct { // FromAddress is the first input address FromAddress string - // ToAddress is the TSS address + // ToAddress is the ZEVM receiver address ToAddress string // Value is the amount of BTC @@ -74,3 +87,133 @@ func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { return InboundProcessabilityGood } + +// DecodeEventMemoBytes decodes the memo bytes as either standard or legacy memo +func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { + // skip decoding donation tx as it won't go through zetacore + if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { + return nil + } + + // try to decode the standard memo as the preferred format + var receiver ethcommon.Address + memoStd, err := memo.DecodeFromBytes(event.MemoBytes) + switch { + case err == nil: + // NoAssetCall will be disabled for Bitcoin until full V2 support + if memoStd.OpCode == memo.OpCodeCall { + return errors.New("NoAssetCall is disabled") + } + + // ensure the revert address is valid and supported + revertAddress := memoStd.RevertOptions.RevertAddress + if revertAddress != "" { + btcAddress, err := chains.DecodeBtcAddress(revertAddress, ob.Chain().ChainId) + if err != nil { + return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) + } + if !chains.IsBtcAddressSupported(btcAddress) { + return fmt.Errorf("unsupported revert address in memo: %s", revertAddress) + } + } + event.MemoStd = memoStd + receiver = memoStd.Receiver + default: + // fallback to legacy memo if standard memo decoding fails + parsedAddress, _, err := memo.DecodeLegacyMemoHex(hex.EncodeToString(event.MemoBytes)) + if err != nil { // never happens + return errors.Wrap(err, "invalid legacy memo") + } + receiver = parsedAddress + } + + // ensure the receiver is valid + if crypto.IsEmptyAddress(receiver) { + return errors.New("got empty receiver address from memo") + } + event.ToAddress = receiver.Hex() + + return nil +} + +// CheckEventProcessability checks if the inbound event is processable +func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { + // check if the event is processable + result := event.CheckProcessability() + switch result { + case InboundProcessabilityGood: + return true + case InboundProcessabilityDonation: + logFields := map[string]any{ + logs.FieldChain: ob.Chain().ChainId, + logs.FieldTx: event.TxHash, + } + ob.Logger().Inbound.Info().Fields(logFields).Msgf("thank you rich folk for your donation!") + return false + case InboundProcessabilityComplianceViolation: + compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance, + false, ob.Chain().ChainId, event.TxHash, event.FromAddress, event.ToAddress, "BTC") + return false + } + + return true +} + +// NewInboundVoteV1 creates a MsgVoteInbound message for inbound that uses legacy memo +func (ob *Observer) NewInboundVoteV1(event *BTCInboundEvent, amountSats *big.Int) *crosschaintypes.MsgVoteInbound { + message := hex.EncodeToString(event.MemoBytes) + + return crosschaintypes.NewMsgVoteInbound( + ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), + event.FromAddress, + ob.Chain().ChainId, + event.FromAddress, + event.ToAddress, + ob.ZetacoreClient().Chain().ChainId, + cosmosmath.NewUintFromBigInt(amountSats), + message, + event.TxHash, + event.BlockNumber, + 0, + coin.CoinType_Gas, + "", + 0, + crosschaintypes.ProtocolContractVersion_V1, + false, // not relevant for v1 + ) +} + +// NewInboundVoteMemoStd creates a MsgVoteInbound message for inbound that uses standard memo +// TODO: rename this function as 'EventToInboundVoteV2' to use ProtocolContractVersion_V2 and enable more options +// https://github.com/zeta-chain/node/issues/2711 +func (ob *Observer) NewInboundVoteMemoStd(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 + } + + // make a legacy message so that zetacore can process it as V1 + msgBytes := append(event.MemoStd.Receiver.Bytes(), event.MemoStd.Payload...) + message := hex.EncodeToString(msgBytes) + + return crosschaintypes.NewMsgVoteInbound( + ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), + sender, + ob.Chain().ChainId, + event.FromAddress, + event.ToAddress, + ob.ZetacoreClient().Chain().ChainId, + cosmosmath.NewUintFromBigInt(amountSats), + message, + event.TxHash, + event.BlockNumber, + 0, + coin.CoinType_Gas, + "", + 0, + crosschaintypes.ProtocolContractVersion_V1, + false, // not relevant for v1 + ) +} diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index abe43f85b3..964bb086b7 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -1,30 +1,51 @@ package observer_test import ( + "encoding/hex" + "math/big" "testing" + cosmosmath "cosmossdk.io/math" + "github.com/btcsuite/btcd/chaincfg" + "github.com/zeta-chain/node/testutil" crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" + "github.com/zeta-chain/node/pkg/chains" + "github.com/zeta-chain/node/pkg/coin" "github.com/zeta-chain/node/pkg/constant" "github.com/zeta-chain/node/pkg/memo" "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/zetaclient/chains/bitcoin/observer" "github.com/zeta-chain/node/zetaclient/config" + "github.com/zeta-chain/node/zetaclient/keys" "github.com/zeta-chain/node/zetaclient/testutils" + "github.com/zeta-chain/node/zetaclient/testutils/mocks" ) +// createTestBtcEvent creates a test BTC inbound event +func createTestBtcEvent( + t *testing.T, + net *chaincfg.Params, + memo []byte, + memoStd *memo.InboundMemo, +) *observer.BTCInboundEvent { + return &observer.BTCInboundEvent{ + FromAddress: sample.BtcAddressP2WPKH(t, net), + ToAddress: sample.EthAddress().Hex(), + MemoBytes: memo, + MemoStd: memoStd, + TxHash: sample.Hash().Hex(), + BlockNumber: 123456, + } +} + func Test_CheckProcessability(t *testing.T) { // setup compliance config cfg := config.Config{ - ComplianceConfig: config.ComplianceConfig{}, + ComplianceConfig: sample.ComplianceConfig(), } - - // add restricted address - restrictedBtcAddress := sample.RestrictedBtcAddressTest - restrictedEvmAddress := sample.RestrictedEVMAddressTest - cfg.ComplianceConfig.RestrictedAddresses = []string{restrictedBtcAddress, restrictedEvmAddress} config.LoadComplianceConfig(cfg) // test cases @@ -44,7 +65,7 @@ func Test_CheckProcessability(t *testing.T) { { name: "should return InboundProcessabilityComplianceViolation for a restricted sender address", event: &observer.BTCInboundEvent{ - FromAddress: restrictedBtcAddress, + FromAddress: sample.RestrictedBtcAddressTest, ToAddress: testutils.TSSAddressBTCAthens3, }, expected: observer.InboundProcessabilityComplianceViolation, @@ -56,7 +77,7 @@ func Test_CheckProcessability(t *testing.T) { ToAddress: testutils.TSSAddressBTCAthens3, MemoStd: &memo.InboundMemo{ FieldsV0: memo.FieldsV0{ - Receiver: common.HexToAddress(restrictedEvmAddress), + Receiver: common.HexToAddress(sample.RestrictedEVMAddressTest), }, }, }, @@ -70,7 +91,7 @@ func Test_CheckProcessability(t *testing.T) { MemoStd: &memo.InboundMemo{ FieldsV0: memo.FieldsV0{ RevertOptions: crosschaintypes.RevertOptions{ - RevertAddress: restrictedBtcAddress, + RevertAddress: sample.RestrictedBtcAddressTest, }, }, }, @@ -95,3 +116,275 @@ func Test_CheckProcessability(t *testing.T) { }) } } + +func Test_DecodeEventMemoBytes(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinTestnet + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + + // test cases + tests := []struct { + name string + event *observer.BTCInboundEvent + expectedMemoStd *memo.InboundMemo + expectedReceiver common.Address + donation bool + errMsg string + }{ + { + name: "should decode standard memo bytes successfully", + event: &observer.BTCInboundEvent{ + // a deposit and call + MemoBytes: testutil.HexToBytes( + t, + "5a0110032d07a9cbd57dcca3e2cf966c88bc874445b6e3b60d68656c6c6f207361746f736869", + ), + }, + expectedMemoStd: &memo.InboundMemo{ + Header: memo.Header{ + Version: 0, + EncodingFmt: memo.EncodingFmtCompactShort, + OpCode: memo.OpCodeDepositAndCall, + DataFlags: 3, // reciever + payload + }, + FieldsV0: memo.FieldsV0{ + Receiver: common.HexToAddress("0x2D07A9CBd57DCca3E2cF966C88Bc874445b6E3B6"), + Payload: []byte("hello satoshi"), + }, + }, + }, + { + name: "should fall back to legacy memo successfully", + event: &observer.BTCInboundEvent{ + // raw address + payload + MemoBytes: testutil.HexToBytes(t, "2d07a9cbd57dcca3e2cf966c88bc874445b6e3b668656c6c6f207361746f736869"), + }, + expectedReceiver: common.HexToAddress("0x2D07A9CBd57DCca3E2cF966C88Bc874445b6E3B6"), + }, + { + name: "should do nothing for donation message", + event: &observer.BTCInboundEvent{ + MemoBytes: []byte(constant.DonationMessage), + }, + donation: true, + }, + { + name: "NoAssetCall is not disabled at the moment", + event: &observer.BTCInboundEvent{ + // a no asset call + MemoBytes: testutil.HexToBytes( + t, + "5a0120032d07a9cbd57dcca3e2cf966c88bc874445b6e3b60d68656c6c6f207361746f736869", + ), + }, + errMsg: "NoAssetCall is disabled", + }, + { + name: "should return error on invalid revert address", + event: &observer.BTCInboundEvent{ + // raw address + payload + revert address + // but the address is "bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2" which is not a testnet address + MemoBytes: testutil.HexToBytes( + t, + "5a0110075ab13400c33b83ca9d3ee5587486c26639a5ef190961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432", + ), + }, + errMsg: "invalid revert address in memo", + }, + { + name: "should return error if revert address is not a supported address type", + event: &observer.BTCInboundEvent{ + // raw address + payload + revert address + // but the revert address is "035e4ae279bd416b5da724972c9061ec6298dac020d1e3ca3f06eae715135cdbec" and it's not supported + MemoBytes: testutil.HexToBytes( + t, + "5a0110072d07a9cbd57dcca3e2cf966c88bc874445b6e3b60961207061796c6f616442303335653461653237396264343136623564613732343937326339303631656336323938646163303230643165336361336630366561653731353133356364626563", + ), + }, + errMsg: "unsupported revert address in memo", + }, + { + name: "should return error on empty receiver address", + event: &observer.BTCInboundEvent{ + // standard memo that carries payload only, receiver address is empty + MemoBytes: testutil.HexToBytes(t, "5a0110020d68656c6c6f207361746f736869"), + }, + errMsg: "got empty receiver address from memo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ob.DecodeEventMemoBytes(tt.event) + if tt.errMsg != "" { + require.Contains(t, err.Error(), tt.errMsg) + return + } + require.NoError(t, err) + + // donation message will skip decoding, so ToAddress will be left empty + if tt.donation { + require.Empty(t, tt.event.ToAddress) + return + } + + // if it's a standard memo + if tt.expectedMemoStd != nil { + require.NotNil(t, tt.event.MemoStd) + require.Equal(t, tt.expectedMemoStd.Receiver.Hex(), tt.event.ToAddress) + require.Equal(t, tt.expectedMemoStd, tt.event.MemoStd) + } else { + // if it's a legacy memo, check receiver address only + require.Equal(t, tt.expectedReceiver.Hex(), tt.event.ToAddress) + } + }) + } +} + +func Test_CheckEventProcessability(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinTestnet + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + + // setup compliance config + cfg := config.Config{ + ComplianceConfig: sample.ComplianceConfig(), + } + config.LoadComplianceConfig(cfg) + + // test cases + tests := []struct { + name string + event *observer.BTCInboundEvent + result bool + }{ + { + name: "should return true for processable event", + event: createTestBtcEvent(t, &chaincfg.MainNetParams, []byte("a memo"), nil), + result: true, + }, + { + name: "should return false on donation message", + event: createTestBtcEvent(t, &chaincfg.MainNetParams, []byte(constant.DonationMessage), nil), + result: false, + }, + { + name: "should return false on compliance violation", + event: createTestBtcEvent(t, &chaincfg.MainNetParams, []byte("a memo"), &memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + Receiver: common.HexToAddress(sample.RestrictedEVMAddressTest), + }, + }), + result: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ob.CheckEventProcessability(tt.event) + require.Equal(t, tt.result, result) + }) + } +} + +func Test_NewInboundVoteV1(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinMainnet + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + zetacoreClient := mocks.NewZetacoreClient(t).WithKeys(&keys.Keys{}).WithZetaChain() + ob.WithZetacoreClient(zetacoreClient) + + t.Run("should create new inbound vote msg V1", func(t *testing.T) { + // create test event + event := createTestBtcEvent(t, &chaincfg.MainNetParams, []byte("dummy memo"), nil) + + // test amount + amountSats := big.NewInt(1000) + + // expected vote + expectedVote := crosschaintypes.MsgVoteInbound{ + Sender: event.FromAddress, + SenderChainId: chain.ChainId, + TxOrigin: event.FromAddress, + Receiver: event.ToAddress, + ReceiverChain: ob.ZetacoreClient().Chain().ChainId, + Amount: cosmosmath.NewUint(amountSats.Uint64()), + Message: hex.EncodeToString(event.MemoBytes), + InboundHash: event.TxHash, + InboundBlockHeight: event.BlockNumber, + CallOptions: &crosschaintypes.CallOptions{ + GasLimit: 0, + }, + CoinType: coin.CoinType_Gas, + ProtocolContractVersion: crosschaintypes.ProtocolContractVersion_V1, + RevertOptions: crosschaintypes.NewEmptyRevertOptions(), // ignored by V1 + } + + // create new inbound vote V1 + vote := ob.NewInboundVoteV1(event, amountSats) + require.Equal(t, expectedVote, *vote) + }) +} + +func Test_NewInboundVoteMemoStd(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinMainnet + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + zetacoreClient := mocks.NewZetacoreClient(t).WithKeys(&keys.Keys{}).WithZetaChain() + ob.WithZetacoreClient(zetacoreClient) + + t.Run("should create new inbound vote msg with standard memo", func(t *testing.T) { + // create revert options + revertOptions := crosschaintypes.NewEmptyRevertOptions() + revertOptions.RevertAddress = sample.BtcAddressP2WPKH(t, &chaincfg.MainNetParams) + + // create test event + receiver := sample.EthAddress() + event := createTestBtcEvent(t, &chaincfg.MainNetParams, []byte("dymmy"), &memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + Receiver: receiver, + Payload: []byte("some payload"), + RevertOptions: revertOptions, + }, + }) + + // test amount + amountSats := big.NewInt(1000) + + // expected vote + memoBytesExpected := append(event.MemoStd.Receiver.Bytes(), event.MemoStd.Payload...) + expectedVote := crosschaintypes.MsgVoteInbound{ + Sender: revertOptions.RevertAddress, // should be overridden by revert address + SenderChainId: chain.ChainId, + TxOrigin: event.FromAddress, + Receiver: event.ToAddress, + ReceiverChain: ob.ZetacoreClient().Chain().ChainId, + Amount: cosmosmath.NewUint(amountSats.Uint64()), + Message: hex.EncodeToString(memoBytesExpected), // a simulated legacy memo + InboundHash: event.TxHash, + InboundBlockHeight: event.BlockNumber, + CallOptions: &crosschaintypes.CallOptions{ + GasLimit: 0, + }, + CoinType: coin.CoinType_Gas, + ProtocolContractVersion: crosschaintypes.ProtocolContractVersion_V1, + RevertOptions: crosschaintypes.NewEmptyRevertOptions(), // ignored by V1 + } + + // create new inbound vote V1 with standard memo + vote := ob.NewInboundVoteMemoStd(event, amountSats) + require.Equal(t, expectedVote, *vote) + }) +} diff --git a/zetaclient/chains/bitcoin/observer/inbound.go b/zetaclient/chains/bitcoin/observer/inbound.go index e76dc0a1e0..fdde56b20a 100644 --- a/zetaclient/chains/bitcoin/observer/inbound.go +++ b/zetaclient/chains/bitcoin/observer/inbound.go @@ -1,30 +1,22 @@ package observer import ( - "bytes" "context" "encoding/hex" "fmt" "math/big" - cosmosmath "cosmossdk.io/math" "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" - ethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" "github.com/rs/zerolog" - "github.com/zeta-chain/node/pkg/chains" "github.com/zeta-chain/node/pkg/coin" - "github.com/zeta-chain/node/pkg/constant" - "github.com/zeta-chain/node/pkg/crypto" - "github.com/zeta-chain/node/pkg/memo" crosschaintypes "github.com/zeta-chain/node/x/crosschain/types" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/chains/interfaces" - "github.com/zeta-chain/node/zetaclient/compliance" zctx "github.com/zeta-chain/node/zetaclient/context" "github.com/zeta-chain/node/zetaclient/logs" "github.com/zeta-chain/node/zetaclient/types" @@ -351,72 +343,6 @@ func FilterAndParseIncomingTx( return events, nil } -// CheckEventProcessability checks if the inbound event is processable -func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { - // check if the event is processable - result := event.CheckProcessability() - switch result { - case InboundProcessabilityGood: - return true - case InboundProcessabilityDonation: - logFields := map[string]any{ - logs.FieldChain: ob.Chain().ChainId, - logs.FieldTx: event.TxHash, - } - ob.Logger().Inbound.Info().Fields(logFields).Msgf("thank you rich folk for your donation!") - return false - case InboundProcessabilityComplianceViolation: - compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance, - false, ob.Chain().ChainId, event.TxHash, event.FromAddress, event.ToAddress, "BTC") - return false - } - - return true -} - -// DecodeEventMemoBytes decodes the memo bytes as either standard or legacy memo -func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { - // skip decoding donation tx as it won't go through zetacore - if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { - return nil - } - - // try to decode the standard memo as the preferred format - var receiver ethcommon.Address - memoStd, err := memo.DecodeFromBytes(event.MemoBytes) - if err == nil { - event.MemoStd = memoStd - receiver = memoStd.Receiver - } else { - // fallback to legacy memo if standard memo decoding fails - parsedAddress, _, err := memo.DecodeLegacyMemoHex(hex.EncodeToString(event.MemoBytes)) - if err != nil { - return errors.Wrap(err, "invalid legacy memo") - } - receiver = parsedAddress - } - - // ensure the receiver is valid - if crypto.IsEmptyAddress(receiver) { - return errors.New("got empty receiver address from memo") - } - event.ToAddress = receiver.Hex() - - // ensure the revert address is valid and supported - revertAddress := memoStd.RevertOptions.RevertAddress - if event.MemoStd != nil && revertAddress != "" { - btcAddress, err := chains.DecodeBtcAddress(revertAddress, ob.Chain().ChainId) - if err != nil { - return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) - } - if !chains.IsBtcAddressSupported(btcAddress) { - return fmt.Errorf("unsupported revert address in memo: %s", revertAddress) - } - } - - return nil -} - // GetInboundVoteFromBtcEvent converts a BTCInboundEvent to a MsgVoteInbound to enable voting on the inbound on zetacore func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosschaintypes.MsgVoteInbound { // prepare logger fields @@ -453,65 +379,6 @@ func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosscha return ob.NewInboundVoteMemoStd(event, amountInt) } -// NewInboundVoteV1 creates a MsgVoteInbound message for inbound that uses legacy memo -func (ob *Observer) NewInboundVoteV1(event *BTCInboundEvent, amountSats *big.Int) *crosschaintypes.MsgVoteInbound { - message := hex.EncodeToString(event.MemoBytes) - - return crosschaintypes.NewMsgVoteInbound( - ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), - event.FromAddress, - ob.Chain().ChainId, - event.FromAddress, - event.ToAddress, - ob.ZetacoreClient().Chain().ChainId, - cosmosmath.NewUintFromBigInt(amountSats), - message, - event.TxHash, - event.BlockNumber, - 0, - coin.CoinType_Gas, - "", - 0, - crosschaintypes.ProtocolContractVersion_V1, - false, // not relevant for v1 - ) -} - -// NewInboundVoteMemoStd creates a MsgVoteInbound message for inbound that uses standard memo -// TODO: rename this function as 'EventToInboundVoteV2' to use ProtocolContractVersion_V2 and enable more options -// https://github.com/zeta-chain/node/issues/2711 -func (ob *Observer) NewInboundVoteMemoStd(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 - } - - // make a legacy message so that zetacore can process it - msgBytes := append(event.MemoStd.Receiver.Bytes(), event.MemoStd.Payload...) - message := hex.EncodeToString(msgBytes) - - return crosschaintypes.NewMsgVoteInbound( - ob.ZetacoreClient().GetKeys().GetOperatorAddress().String(), - sender, - ob.Chain().ChainId, - event.FromAddress, - event.ToAddress, - ob.ZetacoreClient().Chain().ChainId, - cosmosmath.NewUintFromBigInt(amountSats), - message, - event.TxHash, - event.BlockNumber, - 0, - coin.CoinType_Gas, - "", - 0, - crosschaintypes.ProtocolContractVersion_V1, - false, // not relevant for v1 - ) -} - // GetBtcEvent returns a valid BTCInboundEvent or nil // it uses witness data to extract the sender address, except for mainnet func GetBtcEvent( diff --git a/zetaclient/chains/bitcoin/observer/inbound_test.go b/zetaclient/chains/bitcoin/observer/inbound_test.go index 293987196d..a33de19194 100644 --- a/zetaclient/chains/bitcoin/observer/inbound_test.go +++ b/zetaclient/chains/bitcoin/observer/inbound_test.go @@ -2,9 +2,7 @@ package observer_test import ( "bytes" - "encoding/binary" "encoding/hex" - "fmt" "math" "path" "strings" @@ -14,16 +12,19 @@ import ( "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" - "github.com/btcsuite/btcd/txscript" "github.com/pkg/errors" "github.com/rs/zerolog/log" "github.com/stretchr/testify/mock" "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" + "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/chains/bitcoin/observer" clientcommon "github.com/zeta-chain/node/zetaclient/common" + "github.com/zeta-chain/node/zetaclient/keys" "github.com/zeta-chain/node/zetaclient/testutils" "github.com/zeta-chain/node/zetaclient/testutils/mocks" "github.com/zeta-chain/node/zetaclient/testutils/testrpc" @@ -141,6 +142,69 @@ func TestAvgFeeRateBlock828440Errors(t *testing.T) { }) } +func Test_GetInboundVoteFromBtcEvent(t *testing.T) { + // can use any bitcoin chain for testing + chain := chains.BitcoinMainnet + params := mocks.MockChainParams(chain.ChainId, 10) + + // create test observer + ob := MockBTCObserver(t, chain, params, nil) + zetacoreClient := mocks.NewZetacoreClient(t).WithKeys(&keys.Keys{}).WithZetaChain() + ob.WithZetacoreClient(zetacoreClient) + + // test cases + tests := []struct { + name string + event *observer.BTCInboundEvent + nilVote bool + }{ + { + name: "should return vote for standard memo", + event: &observer.BTCInboundEvent{ + FromAddress: sample.BtcAddressP2WPKH(t, &chaincfg.MainNetParams), + // a deposit and call + MemoBytes: testutil.HexToBytes( + t, + "5a0110032d07a9cbd57dcca3e2cf966c88bc874445b6e3b60d68656c6c6f207361746f736869", + ), + }, + }, + { + name: "should return vote for legacy memo", + event: &observer.BTCInboundEvent{ + // raw address + payload + MemoBytes: testutil.HexToBytes(t, "2d07a9cbd57dcca3e2cf966c88bc874445b6e3b668656c6c6f207361746f736869"), + }, + }, + { + name: "should return nil if unable to decode memo", + event: &observer.BTCInboundEvent{ + // standard memo that carries payload only, receiver address is empty + MemoBytes: testutil.HexToBytes(t, "5a0110020d68656c6c6f207361746f736869"), + }, + nilVote: true, + }, + { + name: "should return nil on donation message", + event: &observer.BTCInboundEvent{ + MemoBytes: []byte(constant.DonationMessage), + }, + nilVote: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msg := ob.GetInboundVoteFromBtcEvent(tt.event) + if tt.nilVote { + require.Nil(t, msg) + } else { + require.NotNil(t, msg) + } + }) + } +} + func TestGetSenderAddressByVin(t *testing.T) { // https://mempool.space/tx/3618e869f9e87863c0f1cc46dbbaa8b767b4a5d6d60b143c2c50af52b257e867 txHash := "3618e869f9e87863c0f1cc46dbbaa8b767b4a5d6d60b143c2c50af52b257e867" @@ -192,40 +256,6 @@ func TestGetSenderAddressByVin(t *testing.T) { }) } -func Test_DecodeEventMemoBytes(t *testing.T) { - // can use any bitcoin chain for testing - chain := chains.BitcoinRegtest - params := mocks.MockChainParams(chain.ChainId, 10) - - // create test observer - ob := MockBTCObserver(t, chain, params, nil) - - buf := make([]byte, 2) - binary.LittleEndian.PutUint16(buf, uint16(256)) - - t.Run("should decode standard memo bytes", func(t *testing.T) { - // create test standard memo bytes - //memoHex := "5a010001283d810090edf4043e75247eaebce848806237fd" - memoHex := "5a01100757bcbefff7c37b24602e378ce938a1a1851513e40961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432" + - "5a01100757bcbefff7c37b24602e378ce938a1a1851513e40961207061796c6f61642c626372743171793970716d6b3270" - memo, _ := hex.DecodeString(memoHex) - nullData, err := txscript.NullDataScript(memo) - require.NoError(t, err) - fmt.Printf("nullData: %s\n", hex.EncodeToString(nullData)) - - memoBytes, err := hex.DecodeString(memoHex) - require.NoError(t, err) - - event := &observer.BTCInboundEvent{ - MemoBytes: memoBytes, - } - - // decode memo bytes - err = ob.DecodeEventMemoBytes(event) - require.NoError(t, err) - }) -} - func TestGetBtcEventWithoutWitness(t *testing.T) { // load archived inbound P2WPKH raw result // https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa From 7a537bb46a5de1d5bf38580e8a47634149c3b2d1 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Tue, 22 Oct 2024 12:16:28 -0500 Subject: [PATCH 04/10] disable standard memo for Bitcoin mainnet --- pkg/memo/fields.go | 2 +- pkg/memo/fields_v0.go | 9 ++----- pkg/memo/fields_v0_test.go | 19 +------------- pkg/memo/memo.go | 17 ++++++++----- pkg/memo/memo_test.go | 25 ++++++++++++++++++- zetaclient/chains/bitcoin/observer/event.go | 20 ++++++++++++--- .../chains/bitcoin/observer/event_test.go | 11 ++++++++ 7 files changed, 67 insertions(+), 36 deletions(-) diff --git a/pkg/memo/fields.go b/pkg/memo/fields.go index fff853f955..e0415e0636 100644 --- a/pkg/memo/fields.go +++ b/pkg/memo/fields.go @@ -6,7 +6,7 @@ type Fields interface { Pack(opCode OpCode, encodingFmt EncodingFormat, dataFlags uint8) ([]byte, error) // Unpack decodes the memo fields - Unpack(opCode OpCode, encodingFmt EncodingFormat, dataFlags uint8, data []byte) error + Unpack(encodingFmt EncodingFormat, dataFlags uint8, data []byte) error // Validate checks if the fields are valid Validate(opCode OpCode, dataFlags uint8) error diff --git a/pkg/memo/fields_v0.go b/pkg/memo/fields_v0.go index a8f79d99ba..d30745b6a3 100644 --- a/pkg/memo/fields_v0.go +++ b/pkg/memo/fields_v0.go @@ -54,18 +54,13 @@ func (f *FieldsV0) Pack(opCode OpCode, encodingFmt EncodingFormat, dataFlags uin } // Unpack decodes the memo fields -func (f *FieldsV0) Unpack(opCode OpCode, encodingFmt EncodingFormat, dataFlags uint8, data []byte) error { +func (f *FieldsV0) Unpack(encodingFmt EncodingFormat, dataFlags uint8, data []byte) error { codec, err := GetCodec(encodingFmt) if err != nil { return errors.Wrap(err, "unable to get codec") } - err = f.unpackFields(codec, dataFlags, data) - if err != nil { - return err - } - - return f.Validate(opCode, dataFlags) + return f.unpackFields(codec, dataFlags, data) } // Validate checks if the fields are valid diff --git a/pkg/memo/fields_v0_test.go b/pkg/memo/fields_v0_test.go index 13742422c2..11cff3e1bd 100644 --- a/pkg/memo/fields_v0_test.go +++ b/pkg/memo/fields_v0_test.go @@ -125,7 +125,6 @@ func Test_V0_Unpack(t *testing.T) { tests := []struct { name string - opCode memo.OpCode encodeFmt memo.EncodingFormat dataFlags byte data []byte @@ -134,7 +133,6 @@ func Test_V0_Unpack(t *testing.T) { }{ { name: "unpack all fields with ABI encoding", - opCode: memo.OpCodeDepositAndCall, encodeFmt: memo.EncodingFmtABI, dataFlags: flagsAllFieldsSet, // all fields are set data: ABIPack(t, @@ -156,7 +154,6 @@ func Test_V0_Unpack(t *testing.T) { }, { name: "unpack all fields with compact encoding", - opCode: memo.OpCodeDepositAndCall, encodeFmt: memo.EncodingFmtCompactShort, dataFlags: flagsAllFieldsSet, // all fields are set data: CompactPack( @@ -179,7 +176,6 @@ func Test_V0_Unpack(t *testing.T) { }, { name: "unpack empty ABI encoded payload if flag is set", - opCode: memo.OpCodeDepositAndCall, encodeFmt: memo.EncodingFmtABI, dataFlags: 0b00000010, // payload flags are set data: ABIPack(t, @@ -188,7 +184,6 @@ func Test_V0_Unpack(t *testing.T) { }, { name: "unpack empty compact encoded payload if flag is set", - opCode: memo.OpCodeDepositAndCall, encodeFmt: memo.EncodingFmtCompactShort, dataFlags: 0b00000010, // payload flag is set data: CompactPack( @@ -198,7 +193,6 @@ func Test_V0_Unpack(t *testing.T) { }, { name: "unable to get codec on invalid encoding format", - opCode: memo.OpCodeDepositAndCall, encodeFmt: 0x0F, dataFlags: 0b00000001, data: []byte{}, @@ -206,7 +200,6 @@ func Test_V0_Unpack(t *testing.T) { }, { name: "failed to unpack ABI encoded data with compact encoding format", - opCode: memo.OpCodeDepositAndCall, encodeFmt: memo.EncodingFmtCompactShort, dataFlags: 0b00000011, // receiver and payload flags are set data: ABIPack(t, @@ -214,23 +207,13 @@ func Test_V0_Unpack(t *testing.T) { memo.ArgPayload(fBytes)), errMsg: "failed to unpack arguments", }, - { - name: "fields validation failed due to empty receiver address", - opCode: memo.OpCodeDepositAndCall, - encodeFmt: memo.EncodingFmtABI, - dataFlags: 0b00000011, // receiver and payload flags are set - data: ABIPack(t, - memo.ArgReceiver(common.Address{}), - memo.ArgPayload(fBytes)), - errMsg: "receiver address is empty", - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // unpack the fields fields := memo.FieldsV0{} - err := fields.Unpack(tc.opCode, tc.encodeFmt, tc.dataFlags, tc.data) + err := fields.Unpack(tc.encodeFmt, tc.dataFlags, tc.data) // validate the error message if tc.errMsg != "" { diff --git a/pkg/memo/memo.go b/pkg/memo/memo.go index 30670952b0..b1b801c539 100644 --- a/pkg/memo/memo.go +++ b/pkg/memo/memo.go @@ -51,7 +51,7 @@ func (m *InboundMemo) EncodeToBytes() ([]byte, error) { // DecodeFromBytes decodes a InboundMemo struct from raw bytes // -// Returns an error if given data is not a valid memo +// Returns nil memo if given data can't be decoded as a memo. func DecodeFromBytes(data []byte) (*InboundMemo, error) { memo := &InboundMemo{} @@ -64,15 +64,20 @@ func DecodeFromBytes(data []byte) (*InboundMemo, error) { // decode fields based on version switch memo.Version { case 0: - err = memo.FieldsV0.Unpack(memo.OpCode, memo.EncodingFmt, memo.Header.DataFlags, data[HeaderSize:]) + // unpack fields + err = memo.FieldsV0.Unpack(memo.EncodingFmt, memo.Header.DataFlags, data[HeaderSize:]) + if err != nil { + return nil, errors.Wrap(err, "failed to unpack memo FieldsV0") + } + + // validate fields + // Note: a well-formatted memo may still contain improper field values + err = memo.FieldsV0.Validate(memo.OpCode, memo.Header.DataFlags) default: return nil, fmt.Errorf("invalid memo version: %d", memo.Version) } - if err != nil { - return nil, errors.Wrapf(err, "failed to unpack memo fields version: %d", memo.Version) - } - return memo, nil + return memo, err } // DecodeLegacyMemoHex decodes hex encoded memo message into address and calldata diff --git a/pkg/memo/memo_test.go b/pkg/memo/memo_test.go index e6cb067793..381f256a79 100644 --- a/pkg/memo/memo_test.go +++ b/pkg/memo/memo_test.go @@ -155,6 +155,7 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { head []byte data []byte expectedMemo memo.InboundMemo + invalidField bool errMsg string }{ { @@ -251,7 +252,22 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { memo.EncodingFmtCompactShort, memo.ArgReceiver(fAddress), ), // but data is compact encoded - errMsg: "failed to unpack memo fields", + errMsg: "failed to unpack memo FieldsV0", + }, + { + name: "should return both memo struct and error if fields validation fails", + head: MakeHead( + 0, + uint8(memo.EncodingFmtABI), + uint8(memo.OpCodeDepositAndCall), + 0, + 0b00000011, // receiver flag is set + ), + data: ABIPack(t, + memo.ArgReceiver(common.Address{}), // empty receiver address provided + memo.ArgPayload(fBytes)), + invalidField: true, + errMsg: "receiver address is empty", }, } @@ -261,6 +277,13 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { memo, err := memo.DecodeFromBytes(data) if tt.errMsg != "" { require.ErrorContains(t, err, tt.errMsg) + + // invalid field values may still produce a memo + if tt.invalidField { + require.NotNil(t, memo) + } else { + require.Nil(t, memo) + } return } require.NoError(t, err) diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index 2021c19632..43def388a5 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -90,16 +90,30 @@ func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { // DecodeEventMemoBytes decodes the memo bytes as either standard or legacy memo func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { + var ( + err error + memoStd *memo.InboundMemo + receiver ethcommon.Address + ) + // skip decoding donation tx as it won't go through zetacore if bytes.Equal(event.MemoBytes, []byte(constant.DonationMessage)) { return nil } // try to decode the standard memo as the preferred format - var receiver ethcommon.Address - memoStd, err := memo.DecodeFromBytes(event.MemoBytes) + // the standard memo is NOT enabled for Bitcoin mainnet + if ob.Chain().ChainId != chains.BitcoinMainnet.ChainId { + memoStd, err = memo.DecodeFromBytes(event.MemoBytes) + } + switch { - case err == nil: + case memoStd != nil: + // skip memo that carries improper data + if err != nil { + return errors.Wrap(err, "standard memo contains improper data") + } + // NoAssetCall will be disabled for Bitcoin until full V2 support if memoStd.OpCode == memo.OpCodeCall { return errors.New("NoAssetCall is disabled") diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index 964bb086b7..0c8cdb949e 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -171,6 +171,17 @@ func Test_DecodeEventMemoBytes(t *testing.T) { }, donation: true, }, + { + name: "should skip standard memo that contains improper data", + event: &observer.BTCInboundEvent{ + // a deposit and call, receiver is empty ZEVM address + MemoBytes: testutil.HexToBytes( + t, + "5a01100300000000000000000000000000000000000000000d68656c6c6f207361746f736869", + ), + }, + errMsg: "standard memo contains improper data", + }, { name: "NoAssetCall is not disabled at the moment", event: &observer.BTCInboundEvent{ From b9b64caaf6fad2f9e831ce3852e41c6416479609 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Tue, 22 Oct 2024 12:41:50 -0500 Subject: [PATCH 05/10] fix gosec --- tests/simulation/sim/sim_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/simulation/sim/sim_state.go b/tests/simulation/sim/sim_state.go index 2f314b6c78..c8df9f4f31 100644 --- a/tests/simulation/sim/sim_state.go +++ b/tests/simulation/sim/sim_state.go @@ -248,7 +248,7 @@ func AppStateFromGenesisFileFn( cdc codec.JSONCodec, genesisFile string, ) (tmtypes.GenesisDoc, []simtypes.Account, error) { - bytes, err := os.ReadFile(genesisFile) + bytes, err := os.ReadFile(genesisFile) // #nosec G304 -- genesisFile value is controlled if err != nil { panic(err) } From 55091d8b56b02d69ad05162334aadb67a3ae38b2 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Tue, 22 Oct 2024 12:44:30 -0500 Subject: [PATCH 06/10] uncomment e2e tests --- cmd/zetae2e/local/local.go | 86 +++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index 922245e894..9b11a9a3f2 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -268,78 +268,78 @@ func localE2ETest(cmd *cobra.Command, _ []string) { if !skipRegular { // defines all tests, if light is enabled, only the most basic tests are run and advanced are skipped erc20Tests := []string{ - // e2etests.TestERC20WithdrawName, - // e2etests.TestMultipleERC20WithdrawsName, - // e2etests.TestERC20DepositAndCallRefundName, - // e2etests.TestZRC20SwapName, + e2etests.TestERC20WithdrawName, + e2etests.TestMultipleERC20WithdrawsName, + e2etests.TestERC20DepositAndCallRefundName, + e2etests.TestZRC20SwapName, } erc20AdvancedTests := []string{ - //e2etests.TestERC20DepositRestrictedName, + e2etests.TestERC20DepositRestrictedName, } zetaTests := []string{ - // e2etests.TestZetaWithdrawName, - // e2etests.TestMessagePassingExternalChainsName, - // e2etests.TestMessagePassingRevertFailExternalChainsName, - // e2etests.TestMessagePassingRevertSuccessExternalChainsName, + e2etests.TestZetaWithdrawName, + e2etests.TestMessagePassingExternalChainsName, + e2etests.TestMessagePassingRevertFailExternalChainsName, + e2etests.TestMessagePassingRevertSuccessExternalChainsName, } zetaAdvancedTests := []string{ - // e2etests.TestZetaDepositRestrictedName, - // e2etests.TestZetaDepositName, - // e2etests.TestZetaDepositNewAddressName, + e2etests.TestZetaDepositRestrictedName, + e2etests.TestZetaDepositName, + e2etests.TestZetaDepositNewAddressName, } zevmMPTests := []string{} zevmMPAdvancedTests := []string{ - // e2etests.TestMessagePassingZEVMToEVMName, - // e2etests.TestMessagePassingEVMtoZEVMName, - // e2etests.TestMessagePassingEVMtoZEVMRevertName, - // e2etests.TestMessagePassingZEVMtoEVMRevertName, - // e2etests.TestMessagePassingZEVMtoEVMRevertFailName, - // e2etests.TestMessagePassingEVMtoZEVMRevertFailName, + e2etests.TestMessagePassingZEVMToEVMName, + e2etests.TestMessagePassingEVMtoZEVMName, + e2etests.TestMessagePassingEVMtoZEVMRevertName, + e2etests.TestMessagePassingZEVMtoEVMRevertName, + e2etests.TestMessagePassingZEVMtoEVMRevertFailName, + e2etests.TestMessagePassingEVMtoZEVMRevertFailName, } bitcoinTests := []string{ - // e2etests.TestBitcoinDepositName, - // e2etests.TestBitcoinDepositAndCallName, + e2etests.TestBitcoinDepositName, + e2etests.TestBitcoinDepositAndCallName, e2etests.TestBitcoinDepositAndCallRevertName, e2etests.TestBitcoinDonationName, e2etests.TestBitcoinStdMemoDepositName, e2etests.TestBitcoinStdMemoDepositAndCallName, e2etests.TestBitcoinStdMemoDepositAndCallRevertName, e2etests.TestBitcoinStdMemoDepositAndCallRevertOtherAddressName, - // e2etests.TestBitcoinWithdrawSegWitName, - // e2etests.TestBitcoinWithdrawInvalidAddressName, - // e2etests.TestZetaWithdrawBTCRevertName, - // e2etests.TestCrosschainSwapName, + e2etests.TestBitcoinWithdrawSegWitName, + e2etests.TestBitcoinWithdrawInvalidAddressName, + e2etests.TestZetaWithdrawBTCRevertName, + e2etests.TestCrosschainSwapName, } bitcoinAdvancedTests := []string{ - // e2etests.TestBitcoinWithdrawTaprootName, - // e2etests.TestBitcoinWithdrawLegacyName, - // e2etests.TestBitcoinWithdrawMultipleName, - // e2etests.TestBitcoinWithdrawP2SHName, - // e2etests.TestBitcoinWithdrawP2WSHName, - // e2etests.TestBitcoinWithdrawRestrictedName, + e2etests.TestBitcoinWithdrawTaprootName, + e2etests.TestBitcoinWithdrawLegacyName, + e2etests.TestBitcoinWithdrawMultipleName, + e2etests.TestBitcoinWithdrawP2SHName, + e2etests.TestBitcoinWithdrawP2WSHName, + e2etests.TestBitcoinWithdrawRestrictedName, } ethereumTests := []string{ - // e2etests.TestEtherWithdrawName, - // e2etests.TestContextUpgradeName, - // e2etests.TestEtherDepositAndCallName, - // e2etests.TestEtherDepositAndCallRefundName, + e2etests.TestEtherWithdrawName, + e2etests.TestContextUpgradeName, + e2etests.TestEtherDepositAndCallName, + e2etests.TestEtherDepositAndCallRefundName, } ethereumAdvancedTests := []string{ - //e2etests.TestEtherWithdrawRestrictedName, + e2etests.TestEtherWithdrawRestrictedName, } precompiledContractTests := []string{} if !skipPrecompiles { precompiledContractTests = []string{ - // e2etests.TestPrecompilesPrototypeName, - // e2etests.TestPrecompilesPrototypeThroughContractName, - // e2etests.TestPrecompilesStakingName, - // // Disabled until further notice, check https://github.com/zeta-chain/node/issues/3005. - // // e2etests.TestPrecompilesStakingThroughContractName, - // e2etests.TestPrecompilesBankName, - // e2etests.TestPrecompilesBankFailName, - // e2etests.TestPrecompilesBankThroughContractName, + e2etests.TestPrecompilesPrototypeName, + e2etests.TestPrecompilesPrototypeThroughContractName, + e2etests.TestPrecompilesStakingName, + // Disabled until further notice, check https://github.com/zeta-chain/node/issues/3005. + // e2etests.TestPrecompilesStakingThroughContractName, + e2etests.TestPrecompilesBankName, + e2etests.TestPrecompilesBankFailName, + e2etests.TestPrecompilesBankThroughContractName, } } From 7a2880bf21e47dfa10d0f3a9dd3756064237afbd Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Wed, 23 Oct 2024 21:28:08 -0500 Subject: [PATCH 07/10] a few renamings; better comments and unit test --- e2e/e2etests/e2etests.go | 18 ++--- e2e/e2etests/test_bitcoin_deposit_call.go | 4 +- e2e/e2etests/test_bitcoin_donation.go | 4 +- .../test_bitcoin_std_deposit_and_call.go | 4 +- ...est_bitcoin_std_deposit_and_call_revert.go | 2 +- ...d_deposit_and_call_revert_other_address.go | 2 +- e2e/runner/bitcoin.go | 2 +- e2e/utils/zetacore.go | 20 ++--- pkg/memo/memo.go | 7 +- zetaclient/chains/bitcoin/observer/event.go | 39 ++++++---- .../chains/bitcoin/observer/event_test.go | 27 +++---- zetaclient/chains/bitcoin/observer/inbound.go | 20 ++--- .../chains/bitcoin/observer/inbound_test.go | 8 ++ zetaclient/chains/bitcoin/tx_script_test.go | 78 +++++++++++-------- 14 files changed, 132 insertions(+), 103 deletions(-) diff --git a/e2e/e2etests/e2etests.go b/e2e/e2etests/e2etests.go index 1710e9d8cd..98eac4397d 100644 --- a/e2e/e2etests/e2etests.go +++ b/e2e/e2etests/e2etests.go @@ -471,6 +471,13 @@ var AllE2ETests = []runner.E2ETest{ /* Bitcoin tests */ + runner.NewE2ETest( + TestBitcoinDonationName, + "donate Bitcoin to TSS address", []runner.ArgDefinition{ + {Description: "amount in btc", DefaultValue: "0.1"}, + }, + TestBitcoinDonation, + ), runner.NewE2ETest( TestExtractBitcoinInscriptionMemoName, "extract memo from BTC inscription", []runner.ArgDefinition{ @@ -501,18 +508,11 @@ var AllE2ETests = []runner.E2ETest{ }, TestBitcoinDepositAndCallRevert, ), - runner.NewE2ETest( - TestBitcoinDonationName, - "donate Bitcoin to TSS address", []runner.ArgDefinition{ - {Description: "amount in btc", DefaultValue: "0.001"}, - }, - TestBitcoinDonation, - ), runner.NewE2ETest( TestBitcoinStdMemoDepositName, "deposit Bitcoin into ZEVM with standard memo", []runner.ArgDefinition{ - {Description: "amount in btc", DefaultValue: "0.1"}, + {Description: "amount in btc", DefaultValue: "0.2"}, }, TestBitcoinStdMemoDeposit, ), @@ -520,7 +520,7 @@ var AllE2ETests = []runner.E2ETest{ TestBitcoinStdMemoDepositAndCallName, "deposit Bitcoin into ZEVM and call a contract with standard memo", []runner.ArgDefinition{ - {Description: "amount in btc", DefaultValue: "0.1"}, + {Description: "amount in btc", DefaultValue: "0.5"}, }, TestBitcoinStdMemoDepositAndCall, ), diff --git a/e2e/e2etests/test_bitcoin_deposit_call.go b/e2e/e2etests/test_bitcoin_deposit_call.go index c79ca9c1b8..d3d6917c59 100644 --- a/e2e/e2etests/test_bitcoin_deposit_call.go +++ b/e2e/e2etests/test_bitcoin_deposit_call.go @@ -49,7 +49,7 @@ func TestBitcoinDepositAndCall(r *runner.E2ERunner, args []string) { utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) // check if example contract has been called, 'bar' value should be set to amount - amoutSats, err := zetabitcoin.GetSatoshis(amount) + amountSats, err := zetabitcoin.GetSatoshis(amount) require.NoError(r, err) - utils.MustHaveCalledExampleContract(r, contract, big.NewInt(amoutSats)) + utils.MustHaveCalledExampleContract(r, contract, big.NewInt(amountSats)) } diff --git a/e2e/e2etests/test_bitcoin_donation.go b/e2e/e2etests/test_bitcoin_donation.go index 54a3eefdaf..1dd5a34859 100644 --- a/e2e/e2etests/test_bitcoin_donation.go +++ b/e2e/e2etests/test_bitcoin_donation.go @@ -36,8 +36,8 @@ func TestBitcoinDonation(r *runner.E2ERunner, args []string) { txHash, err := r.SendToTSSFromDeployerWithMemo(amountTotal, utxos, memo) require.NoError(r, err) - // ASSERT after 20 seconds - time.Sleep(time.Second * 20) + // ASSERT after 4 Zeta blocks + time.Sleep(constant.ZetaBlockTime * 4) req := &crosschaintypes.QueryInboundHashToCctxDataRequest{InboundHash: txHash.String()} _, err = r.CctxClient.InTxHashToCctxData(r.Ctx, req) require.Error(r, err) diff --git a/e2e/e2etests/test_bitcoin_std_deposit_and_call.go b/e2e/e2etests/test_bitcoin_std_deposit_and_call.go index 365bea556c..7a9c6ca255 100644 --- a/e2e/e2etests/test_bitcoin_std_deposit_and_call.go +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call.go @@ -51,7 +51,7 @@ func TestBitcoinStdMemoDepositAndCall(r *runner.E2ERunner, args []string) { utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) // check if example contract has been called, 'bar' value should be set to amount - amoutSats, err := zetabitcoin.GetSatoshis(amount) + amountSats, err := zetabitcoin.GetSatoshis(amount) require.NoError(r, err) - utils.MustHaveCalledExampleContract(r, contract, big.NewInt(amoutSats)) + utils.MustHaveCalledExampleContract(r, contract, big.NewInt(amountSats)) } diff --git a/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go index d346049802..76bf128aad 100644 --- a/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go +++ b/e2e/e2etests/test_bitcoin_std_deposit_and_call_revert.go @@ -49,5 +49,5 @@ func TestBitcoinStdMemoDepositAndCallRevert(r *runner.E2ERunner, args []string) assert.Equal(r, r.BTCDeployerAddress.EncodeAddress(), receiver) assert.Positive(r, value) - r.Logger.Print("Sent %f BTC to TSS to call non-existing contract, got refund of %d satoshis", amount, value) + r.Logger.Info("Sent %f BTC to TSS to call non-existing contract, got refund of %d satoshis", amount, value) } 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 d30f93409d..c6da1b1696 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 @@ -54,7 +54,7 @@ func TestBitcoinStdMemoDepositAndCallRevertOtherAddress(r *runner.E2ERunner, arg assert.Equal(r, revertAddress, receiver) assert.Positive(r, value) - r.Logger.Print( + r.Logger.Info( "Sent %f BTC to TSS to call non-existing contract, got refund of %d satoshis to other address", amount, value, diff --git a/e2e/runner/bitcoin.go b/e2e/runner/bitcoin.go index 21cd2cac38..a1ff30c084 100644 --- a/e2e/runner/bitcoin.go +++ b/e2e/runner/bitcoin.go @@ -399,7 +399,7 @@ func (r *E2ERunner) QueryOutboundReceiverAndAmount(txid string) (string, int64) // query outbound raw transaction revertTx, err := r.BtcRPCClient.GetRawTransaction(txHash) require.NoError(r, err, revertTx) - require.True(r, len(revertTx.MsgTx().TxOut) >= 2) + require.True(r, len(revertTx.MsgTx().TxOut) >= 2, "bitcoin outbound must have at least two outputs") // parse receiver address from pkScript txOutput := revertTx.MsgTx().TxOut[1] diff --git a/e2e/utils/zetacore.go b/e2e/utils/zetacore.go index 186b135388..33f5d68262 100644 --- a/e2e/utils/zetacore.go +++ b/e2e/utils/zetacore.go @@ -187,13 +187,22 @@ func WaitCCTXMinedByIndex( type WaitOpts func(c *waitConfig) -// MatchStatus waits for a specific CCTX status. +// MatchStatus is the WaitOpts that matches CCTX with the given status. func MatchStatus(s crosschaintypes.CctxStatus) WaitOpts { return Matches(func(tx crosschaintypes.CrossChainTx) bool { return tx.CctxStatus != nil && tx.CctxStatus.Status == s }) } +// MatchReverted is the WaitOpts that matches reverted CCTX. +func MatchReverted() WaitOpts { + return Matches(func(tx crosschaintypes.CrossChainTx) bool { + return tx.GetCctxStatus().Status == crosschaintypes.CctxStatus_Reverted && + len(tx.OutboundParams) == 2 && + tx.OutboundParams[1].Hash != "" + }) +} + // Matches adds a filter to WaitCctxByInboundHash that checks cctxs match provided callback. // ALL cctxs should match this filter. func Matches(fn func(tx crosschaintypes.CrossChainTx) bool) WaitOpts { @@ -211,15 +220,8 @@ func WaitCctxRevertedByInboundHash( hash string, c CCTXClient, ) crosschaintypes.CrossChainTx { - // criteria for reverted cctx - searchForCCTXReverted := Matches(func(tx crosschaintypes.CrossChainTx) bool { - return tx.GetCctxStatus().Status == crosschaintypes.CctxStatus_Reverted && - len(tx.OutboundParams) == 2 && - tx.OutboundParams[1].Hash != "" - }) - // wait for cctx to be reverted - cctxs := WaitCctxByInboundHash(ctx, t, hash, c, searchForCCTXReverted) + cctxs := WaitCctxByInboundHash(ctx, t, hash, c, MatchReverted()) require.Len(t, cctxs, 1) return cctxs[0] diff --git a/pkg/memo/memo.go b/pkg/memo/memo.go index b1b801c539..a4b0834f73 100644 --- a/pkg/memo/memo.go +++ b/pkg/memo/memo.go @@ -8,6 +8,11 @@ import ( "github.com/pkg/errors" ) +const ( + // version0 is the latest version of the memo + version0 uint8 = 0 +) + // InboundMemo represents the inbound memo structure for non-EVM chains type InboundMemo struct { // Header contains the memo header @@ -37,7 +42,7 @@ func (m *InboundMemo) EncodeToBytes() ([]byte, error) { // encode fields based on version var data []byte switch m.Version { - case 0: + case version0: data, err = m.FieldsV0.Pack(m.OpCode, m.EncodingFmt, dataFlags) default: return nil, fmt.Errorf("invalid memo version: %d", m.Version) diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index 43def388a5..c8f6acac1f 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -62,8 +62,8 @@ type BTCInboundEvent struct { TxHash string } -// IsProcessable checks if the inbound event is processable -func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { +// Processability returns the processability of the inbound event +func (event *BTCInboundEvent) Processability() InboundProcessability { // compliance check on sender and receiver addresses if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { return InboundProcessabilityComplianceViolation @@ -88,8 +88,8 @@ func (event *BTCInboundEvent) CheckProcessability() InboundProcessability { return InboundProcessabilityGood } -// DecodeEventMemoBytes decodes the memo bytes as either standard or legacy memo -func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { +// DecodeMemoBytes decodes the contained memo bytes as either standard or legacy memo +func (event *BTCInboundEvent) DecodeMemoBytes(chainID int64) error { var ( err error memoStd *memo.InboundMemo @@ -103,7 +103,7 @@ func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { // try to decode the standard memo as the preferred format // the standard memo is NOT enabled for Bitcoin mainnet - if ob.Chain().ChainId != chains.BitcoinMainnet.ChainId { + if chainID != chains.BitcoinMainnet.ChainId { memoStd, err = memo.DecodeFromBytes(event.MemoBytes) } @@ -115,6 +115,7 @@ func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { } // NoAssetCall will be disabled for Bitcoin until full V2 support + // https://github.com/zeta-chain/node/issues/2711 if memoStd.OpCode == memo.OpCodeCall { return errors.New("NoAssetCall is disabled") } @@ -122,7 +123,7 @@ func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { // ensure the revert address is valid and supported revertAddress := memoStd.RevertOptions.RevertAddress if revertAddress != "" { - btcAddress, err := chains.DecodeBtcAddress(revertAddress, ob.Chain().ChainId) + btcAddress, err := chains.DecodeBtcAddress(revertAddress, chainID) if err != nil { return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) } @@ -151,10 +152,9 @@ func (ob *Observer) DecodeEventMemoBytes(event *BTCInboundEvent) error { } // CheckEventProcessability checks if the inbound event is processable -func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { +func (ob *Observer) CheckEventProcessability(event BTCInboundEvent) bool { // check if the event is processable - result := event.CheckProcessability() - switch result { + switch result := event.Processability(); result { case InboundProcessabilityGood: return true case InboundProcessabilityDonation: @@ -168,13 +168,17 @@ func (ob *Observer) CheckEventProcessability(event *BTCInboundEvent) bool { compliance.PrintComplianceLog(ob.logger.Inbound, ob.logger.Compliance, false, ob.Chain().ChainId, event.TxHash, event.FromAddress, event.ToAddress, "BTC") return false + default: + ob.Logger().Inbound.Error().Msgf("unreachable code got InboundProcessability: %v", result) + return false } - - return true } -// NewInboundVoteV1 creates a MsgVoteInbound message for inbound that uses legacy memo -func (ob *Observer) NewInboundVoteV1(event *BTCInboundEvent, amountSats *big.Int) *crosschaintypes.MsgVoteInbound { +// NewInboundVoteFromLegacyMemo creates a MsgVoteInbound message for inbound that uses legacy memo +func (ob *Observer) NewInboundVoteFromLegacyMemo( + event *BTCInboundEvent, + amountSats *big.Int, +) *crosschaintypes.MsgVoteInbound { message := hex.EncodeToString(event.MemoBytes) return crosschaintypes.NewMsgVoteInbound( @@ -197,10 +201,13 @@ func (ob *Observer) NewInboundVoteV1(event *BTCInboundEvent, amountSats *big.Int ) } -// NewInboundVoteMemoStd creates a MsgVoteInbound message for inbound that uses standard memo -// TODO: rename this function as 'EventToInboundVoteV2' to use ProtocolContractVersion_V2 and enable more options +// NewInboundVoteFromStdMemo creates a MsgVoteInbound message for inbound that uses standard memo +// TODO: upgrade to ProtocolContractVersion_V2 and enable more options // https://github.com/zeta-chain/node/issues/2711 -func (ob *Observer) NewInboundVoteMemoStd(event *BTCInboundEvent, amountSats *big.Int) *crosschaintypes.MsgVoteInbound { +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 diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index 0c8cdb949e..a265866eee 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -30,8 +30,8 @@ func createTestBtcEvent( net *chaincfg.Params, memo []byte, memoStd *memo.InboundMemo, -) *observer.BTCInboundEvent { - return &observer.BTCInboundEvent{ +) observer.BTCInboundEvent { + return observer.BTCInboundEvent{ FromAddress: sample.BtcAddressP2WPKH(t, net), ToAddress: sample.EthAddress().Hex(), MemoBytes: memo, @@ -111,20 +111,13 @@ func Test_CheckProcessability(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.event.CheckProcessability() + result := tt.event.Processability() require.Equal(t, tt.expected, result) }) } } func Test_DecodeEventMemoBytes(t *testing.T) { - // can use any bitcoin chain for testing - chain := chains.BitcoinTestnet - params := mocks.MockChainParams(chain.ChainId, 10) - - // create test observer - ob := MockBTCObserver(t, chain, params, nil) - // test cases tests := []struct { name string @@ -229,7 +222,7 @@ func Test_DecodeEventMemoBytes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ob.DecodeEventMemoBytes(tt.event) + err := tt.event.DecodeMemoBytes(chains.BitcoinTestnet.ChainId) if tt.errMsg != "" { require.Contains(t, err.Error(), tt.errMsg) return @@ -257,7 +250,7 @@ func Test_DecodeEventMemoBytes(t *testing.T) { func Test_CheckEventProcessability(t *testing.T) { // can use any bitcoin chain for testing - chain := chains.BitcoinTestnet + chain := chains.BitcoinMainnet params := mocks.MockChainParams(chain.ChainId, 10) // create test observer @@ -272,7 +265,7 @@ func Test_CheckEventProcessability(t *testing.T) { // test cases tests := []struct { name string - event *observer.BTCInboundEvent + event observer.BTCInboundEvent result bool }{ { @@ -304,7 +297,7 @@ func Test_CheckEventProcessability(t *testing.T) { } } -func Test_NewInboundVoteV1(t *testing.T) { +func Test_NewInboundVoteFromLegacyMemo(t *testing.T) { // can use any bitcoin chain for testing chain := chains.BitcoinMainnet params := mocks.MockChainParams(chain.ChainId, 10) @@ -341,12 +334,12 @@ func Test_NewInboundVoteV1(t *testing.T) { } // create new inbound vote V1 - vote := ob.NewInboundVoteV1(event, amountSats) + vote := ob.NewInboundVoteFromLegacyMemo(&event, amountSats) require.Equal(t, expectedVote, *vote) }) } -func Test_NewInboundVoteMemoStd(t *testing.T) { +func Test_NewInboundVoteFromStdMemo(t *testing.T) { // can use any bitcoin chain for testing chain := chains.BitcoinMainnet params := mocks.MockChainParams(chain.ChainId, 10) @@ -395,7 +388,7 @@ func Test_NewInboundVoteMemoStd(t *testing.T) { } // create new inbound vote V1 with standard memo - vote := ob.NewInboundVoteMemoStd(event, amountSats) + vote := ob.NewInboundVoteFromStdMemo(&event, amountSats) require.Equal(t, expectedVote, *vote) }) } diff --git a/zetaclient/chains/bitcoin/observer/inbound.go b/zetaclient/chains/bitcoin/observer/inbound.go index fdde56b20a..b08fbea18c 100644 --- a/zetaclient/chains/bitcoin/observer/inbound.go +++ b/zetaclient/chains/bitcoin/observer/inbound.go @@ -7,7 +7,6 @@ import ( "math/big" "github.com/btcsuite/btcd/btcjson" - "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/pkg/errors" @@ -348,35 +347,38 @@ func (ob *Observer) GetInboundVoteFromBtcEvent(event *BTCInboundEvent) *crosscha // prepare logger fields lf := map[string]any{ logs.FieldModule: logs.ModNameInbound, - logs.FieldMethod: "GetInboundVoteMessageFromBtcEvent", + logs.FieldMethod: "GetInboundVoteFromBtcEvent", logs.FieldChain: ob.Chain().ChainId, logs.FieldTx: event.TxHash, } // decode event memo bytes - err := ob.DecodeEventMemoBytes(event) + err := event.DecodeMemoBytes(ob.Chain().ChainId) if err != nil { ob.Logger().Inbound.Info().Fields(lf).Msgf("invalid memo bytes: %s", hex.EncodeToString(event.MemoBytes)) return nil } // check if the event is processable - if !ob.CheckEventProcessability(event) { + if !ob.CheckEventProcessability(*event) { return nil } // convert the amount to integer (satoshis) - amount := big.NewFloat(event.Value) - amount = amount.Mul(amount, big.NewFloat(btcutil.SatoshiPerBitcoin)) - amountInt, _ := amount.Int(nil) + amountSats, err := bitcoin.GetSatoshis(event.Value) + if err != nil { + ob.Logger().Inbound.Error().Err(err).Fields(lf).Msgf("can't convert value %f to satoshis", event.Value) + return nil + } + amountInt := big.NewInt(amountSats) // create inbound vote message contract V1 for legacy memo if event.MemoStd == nil { - return ob.NewInboundVoteV1(event, amountInt) + return ob.NewInboundVoteFromLegacyMemo(event, amountInt) } // create inbound vote message for standard memo - return ob.NewInboundVoteMemoStd(event, amountInt) + return ob.NewInboundVoteFromStdMemo(event, amountInt) } // GetBtcEvent returns a valid BTCInboundEvent or nil diff --git a/zetaclient/chains/bitcoin/observer/inbound_test.go b/zetaclient/chains/bitcoin/observer/inbound_test.go index a33de19194..838315b7b8 100644 --- a/zetaclient/chains/bitcoin/observer/inbound_test.go +++ b/zetaclient/chains/bitcoin/observer/inbound_test.go @@ -191,6 +191,14 @@ func Test_GetInboundVoteFromBtcEvent(t *testing.T) { }, nilVote: true, }, + { + name: "should return nil on invalid deposit value", + event: &observer.BTCInboundEvent{ + Value: -1, // invalid value + MemoBytes: testutil.HexToBytes(t, "2d07a9cbd57dcca3e2cf966c88bc874445b6e3b668656c6c6f207361746f736869"), + }, + nilVote: true, + }, } for _, tt := range tests { diff --git a/zetaclient/chains/bitcoin/tx_script_test.go b/zetaclient/chains/bitcoin/tx_script_test.go index 1af2966223..394a5d8608 100644 --- a/zetaclient/chains/bitcoin/tx_script_test.go +++ b/zetaclient/chains/bitcoin/tx_script_test.go @@ -1,6 +1,7 @@ package bitcoin_test import ( + "bytes" "encoding/hex" "path" "strings" @@ -11,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/zeta-chain/node/pkg/chains" + "github.com/zeta-chain/node/testutil" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/testutils" ) @@ -330,40 +332,50 @@ func TestDecodeVoutP2PKHErrors(t *testing.T) { } func TestDecodeOpReturnMemo(t *testing.T) { - // load archived inbound raw result - // https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa - chain := chains.BitcoinMainnet - txHash := "847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa" - scriptHex := "6a1467ed0bcc4e1256bc2ce87d22e190d63a120114bf" - rawResult := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) - require.True(t, len(rawResult.Vout) >= 2) - require.Equal(t, scriptHex, rawResult.Vout[1].ScriptPubKey.Hex) - - t.Run("should decode memo from OP_RETURN output", func(t *testing.T) { - memo, found, err := bitcoin.DecodeOpReturnMemo(rawResult.Vout[1].ScriptPubKey.Hex) - require.NoError(t, err) - require.True(t, found) - // [OP_RETURN, 0x14,<20-byte-hash>] - require.Equal(t, scriptHex[4:], hex.EncodeToString(memo)) - }) - - t.Run("should return nil memo non-OP_RETURN output", func(t *testing.T) { - // modify the OP_RETURN to OP_1 - scriptInvalid := strings.Replace(scriptHex, "6a", "51", 1) - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid) - require.NoError(t, err) - require.False(t, found) - require.Nil(t, memo) - }) + tests := []struct { + name string + scriptHex string + found bool + expected []byte + }{ + { + name: "should decode memo from OP_RETURN data, size < 76(OP_PUSHDATA1)", + scriptHex: "6a1467ed0bcc4e1256bc2ce87d22e190d63a120114bf", + found: true, + expected: testutil.HexToBytes(t, "67ed0bcc4e1256bc2ce87d22e190d63a120114bf"), + }, + { + name: "should decode memo from OP_RETURN data, size >= 76(OP_PUSHDATA1)", + scriptHex: "6a4c4f" + // 79 bytes memo + "5a0110070a30d55c1031d30dab3b3d85f47b8f1d03df2d480961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432", + found: true, + expected: testutil.HexToBytes( + t, + "5a0110070a30d55c1031d30dab3b3d85f47b8f1d03df2d480961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432", + ), + }, + { + name: "should return nil memo for non-OP_RETURN script", + scriptHex: "511467ed0bcc4e1256bc2ce87d22e190d63a120114bf", // 0x51, OP_1 + found: false, + expected: nil, + }, + { + name: "should return nil memo for script less than 2 bytes", + scriptHex: "00", // 1 byte only + found: false, + expected: nil, + }, + } - t.Run("should return nil memo on script less than 2 byte", func(t *testing.T) { - // use known short script - scriptInvalid := "00" - memo, found, err := bitcoin.DecodeOpReturnMemo(scriptInvalid) - require.NoError(t, err) - require.False(t, found) - require.Nil(t, memo) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + memo, found, err := bitcoin.DecodeOpReturnMemo(tt.scriptHex) + require.NoError(t, err) + require.Equal(t, tt.found, found) + require.True(t, bytes.Equal(tt.expected, memo)) + }) + } } func TestDecodeOpReturnMemoErrors(t *testing.T) { From b2c149c4124f27c83f2ef6f4a55f76f8c3534433 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Wed, 23 Oct 2024 23:25:19 -0500 Subject: [PATCH 08/10] code refactor to make DecodeMemoBytes more redable --- cmd/zetae2e/local/local.go | 2 +- pkg/memo/memo.go | 22 ++-- pkg/memo/memo_test.go | 40 ++++--- zetaclient/chains/bitcoin/observer/event.go | 65 ++++++----- .../chains/bitcoin/observer/event_test.go | 107 ++++++++++++------ 5 files changed, 152 insertions(+), 84 deletions(-) diff --git a/cmd/zetae2e/local/local.go b/cmd/zetae2e/local/local.go index 9b11a9a3f2..4c6624451c 100644 --- a/cmd/zetae2e/local/local.go +++ b/cmd/zetae2e/local/local.go @@ -298,10 +298,10 @@ func localE2ETest(cmd *cobra.Command, _ []string) { } bitcoinTests := []string{ + e2etests.TestBitcoinDonationName, e2etests.TestBitcoinDepositName, e2etests.TestBitcoinDepositAndCallName, e2etests.TestBitcoinDepositAndCallRevertName, - e2etests.TestBitcoinDonationName, e2etests.TestBitcoinStdMemoDepositName, e2etests.TestBitcoinStdMemoDepositAndCallName, e2etests.TestBitcoinStdMemoDepositAndCallRevertName, diff --git a/pkg/memo/memo.go b/pkg/memo/memo.go index a4b0834f73..ca429a5f38 100644 --- a/pkg/memo/memo.go +++ b/pkg/memo/memo.go @@ -56,33 +56,41 @@ func (m *InboundMemo) EncodeToBytes() ([]byte, error) { // DecodeFromBytes decodes a InboundMemo struct from raw bytes // -// Returns nil memo if given data can't be decoded as a memo. -func DecodeFromBytes(data []byte) (*InboundMemo, error) { +// Returns: +// - [memo, true, nil] if given data is successfully decoded as a memo. +// - [nil, true, err] if given data is successfully decoded as a memo but contains improper field values. +// - [nil, false, err] if given data can't be decoded as a memo. +// +// Note: we won't have to differentiate between the two 'true' cases if legacy memo phase out is completed. +func DecodeFromBytes(data []byte) (*InboundMemo, bool, error) { memo := &InboundMemo{} // decode header err := memo.Header.DecodeFromBytes(data) if err != nil { - return nil, errors.Wrap(err, "failed to decode memo header") + return nil, false, errors.Wrap(err, "failed to decode memo header") } // decode fields based on version switch memo.Version { - case 0: + case version0: // unpack fields err = memo.FieldsV0.Unpack(memo.EncodingFmt, memo.Header.DataFlags, data[HeaderSize:]) if err != nil { - return nil, errors.Wrap(err, "failed to unpack memo FieldsV0") + return nil, false, errors.Wrap(err, "failed to unpack memo FieldsV0") } // validate fields // Note: a well-formatted memo may still contain improper field values err = memo.FieldsV0.Validate(memo.OpCode, memo.Header.DataFlags) + if err != nil { + return nil, true, errors.Wrap(err, "failed to validate memo FieldsV0") + } default: - return nil, fmt.Errorf("invalid memo version: %d", memo.Version) + return nil, false, fmt.Errorf("invalid memo version: %d", memo.Version) } - return memo, err + return memo, true, nil } // DecodeLegacyMemoHex decodes hex encoded memo message into address and calldata diff --git a/pkg/memo/memo_test.go b/pkg/memo/memo_test.go index 381f256a79..4eabd6a18f 100644 --- a/pkg/memo/memo_test.go +++ b/pkg/memo/memo_test.go @@ -137,7 +137,8 @@ func Test_Memo_EncodeToBytes(t *testing.T) { require.Equal(t, append(tt.expectedHead, tt.expectedData...), data) // decode the memo and compare with the original - decodedMemo, err := memo.DecodeFromBytes(data) + decodedMemo, isMemo, err := memo.DecodeFromBytes(data) + require.True(t, isMemo) require.NoError(t, err) require.Equal(t, tt.memo, decodedMemo) }) @@ -154,8 +155,8 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { name string head []byte data []byte + isMemo bool expectedMemo memo.InboundMemo - invalidField bool errMsg string }{ { @@ -173,6 +174,7 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { memo.ArgRevertAddress(fString), memo.ArgAbortAddress(fAddress), memo.ArgRevertMessage(fBytes)), + isMemo: true, expectedMemo: memo.InboundMemo{ Header: memo.Header{ Version: 0, @@ -208,6 +210,7 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { memo.ArgRevertAddress(fString), memo.ArgAbortAddress(fAddress), memo.ArgRevertMessage(fBytes)), + isMemo: true, expectedMemo: memo.InboundMemo{ Header: memo.Header{ Version: 0, @@ -255,7 +258,7 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { errMsg: "failed to unpack memo FieldsV0", }, { - name: "should return both memo struct and error if fields validation fails", + name: "should return [nil, true, err] if fields validation fails", head: MakeHead( 0, uint8(memo.EncodingFmtABI), @@ -266,28 +269,37 @@ func Test_Memo_DecodeFromBytes(t *testing.T) { data: ABIPack(t, memo.ArgReceiver(common.Address{}), // empty receiver address provided memo.ArgPayload(fBytes)), - invalidField: true, - errMsg: "receiver address is empty", + isMemo: true, // it's still a memo, but with invalid field values + errMsg: "failed to validate memo FieldsV0", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { data := append(tt.head, tt.data...) - memo, err := memo.DecodeFromBytes(data) + memo, isMemo, err := memo.DecodeFromBytes(data) + + // check error message if tt.errMsg != "" { + require.Nil(t, memo) require.ErrorContains(t, err, tt.errMsg) + return + } - // invalid field values may still produce a memo - if tt.invalidField { - require.NotNil(t, memo) - } else { - require.Nil(t, memo) - } + // a standard memo or not + require.Equal(t, tt.isMemo, isMemo) + if !isMemo { + require.Nil(t, memo) return } - require.NoError(t, err) - require.Equal(t, tt.expectedMemo, *memo) + + // if it's a standard memo, depending on validation result + if err != nil { + require.Nil(t, memo) + } else { + require.NotNil(t, memo) + require.Equal(t, tt.expectedMemo, *memo) + } }) } } diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index c8f6acac1f..81f0d154a3 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -63,7 +63,7 @@ type BTCInboundEvent struct { } // Processability returns the processability of the inbound event -func (event *BTCInboundEvent) Processability() InboundProcessability { +func (event BTCInboundEvent) Processability() InboundProcessability { // compliance check on sender and receiver addresses if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { return InboundProcessabilityComplianceViolation @@ -91,9 +91,10 @@ func (event *BTCInboundEvent) Processability() InboundProcessability { // DecodeMemoBytes decodes the contained memo bytes as either standard or legacy memo func (event *BTCInboundEvent) DecodeMemoBytes(chainID int64) error { var ( - err error - memoStd *memo.InboundMemo - receiver ethcommon.Address + err error + isStandardMemo bool + memoStd *memo.InboundMemo + receiver ethcommon.Address ) // skip decoding donation tx as it won't go through zetacore @@ -103,40 +104,29 @@ func (event *BTCInboundEvent) DecodeMemoBytes(chainID int64) error { // try to decode the standard memo as the preferred format // the standard memo is NOT enabled for Bitcoin mainnet + if chainID != chains.BitcoinMainnet.ChainId { - memoStd, err = memo.DecodeFromBytes(event.MemoBytes) + memoStd, isStandardMemo, err = memo.DecodeFromBytes(event.MemoBytes) } - switch { - case memoStd != nil: - // skip memo that carries improper data + // process standard memo or fallback to legacy memo + if isStandardMemo { + // skip standard memo that carries improper data if err != nil { return errors.Wrap(err, "standard memo contains improper data") } - // NoAssetCall will be disabled for Bitcoin until full V2 support - // https://github.com/zeta-chain/node/issues/2711 - if memoStd.OpCode == memo.OpCodeCall { - return errors.New("NoAssetCall is disabled") + // validate the content of the standard memo + err = ValidateStandardMemo(*memoStd, chainID) + if err != nil { + return errors.Wrap(err, "invalid standard memo for bitcoin") } - // ensure the revert address is valid and supported - revertAddress := memoStd.RevertOptions.RevertAddress - if revertAddress != "" { - btcAddress, err := chains.DecodeBtcAddress(revertAddress, chainID) - if err != nil { - return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) - } - if !chains.IsBtcAddressSupported(btcAddress) { - return fmt.Errorf("unsupported revert address in memo: %s", revertAddress) - } - } event.MemoStd = memoStd receiver = memoStd.Receiver - default: - // fallback to legacy memo if standard memo decoding fails + } else { parsedAddress, _, err := memo.DecodeLegacyMemoHex(hex.EncodeToString(event.MemoBytes)) - if err != nil { // never happens + if err != nil { // unreachable code return errors.Wrap(err, "invalid legacy memo") } receiver = parsedAddress @@ -151,6 +141,29 @@ func (event *BTCInboundEvent) DecodeMemoBytes(chainID int64) error { return nil } +// ValidateStandardMemo validates the standard memo in Bitcoin context +func ValidateStandardMemo(memoStd memo.InboundMemo, chainID int64) error { + // NoAssetCall will be disabled for Bitcoin until full V2 support + // https://github.com/zeta-chain/node/issues/2711 + if memoStd.OpCode == memo.OpCodeCall { + return errors.New("NoAssetCall is disabled for Bitcoin") + } + + // ensure the revert address is a valid and supported BTC address + revertAddress := memoStd.RevertOptions.RevertAddress + if revertAddress != "" { + btcAddress, err := chains.DecodeBtcAddress(revertAddress, chainID) + if err != nil { + return errors.Wrapf(err, "invalid revert address in memo: %s", revertAddress) + } + if !chains.IsBtcAddressSupported(btcAddress) { + return fmt.Errorf("unsupported revert address in memo: %s", revertAddress) + } + } + + return nil +} + // CheckEventProcessability checks if the inbound event is processable func (ob *Observer) CheckEventProcessability(event BTCInboundEvent) bool { // check if the event is processable diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index a265866eee..3c17e8c5ed 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -165,7 +165,7 @@ func Test_DecodeEventMemoBytes(t *testing.T) { donation: true, }, { - name: "should skip standard memo that contains improper data", + name: "should return error if standard memo contains improper data", event: &observer.BTCInboundEvent{ // a deposit and call, receiver is empty ZEVM address MemoBytes: testutil.HexToBytes( @@ -176,47 +176,15 @@ func Test_DecodeEventMemoBytes(t *testing.T) { errMsg: "standard memo contains improper data", }, { - name: "NoAssetCall is not disabled at the moment", + name: "should return error if standard memo validation failed", event: &observer.BTCInboundEvent{ - // a no asset call + // a no asset call opCode passed, not supported at the moment MemoBytes: testutil.HexToBytes( t, "5a0120032d07a9cbd57dcca3e2cf966c88bc874445b6e3b60d68656c6c6f207361746f736869", ), }, - errMsg: "NoAssetCall is disabled", - }, - { - name: "should return error on invalid revert address", - event: &observer.BTCInboundEvent{ - // raw address + payload + revert address - // but the address is "bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2" which is not a testnet address - MemoBytes: testutil.HexToBytes( - t, - "5a0110075ab13400c33b83ca9d3ee5587486c26639a5ef190961207061796c6f61642c626372743171793970716d6b32706439737636336732376a7438723635377779306439756565347832647432", - ), - }, - errMsg: "invalid revert address in memo", - }, - { - name: "should return error if revert address is not a supported address type", - event: &observer.BTCInboundEvent{ - // raw address + payload + revert address - // but the revert address is "035e4ae279bd416b5da724972c9061ec6298dac020d1e3ca3f06eae715135cdbec" and it's not supported - MemoBytes: testutil.HexToBytes( - t, - "5a0110072d07a9cbd57dcca3e2cf966c88bc874445b6e3b60961207061796c6f616442303335653461653237396264343136623564613732343937326339303631656336323938646163303230643165336361336630366561653731353133356364626563", - ), - }, - errMsg: "unsupported revert address in memo", - }, - { - name: "should return error on empty receiver address", - event: &observer.BTCInboundEvent{ - // standard memo that carries payload only, receiver address is empty - MemoBytes: testutil.HexToBytes(t, "5a0110020d68656c6c6f207361746f736869"), - }, - errMsg: "got empty receiver address from memo", + errMsg: "invalid standard memo for bitcoin", }, } @@ -248,6 +216,73 @@ func Test_DecodeEventMemoBytes(t *testing.T) { } } +func Test_ValidateStandardMemo(t *testing.T) { + // test cases + tests := []struct { + name string + memo memo.InboundMemo + errMsg string + }{ + { + name: "validation should pass for a valid standard memo", + memo: memo.InboundMemo{ + Header: memo.Header{ + OpCode: memo.OpCodeDepositAndCall, + }, + FieldsV0: memo.FieldsV0{ + RevertOptions: crosschaintypes.RevertOptions{ + RevertAddress: sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params), + }, + }, + }, + }, + { + name: "NoAssetCall is disabled for Bitcoin", + memo: memo.InboundMemo{ + Header: memo.Header{ + OpCode: memo.OpCodeCall, + }, + }, + errMsg: "NoAssetCall is disabled for Bitcoin", + }, + { + name: "should return error on invalid revert address", + memo: memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + RevertOptions: crosschaintypes.RevertOptions{ + // not a BTC address + RevertAddress: "0x2D07A9CBd57DCca3E2cF966C88Bc874445b6E3B6", + }, + }, + }, + errMsg: "invalid revert address in memo", + }, + { + name: "should return error if revert address is not a supported address type", + memo: memo.InboundMemo{ + FieldsV0: memo.FieldsV0{ + RevertOptions: crosschaintypes.RevertOptions{ + // address not supported + RevertAddress: "035e4ae279bd416b5da724972c9061ec6298dac020d1e3ca3f06eae715135cdbec", + }, + }, + }, + errMsg: "unsupported revert address in memo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := observer.ValidateStandardMemo(tt.memo, chains.BitcoinTestnet.ChainId) + if tt.errMsg != "" { + require.Contains(t, err.Error(), tt.errMsg) + return + } + require.NoError(t, err) + }) + } +} + func Test_CheckEventProcessability(t *testing.T) { // can use any bitcoin chain for testing chain := chains.BitcoinMainnet From 7e6368f3ed07f0a3afaa55f2270a65a0da79746a Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Thu, 24 Oct 2024 17:38:53 -0500 Subject: [PATCH 09/10] add more description for function; add unit test to ensure that standard memo is disabled for mainnet --- e2e/e2etests/test_bitcoin_std_deposit.go | 4 +++ e2e/runner/bitcoin.go | 11 +++++-- .../chains/bitcoin/observer/event_test.go | 30 +++++++++++++++---- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/e2e/e2etests/test_bitcoin_std_deposit.go b/e2e/e2etests/test_bitcoin_std_deposit.go index a98a34a3f1..fa123b94cd 100644 --- a/e2e/e2etests/test_bitcoin_std_deposit.go +++ b/e2e/e2etests/test_bitcoin_std_deposit.go @@ -17,6 +17,10 @@ func TestBitcoinStdMemoDeposit(r *runner.E2ERunner, args []string) { // setup deployer BTC address r.SetBtcAddress(r.Name, false) + // start mining blocks if local bitcoin + stop := r.MineBlocksIfLocalBitcoin() + defer stop() + // parse amount to deposit require.Len(r, args, 1) amount := parseFloat(r, args[0]) diff --git a/e2e/runner/bitcoin.go b/e2e/runner/bitcoin.go index a1ff30c084..3d65589fa5 100644 --- a/e2e/runner/bitcoin.go +++ b/e2e/runner/bitcoin.go @@ -77,7 +77,7 @@ func (r *E2ERunner) GetTop20UTXOsForTssAddress() ([]btcjson.ListUnspentResult, e return utxos, nil } -// DepositBTCWithAmount deposits BTC into ZetaChain with a specific amount +// DepositBTCWithAmount deposits BTC into ZetaChain with a specific amount and memo func (r *E2ERunner) DepositBTCWithAmount(amount float64, memo *memo.InboundMemo) *chainhash.Hash { // list deployer utxos utxos, err := r.ListDeployerUTXOs() @@ -176,16 +176,21 @@ func (r *E2ERunner) DepositBTC() { } // DepositBTCWithLegacyMemo deposits BTC from the deployer address to the TSS using legacy memo +// +// The legacy memo layout: [20-byte receiver] + [payload] func (r *E2ERunner) DepositBTCWithLegacyMemo( amount float64, inputUTXOs []btcjson.ListUnspentResult, ) (*chainhash.Hash, error) { r.Logger.Info("⏳ depositing BTC into ZEVM with legacy memo") - return r.SendToTSSFromDeployerWithMemo(amount, inputUTXOs, r.EVMAddress().Bytes()) + // payload is not needed for pure deposit + memoBytes := r.EVMAddress().Bytes() + + return r.SendToTSSFromDeployerWithMemo(amount, inputUTXOs, memoBytes) } -// DepositBTCWithStandardMemo deposits BTC from the deployer address to the TSS using standard memo +// DepositBTCWithStandardMemo deposits BTC from the deployer address to the TSS using standard `InboundMemo` struct func (r *E2ERunner) DepositBTCWithStandardMemo( amount float64, inputUTXOs []btcjson.ListUnspentResult, diff --git a/zetaclient/chains/bitcoin/observer/event_test.go b/zetaclient/chains/bitcoin/observer/event_test.go index 3c17e8c5ed..5ed8e9b103 100644 --- a/zetaclient/chains/bitcoin/observer/event_test.go +++ b/zetaclient/chains/bitcoin/observer/event_test.go @@ -121,6 +121,7 @@ func Test_DecodeEventMemoBytes(t *testing.T) { // test cases tests := []struct { name string + chainID int64 event *observer.BTCInboundEvent expectedMemoStd *memo.InboundMemo expectedReceiver common.Address @@ -128,7 +129,8 @@ func Test_DecodeEventMemoBytes(t *testing.T) { errMsg string }{ { - name: "should decode standard memo bytes successfully", + name: "should decode standard memo bytes successfully", + chainID: chains.BitcoinTestnet.ChainId, event: &observer.BTCInboundEvent{ // a deposit and call MemoBytes: testutil.HexToBytes( @@ -150,7 +152,8 @@ func Test_DecodeEventMemoBytes(t *testing.T) { }, }, { - name: "should fall back to legacy memo successfully", + name: "should fall back to legacy memo successfully", + chainID: chains.BitcoinTestnet.ChainId, event: &observer.BTCInboundEvent{ // raw address + payload MemoBytes: testutil.HexToBytes(t, "2d07a9cbd57dcca3e2cf966c88bc874445b6e3b668656c6c6f207361746f736869"), @@ -158,14 +161,28 @@ func Test_DecodeEventMemoBytes(t *testing.T) { expectedReceiver: common.HexToAddress("0x2D07A9CBd57DCca3E2cF966C88Bc874445b6E3B6"), }, { - name: "should do nothing for donation message", + name: "should disable standard memo for Bitcoin mainnet", + chainID: chains.BitcoinMainnet.ChainId, + event: &observer.BTCInboundEvent{ + // a deposit and call + MemoBytes: testutil.HexToBytes( + t, + "5a0110032d07a9cbd57dcca3e2cf966c88bc874445b6e3b60d68656c6c6f207361746f736869", + ), + }, + expectedReceiver: common.HexToAddress("0x5A0110032d07A9cbd57dcCa3e2Cf966c88bC8744"), + }, + { + name: "should do nothing for donation message", + chainID: chains.BitcoinTestnet.ChainId, event: &observer.BTCInboundEvent{ MemoBytes: []byte(constant.DonationMessage), }, donation: true, }, { - name: "should return error if standard memo contains improper data", + name: "should return error if standard memo contains improper data", + chainID: chains.BitcoinTestnet.ChainId, event: &observer.BTCInboundEvent{ // a deposit and call, receiver is empty ZEVM address MemoBytes: testutil.HexToBytes( @@ -176,7 +193,8 @@ func Test_DecodeEventMemoBytes(t *testing.T) { errMsg: "standard memo contains improper data", }, { - name: "should return error if standard memo validation failed", + name: "should return error if standard memo validation failed", + chainID: chains.BitcoinTestnet.ChainId, event: &observer.BTCInboundEvent{ // a no asset call opCode passed, not supported at the moment MemoBytes: testutil.HexToBytes( @@ -190,7 +208,7 @@ func Test_DecodeEventMemoBytes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.event.DecodeMemoBytes(chains.BitcoinTestnet.ChainId) + err := tt.event.DecodeMemoBytes(tt.chainID) if tt.errMsg != "" { require.Contains(t, err.Error(), tt.errMsg) return From d0e7b08878d1c5a502a3ab487795538161205188 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Fri, 25 Oct 2024 11:18:17 -0500 Subject: [PATCH 10/10] revert func Processability() to pointer receiver --- zetaclient/chains/bitcoin/observer/event.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zetaclient/chains/bitcoin/observer/event.go b/zetaclient/chains/bitcoin/observer/event.go index 81f0d154a3..69657d29f1 100644 --- a/zetaclient/chains/bitcoin/observer/event.go +++ b/zetaclient/chains/bitcoin/observer/event.go @@ -63,7 +63,7 @@ type BTCInboundEvent struct { } // Processability returns the processability of the inbound event -func (event BTCInboundEvent) Processability() InboundProcessability { +func (event *BTCInboundEvent) Processability() InboundProcessability { // compliance check on sender and receiver addresses if config.ContainRestrictedAddress(event.FromAddress, event.ToAddress) { return InboundProcessabilityComplianceViolation