fix(zetaclient): return early in Bitcoin refreshPendingNonce when pending nonces query fails - #4644
Open
eastagiletracker wants to merge 2 commits into
Conversation
…ding nonces query fails
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes a fix for the nil pointer dereference in the Bitcoin observer's
refreshPendingNoncewhen the pending nonces query to zetacore fails (Fixes #4412). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/779. You can sign in with your GitHub ID to claim ownership of the project.Description
refreshPendingNonceinzetaclient/chains/bitcoin/observer/outbound.gologs the error fromZetaRepo().GetPendingNoncesbut then falls through and readsp.NonceLow. On errorGetPendingNoncesreturns a nil*PendingNonces, so the function panics. It runs at the top ofFetchUTXOs, which is both thefetch_utxosinterval task registered inzetaclient/chains/bitcoin/bitcoin.goand the first step ofsigner.SignWithdrawTx, so a single failed zetacore query aborts that tick (the panic is recovered by the ticker, as the chaos-mode log in the issue shows). The fix is the one described in the issue: return right after logging the error, leaving the artificial pending nonce as it was. The otherGetPendingNoncescallers (mempool.go, the EVM scheduler, the batch signer) already return on error, so Bitcoin'srefreshPendingNoncewas the only place with this pattern.Reproduction on current
main(4ceb665), with a mockedGetPendingNoncesByChainthat returns an error:How Has This Been Tested?
Added
Test_FetchUTXOsPendingNoncesError(UTXOs are still fetched and the pending nonce is unchanged when zetacore errors) and a table-drivenTest_RefreshPendingNonce(nonce raised when lagging, never lowered, kept on error). Both panic without the one-line change and pass with it.go test ./zetaclient/chains/bitcoin/...is green before and after the change (client, common, observer, signer), andgo vetis clean on the observer package.How this was managed
This work was tracked as Minor zetaclient nil pointer dereference on a board imported from this repository's issues and pull requests (4,504 stories), which we used to manage the fix: https://eastagiletracker.com/projects/779
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com
Note
Low Risk
Single early-return on an existing error path in Bitcoin outbound nonce refresh, with unit tests and no change to successful behavior.
Overview
Fixes a nil pointer panic in the Bitcoin observer when zetacore’s pending-nonces query fails.
refreshPendingNoncenow returns immediately after logging the error instead of readingp.NonceLowfrom a nil result, so the in-memory pending nonce is left unchanged.That path runs at the start of
FetchUTXOs, so a transient zetacore error no longer aborts UTXO refresh or withdraw signing for that tick.New tests cover
FetchUTXOswhen pending nonces fail (UTXOs still load, nonce unchanged) and table-driven cases forrefreshPendingNonce(catch-up when lagging, never lowering nonce, no change on error).Reviewed by Cursor Bugbot for commit ff9f4da. Configure here.
The PR appears safe to merge; no actionable issue was identified.
Summary
The PR returns early when Bitcoin’s pending-nonces query fails, preventing a nil dereference while leaving the local pending nonce unchanged. It adds regression coverage for UTXO fetching and nonce refresh behavior.
Reviews (1) · Last reviewed commit: "fix(zetaclient): return early in Bitcoin..."