From a22c1fdbdff7674cc55662dc4894f6c8ccef47e3 Mon Sep 17 00:00:00 2001 From: Charlie Chen <34498985+ws4charlie@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:38:33 -0600 Subject: [PATCH] fix: use chain parameter ConfirmationCount for bitcoin outtx (#1674) * use chain param ConfirmationCount for bitcoin outtx * update changelog * added unit tests for ConfirmationsThreshold * added some description for constants --- changelog.md | 1 + zetaclient/bitcoin_client.go | 17 ++- zetaclient/bitcoin_client_rpc_test.go | 163 ++++++++++++++++++++++++ zetaclient/bitcoin_client_test.go | 173 +++----------------------- 4 files changed, 194 insertions(+), 160 deletions(-) create mode 100644 zetaclient/bitcoin_client_rpc_test.go diff --git a/changelog.md b/changelog.md index 3c82c9f527..86157860ba 100644 --- a/changelog.md +++ b/changelog.md @@ -33,6 +33,7 @@ * [1663](https://github.com/zeta-chain/node/issues/1663) - skip Mumbai empty block if ethclient sanity check fails * [1661](https://github.com/zeta-chain/node/issues/1661) - use estimated SegWit tx size for Bitcoin gas fee calculation * [1667](https://github.com/zeta-chain/node/issues/1667) - estimate SegWit tx size in uinit of vByte +* [1675](https://github.com/zeta-chain/node/issues/1675) - use chain param ConfirmationCount for bitcoin confirmation ## Version: v12.1.0 diff --git a/zetaclient/bitcoin_client.go b/zetaclient/bitcoin_client.go index b67f8c0dd6..44b19a7b8b 100644 --- a/zetaclient/bitcoin_client.go +++ b/zetaclient/bitcoin_client.go @@ -73,9 +73,10 @@ type BitcoinChainClient struct { } const ( - minConfirmations = 0 - maxHeightDiff = 10000 - btcBlocksPerDay = 144 + maxHeightDiff = 10000 // in case the last block is too old when the observer starts + btcBlocksPerDay = 144 // for LRU block cache size + bigValueSats = 200000000 // 2 BTC + bigValueConfirmationCount = 6 // 6 confirmations for value >= 2 BTC ) func (ob *BitcoinChainClient) WithZetaClient(bridge *ZetaCoreBridge) { @@ -459,10 +460,14 @@ func (ob *BitcoinChainClient) observeInTx() error { // ConfirmationsThreshold returns number of required Bitcoin confirmations depending on sent BTC amount. func (ob *BitcoinChainClient) ConfirmationsThreshold(amount *big.Int) int64 { - if amount.Cmp(big.NewInt(200000000)) >= 0 { - return 6 + if amount.Cmp(big.NewInt(bigValueSats)) >= 0 { + return bigValueConfirmationCount } - return 2 + if bigValueConfirmationCount < ob.GetChainParams().ConfirmationCount { + return bigValueConfirmationCount + } + // #nosec G701 always in range + return int64(ob.GetChainParams().ConfirmationCount) } // IsSendOutTxProcessed returns isIncluded(or inMempool), isConfirmed, Error diff --git a/zetaclient/bitcoin_client_rpc_test.go b/zetaclient/bitcoin_client_rpc_test.go new file mode 100644 index 0000000000..49685b1f58 --- /dev/null +++ b/zetaclient/bitcoin_client_rpc_test.go @@ -0,0 +1,163 @@ +//go:build btc_regtest +// +build btc_regtest + +package zetaclient + +import ( + "encoding/hex" + "math/big" + "testing" + + "github.com/btcsuite/btcd/btcjson" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/ethereum/go-ethereum/crypto" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/suite" + "github.com/zeta-chain/zetacore/common" +) + +type BitcoinClientTestSuite struct { + suite.Suite + BitcoinChainClient *BitcoinChainClient +} + +func (suite *BitcoinClientTestSuite) SetupTest() { + // test private key with EVM address + //// EVM: 0x236C7f53a90493Bb423411fe4117Cb4c2De71DfB + // BTC testnet3: muGe9prUBjQwEnX19zG26fVRHNi8z7kSPo + skHex := "7b8507ba117e069f4a3f456f505276084f8c92aee86ac78ae37b4d1801d35fa8" + privateKey, err := crypto.HexToECDSA(skHex) + suite.Require().NoError(err) + pkBytes := crypto.FromECDSAPub(&privateKey.PublicKey) + suite.T().Logf("pubkey: %d", len(pkBytes)) + + tss := TestSigner{ + PrivKey: privateKey, + } + //client, err := NewBitcoinClient(common.BtcTestNetChain(), nil, tss, "", nil) + client, err := NewBitcoinClient(common.BtcRegtestChain(), nil, tss, "/tmp", nil) + suite.Require().NoError(err) + suite.BitcoinChainClient = client + skBytes, err := hex.DecodeString(skHex) + suite.Require().NoError(err) + suite.T().Logf("skBytes: %d", len(skBytes)) + + btc := client.rpcClient + + _, err = btc.CreateWallet("smoketest") + suite.Require().NoError(err) + addr, err := btc.GetNewAddress("test") + suite.Require().NoError(err) + suite.T().Logf("deployer address: %s", addr) + //err = btc.ImportPrivKey(privkeyWIF) + //suite.Require().NoError(err) + + btc.GenerateToAddress(101, addr, nil) + suite.Require().NoError(err) + + bal, err := btc.GetBalance("*") + suite.Require().NoError(err) + suite.T().Logf("balance: %f", bal.ToBTC()) + + utxo, err := btc.ListUnspent() + suite.Require().NoError(err) + suite.T().Logf("utxo: %d", len(utxo)) + for _, u := range utxo { + suite.T().Logf("utxo: %s %f", u.Address, u.Amount) + } +} + +func (suite *BitcoinClientTestSuite) TearDownSuite() { + +} + +func (suite *BitcoinClientTestSuite) Test0() { + +} + +// All methods that begin with "Test" are run as tests within a +// suite. +func (suite *BitcoinClientTestSuite) Test1() { + feeResult, err := suite.BitcoinChainClient.rpcClient.EstimateSmartFee(1, nil) + suite.Require().NoError(err) + suite.T().Logf("fee result: %f", *feeResult.FeeRate) + bn, err := suite.BitcoinChainClient.rpcClient.GetBlockCount() + suite.Require().NoError(err) + suite.T().Logf("block %d", bn) + + hashStr := "0000000000000032cb372f5d5d99c1ebf4430a3059b67c47a54dd626550fb50d" + var hash chainhash.Hash + err = chainhash.Decode(&hash, hashStr) + suite.Require().NoError(err) + + //:= suite.BitcoinChainClient.rpcClient.GetBlock(&hash) + block, err := suite.BitcoinChainClient.rpcClient.GetBlockVerboseTx(&hash) + suite.Require().NoError(err) + suite.T().Logf("block confirmation %d", block.Confirmations) + suite.T().Logf("block txs len %d", len(block.Tx)) + + inTxs := FilterAndParseIncomingTx( + block.Tx, + uint64(block.Height), + "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2", + &log.Logger, + common.BtcRegtestChain().ChainId, + ) + + suite.Require().Equal(1, len(inTxs)) + suite.Require().Equal(inTxs[0].Value, 0.0001) + suite.Require().Equal(inTxs[0].ToAddress, "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2") + // the text memo is base64 std encoded string:DSRR1RmDCwWmxqY201/TMtsJdmA= + // see https://blockstream.info/testnet/tx/889bfa69eaff80a826286d42ec3f725fd97c3338357ddc3a1f543c2d6266f797 + memo, err := hex.DecodeString("0d2451D519830B05a6C6a636d35fd332dB097660") + suite.Require().NoError(err) + suite.Require().Equal((inTxs[0].MemoBytes), memo) + suite.Require().Equal(inTxs[0].FromAddress, "tb1qyslx2s8evalx67n88wf42yv7236303ezj3tm2l") + suite.T().Logf("from: %s", inTxs[0].FromAddress) + suite.Require().Equal(inTxs[0].BlockNumber, uint64(2406185)) + suite.Require().Equal(inTxs[0].TxHash, "889bfa69eaff80a826286d42ec3f725fd97c3338357ddc3a1f543c2d6266f797") +} + +// a tx with memo around 81B (is this allowed1?) +func (suite *BitcoinClientTestSuite) Test2() { + hashStr := "000000000000002fd8136dbf91708898da9d6ae61d7c354065a052568e2f2888" + var hash chainhash.Hash + err := chainhash.Decode(&hash, hashStr) + suite.Require().NoError(err) + + //:= suite.BitcoinChainClient.rpcClient.GetBlock(&hash) + block, err := suite.BitcoinChainClient.rpcClient.GetBlockVerboseTx(&hash) + suite.Require().NoError(err) + suite.T().Logf("block confirmation %d", block.Confirmations) + suite.T().Logf("block height %d", block.Height) + suite.T().Logf("block txs len %d", len(block.Tx)) + + inTxs := FilterAndParseIncomingTx( + block.Tx, + uint64(block.Height), + "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2", + &log.Logger, + common.BtcRegtestChain().ChainId, + ) + + suite.Require().Equal(0, len(inTxs)) +} + +func (suite *BitcoinClientTestSuite) Test3() { + client := suite.BitcoinChainClient.rpcClient + res, err := client.EstimateSmartFee(1, &btcjson.EstimateModeConservative) + suite.Require().NoError(err) + suite.T().Logf("fee: %f", *res.FeeRate) + suite.T().Logf("blocks: %d", res.Blocks) + suite.T().Logf("errors: %s", res.Errors) + gasPrice := big.NewFloat(0) + gasPriceU64, _ := gasPrice.Mul(big.NewFloat(*res.FeeRate), big.NewFloat(1e8)).Uint64() + suite.T().Logf("gas price: %d", gasPriceU64) + + bn, err := client.GetBlockCount() + suite.Require().NoError(err) + suite.T().Logf("block number %d", bn) +} +func TestBitcoinChainClient(t *testing.T) { + suite.Run(t, new(BitcoinClientTestSuite)) +} diff --git a/zetaclient/bitcoin_client_test.go b/zetaclient/bitcoin_client_test.go index 49685b1f58..84d69c4a2c 100644 --- a/zetaclient/bitcoin_client_test.go +++ b/zetaclient/bitcoin_client_test.go @@ -1,163 +1,28 @@ -//go:build btc_regtest -// +build btc_regtest - package zetaclient import ( - "encoding/hex" "math/big" + "sync" "testing" - "github.com/btcsuite/btcd/btcjson" - "github.com/btcsuite/btcd/chaincfg/chainhash" - "github.com/ethereum/go-ethereum/crypto" - "github.com/rs/zerolog/log" - "github.com/stretchr/testify/suite" - "github.com/zeta-chain/zetacore/common" + "github.com/stretchr/testify/require" + observertypes "github.com/zeta-chain/zetacore/x/observer/types" ) -type BitcoinClientTestSuite struct { - suite.Suite - BitcoinChainClient *BitcoinChainClient -} - -func (suite *BitcoinClientTestSuite) SetupTest() { - // test private key with EVM address - //// EVM: 0x236C7f53a90493Bb423411fe4117Cb4c2De71DfB - // BTC testnet3: muGe9prUBjQwEnX19zG26fVRHNi8z7kSPo - skHex := "7b8507ba117e069f4a3f456f505276084f8c92aee86ac78ae37b4d1801d35fa8" - privateKey, err := crypto.HexToECDSA(skHex) - suite.Require().NoError(err) - pkBytes := crypto.FromECDSAPub(&privateKey.PublicKey) - suite.T().Logf("pubkey: %d", len(pkBytes)) - - tss := TestSigner{ - PrivKey: privateKey, - } - //client, err := NewBitcoinClient(common.BtcTestNetChain(), nil, tss, "", nil) - client, err := NewBitcoinClient(common.BtcRegtestChain(), nil, tss, "/tmp", nil) - suite.Require().NoError(err) - suite.BitcoinChainClient = client - skBytes, err := hex.DecodeString(skHex) - suite.Require().NoError(err) - suite.T().Logf("skBytes: %d", len(skBytes)) - - btc := client.rpcClient - - _, err = btc.CreateWallet("smoketest") - suite.Require().NoError(err) - addr, err := btc.GetNewAddress("test") - suite.Require().NoError(err) - suite.T().Logf("deployer address: %s", addr) - //err = btc.ImportPrivKey(privkeyWIF) - //suite.Require().NoError(err) - - btc.GenerateToAddress(101, addr, nil) - suite.Require().NoError(err) - - bal, err := btc.GetBalance("*") - suite.Require().NoError(err) - suite.T().Logf("balance: %f", bal.ToBTC()) - - utxo, err := btc.ListUnspent() - suite.Require().NoError(err) - suite.T().Logf("utxo: %d", len(utxo)) - for _, u := range utxo { - suite.T().Logf("utxo: %s %f", u.Address, u.Amount) - } -} - -func (suite *BitcoinClientTestSuite) TearDownSuite() { - -} - -func (suite *BitcoinClientTestSuite) Test0() { - -} - -// All methods that begin with "Test" are run as tests within a -// suite. -func (suite *BitcoinClientTestSuite) Test1() { - feeResult, err := suite.BitcoinChainClient.rpcClient.EstimateSmartFee(1, nil) - suite.Require().NoError(err) - suite.T().Logf("fee result: %f", *feeResult.FeeRate) - bn, err := suite.BitcoinChainClient.rpcClient.GetBlockCount() - suite.Require().NoError(err) - suite.T().Logf("block %d", bn) - - hashStr := "0000000000000032cb372f5d5d99c1ebf4430a3059b67c47a54dd626550fb50d" - var hash chainhash.Hash - err = chainhash.Decode(&hash, hashStr) - suite.Require().NoError(err) - - //:= suite.BitcoinChainClient.rpcClient.GetBlock(&hash) - block, err := suite.BitcoinChainClient.rpcClient.GetBlockVerboseTx(&hash) - suite.Require().NoError(err) - suite.T().Logf("block confirmation %d", block.Confirmations) - suite.T().Logf("block txs len %d", len(block.Tx)) - - inTxs := FilterAndParseIncomingTx( - block.Tx, - uint64(block.Height), - "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2", - &log.Logger, - common.BtcRegtestChain().ChainId, - ) - - suite.Require().Equal(1, len(inTxs)) - suite.Require().Equal(inTxs[0].Value, 0.0001) - suite.Require().Equal(inTxs[0].ToAddress, "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2") - // the text memo is base64 std encoded string:DSRR1RmDCwWmxqY201/TMtsJdmA= - // see https://blockstream.info/testnet/tx/889bfa69eaff80a826286d42ec3f725fd97c3338357ddc3a1f543c2d6266f797 - memo, err := hex.DecodeString("0d2451D519830B05a6C6a636d35fd332dB097660") - suite.Require().NoError(err) - suite.Require().Equal((inTxs[0].MemoBytes), memo) - suite.Require().Equal(inTxs[0].FromAddress, "tb1qyslx2s8evalx67n88wf42yv7236303ezj3tm2l") - suite.T().Logf("from: %s", inTxs[0].FromAddress) - suite.Require().Equal(inTxs[0].BlockNumber, uint64(2406185)) - suite.Require().Equal(inTxs[0].TxHash, "889bfa69eaff80a826286d42ec3f725fd97c3338357ddc3a1f543c2d6266f797") -} - -// a tx with memo around 81B (is this allowed1?) -func (suite *BitcoinClientTestSuite) Test2() { - hashStr := "000000000000002fd8136dbf91708898da9d6ae61d7c354065a052568e2f2888" - var hash chainhash.Hash - err := chainhash.Decode(&hash, hashStr) - suite.Require().NoError(err) - - //:= suite.BitcoinChainClient.rpcClient.GetBlock(&hash) - block, err := suite.BitcoinChainClient.rpcClient.GetBlockVerboseTx(&hash) - suite.Require().NoError(err) - suite.T().Logf("block confirmation %d", block.Confirmations) - suite.T().Logf("block height %d", block.Height) - suite.T().Logf("block txs len %d", len(block.Tx)) - - inTxs := FilterAndParseIncomingTx( - block.Tx, - uint64(block.Height), - "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2", - &log.Logger, - common.BtcRegtestChain().ChainId, - ) - - suite.Require().Equal(0, len(inTxs)) -} - -func (suite *BitcoinClientTestSuite) Test3() { - client := suite.BitcoinChainClient.rpcClient - res, err := client.EstimateSmartFee(1, &btcjson.EstimateModeConservative) - suite.Require().NoError(err) - suite.T().Logf("fee: %f", *res.FeeRate) - suite.T().Logf("blocks: %d", res.Blocks) - suite.T().Logf("errors: %s", res.Errors) - gasPrice := big.NewFloat(0) - gasPriceU64, _ := gasPrice.Mul(big.NewFloat(*res.FeeRate), big.NewFloat(1e8)).Uint64() - suite.T().Logf("gas price: %d", gasPriceU64) - - bn, err := client.GetBlockCount() - suite.Require().NoError(err) - suite.T().Logf("block number %d", bn) -} -func TestBitcoinChainClient(t *testing.T) { - suite.Run(t, new(BitcoinClientTestSuite)) +func TestConfirmationThreshold(t *testing.T) { + client := &BitcoinChainClient{Mu: &sync.Mutex{}} + t.Run("should return confirmations in chain param", func(t *testing.T) { + client.SetChainParams(observertypes.ChainParams{ConfirmationCount: 3}) + require.Equal(t, int64(3), client.ConfirmationsThreshold(big.NewInt(1000))) + }) + + t.Run("should return big value confirmations", func(t *testing.T) { + client.SetChainParams(observertypes.ChainParams{ConfirmationCount: 3}) + require.Equal(t, int64(bigValueConfirmationCount), client.ConfirmationsThreshold(big.NewInt(bigValueSats))) + }) + + t.Run("big value confirmations is the upper cap", func(t *testing.T) { + client.SetChainParams(observertypes.ChainParams{ConfirmationCount: bigValueConfirmationCount + 1}) + require.Equal(t, int64(bigValueConfirmationCount), client.ConfirmationsThreshold(big.NewInt(1000))) + }) }