BIP93: Fix checksum selection bounds and restrict ms sizes

[BenWestgate]

Motivation

BIP93 currently admits expanded codewords beyond the checksum’s stated period, so its detection guarantee no longer applies to the complete codeword. This is a problem for a generic codex32_decode function in later PRs. Both regular and long checksums permitted codewords in excess of their period.

Meanwhile, fixing the above will invalidate very uncommon ms strings or leave an ugly disjoint checksum selection rule for len(hrp) == 2. A better solution is immediately follow up with by restricting master seeds to the most commonly used seed and entropy sizes. This reduces target length ambiguity for optional insert/delete correcting wallets and my benchmarks show improvements in both false-correction risk and performance at the same indel search depths.

Changes

• ms32_verify_checksum and ms32_create_checksum now include the expanded "ms" HRP length when selecting regular vs. long checksums.
• ms32_create_checksum uses if 5 + len(data) > 80: as the switch to ms32_create_long_checksum.
• added ms32_create_regular_checksum and ms32_verify_regular_checksum.
• The Python reference code, specification, rationale, and vectors are updated.
• ms32_decode enforces the new ms string lengths.
• Obsolete tests/vectors from the seed size restrictions are removed.

codex32 lengths:

• Regular codewords will now be <= 93 expanded values
• Long codewords will now be >= 96 and <= 1023 expanded values

invalid codex32 lengths:

• Expanded values 94, 95 and > 1023.

codex32-encoded master seed sizes:

• 16-, 20-, 24-, 28-, 32- and 64-bytes

unsupported ms sizes:

• All from 16- to 64-bytes besides the six above.

Compatibility

This is a breaking change for old strings encoding deprecated byte-length seeds. However the authors do not see this is a problem due to the exceedingly rare possibility these have ever been created and used.

Testing

Thoroughly reviewed the complete diff.
Checked the new vectors.
Checked that expanded length 1023 verifies and 1024 fails.
Checked that the legacy short-checksum vectors should fail.
Checked that ms32_decode rejects newly invalid lengths, even when they have valid header, incomplete group and checksum.

Discussion

Proposal: #2040 (comment)
cACK #2040 (comment)
Reference impl issue: BlockstreamResearch/codex32#75
rust-codex32 PR: BlockstreamResearch/codex32#76
Restrict ms sizes suggestion: #2258 (comment)

[murchandamus]

cc: @roconnor-blockstream, @apoelstra

[BenWestgate]

Remaining gate: reproducible boundary vectors.

I added "reproducible boundary vectors."

Edit: Unsure if I should have done this to satisfy a review comment that was later marked "low quality".

For vectors, we need at minimum:

• add regular checksum 44-, 45-, and 46-byte seeds to These examples use the wrong checksum for their given data sizes
• add Expanded values 94, 95 and > 1023 to These examples have improper lengths
• remove any vectors that are now accepted

Willing to revert the "reproducible" vectors added in a9d8246 if reviews also find it overkill or open to another suggestion.

What I would keep:
Separated checksum verification into regular/long primitives that check expanded length is within the checksum period and residue is valid. While ms32_verify_checksum owns the switch to long primitive at 96 expanded symbol codewords and calls the above.

Similarly ms32_create_regular_checksum creates the short checksum for data, while ms32_create_checksum selects the required checksum by expanded codewode length and creates it.

It should be easier to generate test vectors and test them now, as well as to reuse our checksums for non-codex32 data.

[roconnor]

Based on my comment at #2040 (comment) which received some thumbs up by all parties here, I wonder if it everyone would find it preferable to simply restrict BIP-93 to seed lengths 128, 160, 192, 224, 256 and 512. I think I slightly prefer that, and if we are going to make such a change it would be better to do it from the get go.

[BenWestgate]

cACK

simply restrict BIP-93 to seed lengths 128, 160, 192, 224, 256 and 512. I think I slightly prefer that, and if we are going to make such a change it would be better to do it from the get go.

To prepare to generalize BIP-93 we still must also correct the checksum-period coverage or it leaves the unsafe checksum-selection question for every later application:

For an ms application to restrict to these lengths, codex32_decode must verify the separator, header and checksum, then return HRP and u5 data. If expanded codewords may exceed the checksum period, detection guarantees won't apply, risking undetected cross-application/profile confusion.

Since restricting ms seed lengths can simplify the test vectors, rationale and compatibility sections I will add it here as a separate commit rather than revive #2077.

• Checksum verification primitives enforce their maximum 93/1023 periods.
• Checksum verification selects Long from 96 expanded length.
• Checksum creation selects Long above 80 expanded hrp plus data length.
• Master seed format rules will update and state its valid lengths.
• “Long codex32 only for 512 bits” is an ms-profile rule, not a general rule.
• Compatibility mentions the removed previously valid non-multiple of 4-byte and 33-63-byte sizes and their previous checksum selections.
• SLIP-0039 permits additional lengths, compatibility needs "interconvertible with SLIP-0039 master seeds of supported lengths".
• 43–47-byte master-seed vectors will be removed.
• Checksum-layer assertions for 93/94/95/96/1023/1024 will be added while ms tests will enforce the six seed lengths.

[BenWestgate]

c201536 is ready for review. I drafted the next commit "BIP93: Restrict ms seed lengths", working on the vectors.

Each commit is stand alone complete.