From 46f3d4472c9c2a61eda408550db666f1a81c9843 Mon Sep 17 00:00:00 2001 From: Charlie Chen <34498985+ws4charlie@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:54:38 -0600 Subject: [PATCH] fix: skip depositor fee calculation on irrelevant transactions (#3162) * skip depositor fee calculation on irrelevant txs * add changelog entry * correct PR number in changelog --- changelog.md | 1 + zetaclient/chains/bitcoin/fee.go | 3 + zetaclient/chains/bitcoin/observer/inbound.go | 40 ++++++----- .../chains/bitcoin/observer/inbound_test.go | 68 ++++++++++++++----- zetaclient/chains/bitcoin/observer/witness.go | 8 ++- .../chains/bitcoin/observer/witness_test.go | 34 ++++++++-- 6 files changed, 107 insertions(+), 47 deletions(-) diff --git a/changelog.md b/changelog.md index 4facc05f04..2ac17bbc71 100644 --- a/changelog.md +++ b/changelog.md @@ -27,6 +27,7 @@ * [3106](https://github.com/zeta-chain/node/pull/3106) - prevent blocked CCTX on out of gas during omnichain calls * [3139](https://github.com/zeta-chain/node/pull/3139) - fix config resolution in orchestrator * [3149](https://github.com/zeta-chain/node/pull/3149) - abort the cctx if dust amount is detected in the revert outbound +* [3162](https://github.com/zeta-chain/node/pull/3162) - skip depositor fee calculation if transaction does not involve TSS address ## v21.0.0 diff --git a/zetaclient/chains/bitcoin/fee.go b/zetaclient/chains/bitcoin/fee.go index f56a479364..7ce483a5bf 100644 --- a/zetaclient/chains/bitcoin/fee.go +++ b/zetaclient/chains/bitcoin/fee.go @@ -56,6 +56,9 @@ var ( DefaultDepositorFee = DepositorFee(defaultDepositorFeeRate) ) +// DepositorFeeCalculator is a function type to calculate the Bitcoin depositor fee +type DepositorFeeCalculator func(interfaces.BTCRPCClient, *btcjson.TxRawResult, *chaincfg.Params) (float64, error) + // FeeRateToSatPerByte converts a fee rate in BTC/KB to sat/byte. func FeeRateToSatPerByte(rate float64) *big.Int { // #nosec G115 always in range diff --git a/zetaclient/chains/bitcoin/observer/inbound.go b/zetaclient/chains/bitcoin/observer/inbound.go index 3cd1ab3945..aa4f5667ad 100644 --- a/zetaclient/chains/bitcoin/observer/inbound.go +++ b/zetaclient/chains/bitcoin/observer/inbound.go @@ -255,12 +255,6 @@ func (ob *Observer) CheckReceiptForBtcTxHash(ctx context.Context, txHash string, return "", fmt.Errorf("block %d is not confirmed yet", blockVb.Height) } - // calculate depositor fee - depositorFee, err := bitcoin.CalcDepositorFee(ob.btcClient, tx, ob.netParams) - if err != nil { - return "", errors.Wrapf(err, "error calculating depositor fee for inbound %s", tx.Txid) - } - // #nosec G115 always positive event, err := GetBtcEvent( ob.btcClient, @@ -269,7 +263,7 @@ func (ob *Observer) CheckReceiptForBtcTxHash(ctx context.Context, txHash string, uint64(blockVb.Height), ob.logger.Inbound, ob.netParams, - depositorFee, + bitcoin.CalcDepositorFee, ) if err != nil { return "", err @@ -323,13 +317,7 @@ func FilterAndParseIncomingTx( continue // the first tx is coinbase; we do not process coinbase tx } - // calculate depositor fee - depositorFee, err := bitcoin.CalcDepositorFee(rpcClient, &txs[idx], netParams) - if err != nil { - return nil, errors.Wrapf(err, "error calculating depositor fee for inbound %s", tx.Txid) - } - - event, err := GetBtcEvent(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee) + event, err := GetBtcEvent(rpcClient, tx, tssAddress, blockNumber, logger, netParams, bitcoin.CalcDepositorFee) if err != nil { // unable to parse the tx, the caller should retry return nil, errors.Wrapf(err, "error getting btc event for tx %s in block %d", tx.Txid, blockNumber) @@ -391,12 +379,12 @@ func GetBtcEvent( blockNumber uint64, logger zerolog.Logger, netParams *chaincfg.Params, - depositorFee float64, + feeCalculator bitcoin.DepositorFeeCalculator, ) (*BTCInboundEvent, error) { if netParams.Name == chaincfg.MainNetParams.Name { - return GetBtcEventWithoutWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee) + return GetBtcEventWithoutWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, feeCalculator) } - return GetBtcEventWithWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee) + return GetBtcEventWithWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, feeCalculator) } // GetBtcEventWithoutWitness either returns a valid BTCInboundEvent or nil @@ -409,11 +397,15 @@ func GetBtcEventWithoutWitness( blockNumber uint64, logger zerolog.Logger, netParams *chaincfg.Params, - depositorFee float64, + feeCalculator bitcoin.DepositorFeeCalculator, ) (*BTCInboundEvent, error) { - found := false - var value float64 - var memo []byte + var ( + found bool + value float64 + depositorFee float64 + memo []byte + ) + if len(tx.Vout) >= 2 { // 1st vout must have tss address as receiver with p2wpkh scriptPubKey vout0 := tx.Vout[0] @@ -430,6 +422,12 @@ func GetBtcEventWithoutWitness( return nil, nil } + // calculate depositor fee + depositorFee, err = feeCalculator(rpcClient, &tx, netParams) + if err != nil { + return nil, errors.Wrapf(err, "error calculating depositor fee for inbound %s", tx.Txid) + } + // deposit amount has to be no less than the minimum depositor fee if vout0.Value < depositorFee { logger.Info(). diff --git a/zetaclient/chains/bitcoin/observer/inbound_test.go b/zetaclient/chains/bitcoin/observer/inbound_test.go index 838315b7b8..7ec938aab0 100644 --- a/zetaclient/chains/bitcoin/observer/inbound_test.go +++ b/zetaclient/chains/bitcoin/observer/inbound_test.go @@ -23,6 +23,7 @@ import ( "github.com/zeta-chain/node/testutil/sample" "github.com/zeta-chain/node/zetaclient/chains/bitcoin" "github.com/zeta-chain/node/zetaclient/chains/bitcoin/observer" + "github.com/zeta-chain/node/zetaclient/chains/interfaces" clientcommon "github.com/zeta-chain/node/zetaclient/common" "github.com/zeta-chain/node/zetaclient/keys" "github.com/zeta-chain/node/zetaclient/testutils" @@ -30,6 +31,13 @@ import ( "github.com/zeta-chain/node/zetaclient/testutils/testrpc" ) +// mockDepositFeeCalculator returns a mock depositor fee calculator that returns the given fee and error. +func mockDepositFeeCalculator(fee float64, err error) bitcoin.DepositorFeeCalculator { + return func(interfaces.BTCRPCClient, *btcjson.TxRawResult, *chaincfg.Params) (float64, error) { + return fee, err + } +} + func TestAvgFeeRateBlock828440(t *testing.T) { // load archived block 828440 var blockVb btcjson.GetBlockVerboseTxResult @@ -278,6 +286,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { // fee rate of above tx is 28 sat/vB depositorFee := bitcoin.DepositorFee(28 * clientcommon.BTCOutboundGasPriceMultiplier) + feeCalculator := mockDepositFeeCalculator(depositorFee, nil) // expected result memo, err := hex.DecodeString(tx.Vout[1].ScriptPubKey.Hex[4:]) @@ -309,7 +318,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -333,7 +342,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -357,7 +366,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -381,7 +390,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -405,7 +414,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -425,7 +434,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -445,7 +454,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -459,7 +468,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -479,12 +488,31 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) }) + t.Run("should return error if RPC failed to calculate depositor fee", func(t *testing.T) { + // load tx + tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) + + // get BTC event + rpcClient := mocks.NewBTCRPCClient(t) + event, err := observer.GetBtcEventWithoutWitness( + rpcClient, + *tx, + tssAddress, + blockNumber, + log.Logger, + net, + mockDepositFeeCalculator(0.0, errors.New("rpc error")), + ) + require.ErrorContains(t, err, "rpc error") + require.Nil(t, event) + }) + t.Run("should skip tx if amount is less than depositor fee", func(t *testing.T) { // load tx and modify amount to less than depositor fee tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) @@ -499,7 +527,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -519,7 +547,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -539,7 +567,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -570,7 +598,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -588,6 +616,7 @@ func TestGetBtcEventErrors(t *testing.T) { // fee rate of above tx is 28 sat/vB depositorFee := bitcoin.DepositorFee(28 * clientcommon.BTCOutboundGasPriceMultiplier) + feeCalculator := mockDepositFeeCalculator(depositorFee, nil) t.Run("should return error on invalid Vout[0] script", func(t *testing.T) { // load tx and modify Vout[0] script to invalid script @@ -603,7 +632,7 @@ func TestGetBtcEventErrors(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.Error(t, err) require.Nil(t, event) @@ -623,7 +652,7 @@ func TestGetBtcEventErrors(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.ErrorContains(t, err, "no input found") require.Nil(t, event) @@ -645,7 +674,7 @@ func TestGetBtcEventErrors(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.ErrorContains(t, err, "error getting sender address") require.Nil(t, event) @@ -662,6 +691,8 @@ func TestGetBtcEvent(t *testing.T) { net := &chaincfg.MainNetParams // 2.992e-05, see avgFeeRate https://mempool.space/api/v1/blocks/835640 depositorFee := bitcoin.DepositorFee(22 * clientcommon.BTCOutboundGasPriceMultiplier) + feeCalculator := mockDepositFeeCalculator(depositorFee, nil) + txHash2 := "37777defed8717c581b4c0509329550e344bdc14ac38f71fc050096887e535c8" tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash2, false) rpcClient := mocks.NewBTCRPCClient(t) @@ -673,7 +704,7 @@ func TestGetBtcEvent(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, (*observer.BTCInboundEvent)(nil), event) @@ -693,6 +724,7 @@ func TestGetBtcEvent(t *testing.T) { // fee rate of above tx is 28 sat/vB depositorFee := bitcoin.DepositorFee(28 * clientcommon.BTCOutboundGasPriceMultiplier) + feeCalculator := mockDepositFeeCalculator(depositorFee, nil) // expected result memo, err := hex.DecodeString(tx.Vout[1].ScriptPubKey.Hex[4:]) @@ -723,7 +755,7 @@ func TestGetBtcEvent(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) diff --git a/zetaclient/chains/bitcoin/observer/witness.go b/zetaclient/chains/bitcoin/observer/witness.go index 9625ad3caa..bb85bdc47b 100644 --- a/zetaclient/chains/bitcoin/observer/witness.go +++ b/zetaclient/chains/bitcoin/observer/witness.go @@ -24,7 +24,7 @@ func GetBtcEventWithWitness( blockNumber uint64, logger zerolog.Logger, netParams *chaincfg.Params, - depositorFee float64, + feeCalculator bitcoin.DepositorFeeCalculator, ) (*BTCInboundEvent, error) { if len(tx.Vout) < 1 { logger.Debug().Msgf("no output %s", tx.Txid) @@ -40,6 +40,12 @@ func GetBtcEventWithWitness( return nil, nil } + // calculate depositor fee + depositorFee, err := feeCalculator(client, &tx, netParams) + if err != nil { + return nil, errors.Wrapf(err, "error calculating depositor fee for inbound %s", tx.Txid) + } + isAmountValid, amount := isValidAmount(tx.Vout[0].Value, depositorFee) if !isAmountValid { logger.Info(). diff --git a/zetaclient/chains/bitcoin/observer/witness_test.go b/zetaclient/chains/bitcoin/observer/witness_test.go index af14427c6e..745f2003a9 100644 --- a/zetaclient/chains/bitcoin/observer/witness_test.go +++ b/zetaclient/chains/bitcoin/observer/witness_test.go @@ -61,6 +61,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { // fee rate of above tx is 28 sat/vB depositorFee := bitcoin.DepositorFee(28 * clientcommon.BTCOutboundGasPriceMultiplier) + feeCalculator := mockDepositFeeCalculator(depositorFee, nil) t.Run("decode OP_RETURN ok", func(t *testing.T) { tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) @@ -93,7 +94,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -131,7 +132,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, eventExpected, event) @@ -172,7 +173,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Equal(t, event, eventExpected) @@ -192,12 +193,31 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) }) + t.Run("should return error if RPC failed to calculate depositor fee", func(t *testing.T) { + // load tx + tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) + + // get BTC event + rpcClient := mocks.NewBTCRPCClient(t) + event, err := observer.GetBtcEventWithWitness( + rpcClient, + *tx, + tssAddress, + blockNumber, + log.Logger, + net, + mockDepositFeeCalculator(0.0, errors.New("rpc error")), + ) + require.ErrorContains(t, err, "rpc error") + require.Nil(t, event) + }) + t.Run("should skip tx if amount is less than depositor fee", func(t *testing.T) { // load tx and modify amount to less than depositor fee tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false) @@ -212,7 +232,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event) @@ -234,7 +254,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.ErrorContains(t, err, "rpc error") require.Nil(t, event) @@ -268,7 +288,7 @@ func TestGetBtcEventWithWitness(t *testing.T) { blockNumber, log.Logger, net, - depositorFee, + feeCalculator, ) require.NoError(t, err) require.Nil(t, event)