Fix mnemonic checksum generation for cross-SDK compatibility #2
Conversation
The mnemonic encoding/decoding was using big-endian bit packing instead of the little-endian bit packing required by the Algorand SDK spec. Changes: - Rewrite encode() to use little-endian bit packing via toElevenBit() - Rewrite decode() to use little-endian bit unpacking via fromElevenBit() - Add cross-SDK compatibility tests with verified test vectors The fix ensures mnemonics generated by swift-algorand are now accepted by py-algorand-sdk and other official Algorand SDKs.
0xLeif
force-pushed
the
fix/mnemonic-checksum
branch
from
b5e0f36 to
47c573a
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical compatibility issue in mnemonic encoding/decoding by switching from big-endian to little-endian bit packing, ensuring the swift-algorand SDK generates mnemonics compatible with py-algorand-sdk and other official Algorand SDKs.
- Rewrites
encode()anddecode()methods to use little-endian bit packing via new helper functionstoElevenBit()andfromElevenBit() - Adds comprehensive cross-SDK compatibility tests with verified test vectors from py-algorand-sdk
- Adds robustness testing with multiple generated mnemonics
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Sources/Algorand/Mnemonic.swift | Replaces big-endian string-based bit manipulation with little-endian buffer-based bit packing/unpacking for proper cross-SDK compatibility |
| Tests/AlgorandTests/MnemonicTests.swift | Adds test vectors verified against py-algorand-sdk and multiple-generation round-trip tests to ensure compatibility and correctness |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Test with tampered padding (last word should encode to zeros in padding bits) | ||
| var words = validMnemonic.components(separatedBy: " ") | ||
| words[24] = "ability" // "ability" = index 1, encodes to non-zero | ||
| words[24] = "ability" // "ability" = index 1, encodes to non-zero in padding bits |
There was a problem hiding this comment.
The comment is misleading. With the new little-endian implementation, the last word (word 24) is the checksum, not padding. The test is actually checking that modifying the checksum word to "ability" (index 1) will fail validation because it won't match the computed checksum. Consider updating the comment to: // Test with tampered checksum (last word should match computed checksum)
The mnemonic encoding/decoding was using big-endian bit packing instead of the little-endian bit packing required by the Algorand SDK spec.
Changes:
The fix ensures mnemonics generated by swift-algorand are now accepted by py-algorand-sdk and other official Algorand SDKs.