manager: tell apart missing transactions from mempool transactions - #181
Merged
Conversation
Blockchain::get_transaction_confirmations returned Ok(0) both for a transaction in the mempool and for a transaction the network does not know. With NB_CONFIRMATIONS=0 the manager then moved contracts through states on transactions that were evicted from the mempool or never broadcast. Return a ConfirmationStatus enum (NotFound / InMempool / Confirmed(n)) instead of a bare count. The esplora client queries the transaction itself when the status endpoint reports it unconfirmed, because esplora reports "unconfirmed" for both cases. Transport errors stay errors, so a flaky esplora is not mistaken for a dropped transaction. The manager handles NotFound explicitly: - check_signed_contract does not confirm a contract whose funding transaction is missing. - check_for_spliced_contract does not pre-close the previous contract when the splice funding transaction is missing. - Pending close transactions that are missing are skipped. - check_preclosed_contract broadcasts the closing transaction again; when it cannot enter the mempool and the pre-close has no attestations (splice or cooperative close), the contract moves back to Confirmed. - Entering the terminal Closed state now requires at least one confirmation on-chain, also when NB_CONFIRMATIONS is zero, because no periodic check examines Closed contracts again.
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.
Summary
Redo of #169.
Blockchain::get_transaction_confirmationsreturnedOk(0)both for a transaction in the mempool and for a transaction the network does not know. WithNB_CONFIRMATIONS=0the manager moved contracts through states on transactions that were evicted from the mempool or never broadcast. The trait now returns aConfirmationStatusenum —NotFound/InMempool/Confirmed(n)— and the manager handles each case. There is no recovery scan over closed contracts: the state machine no longer entersClosedon a transaction that is not on-chain, so there is nothing to recover.Changes
ddk-manager/src/lib.rs: newConfirmationStatusenum with aconfirmations()helper.Blockchain::get_transaction_confirmationsreturns it instead of a bareu32. This is a breaking change for trait implementers.ddk/src/chain/esplora.rs: when the status endpoint reports a transaction unconfirmed, query the transaction itself to tell "in the mempool" apart from "not found" (esplora reports both as unconfirmed). Transport errors stayErr, so a flaky esplora is not mistaken for a dropped transaction. The tip-height call now only runs for confirmed transactions.check_signed_contract: a missing funding transaction does not confirm the contract.check_for_spliced_contract: a missing splice funding transaction does not pre-close the previous contract.check_preclosed_contract: a missing closing transaction is broadcast again (safe when the chain source lags — the network accepts a transaction it already knows). If it cannot enter the mempool and the pre-close has no attestations (splice or cooperative close), the funding output is unspent and the contract moves back toConfirmed.Closedstate now requires at least one confirmation on-chain, also whenNB_CONFIRMATIONS=0. No periodic check examinesClosedcontracts again, so a mempool-only transaction that later gets evicted must not put a contract there. This closes the exact incident path: withNB_CONFIRMATIONS=0a splice contract now staysPreCloseduntil its funding transaction is mined, and reverts toConfirmedif that transaction is dropped.ddk/tests/chain.rsasserts the esplora client reportsNotFound,InMempool, andConfirmedcorrectly against a regtest backend.Testing
cargo test -p ddk -p ddk-manager(full non-ignored suites) pass.NB_CONFIRMATIONS=6 cargo test -p ddk-manager --test manager_execution_tests -- --ignored --exact splice_in_enum_single_oracle_test cooperative_close_single_oracle_test enum_single_oracle_refund_testpass.cooperative_close_single_oracle_testfails on master too whenNB_CONFIRMATIONSis unset locally (CI sets it to 6); not related to this change.cargo clippyclean for the changed code;cargo check --workspacepasses.