Skip to content

Commit

Permalink
refactor: remove btc deposit fee v1 and improve unit tests (#2899)
Browse files Browse the repository at this point in the history
* remove CalcDepositorFeeV2; add unit tests; migration mock btc client to Mockery

* replace manual btc rpc mock with Mockery

* move createRPCClientAndLoadTx to testutils package as CreateBTCRPCAndLoadTx

* move GetSenderAddressByVin to inbound.go as it's for inbound observation

* add unit test for IsBlockConfirmed

* add changelog entry

* remove unused constants

* move changelog entry under Refactor section; created sub method DecodeSenderFromScript() and added unit tests
  • Loading branch information
ws4charlie authored Sep 23, 2024
1 parent d14b200 commit 3eba701
Show file tree
Hide file tree
Showing 19 changed files with 1,097 additions and 544 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* [2802](https://github.com/zeta-chain/node/pull/2802) - set default liquidity cap for new ZRC20s
* [2826](https://github.com/zeta-chain/node/pull/2826) - remove unused code from emissions module and add new parameter for fixed block reward amount
* [2890](https://github.com/zeta-chain/node/pull/2890) - refactor `MsgUpdateChainInfo` to accept a single chain, and add `MsgRemoveChainInfo` to remove a chain
* [2899](https://github.com/zeta-chain/node/pull/2899) - remove btc deposit fee v1 and improve unit tests

### Tests

Expand Down
2 changes: 0 additions & 2 deletions e2e/runner/bitcoin.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,15 +267,13 @@ func (r *E2ERunner) SendToTSSFromDeployerWithMemo(
rawtx, err := btcRPC.GetRawTransactionVerbose(txid)
require.NoError(r, err)

depositorFee := zetabitcoin.DefaultDepositorFee
events, err := btcobserver.FilterAndParseIncomingTx(
btcRPC,
[]btcjson.TxRawResult{*rawtx},
0,
r.BTCTSSAddress.EncodeAddress(),
log.Logger,
r.BitcoinParams,
depositorFee,
)
require.NoError(r, err)
r.Logger.Info("bitcoin inbound events:")
Expand Down
9 changes: 9 additions & 0 deletions zetaclient/chains/base/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,15 @@ func (ob *Observer) WithLastBlock(lastBlock uint64) *Observer {
return ob
}

// IsBlockConfirmed checks if the given block number is confirmed.
//
// Note: block 100 is confirmed if the last block is 100 and confirmation count is 1.
func (ob *Observer) IsBlockConfirmed(blockNumber uint64) bool {
lastBlock := ob.LastBlock()
confBlock := blockNumber + ob.chainParams.ConfirmationCount - 1
return lastBlock >= confBlock
}

// LastBlockScanned get last block scanned (not necessarily caught up with the chain; could be slow/paused).
func (ob *Observer) LastBlockScanned() uint64 {
height := atomic.LoadUint64(&ob.lastBlockScanned)
Expand Down
49 changes: 49 additions & 0 deletions zetaclient/chains/base/observer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@ import (
const (
// defaultAlertLatency is the default alert latency (in seconds) for unit tests
defaultAlertLatency = 60

// defaultConfirmationCount is the default confirmation count for unit tests
defaultConfirmationCount = 2
)

// createObserver creates a new observer for testing
func createObserver(t *testing.T, chain chains.Chain, alertLatency int64) *base.Observer {
// constructor parameters
chainParams := *sample.ChainParams(chain.ChainId)
chainParams.ConfirmationCount = defaultConfirmationCount
zetacoreClient := mocks.NewZetacoreClient(t)
tss := mocks.NewTSSMainnet()

Expand Down Expand Up @@ -267,6 +271,51 @@ func TestObserverGetterAndSetter(t *testing.T) {
})
}

func TestIsBlockConfirmed(t *testing.T) {
tests := []struct {
name string
chain chains.Chain
block uint64
lastBlock uint64
confirmed bool
}{
{
name: "should confirm block 100 when confirmation arrives 2",
chain: chains.BitcoinMainnet,
block: 100,
lastBlock: 101, // got 2 confirmations
confirmed: true,
},
{
name: "should not confirm block 100 when confirmation < 2",
chain: chains.BitcoinMainnet,
block: 100,
lastBlock: 100, // got 1 confirmation, need one more
confirmed: false,
},
{
name: "should confirm block 100 when confirmation arrives 2",
chain: chains.Ethereum,
block: 100,
lastBlock: 99, // last block lagging behind, need to wait
confirmed: false,
},
}

// run tests
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// create observer
ob := createObserver(t, tt.chain, defaultAlertLatency)
ob = ob.WithLastBlock(tt.lastBlock)

// check if block is confirmed
confirmed := ob.IsBlockConfirmed(tt.block)
require.Equal(t, tt.confirmed, confirmed)
})
}
}

func TestOutboundID(t *testing.T) {
tests := []struct {
name string
Expand Down
39 changes: 1 addition & 38 deletions zetaclient/chains/bitcoin/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcd/wire"
"github.com/pkg/errors"
"github.com/rs/zerolog"

"github.com/zeta-chain/node/pkg/chains"
"github.com/zeta-chain/node/zetaclient/chains/bitcoin/rpc"
Expand Down Expand Up @@ -44,13 +43,6 @@ const (

// feeRateCountBackBlocks is the default number of blocks to look back for fee rate estimation
feeRateCountBackBlocks = 2

// DynamicDepositorFeeHeight is the mainnet height from which dynamic depositor fee V1 is applied
DynamicDepositorFeeHeight = 834500

// DynamicDepositorFeeHeightV2 is the mainnet height from which dynamic depositor fee V2 is applied
// Height 863400 is approximately a month away (2024-09-28) from the time of writing, allowing enough time for the upgrade
DynamicDepositorFeeHeightV2 = 863400
)

var (
Expand Down Expand Up @@ -226,37 +218,8 @@ func CalcBlockAvgFeeRate(blockVb *btcjson.GetBlockVerboseTxResult, netParams *ch
return txsFees / int64(vBytes), nil
}

// CalcDepositorFee calculates the depositor fee for a given block
// CalcDepositorFee calculates the depositor fee for a given tx result
func CalcDepositorFee(
blockVb *btcjson.GetBlockVerboseTxResult,
chainID int64,
netParams *chaincfg.Params,
logger zerolog.Logger,
) float64 {
// use default fee for regnet
if chains.IsBitcoinRegnet(chainID) {
return DefaultDepositorFee
}
// mainnet dynamic fee takes effect only after a planned upgrade height
if chains.IsBitcoinMainnet(chainID) && blockVb.Height < DynamicDepositorFeeHeight {
return DefaultDepositorFee
}

// calculate deposit fee rate
feeRate, err := CalcBlockAvgFeeRate(blockVb, netParams)
if err != nil {
feeRate = defaultDepositorFeeRate // use default fee rate if calculation fails, should not happen
logger.Error().Err(err).Msgf("cannot calculate fee rate for block %d", blockVb.Height)
}

// #nosec G115 always in range
feeRate = int64(float64(feeRate) * clientcommon.BTCOutboundGasPriceMultiplier)

return DepositorFee(feeRate)
}

// CalcDepositorFeeV2 calculates the depositor fee for a given tx result
func CalcDepositorFeeV2(
rpcClient interfaces.BTCRPCClient,
rawResult *btcjson.TxRawResult,
netParams *chaincfg.Params,
Expand Down
Loading

0 comments on commit 3eba701

Please sign in to comment.