Repository navigation
Conversation
… broken checksums addressToChecksum/address_to_checksum map 11-bit indices (0-2047) into the caller's word list without checking its length. In JS a truncated list silently produced checksums with undefined words (a 100-word list yields ["ahead", undefined, undefined, undefined, undefined] for the Satoshi test address) — a silently broken anti-poisoning checkphrase. In Rust the same input died with an opaque 'index out of bounds' panic that never names the actual contract. The Dart implementation already validates the length in its constructor, and both loaders validate it too; the checksum functions themselves did not. JS now throws 'Word list must contain exactly 2048 words, found N' and the Rust function asserts with the same message its loader uses. Tests: new jest case (100-word, empty, and 2049-word lists all reject; all 1171 vectors still pass) and two Rust should_panic cases pinning the message. cargo test --release 6/6, jest 6/6, cargo fmt --check clean.
n13
approved these changes
Oct 9, 2026
Contributor
There was a problem hiding this comment.
Reviewer model: GPT-6.1 Sol
APPROVE — no blocking findings at 0a6191fbe609a23bdcae08d01dc6bc85d7cc9abc.
The Rust and JavaScript guards enforce the existing 2048-word contract before PBKDF2 runs. Valid-list derivation and public signatures are unchanged. The regression tests verify the explicit failure messages; both implementations still pass all 1,171 shared checksum vectors.
Validation passed in a detached worktree:
cargo test --locked --release— 6 tests passed, including the shared vectors.npm ci --no-audit --no-fund— dependency installation and TypeScript build passed.npm test -- --runInBand— 6 tests passed, including the shared vectors and short/empty/oversized list rejection.cargo clippy --locked --all-targets --all-features -- -D warnings.cargo +nightly fmt --all -- --check.python3 scripts/sync_wordlists.py --checkandgit diff --check.
The latest GitHub CI snapshot has passing format, wordlist sync, JavaScript, Linux Rust, and Clippy/doc checks; macOS/Windows Rust and Dart jobs are still running.
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.
Problem
addressToChecksum(JS) andaddress_to_checksum(Rust) map 11-bit indices (0–2047) into whatever word list the caller passes, without checking its length:["ahead", undefined, undefined, undefined, undefined]— an anti-poisoning checksum with four of its five words missing, and no error anywhere. An empty list returns fiveundefineds.index out of bounds: the len is 100 but the index is 1016, which never tells the caller what the actual contract is.The Dart implementation already validates the length in its constructor (
ArgumentError: Word list must contain exactly 2048 words), and both the Rust and JS loaders validate the lists they ship — but the checksum functions themselves accept any list, so any caller passing a custom or accidentally truncated list hits this.Fix
addressToChecksumthrowsWord list must contain exactly 2048 words, found Nbefore deriving.address_to_checksumasserts the same condition with the same wordingload_word_listalready uses for its error, so the panic names the contract instead of an index.No behavior change for valid 2048-word lists.
Tests
should_paniccases pinning the message for short and empty lists.cargo test --release6/6;cargo fmt --checkclean.Reproduced before the fix with a scratch spec (JS result above) and a scratch Rust test (panic message above); both now fail loudly and clearly instead.
Submitted by @Kshot9000 (https://x.com/kshot9000) — independent builder in the Quantus ecosystem. Tips welcome in QTC: qznY8nwuvWcCCVys4da1oQdysyh8YUZYRjRqgk3S8Wos8kbau
cc @Quantus-Network