Skip to content

fix: cherry pick all hotfix from v10.0.x (zero-amount, precision, etc.) #1235

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.

Sign up for GitHub

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

Merged
ws4charlie merged 8 commits into from
Oct 12, 2023
Merged

fix: cherry pick all hotfix from v10.0.x (zero-amount, precision, etc.) #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e2b72f7
cherry pick all hotfix from v10.0.x
ws4charlie Oct 4, 2023
0d38643
adjusted code to for nosec
ws4charlie Oct 4, 2023
e0cf329
adusted error handling and code comments according to PR review feedback
ws4charlie Oct 4, 2023
5a97527
cherry pick hotfix for bitcoin outbound performance and updated some …
ws4charlie Oct 6, 2023
c656823
Merge branch 'develop' of https://github.com/zeta-chain/node into che…
ws4charlie Oct 6, 2023
3ec7159
cherry pick mock mainnet hotfix for duplicate payment on nonce 0
ws4charlie Oct 10, 2023
7b87eda
Merge branch 'develop' into cherry-pick-hotfix-v10.0.x
ws4charlie Oct 10, 2023
8e1da99
Merge branch 'develop' into cherry-pick-hotfix-v10.0.x
ws4charlie Oct 11, 2023
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
56 changes: 28 additions & 28 deletions zetaclient/bitcoin_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,12 @@ func (ob *BitcoinChainClient) observeInTx() error {

for _, inTx := range inTxs {
ob.logger.WatchInTx.Debug().Msgf("Processing inTx: %s", inTx.TxHash)
amount := big.NewFloat(inTx.Value)
amount = amount.Mul(amount, big.NewFloat(1e8))
amountInt, _ := amount.Int(nil)
sats, err := getSatoshis(inTx.Value)
if err != nil {
ob.logger.WatchInTx.Error().Err(err).Msgf("getSatoshis error: %s", err)
continue
}
amountInt := big.NewInt(sats)
message := hex.EncodeToString(inTx.MemoBytes)
zetaHash, err := ob.zetaClient.PostSend(
inTx.FromAddress,
Expand Down Expand Up @@ -347,22 +350,31 @@ func (ob *BitcoinChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64
res, included := ob.includedTxResults[outTxID]
ob.mu.Unlock()

// Get original cctx parameters
params, err := ob.GetPendingCctxParams(nonce)
if err != nil {
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: can't find pending cctx for nonce %d", nonce)
return false, false, err
}

if !included {
if !broadcasted {
return false, false, nil
}
// Get original cctx parameters
params, err := ob.GetPendingCctxParams(nonce)
if err != nil {
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: can't find pending cctx for nonce %d", nonce)
return false, false, nil
// If the broadcasted outTx is nonce 0, just wait for inclusion and don't schedule more keysign
// Schedule more than one keysign for nonce 0 can lead to duplicate payments.
// One purpose of nonce mark UTXO is to avoid duplicate payment based on the fact that Bitcoin
// prevents double spending of same UTXO. However, for nonce 0, we don't have a prior nonce (e.g., -1)
// for the signer to check against when making the payment. Signer treats nonce 0 as a special case in downstream code.
if nonce == 0 {
return true, false, nil
}

// 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")
return false, false, nil
return false, false, err
}
if inMempool { // to avoid unnecessary Tss keysign
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: outTx %s is still in mempool", outTxID)
Expand All @@ -379,27 +391,16 @@ func (ob *BitcoinChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64
ob.logger.ObserveOutTx.Info().Msgf("IsSendOutTxProcessed: checkNSaveIncludedTx succeeded for outTx %s", outTxID)
}

var amount float64
if res.Amount > 0 {
ob.logger.ObserveOutTx.Warn().Msg("IsSendOutTxProcessed: res.Amount > 0")
amount = res.Amount
} else if res.Amount == 0 {
ob.logger.ObserveOutTx.Error().Msg("IsSendOutTxProcessed: res.Amount == 0")
return false, false, nil
} else {
amount = -res.Amount
}

amountInSat, _ := big.NewFloat(amount * 1e8).Int(nil)
// It's safe to use cctx's amount to post confirmation because it has already been verified in observeOutTx()
amountInSat := params.Amount.BigInt()
if res.Confirmations < ob.ConfirmationsThreshold(amountInSat) {
return true, false, nil
}

logger.Debug().Msgf("Bitcoin outTx confirmed: txid %s, amount %f\n", res.TxID, res.Amount)
logger.Debug().Msgf("Bitcoin outTx confirmed: txid %s, amount %s\n", res.TxID, amountInSat.String())
zetaHash, err := ob.zetaClient.PostReceiveConfirmation(
sendHash,
res.TxID,
// #nosec G701 always positive
uint64(res.BlockIndex),
0, // gas used not used with Bitcoin
nil, // gas price not used with Bitcoin
Expand Down Expand Up @@ -663,10 +664,9 @@ func (ob *BitcoinChainClient) refreshPendingNonce() {
pendingNonce := ob.pendingNonce
ob.mu.Unlock()

// #nosec G701 always positive
// #nosec G701 always non-negative
nonceLow := uint64(p.NonceLow)

if nonceLow > 0 && nonceLow >= pendingNonce {
if nonceLow > pendingNonce {
// get the last included outTx hash
txid, err := ob.getOutTxidByNonce(nonceLow-1, false)
if err != nil {
Expand Down Expand Up @@ -888,9 +888,9 @@ func (ob *BitcoinChainClient) checkNSaveIncludedTx(txHash string, params types.O
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
ob.pendingNonce = params.OutboundTxTssNonce + 1
}
ob.logger.ObserveOutTx.Info().Msgf("checkNSaveIncludedTx: included new bitcoin outTx %s outTxID %s", txHash, outTxID)
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 {
Expand Down
14 changes: 7 additions & 7 deletions zetaclient/evm_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
if err != nil {
logger.Error().Err(err).Msg("error posting confirmation to meta core")
}
logger.Info().Msgf("Zeta tx hash: %s\n", zetaHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaHash, sendHash, nonce)
return true, true, nil

} else if cointype == common.CoinType_Gas { // the outbound is a regular Ether/BNB/Matic transfer; no need to check events
Expand All @@ -278,7 +278,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
if err != nil {
logger.Error().Err(err).Msg("error posting confirmation to meta core")
}
logger.Info().Msgf("Zeta tx hash: %s\n", zetaHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaHash, sendHash, nonce)
return true, true, nil
} else if receipt.Status == 0 { // the same as below events flow
logger.Info().Msgf("Found (failed tx) sendHash %s on chain %s txhash %s", sendHash, ob.chain.String(), receipt.TxHash.Hex())
Expand All @@ -298,7 +298,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
if err != nil {
logger.Error().Err(err).Msgf("PostReceiveConfirmation error in WatchTxHashWithTimeout; zeta tx hash %s", zetaTxHash)
}
logger.Info().Msgf("Zeta tx hash: %s", zetaTxHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaTxHash, sendHash, nonce)
return true, true, nil
}
} else if cointype == common.CoinType_Zeta { // the outbound is a Zeta transfer; need to check events ZetaReceived
Expand Down Expand Up @@ -344,7 +344,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
logger.Error().Err(err).Msg("error posting confirmation to meta core")
continue
}
logger.Info().Msgf("Zeta tx hash: %s\n", zetaHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaHash, sendHash, nonce)
return true, true, nil
}
// #nosec G701 always in range
Expand Down Expand Up @@ -380,7 +380,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
logger.Err(err).Msg("error posting confirmation to meta core")
continue
}
logger.Info().Msgf("Zeta tx hash: %s", metaHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", metaHash, sendHash, nonce)
return true, true, nil
}
// #nosec G701 always in range
Expand All @@ -407,7 +407,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
if err != nil {
logger.Error().Err(err).Msgf("PostReceiveConfirmation error in WatchTxHashWithTimeout; zeta tx hash %s", zetaTxHash)
}
logger.Info().Msgf("Zeta tx hash: %s", zetaTxHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaTxHash, sendHash, nonce)
return true, true, nil
}
} else if cointype == common.CoinType_ERC20 {
Expand Down Expand Up @@ -446,7 +446,7 @@ func (ob *EVMChainClient) IsSendOutTxProcessed(sendHash string, nonce uint64, co
logger.Error().Err(err).Msg("error posting confirmation to meta core")
continue
}
logger.Info().Msgf("Zeta tx hash: %s\n", zetaHash)
logger.Info().Msgf("Zeta tx hash: %s cctx %s nonce %d", zetaHash, sendHash, nonce)
return true, true, nil
}
// #nosec G701 always in range
Expand Down
7 changes: 6 additions & 1 deletion zetaclient/zetacore_observer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,12 @@ func (co *CoreObserver) startSendScheduler() {
outTxID := fmt.Sprintf("%s-%d-%d", cctx.Index, params.ReceiverChainId, nonce) // would outTxID a better ID?

// Process Bitcoin OutTx
if common.IsBitcoinChain(c.ChainId) && !outTxMan.IsOutTxActive(outTxID) {
if common.IsBitcoinChain(c.ChainId) {
if outTxMan.IsOutTxActive(outTxID) {
// bitcoun outTx is processed sequencially by nonce
// if the current outTx is being processed, there is no need to process outTx with future nonces
break
}
// #nosec G701 positive
if stop := co.processBitcoinOutTx(outTxMan, uint64(idx), cctx, signer, ob, currentHeight); stop {
break
Expand Down