doc: correct PSBT spec size figures (Appendix A units, §4.5 limits) - #159
Open
augchan42 wants to merge 2 commits into
Open
doc: correct PSBT spec size figures (Appendix A units, §4.5 limits)#159augchan42 wants to merge 2 commits into
augchan42 wants to merge 2 commits into
Conversation
The arithmetic in Appendix A is right; the unit is wrong by a factor of 1000. 2 x (1344 + 2421) is 7,530 bytes, so ~7.5 KB binary and ~10 KB base64, not MB. The document contradicts itself as written. Section 4.5 caps MAX_DILITHIUM_PARTIAL_SIG_PAYLOAD_PER_INPUT at 1,500,000 bytes. At the stated 7.5 MB, every conforming 2-of-3 PSBT would exceed the spec's own per-input limit and be rejected on load. Reading the figure as KB resolves it: 7,530 is comfortably under 1,500,000. A real 2-of-3 PSBT produced on testnet measured about 12 KB, which is consistent with the KB reading once the non-partial-sig fields are included. This matters more than a typo usually would. The abstract states that two independent teams must be able to produce byte-compatible PSBTs from this document alone, and Appendix A is the worked example they would size transport against. An implementer reading 10 MB would reasonably conclude that email attachment is infeasible and design around a constraint that does not exist. Unit only. No change to the arithmetic, the field layout, or the guidance that these are file-transfer sized and not QR sized.
MAX_CONTROL_BLOCK_SIZE was given as 513 while citing P2MR_CONTROL_MAX_SIZE as its source, but that constant is 1 + 32*128 = 4097. Since §4.7 makes the cap normative, an implementation built from the spec would reject control blocks for leaves at depth 17 and deeper, which are consensus-valid and reachable through createp2mr. That would make an output spendable on-chain but unspendable through a conformant PSBT implementation, and would diverge from btq-core, whose parser already bounds the control block by P2MR_CONTROL_MAX_SIZE itself. Drop the separate parse cap so the limit is the consensus constant, and fix §4.9.1, which stated 4129. That figure is taproot's, where the control block carries a 33-byte base for an internal key P2MR has no equivalent of. MAX_P2MR_LEAF_SCRIPT_SIZE was 10000, capping accumulator leaves at seven Dilithium keys. Consensus imposes no script-size limit here: MAX_SCRIPT_SIZE applies only to SigVersion::BASE and WITNESS_V0, and MAX_SCRIPT_ELEMENT_SIZE does not bind the leaf either, because it is popped from the witness stack before ExecuteWitnessScript checks the remaining elements. Record the value the parser uses, and note that key count is bounded by MAX_DILITHIUM_PARTIAL_SIGS_PER_INPUT rather than by leaf size.
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.
Two size-figure corrections to the BTQ-PSBT spec merged in #99.
1. Appendix A unit error (MB where KB was meant)
design/DILITHIUM_PSBT_DESIGN.md, Appendix A:The arithmetic is correct; the unit is wrong by a factor of 1000.
(1344 = 1312-byte Dilithium pubkey + 32-byte leaf hash, per §4.2. 2421 = 2420-byte sig + 1 sighash byte, per §4.5.)
Why this is a real defect and not a typo
The document contradicts itself as written. §4.5 sets:
MAX_DILITHIUM_PARTIAL_SIG_PAYLOAD_PER_INPUTAt Appendix A's stated 7.5 MB, every conforming 2-of-3 PSBT would exceed the spec's own per-input limit and be rejected on load — the worked example would be unimplementable under the same document's validation rules. Reading the figure as KB resolves the contradiction cleanly:
Independent confirmation from a real PSBT. A 2-of-3 Dilithium PSBT produced on testnet measured ~12 KB, consistent with the KB reading once the non-partial-sig fields (witness UTXO, leaf script, control block, unsigned tx) are added on top of the ~10 KB base64 partial-sig payload.
Why it is worth fixing before anyone implements
The abstract states:
Appendix A is the worked example an implementer sizes transport against. Someone reading 10 MB would reasonably conclude email attachment is infeasible and design around a constraint that does not exist — landing on chunking or a transfer service for a payload that comfortably fits in a mail attachment. That is the opposite of the intended guidance, and §11 (Transport guidance) reads very differently depending on which number you arrived with.
2. §4.5 size limits disagreed with consensus
Flagged by @augchan42 in review on #99 as one of the two findings to settle before landing. They were not settled, so they are still on master.
Control block cap
§4.5 gave:
MAX_CONTROL_BLOCK_SIZEP2MR_CONTROL_MAX_SIZEIt names
P2MR_CONTROL_MAX_SIZEas its source, but that constant is 4097:513 is 1 + 32*16, a 16-level tree; consensus permits 128 levels. §4.7 makes the cap normative (
control_block size ∈ [P2MR_CONTROL_BASE_SIZE, MAX_CONTROL_BLOCK_SIZE]), so an implementation built from this spec rejects control blocks for leaves at depth 17 and deeper. Those are consensus-valid and reachable today:createp2mraccepts an arbitrary DFS tree and validates depth only againstP2MR_CONTROL_MAX_NODE_COUNT.btq-core itself is not affected.
psbt.h:725already bounds the key by1 + P2MR_CONTROL_MAX_SIZE, so the parse cap is the consensus cap by construction and cannot drift below it. Checked rather than assumed: a leaf at depth 128 carrying a 4097-byte control block round-trips throughDecodeBase64PSBTintact. The consequence of the doc bug is therefore not a broken btq-core but a second implementation that refuses PSBTs btq-core produces, which is harder to diagnose than an outright unspendable output.§4.9.1 separately stated
P2MR_CONTROL_MAX_SIZE = 1 + 32 * 128 = 4129. 4129 is taproot'sTAPROOT_CONTROL_MAX_SIZE, where the base is 33 bytes to carry an internal key P2MR has no equivalent of.Leaf script cap
§4.5 gave
MAX_P2MR_LEAF_SCRIPT_SIZEas 10000, which caps an accumulator leaf at seven 1312-byte Dilithium keys, below the production m-of-n case §2.2 calls critical.Two candidate consensus bounds came up in review; neither actually binds the leaf:
MAX_SCRIPT_SIZE(100000) is enforced only forSigVersion::BASEandWITNESS_V0(interpreter.cpp:481), so P2MR tapscript is unbounded at the script level, as in BIP342.MAX_SCRIPT_ELEMENT_SIZE(15000) does not apply either, because the leaf script is popped from the witness stack atinterpreter.cpp:2268before the element-size loop inExecuteWitnessScriptsees the remaining stack.Consensus therefore bounds leaf size only by transaction and block weight. §4.5 now records the value the parser actually uses, states that it is a parser-memory bound rather than a consensus mirror, and points at
MAX_DILITHIUM_PARTIAL_SIGS_PER_INPUT(20, trackingMAX_PUBKEYS_PER_MULTISIG) as the real ceiling on key count.Both entries now carry a note that a PSBT cap sitting below a consensus limit is the defect to avoid, and that the control block bound must be derived from the consensus constants rather than restated, so the two cannot drift apart again. That is the same failure mode as the
-blocksonly/-maxmempoolmismatch in #146, with the drift in the spec rather than in the code.Scope
Documentation only. No change to the arithmetic in Appendix A, to the field layout, or to the standing guidance that these payloads are file-transfer sized and explicitly not QR sized. The §4.5 changes bring the stated limits into agreement with the constants they cite; no btq-core behaviour changes, because the parser already used the consensus constants.
Test plan
src/script/interpreter.h:252-255src/psbt.h:723-728MAX_SCRIPT_SIZEapplies only to BASE/WITNESS_V0 atsrc/script/interpreter.cpp:481src/script/interpreter.cpp:2268MAX_CONTROL_BLOCK_SIZE, 513, 4129 or 10000 references remain in the spec