Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A confirmed version-propagation bug in PSBTScope.read_from() can prevent correct parsing/validation of versioned fields for any scope class using the base implementation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds full PSBTv2 (BIP-370) support across PSBT and streaming PSBTView, and hardens parsing/signing to consistently reject malformed PSBTs with PSBTError while improving PSBTView signing performance via single-pass hashing and caching.
Changes:
- Implement PSBTv2 global/input/output parsing, validation, serialization, locktime derivation, Creator/Constructor helpers, and BIP-370
PSBT_GLOBAL_TX_MODIFIABLEupdates on signing. - Fix Taproot sighash correctness (ANYONECANPAY outpoint hashing; SIGHASH_SINGLE sha_single_output hashing) and align
PSBT/PSBTViewoutput bytes + locktime APIs. - Harden compact-int parsing and PSBT framing (canonical compact sizes, bounded non-witness-utxo length, duplicate-key rejection, trailing-bytes rejection) and add/extend regression + vector tests.
File summaries
| File | Description |
|---|---|
| tests/tests/test_psbtview.py | Strengthens PSBTView round-trip assertion to byte-for-byte equality. |
| tests/tests/test_psbt.py | Refactors signing test vectors into constants for reuse/clarity. |
| tests/tests/test_psbt_versions.py | Adds PSBTv2 negative-int64 amount rejection coverage; minor formatting. |
| tests/tests/test_liquid.py | Adds PSETv2 signing/locktime/sequence/asset-required and corruption tests; refactors fixtures. |
| src/embit/transaction.py | Adds segwit-aware parsing for unsigned txs and fixes Taproot sighash edge cases. |
| src/embit/psbtview.py | Implements PSBTv2 global handling, strict streaming validation, locktime derivation, caching, and tx_modifiable updates. |
| src/embit/psbt.py | Implements PSBTv2 core support, hardened parsing helpers, locktime determination, tx_modifiable semantics, and signing refactors. |
| src/embit/liquid/transaction.py | Mirrors Transaction.read_from(segwit=...) signature for Liquid transactions. |
| src/embit/liquid/psetview.py | Aligns parsing hardening with PSBTView and ensures derived locktime is used in sighash. |
| src/embit/liquid/pset.py | Fixes v2 sequence defaulting, ensures derived locktime in blinded tx, and adds v2 output validation (asset/commitment). |
| src/embit/compact.py | Hardens compact-size parsing to reject truncated and non-canonical encodings. |
| CHANGELOG.md | Documents PSBTv2, hardening, behavior changes, and correctness fixes. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Locktime determination can incorrectly accept mixed (height+time) locktime requirements (including the case where an input specifies both), which should be rejected to avoid deriving an invalid signing locktime.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/embit/psbt.py:212
- choose_locktime() can return a height-based locktime even when at least one time-based requirement is present (e.g., when an input sets both PSBT_IN_REQUIRED_HEIGHT_LOCKTIME and PSBT_IN_REQUIRED_TIME_LOCKTIME). In that case there is no single nLockTime that can satisfy both requirement domains, so this should be rejected instead of silently preferring height.
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
Collaborator
Author
Rejecting the "both" case would fail the first two BIP vectors and diverge from Bitcoin Core, whose ComputeTimeLock treats an input carrying both fields as satisfiable by either type. |
notTanveer
reviewed
Sep 3, 2026
Comment on lines
+667
to
670
| num_leaf_hashes = read_compact(b) | ||
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] | ||
| if not all([len(leaf) == 32 for leaf in leaf_hashes]): | ||
| raise PSBTError("Invalid length of taproot leaf hashes") |
Collaborator
There was a problem hiding this comment.
check before allocating memory to prevent OOM attacks
Suggested change
| num_leaf_hashes = read_compact(b) | |
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] | |
| if not all([len(leaf) == 32 for leaf in leaf_hashes]): | |
| raise PSBTError("Invalid length of taproot leaf hashes") | |
| num_leaf_hashes = read_compact(b) | |
| if num_leaf_hashes * 32 > len(v) - b.tell(): | |
| raise PSBTError("Invalid length of taproot leaf hashes") | |
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] |
notTanveer
reviewed
Sep 3, 2026
Comment on lines
+939
to
942
| num_leaf_hashes = read_compact(b) | ||
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] | ||
| if not all([len(leaf) == 32 for leaf in leaf_hashes]): | ||
| raise PSBTError("Invalid length of taproot leaf hashes") |
Collaborator
There was a problem hiding this comment.
Suggested change
| num_leaf_hashes = read_compact(b) | |
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] | |
| if not all([len(leaf) == 32 for leaf in leaf_hashes]): | |
| raise PSBTError("Invalid length of taproot leaf hashes") | |
| num_leaf_hashes = read_compact(b) | |
| if num_leaf_hashes * 32 > len(v) - b.tell(): | |
| raise PSBTError("Invalid length of taproot leaf hashes") | |
| leaf_hashes = [b.read(32) for i in range(num_leaf_hashes)] |
Collaborator
Author
There was a problem hiding this comment.
Thank you @notTanveer ! Both suggestions and a test added as a new commit, co-authoring you.
There was a problem hiding this comment.
🟡 Changes recommended
PSBTView’s PSBTv2 global-scope reconstruction currently buffers deferred unknown global values into memory, enabling a potential memory-exhaustion DoS via oversized unknown global fields.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
PSBTView’s tx_modifiable_flags setter can currently raise ValueError on invalid input, violating the PR’s stated “raise PSBTError” hardening guarantee.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/embit/psbtview.py:285
- tx_modifiable_flags setter can raise ValueError (bytes([value])) for out-of-range or non-int inputs, which leaks a non-PSBTError exception path. Since this is part of the PSBTv2 hardening surface, validate the type/range and raise PSBTError with a clear message instead.
src/embit/psbtview.py:427 - The _scan_scope_values() docstring claims that scope_cls._validate_key() performs “the same checks the full scope parser applies”, but _validate_key() only checks key shape (e.g., PSBTv2 keyed-field rule) and does not validate per-key value formats like read_from()/read_value() do. This is misleading for callers relying on it for full validation.
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
Collaborator
|
tested this at 12deedc and found a few things, fixes are here as separate commits on top of your branch: https://github.com/odudex/embit/compare/feat/psbt-v2...notTanveer:embit:pr-152?expand=1 each commit has its own tests, so cherry-pick whatever you like. |
Closed
Co-authored-by: odudex <5049478+odudex@users.noreply.github.com>
…SBTView Extract next_tx_modifiable() in psbt.py. PSBT and PSBTView each carried a byte-identical copy of the BIP-370 sighash -> flag rules, so a later correction to one would silently leave the constrained-device path wrong. Going through a pure function also removes a None &= -2 TypeError on a PSBTView built with version=2 and no global_kvs, whose flags setter no-ops. Drop the trailing tx_modifiable_flags block in PSBTView.sign_with(). Every path in sign_input() that adds a signature already updates the flags, so the block could only fire after signing nothing, stamping TX_MODIFIABLE=0 onto a PSBT this pass never touched. PSBT.sign_with() does not do this. (PSBT-only half of 59908f5 on feat/silent-payments-V2.)
… rebase Rebasing feat/silent-payments-V2 onto upstream/master's PSBTv2 hardening series (eb6104f) merged in upstream's declarative _validate_global_key, V2_FIELDS/_validate_key scope checks, and _read_global_value eager-shape validation, but left _validate_global_fields (canonical count-field encoding, fixed-length checks, required-field presence) unwired in both PSBT.read_from and PSBTView.view. Malformed PSBT_GLOBAL_INPUT_COUNT / PSBT_GLOBAL_OUTPUT_COUNT values crashed with RuntimeError/ValueError instead of PSBTError, non-canonical compact encodings went unrejected, and PSBTView accepted a v2 PSBT missing PSBT_GLOBAL_TX_VERSION until something later touched the lazy .tx_version property. Wire _validate_global_fields into both parse paths, replacing the now-duplicate manual v0/v2 field-exclusion checks it subsumes. All 25 of upstream's tests/tests/test_psbt_versions.py cases now pass.
…signed byte helpers Parsing tx_version unsigned closes the same Transaction(version=tx_version) overflow the helpers were added for.
…oot_keypath Subclasses that tweak the key differently (BIP-376 silent payment spends) encode and store the BIP-341 key-path signature identically, so share that step instead of duplicating it. (PSBT-only half of 533cd85 on feat/silent-payments-V2.)
…like PSBT PSBTView.vin()/vout() looked up PSBT_IN_PREVIOUS_TXID, OUTPUT_INDEX, SEQUENCE, PSBT_OUT_AMOUNT and SCRIPT with seek_to_value(), which matches by key prefix and returns the first hit. A keyless BIP-370 field carrying keydata (0x0e||xx) or a duplicate of the field was therefore accepted, and its value fed hashPrevouts/hashSequence/hashOutputs. PSBT.parse rejects both. On a two-input PSBT with the bogus key in input 1, PSBTView.sign_with() computed input 0's sighash over the wrong prevout and wrote its signature to the sig stream before input 1 was ever parsed. vin()/vout() now go through _scan_scope_values(), which matches keys exactly, rejects duplicates of the requested keys and runs the scope class's _validate_key() on every key it skips over, so a malformed map is rejected while hashing, before any signature is emitted. The same walk serves _determine_locktime_v2(). input()/output() now run the PSBT_CLS._validate_v2_input/_output hooks (PSET overrides them), so a v2 map missing a required field fails in input(i) instead of only in vin(i). tx_version and the fallback locktime are read from the materialised _global_kvs instead of re-walking the global map with a prefix match.
…ence PSET.blinded_tx, which PSET.sighash_segwit/sighash_legacy hash against, still built the transaction with the raw fallback locktime while PSBT.tx went through determine_locktime(). A PSETv2 with a required height or time locktime therefore signed one transaction and extracted another, and PSET.sign_with() disagreed with PSETView.sign_with(), which already used the derived value. LInputScope.vin/blinded_vin also rewrote sequence == 0 to 0xffffffff via `self.sequence or 0xFFFFFFFF`, the bug the base InputScope.vin fixed on this branch, so the same zero-sequence input hashed and serialized differently depending on which scope class held it.
Truncated or corrupted input reached callers as RuntimeError from compact.read_from, ValueError/TransactionError/ECError/HDError from the nested transaction, script and key parsers, IndexError from a one-byte PSBT_GLOBAL_XPUB key, OverflowError from an absurd length reaching BytesIO.read/seek, and the base-class EmbitError for trailing bytes. Only read_string() wrapped its length read; the prescan added on this branch, PSBTView's deferred-global and scope walks, and GlobalTransactionView all called compact.read_from directly. - compact.read_from now fails on a truncated multi-byte prefix instead of returning int.from_bytes(b"") == 0, and rejects non-canonical encodings the way Bitcoin Core's ReadCompactSize does. A separator written as fd0000 was accepted as an empty key by both parsers, and PSBTView's offset bookkeeping assumes canonical lengths. - read_compact/read_exact/skip_exact wrap the low-level reads; every scope and global read goes through them and through _read_value_checked(), which reports nested parser errors as PSBTError naming the key. - PSBT.parse raises PSBTError on trailing bytes; PSBT_GLOBAL_XPUB keys are length-checked in the shared _validate_global_key; PSBTView bounds the global tx input/output counts by the declared tx length. - Serialization of out-of-range v2 fields (tx_modifiable_flags=300, locktime=2**32, negative sequence, 31-byte txid, amount >= 2**63) raises PSBTError instead of OverflowError/ValueError. Tests truncate and bit-flip three sample PSBTs at every byte and assert both parsers only ever raise PSBTError.
…wing PSBT.parse fails on anything after the last output map (as Bitcoin Core's DecodeRawPSBT does) and on a PSBT_GLOBAL_INPUT_COUNT larger than the maps present, but PSBTView accepted both silently: it only ever walked as far as the scope being asked for, so an extra map, trailing garbage or a missing map surfaced late, in whatever accessor happened to reach it, or not at all. PSBTView.view() now walks all the input and output maps once, reading only the key and value lengths, and fails if the maps run out or bytes remain. Every proper prefix of a PSBT now fails in view() instead of in a later lazy read.
…igs and errors
Several places where the two implementations disagreed on the same PSBT:
- PSBTView.locktime returned the BIP-370 derived locktime while PSBT.locktime
is the raw PSBT_GLOBAL_FALLBACK_LOCKTIME field, so test_psbtview's equality
assertion only held for vectors without per-input requirements. PSBTView
now mirrors PSBT: .locktime is the raw field and determine_locktime() is
the transaction locktime, which is what every sighash uses.
- PSBTView wrote the v2 global map in sorted key order, PSBT in xpubs /
BIP-370 fields / unknowns order, so a PSBT with an unknown global field
round-tripped to different bytes through each. PSBTView now uses PSBT's
order and keeps the source order of unknowns with an OrderedDict
(MicroPython dicts are unordered).
- PSBTView's taproot key-path signer filled only final_scriptwitness while
PSBT also stores PSBT_IN_TAP_KEY_SIG, so the signed PSBTs differed. Both
now fill both fields and the sig stream carries 0x13 as well.
- PSBTScope(unknown={...}) could not carry PSBTv2 fields because
parse_unknowns() never passed a version, and any unknown key raised
"Duplicated key" because the entry was never popped before read_value()
re-added it. Scopes take version= and consume the dict as they parse.
- An input without a utxo raised AttributeError from is_taproot when
signing; it is now PSBTError("Missing previous utxo on input N") from
sighash() in both PSBT and PSBTView, matching PSBT.fee().
- Documented that PSBTView.sign_with() keeps the TX_MODIFIABLE update on the
view, not in the sig stream, so the same view must write the result.
Parity tests sign taproot key-path and script-path PSBTs, in v0 and v2 and
with KEEP_ALL and PARTIAL compression, through PSBT and PSBTView and compare
the resulting bytes.
A declared length that did not match the transaction used to shift the key/value framing of the rest of the input map, so PSBT.parse and PSBTView disagreed on the same bytes. The value is already parsed through _BoundedReader; add regression coverage for a short and a long length in both the streaming (compress) and the full parse.
- psetview imported PSBTError-wrapping helpers through `from ..psbtview import *`, which skips underscored names, so GlobalLTransactionView's `except _PARSE_ERRORS` raised NameError on the first corrupted global transaction instead of PSBTError. Import it explicitly. - Cover a PSBT_IN_NON_WITNESS_UTXO declared 1-4 bytes short: a bounded reader serving short reads would let Transaction.read_from finish on a truncated transaction and hand the cut bytes to the next key, so with a crafted unknown key PSBT.parse accepted the input with locktime 0 while the PARTIAL parse rejected it. _BoundedReader makes a read past the declared length a PSBTError. - PSET._validate_v2_output requires PSBT_ELEMENTS_OUT_ASSET or its commitment; without either the output could be parsed but hashing or extracting it raised TypeError from LTransactionOutput.write_to. - PSBTView._scan_scope_values rejected duplicates only of the keys it was asked for, so a lone sign_input(0) signed a PSBT whose other input repeated e.g. PSBT_IN_WITNESS_UTXO, which PSBT.parse rejects. Every repeated key in the map is now rejected. Co-authored-by: notTanveer <tanveer.x.ansari@gmail.com>
…Y and SINGLE Two deviations from the BIP-341 SigMsg, present in both Transaction and PSBTView.sighash_taproot, reported in diybitcoinhardware#65 and fixed the same way as diybitcoinhardware#66: - with ANYONECANPAY the message hashed the whole serialized input (41 bytes, scriptSig length included) instead of the 36-byte outpoint; - with SIGHASH_SINGLE it hashed the corresponding output itself instead of sha_single_output, its SHA256. Any taproot input signed with a non-default sighash therefore produced a signature over the wrong message. SINGLE with no corresponding output now raises instead of an IndexError. Every hash type in the BIP-341 wallet test vectors (0x00, 0x01, 0x02, 0x03, 0x81, 0x82, 0x83) is now checked against Transaction, PSBT and PSBTView, in PSBTv0 and PSBTv2. Co-authored-by: Marcel Hernandez <e6990620@gmail.com>
BIP-174 serializes PSBT_GLOBAL_UNSIGNED_TX without witness data, so a transaction with no inputs yet (a Creator-stage PSBT) starts with a plain 0x00 input count. Transaction.read_from took that byte for the BIP-144 segwit marker and failed on the flag, so PSBT.parse rejected such PSBTs while PSBTView, which never looks for a marker, accepted them. Transaction.read_from gains segwit=True; PSBT passes segwit=False for the global transaction, as Bitcoin Core does. Network transactions keep the marker detection. Same approach as diybitcoinhardware#117. Co-authored-by: Sinan <sinanrango@gmail.com>
The base PSBTScope.read_from popped the version keyword to apply the v0/v2 field-scope check but called read_value() without it, so any scope class relying on the base implementation parsed versioned fields as if the PSBT were v0. InputScope and OutputScope override read_from and were not affected; found by the CI review on the PR.
A crafted PSBT_IN/OUT_TAP_BIP32_DERIVATION value with a huge compact count made the parser build a list of up to 2^64 empty reads before the length check ran, exhausting memory on constrained devices. Co-authored-by: notTanveer <tanveer.x.ansari@gmail.com>
InputScope.update() kept _verified set after merging a new witness_utxo, so fee() used an unauthenticated amount. It also merged a compressed _utxo without its _txhash, pairing a foreign output with our authenticated hash, so even a fresh verify() passed. Merge the pair together and clear the verified state whenever prevout or UTXO data is merged; signature-only merges keep it.
sign_input_with_tapkey and _sign_taproot_keypath now update PSBT_GLOBAL_TX_MODIFIABLE for every signature they store, so the callers in PSBT.sign_with and PSBTView.sign_input no longer need to.
- Serialize the blanked outputs with nValue -1 (ff x8) in Transaction and PSBTView instead of 0xFFFFFFFF - Add Bitcoin Core LegacySignatureHash vectors for SINGLE and SINGLE|ANYONECANPAY at input indexes 1 and 2
- Raise PSBTError when PSBT_IN_NON_WITNESS_UTXO arrives before the output index is known under compress, instead of buffering the whole previous transaction - Add a test that parses the same PSBTv2 from a read-only stream with and without compress
- Check PSBT_IN_OUTPUT_INDEX against the full non_witness_utxo in InputScope.read_from, so KEEP_ALL raises PSBTError like compress does instead of IndexError when signing - Add a v0 and v2 test that covers PSBT.parse in both modes and PSBTView signing
- Track the raw keys seen in InputScope.read_from and OutputScope.read_from, since compress skips values and bypasses the per-field duplicate checks - Catch duplicated PSET rangeproof keys, which LOutputScope overwrote or skipped without a check - Add tests for duplicated input keys under KEEP_ALL and PARTIAL and for a duplicated PSET rangeproof
- Run _validate_v2_input and _validate_v2_output inside the parse loops, so a huge declared count fails on the first bad map instead of allocating a scope per input byte first - Add a test with INPUT_COUNT 2^40 followed by empty maps
- Parse the xpub key and check the value length in PSBTView.view without reading the value - Require a full 4-byte fingerprint in DerivationPath.read_from instead of accepting a short read - Check the garbage xpub case against PSBTView and add value length tests for both parsers
- Add an exact flag to PSBTView.seek_to_value, keeping the prefix match as the default for existing callers - Use exact lookups for the rangeproof and surjection proof keys in PSETView.hash_rangeproofs, so a longer unknown key is not hashed as a proof - Add a test that PSET and PSETView sign the same message with ALL|RANGEPROOF when such keys are present
- Replace the inline fingerprint and origin derivation code with resolve_signing_root and derive_hdkey from psbt.py - Add a PSBT vs PSBTView signing test with descriptor keys whose origin matches, doesn't match, or is public only
…_output - Drop the _raw_input_count_from_global and _raw_output_count_from_global updates, nothing reads them after parsing and write_to uses len(inputs) and len(outputs) - Check the serialized counts in the constructor tests instead of the private attributes
Comment on lines
+193
to
+198
| b"\xfc\x04pset\x04", from_current=True, exact=True | ||
| ) | ||
| if not rangeproof_offset: | ||
| self.stream.seek(off) | ||
| rangeproof_offset = self.seek_to_value( | ||
| b"\xfc\x08elements\x04", from_current=True | ||
| b"\xfc\x08elements\x04", from_current=True, exact=True |
Collaborator
There was a problem hiding this comment.
not a bug. the offset seek_to_value returns includes the key itself, so it's never 0 when the key is found (8 for pset\x04, 12 for elements\x04). it only returns None when the key is missing, so if not works the same as is None here.
tested it with the proof keys moved to the front of every output, PSET and PSETView give the same hash.
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.
This PR carves the PSBT work out of #145 (Full Silent Payment Implementation) into a smaller, self-contained branch so it can be reviewed and merged first. It adds PSBTv2 support to
PSBTandPSBTView, then fixes a series of correctness and hardening gaps found while reviewing PSBT handling end to end. Silent payments (BIP-352/374/375/376/392) will be rebased on top of it as a follow-up PR.Nothing in this PR is silent-payments specific. The only forward-looking piece is
PSBT._sign_taproot_keypath, a small extraction the SP signer subclasses.What is in it
PSBTv2 (BIP-370)
PSBTandPSBTView; version-scoped field checks (v2 fields rejected in v0 and vice versa), canonical count fields, required-field checks.PSBT_GLOBAL_TX_MODIFIABLEupdates on signing, Creator/Constructor helpers (PSBT.create_v2,add_input,add_output).PSBTandPSBTView; valid vectors must round-trip byte for byte through both.Correctness fixes
ANYONECANPAYhashed the whole serialized input instead of the outpoint, andSIGHASH_SINGLEhashed the output instead of its SHA256, inTransactionandPSBTView(Invalid Taproot signatures in transactions with non-standard sighash #65). Verified against every hash type in the BIP-341 wallet test vectors.nSequence = 0was rewritten to0xffffffffandtx_version = 0to2before signing (psbt:nSequence=0is silently rewritten to0xFFFFFFFF, producing signatures over the wrong transaction #146), inInputScope,PSETandPSET.blinded_tx.PSETsigned PSETv2 with the fallback locktime instead of the BIP-370 derived one.PSBTViewmatched v2 prevout/output fields by key prefix, so a keyed or duplicated field could feed a wrong value intohashPrevouts/hashOutputsand a signature could be emitted before the malformed input was parsed.PSBT.parserejected PSBTv0 whose unsigned transaction has no inputs (Support parsing legacy BIP-174 PSBTs with zero inputs in embedded unsigned transactions #117).PSBTandPSBTViewnow produce identical bytes: same v2 global key order, both storePSBT_IN_TAP_KEY_SIGon taproot key-path spends, same locktime API (.locktimeis the raw field,determine_locktime()the derived one).Hardening
PSBTErrorfrom both parsers: truncated input, non-canonical or truncated compact sizes (compact.read_fromnow matches Bitcoin Core), corrupted nested transactions, scripts and keys, absurd lengths, trailing bytes, out-of-range values on serialization. Tests truncate and bit-flip sample PSBTs at every byte.PSBT_IN_NON_WITNESS_UTXOis held to its declared length so a wrong length cannot shift the map framing.PSBTView.view()walks all maps once and rejects trailing data and count/map mismatches, likePSBT.parse.PSBTError.PSBTView performance
Behaviour changes to be aware of
PSBTView.locktimenow means the same asPSBT.locktime: the raw fallback field for v2 (Nonewhen absent). Callers that displayed the derived transaction locktime from a view should usedetermine_locktime().PSBTView.view()rejects a stream with data after the PSBT, asPSBT.parsealways did.compact.read_fromrejects non-canonical encodings, as Bitcoin Core does.PSBT.parseraisesPSBTError(still anEmbitError) instead of a bareEmbitErroron trailing bytes.Issues covered
nSequence=0is silently rewritten to0xFFFFFFFF, producing signatures over the wrong transaction #146 (nSequence=0rewritten before signing).SIGHASH_ALLbyte on Schnorr signatures): embit appends the byte only when the caller asks forSIGHASH.ALLexplicitly, which is what BIP-341 specifies. With the defaultSIGHASH.DEFAULTthe signature is 64 bytes. Left as is, the fix belongs in the caller; noted here so it can be closed if that is agreed.Authorship
feat(BIP-370): PSBTv2 support, the global-field validation and parsing fixes, theTX_MODIFIABLEdedupe and the_sign_taproot_keypathextraction) is authored by @notTanveer and cherry-picked from Full Silent Payment Implementation #145 with authorship preserved.PRs covered or replaced
Transaction.read_from(segwit=), with tests; @sinancrypto is credited as co-author of that commit. Can be closed.PSBT_GLOBAL_UNSIGNED_TX#149 (PSBTv0 missingPSBT_GLOBAL_UNSIGNED_TX): already rejected on master since PSBT: align version handling with BIP 174 and 370 #150; this PR keeps that check in the shared validation. Can be closed.