Skip to content

Commit

Permalink
always use latest gas rate for Bitcoin outbound; enforce gas rate cap
Browse files Browse the repository at this point in the history
  • Loading branch information
ws4charlie committed Jan 8, 2025
1 parent 847e2f1 commit c507687
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 36 deletions.
23 changes: 23 additions & 0 deletions pkg/math/integer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package math

import "math"

// IncreaseIntByPercent is a function that increases integer by a percentage.
// Example1: IncreaseIntByPercent(10, 15, true) = 10 * 1.15 = 12
// Example2: IncreaseIntByPercent(10, 15, false) = 10 + 10 * 0.15 = 11
func IncreaseIntByPercent(value int64, percent uint64, round bool) int64 {
switch {
case percent == 0:
return value
case percent%100 == 0:
// optimization: a simple multiplication
increase := value * int64(percent/100)
return value + increase
default:
increase := float64(value) * float64(percent) / 100
if round {
return value + int64(math.Round(increase))
}
return value + int64(increase)
}
}
30 changes: 30 additions & 0 deletions pkg/math/integer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package math

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

func Test_IncreaseIntByPercent(t *testing.T) {
for i, tt := range []struct {
value int64
percent uint64
round bool
expected int64
}{
{value: 10, percent: 0, round: false, expected: 10},
{value: 10, percent: 15, round: false, expected: 11},
{value: 10, percent: 15, round: true, expected: 12},
{value: 10, percent: 14, round: false, expected: 11},
{value: 10, percent: 14, round: true, expected: 11},
{value: 10, percent: 200, round: false, expected: 30},
{value: 10, percent: 200, round: true, expected: 30},
} {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
result := IncreaseIntByPercent(tt.value, tt.percent, tt.round)
assert.Equal(t, tt.expected, result)
})
}
}
35 changes: 25 additions & 10 deletions zetaclient/chains/bitcoin/signer/fee_bumper.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,22 @@ import (
"github.com/rs/zerolog"

"github.com/zeta-chain/node/pkg/constant"
mathpkg "github.com/zeta-chain/node/pkg/math"
"github.com/zeta-chain/node/zetaclient/chains/bitcoin"
"github.com/zeta-chain/node/zetaclient/chains/bitcoin/rpc"
"github.com/zeta-chain/node/zetaclient/chains/interfaces"
)

const (
// minCPFPFeeBumpFactor is the minimum factor by which the CPFP average fee rate should be bumped.
// gasRateCap is the maximum average gas rate for CPFP fee bumping
// 100 sat/vB is a typical heuristic based on Bitcoin mempool statistics
// see: https://mempool.space/graphs/mempool#3y
gasRateCap = 100

// minCPFPFeeBumpPercent is the minimum percentage by which the CPFP average fee rate should be bumped.
// This value 20% is a heuristic, not mandated by the Bitcoin protocol, designed to balance effectiveness
// in replacing stuck transactions while avoiding excessive sensitivity to fee market fluctuations.
minCPFPFeeBumpFactor = 1.2
minCPFPFeeBumpPercent = 20
)

// MempoolTxsInfoFetcher is a function type to fetch mempool txs information
Expand Down Expand Up @@ -80,7 +86,7 @@ func NewCPFPFeeBumper(
}

// BumpTxFee bumps the fee of the stuck transaction using reserved bump fees
func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, error) {
func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, int64, error) {
// reuse old tx body and clear witness data (e.g., signatures)
newTx := b.Tx.MsgTx().Copy()
for idx := range newTx.TxIn {
Expand All @@ -89,14 +95,14 @@ func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, error) {

// check reserved bump fees amount in the original tx
if len(newTx.TxOut) < 3 {
return nil, 0, errors.New("original tx has no reserved bump fees")
return nil, 0, 0, errors.New("original tx has no reserved bump fees")
}

// tx replacement is triggered only when market fee rate goes 20% higher than current paid fee rate.
// zetacore updates the cctx fee rate evey 10 minutes, we could hold on and retry later.
minBumpRate := int64(math.Ceil(float64(b.AvgFeeRate) * minCPFPFeeBumpFactor))
minBumpRate := mathpkg.IncreaseIntByPercent(b.AvgFeeRate, minCPFPFeeBumpPercent, true)
if b.CCTXRate < minBumpRate {
return nil, 0, fmt.Errorf(
return nil, 0, 0, fmt.Errorf(
"hold on RBF: cctx rate %d is lower than the min bumped rate %d",
b.CCTXRate,
minBumpRate,
Expand All @@ -106,15 +112,21 @@ func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, error) {
// the live rate may continue increasing during network congestion, we should wait until it stabilizes a bit.
// this is to ensure the live rate is not 20%+ higher than the cctx rate, otherwise, the replacement tx may
// also get stuck and need another replacement.
bumpedRate := int64(math.Ceil(float64(b.CCTXRate) * minCPFPFeeBumpFactor))
bumpedRate := mathpkg.IncreaseIntByPercent(b.CCTXRate, minCPFPFeeBumpPercent, true)
if b.LiveRate > bumpedRate {
return nil, 0, fmt.Errorf(
return nil, 0, 0, fmt.Errorf(
"hold on RBF: live rate %d is much higher than the cctx rate %d",
b.LiveRate,
b.CCTXRate,
)
}

// cap the gas rate to avoid excessive fees
gasRateNew := b.CCTXRate
if b.CCTXRate > gasRateCap {
gasRateNew = gasRateCap
}

// calculate minmimum relay fees of the new replacement tx
// the new tx will have almost same size as the old one because the tx body stays the same
txVSize := mempool.GetTxVirtualSize(b.Tx)
Expand All @@ -127,7 +139,7 @@ func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, error) {
// 2. additionalFees >= minRelayTxFees
//
// see: https://github.com/bitcoin/bitcoin/blob/master/src/policy/rbf.cpp#L166-L183
additionalFees := b.TotalVSize*b.CCTXRate - b.TotalFees
additionalFees := b.TotalVSize*gasRateNew - b.TotalFees
if additionalFees < minRelayTxFees {
additionalFees = minRelayTxFees
}
Expand All @@ -142,7 +154,10 @@ func (b *CPFPFeeBumper) BumpTxFee() (*wire.MsgTx, int64, error) {
newTx.TxOut = newTx.TxOut[:2]
}

return newTx, additionalFees, nil
// effective gas rate
gasRateNew = int64(math.Ceil(float64(b.TotalFees+additionalFees) / float64(b.TotalVSize)))

return newTx, additionalFees, gasRateNew, nil
}

// fetchFeeBumpInfo fetches all necessary information needed to bump the stuck tx
Expand Down
59 changes: 42 additions & 17 deletions zetaclient/chains/bitcoin/signer/fee_bumper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func Test_NewCPFPFeeBumper(t *testing.T) {
2, // 2 stuck TSS txs
0.0001, // total fees 0.0001 BTC
1000, // total vsize 1000
10, // average fee rate 10 sat/vbyte
10, // average fee rate 10 sat/vB
"", // no error
),
expected: &signer.CPFPFeeBumper{
Expand Down Expand Up @@ -106,11 +106,12 @@ func Test_BumpTxFee(t *testing.T) {
msgTx := testutils.LoadBTCMsgTx(t, TestDataDir, chain.ChainId, txid)

tests := []struct {
name string
feeBumper *signer.CPFPFeeBumper
errMsg string
additionalFees int64
expectedTx *wire.MsgTx
name string
feeBumper *signer.CPFPFeeBumper
additionalFees int64
expectedNewRate int64
expectedNewTx *wire.MsgTx
errMsg string
}{
{
name: "should bump tx fee successfully",
Expand All @@ -123,8 +124,9 @@ func Test_BumpTxFee(t *testing.T) {
TotalVSize: 579,
AvgFeeRate: 47,
},
additionalFees: 5790,
expectedTx: func() *wire.MsgTx {
additionalFees: 5790,
expectedNewRate: 57,
expectedNewTx: func() *wire.MsgTx {
// deduct additional fees
newTx := copyMsgTx(msgTx)
newTx.TxOut[2].Value -= 5790
Expand All @@ -137,13 +139,14 @@ func Test_BumpTxFee(t *testing.T) {
Tx: btcutil.NewTx(msgTx),
MinRelayFee: 0.00002, // min relay fee will be 579vB * 2 = 1158 sats
CCTXRate: 6,
LiveRate: 8,
LiveRate: 7,
TotalFees: 2895,
TotalVSize: 579,
AvgFeeRate: 5,
},
additionalFees: 1158,
expectedTx: func() *wire.MsgTx {
additionalFees: 1158,
expectedNewRate: 7, // (2895 + 1158) / 579 = 7
expectedNewTx: func() *wire.MsgTx {
// deduct additional fees
newTx := copyMsgTx(msgTx)
newTx.TxOut[2].Value -= 1158
Expand All @@ -166,14 +169,35 @@ func Test_BumpTxFee(t *testing.T) {
TotalVSize: 579,
AvgFeeRate: 47,
},
additionalFees: 5790 + constant.BTCWithdrawalDustAmount - 1, // 6789
expectedTx: func() *wire.MsgTx {
additionalFees: 5790 + constant.BTCWithdrawalDustAmount - 1, // 6789
expectedNewRate: 59, // (27213 + 6789) / 579 ≈ 59
expectedNewTx: func() *wire.MsgTx {
// give up all reserved bump fees
newTx := copyMsgTx(msgTx)
newTx.TxOut = newTx.TxOut[:2]
return newTx
}(),
},
{
name: "should cap new gas rate to 'gasRateCap'",
feeBumper: &signer.CPFPFeeBumper{
Tx: btcutil.NewTx(msgTx),
MinRelayFee: 0.00001,
CCTXRate: 101, // > 100
LiveRate: 120,
TotalFees: 27213,
TotalVSize: 579,
AvgFeeRate: 47,
},
additionalFees: 30687, // (100-47)*579
expectedNewRate: 100,
expectedNewTx: func() *wire.MsgTx {
// deduct additional fees
newTx := copyMsgTx(msgTx)
newTx.TxOut[2].Value -= 30687
return newTx
}(),
},
{
name: "should fail if original tx has no reserved bump fees",
feeBumper: &signer.CPFPFeeBumper{
Expand All @@ -190,7 +214,7 @@ func Test_BumpTxFee(t *testing.T) {
name: "should hold on RBF if CCTX rate is lower than minimum bumpeed rate",
feeBumper: &signer.CPFPFeeBumper{
Tx: btcutil.NewTx(msgTx),
CCTXRate: 56, // 56 < 47 * 120%
CCTXRate: 55, // 56 < 47 * 120%
AvgFeeRate: 47,
},
errMsg: "lower than the min bumped rate",
Expand All @@ -209,15 +233,16 @@ func Test_BumpTxFee(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
newTx, additionalFees, err := tt.feeBumper.BumpTxFee()
newTx, additionalFees, newRate, err := tt.feeBumper.BumpTxFee()
if tt.errMsg != "" {
require.Nil(t, newTx)
require.Zero(t, additionalFees)
require.ErrorContains(t, err, tt.errMsg)
} else {
require.NoError(t, err)
require.Equal(t, tt.expectedTx, newTx)
require.Equal(t, tt.expectedNewTx, newTx)
require.Equal(t, tt.additionalFees, additionalFees)
require.Equal(t, tt.expectedNewRate, newRate)
}
})
}
Expand Down Expand Up @@ -246,7 +271,7 @@ func Test_FetchFeeBumpInfo(t *testing.T) {
2, // 2 stuck TSS txs
0.0001, // total fees 0.0001 BTC
1000, // total vsize 1000
10, // average fee rate 10 sat/vbyte
10, // average fee rate 10 sat/vB
"", // no error
),
expected: &signer.CPFPFeeBumper{
Expand Down
9 changes: 8 additions & 1 deletion zetaclient/chains/bitcoin/signer/outbound_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,19 @@ func NewOutboundData(
return nil, errors.New("can only send gas token to a Bitcoin network")
}

// fee rate
// initial fee rate
feeRate, err := strconv.ParseInt(params.GasPrice, 10, 64)
if err != nil || feeRate < 0 {
return nil, fmt.Errorf("cannot convert gas price %s", params.GasPrice)
}

// use current gas rate if fed by zetacore
newRate, err := strconv.ParseInt(params.GasPriorityFee, 10, 64)
if err == nil && newRate > 0 && newRate != feeRate {
logger.Info().Msgf("use new gas rate %d sat/vB instead of %d sat/vB", newRate, feeRate)
feeRate = newRate
}

// check receiver address
to, err := chains.DecodeBtcAddress(params.Receiver, params.ReceiverChainId)
if err != nil {
Expand Down
28 changes: 28 additions & 0 deletions zetaclient/chains/bitcoin/signer/outbound_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,34 @@ func Test_NewOutboundData(t *testing.T) {
},
errMsg: "",
},
{
name: "create new outbound data using current gas rate instead of old rate",
cctx: sample.CrossChainTx(t, "0x123"),
cctxModifier: func(cctx *crosschaintypes.CrossChainTx) {
cctx.InboundParams.CoinType = coin.CoinType_Gas
cctx.GetCurrentOutboundParam().Receiver = receiver.String()
cctx.GetCurrentOutboundParam().ReceiverChainId = chain.ChainId
cctx.GetCurrentOutboundParam().Amount = sdk.NewUint(1e7) // 0.1 BTC
cctx.GetCurrentOutboundParam().CallOptions.GasLimit = 254 // 254 bytes
cctx.GetCurrentOutboundParam().GasPrice = "10" // 10 sats/vByte
cctx.GetCurrentOutboundParam().GasPriorityFee = "15" // 15 sats/vByte
cctx.GetCurrentOutboundParam().TssNonce = 1
},
chainID: chain.ChainId,
height: 101,
minRelayFee: 0.00001, // 1000 sat/KB
expected: &OutboundData{
chainID: chain.ChainId,
to: receiver,
amount: 0.1,
feeRate: 16, // 15 + 1 (minRelayFee)
txSize: 254,
nonce: 1,
height: 101,
cancelTx: false,
},
errMsg: "",
},
{
name: "cctx is nil",
cctx: nil,
Expand Down
17 changes: 11 additions & 6 deletions zetaclient/chains/bitcoin/signer/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ const (

// the rank below (or equal to) which we consolidate UTXOs
consolidationRank = 10

// reservedRBFFees is the amount of BTC reserved for RBF fee bumping.
// the TSS keysign stops automatically when transactions get stuck in the mempool
// 0.01 BTC can bump 10 transactions (1KB each) by 100 sat/vB
reservedRBFFees = 0.01
)

// SignWithdrawTx signs a BTC withdrawal tx and returns the signed tx
Expand All @@ -35,15 +40,15 @@ func (signer *Signer) SignWithdrawTx(
) (*wire.MsgTx, error) {
nonceMark := chains.NonceMarkAmount(txData.nonce)
estimateFee := float64(txData.feeRate*bitcoin.OutboundBytesMax) / 1e8
totalAmount := txData.amount + estimateFee + reservedRBFFees + float64(nonceMark)*1e-8

// refreshing UTXO list before TSS keysign is important:
// 1. all TSS outbounds have opted-in for RBF to be replaceable
// 2. using old UTXOs may lead to accidental double-spending
// 3. double-spending may trigger unexpected tx replacement (RBF)
// 2. using old UTXOs may lead to accidental double-spending, which may trigger unwanted RBF
//
// Note: unwanted RBF will rarely happen for two reasons:
// Note: unwanted RBF is very unlikely to happen for two reasons:
// 1. it requires 2/3 TSS signers to accidentally sign the same tx using same outdated UTXOs.
// 2. RBF requires a higher fee rate than the original tx.
// 2. RBF requires a higher fee rate than the original tx, otherwise it will fail.
err := ob.FetchUTXOs(ctx)
if err != nil {
return nil, errors.Wrap(err, "FetchUTXOs failed")
Expand All @@ -52,7 +57,7 @@ func (signer *Signer) SignWithdrawTx(
// select N UTXOs to cover the total expense
prevOuts, total, consolidatedUtxo, consolidatedValue, err := ob.SelectUTXOs(
ctx,
txData.amount+estimateFee+float64(nonceMark)*1e-8,
totalAmount,
MaxNoOfInputsPerTx,
txData.nonce,
consolidationRank,
Expand All @@ -79,7 +84,7 @@ func (signer *Signer) SignWithdrawTx(
signer.Logger().Std.Info().
Msgf("txSize %d is less than BtcOutboundBytesWithdrawer %d for nonce %d", txData.txSize, txSize, txData.nonce)
}
if txSize < bitcoin.OutboundBytesMin { // outbound shouldn't be blocked a low sizeLimit
if txSize < bitcoin.OutboundBytesMin { // outbound shouldn't be blocked by low sizeLimit
signer.Logger().Std.Warn().
Msgf("txSize %d is less than outboundBytesMin %d; use outboundBytesMin", txSize, bitcoin.OutboundBytesMin)
txSize = bitcoin.OutboundBytesMin
Expand Down
Loading

0 comments on commit c507687

Please sign in to comment.