Skip to content

Commit

Permalink
Merge branch 'develop' into wt-add-grafana-bs
Browse files Browse the repository at this point in the history
  • Loading branch information
@lumtis
lumtis authored Jan 10, 2024
2 parents 126c590 + 89475e5 commit be4644f
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 106 deletions.
1 change: 1 addition & 0 deletions changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

### Fixes

* [1530](https://github.com/zeta-chain/node/pull/1530) - Outbound tx confirmation/inclusion enhancement
* [1496](https://github.com/zeta-chain/node/issues/1496) - post block header for enabled EVM chains only
* [1518](https://github.com/zeta-chain/node/pull/1518) - Avoid duplicate keysign if an outTx is already pending
* fix Code4rena issue - zetaclients potentially miss inTx when PostSend (or other RPC) fails
Expand Down
138 changes: 84 additions & 54 deletions zetaclient/bitcoin_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ type BitcoinChainClient struct {

Mu *sync.Mutex // lock for all the maps, utxos and core params
pendingNonce uint64
includedTxHashes map[string]uint64 // key: tx hash
includedTxResults map[string]btcjson.GetTransactionResult // key: chain-tss-nonce
broadcastedTx map[string]string // key: chain-tss-nonce, value: outTx hash
includedTxHashes map[string]uint64 // key: tx hash
includedTxResults map[string]*btcjson.GetTransactionResult // key: chain-tss-nonce
broadcastedTx map[string]string // key: chain-tss-nonce, value: outTx hash
utxos []btcjson.ListUnspentResult
params observertypes.ChainParams

Expand Down Expand Up @@ -148,7 +148,7 @@ func NewBitcoinClient(
ob.zetaClient = bridge
ob.Tss = tss
ob.includedTxHashes = make(map[string]uint64)
ob.includedTxResults = make(map[string]btcjson.GetTransactionResult)
ob.includedTxResults = make(map[string]*btcjson.GetTransactionResult)
ob.broadcastedTx = make(map[string]string)
ob.params = btcCfg.ChainParams

Expand Down Expand Up @@ -442,24 +442,23 @@ func (ob *BitcoinChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64
}

// Try including this outTx broadcasted by myself
inMempool, err := ob.checkNSaveIncludedTx(txnHash, params)
if err != nil {
ob.logger.ObserveOutTx.Error().Err(err).Msg("IsSendOutTxProcessed: checkNSaveIncludedTx failed")
txResult, inMempool := ob.checkIncludedTx(txnHash, params)
if txResult == nil {
ob.logger.ObserveOutTx.Error().Err(err).Msg("IsSendOutTxProcessed: checkIncludedTx failed")
return false, false, err
}
if inMempool { // to avoid unnecessary Tss keysign
} else if inMempool { // still in mempool (should avoid unnecessary Tss keysign)
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: outTx %s is still in mempool", outTxID)
return true, false, nil
} else { // included
ob.setIncludedTx(nonce, txResult)
}

// Get tx result again in case it is just included
ob.Mu.Lock()
res, included = ob.includedTxResults[outTxID]
ob.Mu.Unlock()
if !included {
res = ob.getIncludedTx(nonce)
if res == nil {
return false, false, nil
}
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: checkNSaveIncludedTx succeeded for outTx %s", outTxID)
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: setIncludedTx succeeded for outTx %s", outTxID)
}

// It's safe to use cctx's amount to post confirmation because it has already been verified in observeOutTx()
Expand Down Expand Up @@ -830,14 +829,11 @@ func (ob *BitcoinChainClient) refreshPendingNonce() {
}

func (ob *BitcoinChainClient) getOutTxidByNonce(nonce uint64, test bool) (string, error) {
ob.Mu.Lock()
res, included := ob.includedTxResults[ob.GetTxID(nonce)]
ob.Mu.Unlock()

// There are 2 types of txids an observer can trust
// 1. The ones had been verified and saved by observer self.
// 2. The ones had been finalized in zetacore based on majority vote.
if included {
if res := ob.getIncludedTx(nonce); res != nil {
return res.TxID, nil
}
if !test { // if not unit test, get cctx from zetacore
Expand Down Expand Up @@ -1020,13 +1016,28 @@ func (ob *BitcoinChainClient) observeOutTx() {
if len(tracker.HashList) > 1 {
ob.logger.ObserveOutTx.Warn().Msgf("observeOutTx: oops, outTxID %s got multiple (%d) outTx hashes", outTxID, len(tracker.HashList))
}
// verify outTx hashes
// iterate over all txHashes to find the truly included one.
// we do it this (inefficient) way because we don't rely on the first one as it may be a false positive (for unknown reason).
txCount := 0
var txResult *btcjson.GetTransactionResult
for _, txHash := range tracker.HashList {
_, err := ob.checkNSaveIncludedTx(txHash.TxHash, params)
if err != nil {
ob.logger.ObserveOutTx.Error().Err(err).Msg("observeOutTx: checkNSaveIncludedTx failed")
result, inMempool := ob.checkIncludedTx(txHash.TxHash, params)
if result != nil && !inMempool { // included
txCount++
txResult = result
ob.logger.ObserveOutTx.Info().Msgf("observeOutTx: included outTx %s for chain %d nonce %d", txHash.TxHash, ob.chain.ChainId, tracker.Nonce)
if txCount > 1 {
ob.logger.ObserveOutTx.Error().Msgf(
"observeOutTx: checkIncludedTx passed, txCount %d chain %d nonce %d result %v", txCount, ob.chain.ChainId, tracker.Nonce, result)
}
}
}
if txCount == 1 { // should be only one txHash included for each nonce
ob.setIncludedTx(tracker.Nonce, txResult)
} else if txCount > 1 {
ob.removeIncludedTx(tracker.Nonce) // we can't tell which txHash is true, so we remove all (if any) to be safe
ob.logger.ObserveOutTx.Error().Msgf("observeOutTx: included multiple (%d) outTx for chain %d nonce %d", txCount, ob.chain.ChainId, tracker.Nonce)
}
}
ticker.UpdateInterval(ob.GetChainParams().OutTxTicker, ob.logger.ObserveOutTx)
case <-ob.stop:
Expand All @@ -1036,50 +1047,69 @@ func (ob *BitcoinChainClient) observeOutTx() {
}
}

// checkNSaveIncludedTx either includes a new outTx or update an existing outTx result.
// Returns inMempool, error
func (ob *BitcoinChainClient) checkNSaveIncludedTx(txHash string, params types.OutboundTxParams) (bool, error) {
// checkIncludedTx checks if a txHash is included and returns (txResult, inMempool)
// Note: if txResult is nil, then inMempool flag should be ignored.
func (ob *BitcoinChainClient) checkIncludedTx(txHash string, params types.OutboundTxParams) (*btcjson.GetTransactionResult, bool) {
outTxID := ob.GetTxID(params.OutboundTxTssNonce)
hash, getTxResult, err := ob.GetTxResultByHash(txHash)
if err != nil {
return false, errors.Wrapf(err, "checkNSaveIncludedTx: error GetTxResultByHash: %s", txHash)
ob.logger.ObserveOutTx.Error().Err(err).Msgf("checkIncludedTx: error GetTxResultByHash: %s", txHash)
return nil, false
}
if txHash != getTxResult.TxID { // just in case, we'll use getTxResult.TxID later
ob.logger.ObserveOutTx.Error().Msgf("checkIncludedTx: inconsistent txHash %s and getTxResult.TxID %s", txHash, getTxResult.TxID)
return nil, false
}
if getTxResult.Confirmations >= 0 { // check included tx only
err = ob.checkTssOutTxResult(hash, getTxResult, params, params.OutboundTxTssNonce)
if err != nil {
return false, errors.Wrapf(err, "checkNSaveIncludedTx: error verify bitcoin outTx %s outTxID %s", txHash, outTxID)
ob.logger.ObserveOutTx.Error().Err(err).Msgf("checkIncludedTx: error verify bitcoin outTx %s outTxID %s", txHash, outTxID)
return nil, false
}
return getTxResult, false // included
}
return getTxResult, true // in mempool
}

ob.Mu.Lock()
defer ob.Mu.Unlock()
nonce, foundHash := ob.includedTxHashes[txHash]
res, foundRes := ob.includedTxResults[outTxID]

// include new outTx and enforce rigid 1-to-1 mapping: outTxID(nonce) <===> txHash
if !foundHash && !foundRes {
ob.includedTxHashes[txHash] = params.OutboundTxTssNonce
ob.includedTxResults[outTxID] = *getTxResult
if params.OutboundTxTssNonce >= ob.pendingNonce { // try increasing pending nonce on every newly included outTx
ob.pendingNonce = params.OutboundTxTssNonce + 1
}
ob.logger.ObserveOutTx.Info().Msgf("checkNSaveIncludedTx: included new bitcoin outTx %s outTxID %s pending nonce %d", txHash, outTxID, ob.pendingNonce)
}
// update saved tx result as confirmations may increase
if foundHash && foundRes {
ob.includedTxResults[outTxID] = *getTxResult
if getTxResult.Confirmations > res.Confirmations {
ob.logger.ObserveOutTx.Info().Msgf("checkNSaveIncludedTx: bitcoin outTx %s got confirmations %d", txHash, getTxResult.Confirmations)
}
}
if !foundHash && foundRes { // be alert for duplicate payment!!! As we got a new hash paying same cctx. It might happen (e.g. majority of signers get crupted)
ob.logger.ObserveOutTx.Error().Msgf("checkNSaveIncludedTx: duplicate payment by bitcoin outTx %s outTxID %s, prior result %v, current result %v", txHash, outTxID, res, *getTxResult)
// setIncludedTx saves included tx result in memory
func (ob *BitcoinChainClient) setIncludedTx(nonce uint64, getTxResult *btcjson.GetTransactionResult) {
txHash := getTxResult.TxID
outTxID := ob.GetTxID(nonce)

ob.Mu.Lock()
defer ob.Mu.Unlock()
res, found := ob.includedTxResults[outTxID]

if !found { // not found.
ob.includedTxResults[outTxID] = getTxResult // include new outTx and enforce rigid 1-to-1 mapping: nonce <===> txHash
if nonce >= ob.pendingNonce { // try increasing pending nonce on every newly included outTx
ob.pendingNonce = nonce + 1
}
if foundHash && !foundRes {
ob.logger.ObserveOutTx.Error().Msgf("checkNSaveIncludedTx: unreachable code path! outTx %s outTxID %s, prior nonce %d, current nonce %d", txHash, outTxID, nonce, params.OutboundTxTssNonce)
ob.logger.ObserveOutTx.Info().Msgf("setIncludedTx: included new bitcoin outTx %s outTxID %s pending nonce %d", txHash, outTxID, ob.pendingNonce)
} else if txHash == res.TxID { // found same hash.
ob.includedTxResults[outTxID] = getTxResult // update tx result as confirmations may increase
if getTxResult.Confirmations > res.Confirmations {
ob.logger.ObserveOutTx.Info().Msgf("setIncludedTx: bitcoin outTx %s got confirmations %d", txHash, getTxResult.Confirmations)
}
return false, nil
} else { // found other hash.
// be alert for duplicate payment!!! As we got a new hash paying same cctx (for whatever reason).
delete(ob.includedTxResults, outTxID) // we can't tell which txHash is true, so we remove all to be safe
ob.logger.ObserveOutTx.Error().Msgf("setIncludedTx: duplicate payment by bitcoin outTx %s outTxID %s, prior outTx %s", txHash, outTxID, res.TxID)
}
return true, nil // in mempool
}

// getIncludedTx gets the receipt and transaction from memory
func (ob *BitcoinChainClient) getIncludedTx(nonce uint64) *btcjson.GetTransactionResult {
ob.Mu.Lock()
defer ob.Mu.Unlock()
return ob.includedTxResults[ob.GetTxID(nonce)]
}

// removeIncludedTx removes included tx's result from memory
func (ob *BitcoinChainClient) removeIncludedTx(nonce uint64) {
ob.Mu.Lock()
defer ob.Mu.Unlock()
delete(ob.includedTxResults, ob.GetTxID(nonce))
}

// Basic TSS outTX checks:
Expand Down
4 changes: 2 additions & 2 deletions zetaclient/btc_signer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func createTestClient(t *testing.T) *BitcoinChainClient {
client := &BitcoinChainClient{
Tss: tss,
Mu: &sync.Mutex{},
includedTxResults: make(map[string]btcjson.GetTransactionResult),
includedTxResults: make(map[string]*btcjson.GetTransactionResult),
}

// Create 10 dummy UTXOs (22.44 BTC in total)
Expand All @@ -413,7 +413,7 @@ func createTestClient(t *testing.T) *BitcoinChainClient {
func mineTxNSetNonceMark(ob *BitcoinChainClient, nonce uint64, txid string, preMarkIndex int) {
// Mine transaction
outTxID := ob.GetTxID(nonce)
ob.includedTxResults[outTxID] = btcjson.GetTransactionResult{TxID: txid}
ob.includedTxResults[outTxID] = &btcjson.GetTransactionResult{TxID: txid}

// Set nonce mark
tssAddress := ob.Tss.BTCAddressWitnessPubkeyHash().EncodeAddress()
Expand Down
Loading

0 comments on commit be4644f

Please sign in to comment.