Repository navigation
Conversation
…d silent failures
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.
Audit items L52, L20, L55, L53, L54, L49, L30, L45, L48. Open against
dev.Goat nonces and receipts (L52, L20). A transaction took its nonce from the latest block, and the node runs several
GOATClients with the same key, so concurrent sends could sign with the same nonce and replace each other. A send waited 10 seconds for its receipt and then reported failure while the transaction could still be pending. Now:One lock per sender address, shared by every adaptor in the process, is held from reading the nonce until the receipt arrives or the wait ends. Sends from one key are serialized.
The nonce still comes from the latest block, on purpose. With sends serialized it equals the pending nonce, except after a timeout. Then the next send reuses the nonce and replaces the stuck transaction instead of queueing a duplicate behind it. Its caller was already told the send failed.
The receipt wait is 30 seconds, and the timeout error says the transaction may still be pending and may be replaced.
SPV overwrite (L55). With
ENABLE_UPDATE_SPV_CONTRACT, a block-hash mismatch while checking a peg-in or a graph posted the local esplora's hash to the SPV contract at that height, with no depth or continuity check. Both branches are removed and a mismatch is an error. The relayer task, which posts confirmed blocks only, is the one writer. The variable still enables that task.Esplora retries (L53).
BTC_REQUEST_TIMEOUT_SECSbounds a whole esplora call, retries included. esplora-client retries 429 and 5xx six times with doubling backoff from 256 ms, about 16 seconds of sleep, longer than the default 15-second timeout. The client now retries three times (about 1.8 seconds) and each HTTP request has the same timeout.Default explorers (L54). Every network other than mainnet defaulted to the testnet3 explorer, while the default network is testnet4. Testnet4 and signet now default to their own mempool.space APIs; testnet3 is unchanged and regtest needs an explicit URL as before. The unused
mempool_v1_typemodule, which built its URLs from the default explorer regardless of the configured one, is deleted.Subgraph numbers (L49). A non-numeric
blockNumberorrewardAmountSatsfrom the subgraph panicked the event watcher on every restart. These are now errors: the watch round fails and its cursor does not advance.Watch contract writes (L30).
upsert_watch_contractandupdate_watch_contract_statusignored database errors and returnedOk, inside the watcher's transaction. They now return the error.Swap confirmations (L45). The claim confirmation count used unchecked
u32subtraction and wrapped when the node's height was behind the explorer's. It now saturates at 0.Swap claim trace (L48). The claim lookup walked every call frame, including reverted ones, so a reverted inner claim could supply the recorded claim data. A reverted frame and everything under it are now skipped.
Tests: two adaptors with the same key share one send lock and another key does not; each network's default explorer; a malformed block number is an error; watch contract writes report a database error; confirmations do not wrap; a claim in a reverted frame is ignored and the same claim in a successful frame is found. Node, client and store tests, clippy with
-D warningsand fmt pass.The test coverage and validation notes above are from the zkm PR notes. Branch scope and diff formatting were checked locally; builds and tests were not rerun during publication.