Repository navigation
Seal fallback_tx and deprecate extract_tx_to_schedule_broadcast - #1994
Open
spacebear21 wants to merge 3 commits into
Open
spacebear21 wants to merge 3 commits into
spacebear21 wants to merge 3 commits into
Conversation
The sealed `FallbackTx` trait was `pub` inside a private module, which makes it unnameable from outside the crate but still reachable through the public `HasFallbackTx` bound. Rust lets generic code call the methods of a reachable supertrait, so a downstream function bounded on `S: HasFallbackTx` could call `receiver.fallback_tx()` and read the fallback transaction from any state, bypassing the `cancel` path and the Cancelled event it records. Narrow the trait to `pub(crate)`. The compiler then rejects the call with E0624 even inside a generic function, while code written against `HasFallbackTx` keeps compiling. A private supertrait on a public trait has only been the `private_bounds` lint since Rust 1.74, which the MSRV of 1.85 covers, so the two allows mark the mismatch as deliberate. A compile_fail doctest guards the seal; its error code is enforced on nightly and ignored on stable.
Collaborator
Coverage Report for CI Build 39269677647Coverage decreased (-0.02%) to 87.45%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions3 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
spacebear21
marked this pull request as draft
October 19, 2026 14:27
When the Original PSBT arrived, the v2 receiver printed its hex and suggested broadcasting it by hand if the payjoin failed. That is the pattern payjoin is moving away from: a fallback broadcast should go through the cancel path, which records the Cancelled event in the session log and hands back the transaction as Receiver<PendingFallback>, so the session history and the chain agree on what happened. The CLI already implements that path as `payjoin-cli cancel <session-id>`. Replace the hex dump with a pointer to the cancel command, matching the wording used when a session is interrupted or fails. This also removes the CLI's only call to extract_tx_to_schedule_broadcast ahead of its deprecation.
Receiver<MaybeInputsOwned>::extract_tx_to_schedule_broadcast handed out the fallback transaction up front so an application could schedule its broadcast out of band. A broadcast scheduled that way leaves no trace in the session event log: the session stays Active while the fallback is already on the chain, and the log only learns of it if the session later reaches Monitor and check_for_transaction finds the fallback. Scheduling a fallback broadcast should instead go through cancel(). It is available from every state that holds a verified fallback, records the Cancelled event, and yields Receiver<PendingFallback>, whose fallback_tx is the transaction to broadcast before close() ends the session. Applications that need the transaction after the fact can read it from SessionHistory::fallback_tx. Mark the v2 method deprecated and point its docs at cancel(). The v1 receiver keeps its method: it has no session or cancel path. Move the session tests that used it as a shortcut to the original PSBT, drop the unused call from the integration test, and keep the FFI wrapper compiling with an allow while its docstring carries the deprecation, since UniFFI has no deprecation attribute.
spacebear21
force-pushed
the
seal-fallback-tx
branch
from
October 19, 2026 14:31
fa16fc6 to
63d5833
Compare
spacebear21
marked this pull request as ready for review
October 19, 2026 14:32
3 tasks done
xstoicunicornx
left a comment
xstoicunicornx
left a comment
Collaborator
There was a problem hiding this comment.
About ready to ACK these changes but seems like it needs a rebase and a CI job is failing.
Also, these changes must be shipped in the same CLI version as #1971 (which I don't think will be a problem), otherwise when check_for_transaction is called for non-segwit input txs the sessions will close automatically without giving recourse to broadcast fallback tx.
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.
The sealed
FallbackTxtrait behindHasFallbackTxwaspubinside a private module. That keeps it unnameable from outside the crate, but Rust still lets generic code call the methods of a reachable supertrait, so a downstream function bounded onS: HasFallbackTxcould callreceiver.fallback_tx()and read the fallback transaction from any mid-protocol state, includingMonitor, without going throughcancel()and theCancelledevent it records. The first commit narrows the trait topub(crate), which makes the compiler reject that call with E0624 while code written againstHasFallbackTxkeeps compiling, and adds acompile_faildoctest that guards the seal.The remaining commits follow the same principle for the one accessor that handed the fallback out early on purpose.
Receiver<MaybeInputsOwned>::extract_tx_to_schedule_broadcastexists so an application can schedule the fallback broadcast out of band, but a broadcast scheduled that way leaves no trace in the session log. Scheduling it should go throughcancel(), which is available from every state that holds a verified fallback, records the event, and yieldsReceiver<PendingFallback>with the transaction to broadcast. The v2 method is now deprecated with its docs pointing atcancel(), and payjoin-cli stops printing the fallback hex when the Original PSBT arrives and points atpayjoin-cli cancel <session-id>instead.Points for review:
0.0.0-unreleasedplaceholder in the Rust attribute and assince payjoin 0.0.0-unreleasedin the FFI docstring, forbump.shto fill in.extract_tx_to_schedule_broadcastis untouched. v1 has no session or cancel path to point to.#[allow(deprecated)]on the call. The generated bindings are gitignored and built from the FFI source, so no binding files change.