Apply 1.0 freeze-required API fixes (non_exhaustive, ShortIdError, OutputSubstitution, must_use) - #1703
Merged
Conversation
Collaborator
Coverage Report for CI Build 28969165211Coverage decreased (-0.07%) to 85.833%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Adding a variant to a public exhaustive enum is a breaking change, so without #[non_exhaustive] these enums could not grow after 1.0 without a 2.0. Mark only the enums where a wildcard arm has meaningful semantics for integrators: the error enums (ProtocolError, IntoUrlError, UrlParseError, ShortIdError), Version, and the receive/send SessionStatus. Keep the closed sets and state machines exhaustive: ReceiveSession and SendSession, SessionEvent, SessionOutcome, MaybePayjoinExtras, and the persist transition/outcome enums. A wildcard arm has no meaningful semantics for those, since a caller cannot resume a session in an unknown state. Downstream code would be forced into unreachable!() or silent fallback arms that turn a new variant into a runtime failure on live sessions. Adding a state is semantically breaking regardless of exhaustiveness, and new session events already break event-log replay on downgrade, so compatibility there is governed by persistence versioning. The compile error on a new variant is the feature. payjoin-cli keeps wildcard arms where it matches Version, mapping unknown versions to a config error.
ShortIdError is the error type of ShortId's TryFrom<&[u8]> and FromStr impls, but it derived only Debug. Without Display and std::error::Error it could not be `?`-propagated into Box<dyn Error> or anyhow, forcing callers to handle it specially. Implement Display and Error with source() chaining to the wrapped bech32 and slice-conversion errors.
The v2 API returns OutputSubstitution (Receiver<WantsOutputs>:: output_substitution and PayjoinExtras::output_substitution), but its crate-root re-export was gated on the v1 feature. Since v2 does not enable v1, default (v2-only) builds returned a value of a type callers could not name or import. Drop the v1 gate. The re-export lives inside the _core-gated core module, so it is now available whenever v1 or v2 is enabled.
Every state-machine transition returns a transition wrapper whose .save() both persists the session event and yields the next typestate. Dropping the wrapper without calling .save() silently loses the event and the state advance, with no diagnostic. Add #[must_use] to the transition types so that dropping one is a compiler warning.
spacebear21
force-pushed
the
api/1.0-freeze-fixes
branch
from
July 8, 2026 19:16
5b467ea to
6f27ca3
Compare
spacebear21
marked this pull request as ready for review
July 8, 2026 19:49
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
3 participants
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
I ran a 1.0 API audit with Claude that surfaced these relatively low-hanging fruit issues that are best addressed before a 1.0 freeze.
Each commit is standalone:
state/event/status/outcome enums,
ReceiveSession/SendSession,ProtocolError,IntoUrlError,UrlParseError,ShortIdError,Version,MaybePayjoinExtras, and the persist transition/outcome enums. Adds therequired wildcard arms in
payjoin-cli(graceful "unknown" fallbacks indisplay and dispatch) and
payjoin-ffi(unreachable!()in the infalliblemirror-conversion
Fromimpls, which are version-locked topayjoin).Error+DisplayforShortIdErrorso it can be?-propagated — it is theErrtype ofShortId'sTryFrom/FromStrbutderived only
Debug.OutputSubstitutionunder v2 — it was gated on thev1feature,but v2 methods return it and v2 does not enable v1, so default (v2-only)
builds could not name the type.
#[must_use]so dropping a transitionwithout
.save()is a compiler warning instead of a silently lost step.Follows #1702 (bitcoin-ohttp / bitcoin-hpke insulation). Tracking: #976.
Disclosure: co-authored by Claude Code (Opus 4.8)
🤖 Generated with Claude Code