fix(mint): distinguish unparsable witness from absent witness - #1130
Open
KvngMikey wants to merge 3 commits into
Open
fix(mint): distinguish unparsable witness from absent witness#1130KvngMikey wants to merge 3 commits into
KvngMikey wants to merge 3 commits into
Conversation
A witness carrying no signatures must keep reporting code 11000 with "no signatures in proof." — cashu-ts#991 is widening its assertions around exactly these values (cashubtc#1126, question 1). Pins both rows of that contract, non-SIG_ALL and SIG_ALL, plus the {"signatures":[]} witness. That last one reaches the same outcome by a different route: it goes through the parser rather than returning ahead of it, which makes it the case that would move if the parse/no-parse boundary in from_p2pk_witness ever shifted. Asserts the code explicitly rather than the type alone: TransactionError is a base class, so pytest.raises on it would still pass if the code drifted. No source changes.
There was a problem hiding this comment.
Pull request overview
This PR refines mint-side spending-condition error reporting to distinguish unparsable witnesses from absent witnesses (while keeping TransactionError code 11000), and adds unit tests to pin the client-visible (“wire contract”) behavior discussed in #1126.
Changes:
- Add a
malformedflag to witness parsing helpers and raise aTransactionError("witness could not be parsed.")for unparsable P2PK/HTLC witnesses. - Add unit tests that pin absent-witness behavior (
"no signatures in proof.", code11000) and new unparsable-witness behavior ("witness could not be parsed", code11000) across P2PK, HTLC, and SIG_ALL paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cashu/mint/conditions.py |
Tracks malformed witness parsing and raises a specific parse-failure TransactionError. |
tests/mint/test_spending_conditions_unit_p2pk.py |
Adds tests pinning absent/empty-signatures behavior and asserting parse-failure behavior for malformed P2PK witnesses. |
tests/mint/test_spending_conditions_unit_htlc.py |
Adds tests asserting malformed HTLC witness is a parse failure while “missing preimage” remains a distinct error. |
tests/mint/test_spending_conditions_unit_sigall.py |
Adds test pinning SIG_ALL absent-witness behavior (no signatures, code 11000). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1130 +/- ##
==========================================
+ Coverage 74.85% 74.88% +0.03%
==========================================
Files 112 112
Lines 12589 12598 +9
==========================================
+ Hits 9423 9434 +11
+ Misses 3166 3164 -2 ☔ View full report in Codecov by Harness. |
A witness the mint cannot read was reported as "no signatures in proof.", the same message as a witness that was never sent. That points a client author at their signing code when the fault is in their serialization. Flag the parse failure on WitnessForP2pkOrHtlc and report it where the verifier decides what to surface, after both the primary and refund paths have been tried. A malformed witness can never satisfy the primary path -- P2PK requires a signature and has none, HTLC always carries a preimage hash while the witness has no preimage -- and a zero-sig refund path still returns before that point. Every success and failure outcome is unchanged; only the message improves, and it now outranks the refund attempt's own "no signatures in proof." complaint. Raising from the witness constructors instead would be shorter but wrong: they are evaluated as arguments to _verify_p2pk_or_htlc_spending_requirements, so they run before the primary/refund fallback is entered. An expired lock with no refund pubkeys is spendable without a witness signature, and a proof carrying a garbage witness on that path spends today -- raising at parse time would start rejecting it. That is a logic change, not a message change. The absent-witness code and message are unchanged, pinned by the tests in the preceding commit, so cashu-ts#991's widened assertions stay valid. The new message keeps code 11000; no new error class. Note the two witness types differ in what they can reject: P2PKWitness requires "signatures", so wrong-shape JSON is a parse failure, while HTLCWitness declares every field optional, so only unreadable input is. Closes cashubtc#1126
KvngMikey
force-pushed
the
fix/unparsable-witness-error
branch
from
August 24, 2026 09:42
749ad10 to
acb53e4
Compare
a1denvalu3
reviewed
Aug 27, 2026
a1denvalu3
left a comment
a1denvalu3
left a comment
Collaborator
There was a problem hiding this comment.
This works but I think introducing a malformed field that has the only purpose of indicating an error is ugly.
Parse the witness inside the requirements verifier so the constructors can raise directly, dropping the malformed field. Behaviour unchanged.
Member
Author
@a1denvalu3, yes ! I moved the witness parsing into the shared verifier. The constructors now either return a valid normalized witness or raise TransactionError("witness could not be parsed."), so there is no longer any error state on the dataclass. |
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.
Closes #1126
11000.