BRC-220: state the ECDSA digest convention and the batch leaf datum - #246
codenlighten wants to merge 2 commits into
Conversation
Two clarifications, and a batch-mode test vector. Neither changes behaviour: both state what the reference implementation already does, so no issued certificate is affected. The signing sentence now says what "signs the payloadHash directly" means for ECDSA-secp256k1: payloadHash is the digest H of SEC 1 section 4.1.3, read as a big-endian integer and not hashed again. Libraries that re-hash by default, or that read the digest as little-endian, produce signatures conformant verifiers reject. The batch record now binds RFC 6962's generic leaf datum d to the proof's proofHash. Using canonicalBytes instead is equally sound but yields a different root, and the text did not say which. The vector covers five proofs so the tree splits 4/1, and gives the root under both readings of d.
There was a problem hiding this comment.
🟡 Changes recommended
The new batch wording/test-vector still leaves a couple of reproducibility-critical details ambiguous (notably leafIndex base and i encoding/Unicode math notation).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates BRC-220 with clarifying specification text around ECDSA digest handling and batch-mode Merkle leaf construction, and adds a batch-mode test vector to help independent implementations match the reference behavior.
Changes:
- Clarifies that
ECDSA-secp256k1signspayloadHashas SEC 1 digest H (big-endian integer) without re-hashing. - Specifies batch Merkle leaf datum
dasproofHash, and explains leaf ordering /leafIndex. - Adds a 5-leaf batch-mode test vector (including the alternate-root outcome under
d = canonicalBytes).
File summaries
| File | Description |
|---|---|
| apps/0220.md | Adds clarifications for ECDSA digest conventions, batch leaf datum definition, and a new batch-mode test vector. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - **hybrid** (`mode = 1`): as full, but the final two pushes are `SHA-256(publicKey)` and `SHA-256(signature)`; the full blobs live in the certificate (keeps large post-quantum records small on-chain). | ||
| - **batch** (`kind = 2`): `"NOTARYHASH" | u8(1) | u8(2) | merkleRoot(32) | u32be(leafCount)`. One transaction anchors many proofs under an [RFC 6962](https://www.rfc-editor.org/rfc/rfc6962) Merkle root, domain-separated (`leaf = SHA256(0x00 ‖ d)`, `node = SHA256(0x01 ‖ l ‖ r)`, split at the largest power of two `< n`, last leaf never duplicated). | ||
|
|
||
| The leaf datum `d` for each proof is its 32-byte **`proofHash`**, so a leaf is `SHA256(0x00 ‖ proofHash)` — that is, `SHA256(0x00 ‖ SHA256(canonicalBytes))`, not `SHA256(0x00 ‖ canonicalBytes)`. Both are sound, but they produce different roots, so the choice is stated rather than left to RFC 6962's generic `d`. Leaves are in the order the batch was assembled, and `leafIndex` in the certificate's `merkle` object is that position. |
There was a problem hiding this comment.
Fixed in dcb613c: leafIndex is now stated as the position counted from 0.
|
|
||
| #### Batch-mode test vector | ||
|
|
||
| Five proofs, so the tree splits 4/1 and exercises both the largest-power-of-two split and the never-duplicate rule. For `i` in `0..4`: private key `SHA-256("BRC-220/batch-vector/key/" + i)`, `payloadHash` `SHA-256("BRC-220/batch-vector/payload/" + i)`, `createdAtUnix` `1767225600 + 86400·i` (seconds), `algorithm` `ECDSA-secp256k1`, `hashAlgorithm` `SHA-256`, compressed public key, 64-byte `r ‖ s` signature with an RFC 6979 nonce, normalised to low-S. (Two of the five signatures differ without that normalisation, so it is needed to reproduce the values below.) |
There was a problem hiding this comment.
Fixed in dcb613c: the recipe now uses ASCII *, says i is written as one ASCII decimal digit ("0" to "4"), and says the key digest is read as a big-endian integer. The preimage bytes are unchanged, so every value in the vector is unchanged.
…the vector The batch test vector's recipe used a Unicode middle dot and did not say how i becomes bytes. It is one ASCII decimal digit, and the key digest is read as a big-endian integer. The bytes, and so every value in the vector, are unchanged.
There was a problem hiding this comment.
🟢 Approval recommended
Changes are documentation-only clarifications and a test vector addition, with no behavioral impact and only minor reference-link nits noted.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
apps/0220.md:34
- The spec cites SEC 1 sections but doesn’t link to the referenced document, which makes it harder for implementers to verify the exact digest/endianness convention. Prefer an inline link to SEC 1 (as done in other BRCs) so the reference is unambiguous.
apps/0220.md:110 - The batch-mode test vector relies on an “RFC 6979 nonce”, but RFC 6979 isn’t linked here. Adding an inline link avoids ambiguity about which deterministic-nonce scheme is meant.
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
… height, and its fields are filed for BRC-220 (#169) Carries the base64 length rule and the optional header height from #168, and the certificate field definitions from #167, now filed upstream as bsv-blockchain/BRCs#247. The batch-leaf draft says that leafIndex counts from 0 and how i is written in the vector, as review of bsv-blockchain/BRCs#246 asked. Additive, or refuses only malformed input.
Two clarifications to BRC-220, plus a batch-mode test vector. Neither changes behaviour. Each writes down what the reference implementation already does, so no issued certificate is affected. That keeps this within "adding context or wording" rather than a revision that would need a new BRC.
Both ambiguities were hit in practice by a second, independent implementation (
@smartledger/bsv). One of them shipped as a bug before it was caught.1. What "signs the
payloadHashdirectly" means for ECDSA§Algorithms says the signer signs the 32-byte
payloadHashdirectly. ForECDSA-secp256k1that still leaves two common ways to get it wrong:@noble/curvesv2prehash), and so signSHA-256(payloadHash);endian: 'little'mode that reads the digest as little-endian.Each produces a valid-looking signature over a different value. The added sentence pins the convention to SEC 1 §4.1.3:
payloadHashis the digest H, read as a big-endian integer (SEC 1 §2.3.8), and is not hashed again.That is the reference implementation's behaviour (
secp256k1.sign(payloadHash, priv, { prehash: false })). The published certificate golden vector confirms it: its signature verifies under this reading and fails under the two others.2. The batch leaf datum
The batch bullet names the RFC 6962 construction,
leaf = SHA256(0x00 ‖ d), but never says whatdis. Two readings are both reasonable:dproofHashSHA256(0x00 ‖ SHA256(canonicalBytes))canonicalBytesSHA256(0x00 ‖ canonicalBytes)Both are equally secure, and they produce different roots. A certificate built under one fails inclusion under the other, with nothing pointing at the cause. The added paragraph states
d = proofHash, which is what the reference batcher uses (leaves = batch.map(e => e.proofHash)). It also states the leaf order and whatleafIndexrefers to.3. Batch-mode test vector
The existing published vectors don't cover batch mode. This one uses five proofs, so the tree splits 4/1. That tests the largest-power-of-two split and the never-duplicate rule, and a power-of-two leaf count tests neither. Every input is derived from a labelled preimage, so anyone can rebuild it.
It also gives the root under the other reading of
d. An implementation that computes that root has made a reasonable choice against the previous wording, not a coding error, and printing both roots makes the mismatch diagnosable in one line.The vector was rebuilt independently of the code that first generated it:
@noble/curveswith no re-hashing;All eleven values agree.
Not in this PR
The certificate's
encodingfield is still listed as required, with its values never enumerated. In the reference implementation they are"hex"and"base64", meaning howpublicKeyandsignatureare written into the JSON. That, along with the JSON types ofversionandmode, deserves its own clarification. I've kept it separate so this change stays uncontroversial.