#36690 test: check guarantees of bech32 error detection

ReviewabilityReviewsAgreementSize
#36690 bartoli test: check guarantees of bech32 error detectionReady0Crickets2 + 92 tests
  • Verify Bech32m checksum error detection guarantees at standard and silent payment character limits
  • Clarify header comments so developers do not assume checksums can locate or repair errors
  • Ready for review
  • No review verdicts recorded
  • No reviews or external comments yet
  • Awaiting initial review from maintainers or contributors
  • 2 lines added or modified outside tests
  • 92 lines added or modified in tests
  • 2 lines removed in total
  • 2 files, 2 commits
Test infrastructure#40 of 54
P4 · test coverage
  • P4 because it adds unit test coverage for existing behavior rather than addressing bugs or flaky tests
  • Verifies bounds around silent payment limits without changing node runtime logic or test harnesses
  • Ranking pass: Moved up within P4: it tests a real boundary (the 1023-char silent-payments limit) and corrects misleading header comments.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds unit tests in bech32_tests.cpp to verify Bech32m checksum error detection guarantees and error location bounds at both the standard 90-character limit and the 1023-character silent payments limit. Also adjusts comments in bech32.h to state that checksums detect character changes rather than ambiguously implying they can find or fix errors.

Problem: Test coverage does not explicitly validate error detection guarantees or error location behavior around the 1023-character limit introduced for BIP352 silent payments. Furthermore, header comments stating that character limits 'guarantee finding up to N errors' can mislead developers into thinking errors can be accurately located or repaired.

Evidence of importance:

  • Author noted in the description: 'Initially I was just looking at all code around Silent Payments, looked if the bech32 test had been updated to have tests for the new CharLimit::SILENT_PAYMENTS, and saw less tests than I expected.' (PR description)

Risk of a mistake: low (tests, docs). Touches only unit tests and header comments; any error would cause test suite failures without affecting node runtime behavior.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

Agreement

Crickets

no substantive comment either way

Thread notes: Initial record. No external reviews or comments yet; author noted fixing an error in a comment. / Initial record. No reviews yet; only an update comment from the PR author.

Objections, suggestions, and questions: none recorded.

Files

92 lines under test/bench/ci.

  • src/test/bech32_tests.cpp +92/-0
  • src/bech32.h +2/-2

Card

The short text the ranking pass compares across PRs.

This PR adds unit tests verifying Bech32m checksum error detection and error location bounds at both the standard 90-character limit and the 1023-character silent payments limit. It also clarifies comments in bech32.h that character limits guarantee detecting changes rather than locating or fixing errors. The change addresses missing test coverage for the BIP352 silent payment character limits.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-30T01:15, confidence high, input hash e4adc3c6807ff7ca
thread read through 2026-10-29 (1 statements), head 6151f1dd19
labels: Tests

Categories

Test infrastructure

P4 · test coverage (ranking pass: Moved up within P4: it tests a real boundary (the 1023-char silent-payments limit) and corrects misleading header comments.)

Adds unit tests verifying Bech32m error detection guarantees and clarifies header comments without addressing a known bug or flaky test. Test infrastructure prioritizes test harness signal and regression-catching tests, while test coverage added for its own sake ranks low.

Membership: PR has the Tests label and adds unit test coverage in src/test/bech32_tests.cpp with no separate category for encoding/bech32.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0