Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: skip depositor fee calculation on irrelevant transactions #3162

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 @@
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 @@
uint64(blockVb.Height),
ob.logger.Inbound,
ob.netParams,
depositorFee,
bitcoin.CalcDepositorFee,

Check warning on line 266 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L266

Added line #L266 was not covered by tests
)
if err != nil {
return "", err
Expand Down Expand Up @@ -323,13 +317,7 @@
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)

Check warning on line 320 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L320

Added line #L320 was not covered by tests
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 @@
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)

Check warning on line 387 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L387

Added line #L387 was not covered by tests
}

// GetBtcEventWithoutWitness either returns a valid BTCInboundEvent or nil
Expand All @@ -409,11 +397,15 @@
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 @@
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
Loading