Skip to content

Commit

Permalink
fix: skip depositor fee calculation on irrelevant transactions (#3162)
Browse files Browse the repository at this point in the history
* skip depositor fee calculation on irrelevant txs

* add changelog entry

* correct PR number in changelog
  • Loading branch information
ws4charlie authored Nov 14, 2024
1 parent 6fb64ca commit 46f3d44
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 47 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions zetaclient/chains/bitcoin/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 19 additions & 21 deletions zetaclient/chains/bitcoin/observer/inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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().
Expand Down
68 changes: 50 additions & 18 deletions zetaclient/chains/bitcoin/observer/inbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,21 @@ 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"
"github.com/zeta-chain/node/zetaclient/testutils/mocks"
"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
Expand Down Expand Up @@ -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:])
Expand Down Expand Up @@ -309,7 +318,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand All @@ -333,7 +342,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand All @@ -357,7 +366,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand All @@ -381,7 +390,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand All @@ -405,7 +414,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand All @@ -425,7 +434,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -445,7 +454,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -459,7 +468,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -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)
Expand All @@ -499,7 +527,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -519,7 +547,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -539,7 +567,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand Down Expand Up @@ -570,7 +598,7 @@ func TestGetBtcEventWithoutWitness(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Nil(t, event)
Expand All @@ -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
Expand All @@ -603,7 +632,7 @@ func TestGetBtcEventErrors(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.Error(t, err)
require.Nil(t, event)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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:])
Expand Down Expand Up @@ -723,7 +755,7 @@ func TestGetBtcEvent(t *testing.T) {
blockNumber,
log.Logger,
net,
depositorFee,
feeCalculator,
)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
Expand Down
8 changes: 7 additions & 1 deletion zetaclient/chains/bitcoin/observer/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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().
Expand Down
Loading

0 comments on commit 46f3d44

Please sign in to comment.