fix(bns): correct SQLite read-width predicate and make_decrypted helper - #196
Merged
Merged
Conversation
The get<T> templates at beldex_name_system.cpp:301-306 compare sizeof(T) against 32, but sizeof is bytes not bits, so every integer type takes the 32-bit reader and 64-bit columns are silently truncated. The matching bind<T> templates at line 178 already use the correct predicate (<= 4). Mainnet heights and timestamps will start losing precision once they cross INT32_MAX. mapping_value::make_decrypted at line 1584 calls encrypt() instead of decrypt(), and the assert(!result.encrypted) on the next line cannot hold after that call. The function has no callers right now; fixing it before anything starts using it.
Author
|
Closing this PR. On reflection both changes here are dormant: the sizeof predicate fires only when SQLite columns cross INT32_MAX (no current callers hit that range), and make_decrypted has no callers at all. These belong in ordinary maintenance rather than a Major-framed PR. Will let the fixes come back later as one-line changes if the underlying call sites grow. |
sanada08
approved these changes
Aug 12, 2026
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.
Summary
Two correctness bugs in
src/cryptonote_core/beldex_name_system.cpp.1. SQLite read-width predicate compares bytes against bits
At lines 301-306, the templated
get<T>overloads dispatch betweensqlite3_column_int(32-bit) andsqlite3_column_int64(64-bit) based onsizeof(T). The current predicates comparesizeof(T)to32:sizeofreturns bytes, not bits. Every integral type sits comfortably under 32 bytes, so the first overload matches all of them — every column, includinguint64_theights and timestamps, is read throughsqlite3_column_int(signed 32-bit). Values aboveINT32_MAXare silently truncated.The comment on the first template says "Small (<=32 bits)", confirming the intent was 32 bits = 4 bytes. The sibling
bind<T>overloads at line 178 already use the correct predicate:So the get/bind pair is already inconsistent. Mainnet heights and timestamps dont yet exceed 2^31, so this is latent: the truncation will start happening silently when the chain or the time epoch grows past INT32_MAX.
2.
make_decryptedcallsencryptinstead ofdecryptAt lines 1584-1590:
The function name says decrypt; the function body calls encrypt. The assert at line 1588 (
!result.encrypted) is inconsistent with calling encrypt and would fire in any build with asserts enabled if this function were used.mapping_value::decrypthas signaturebool decrypt(std::string_view name, mapping_type type, const crypto::hash* name_hash = nullptr)(seebeldex_name_system.h:95), so the helper needs to accept amapping_typeargument to forward. The siblingmake_encryptedis correctly structured.The function has no callers right now (grep shows only the declaration in the header and the definition here), so this is a quiet correctness bug in dormant API surface. Worth fixing before anything starts depending on it.
Patches
sizeof predicate fix
This mirrors the existing
bind<T>overloads. No behavior change for current heights; correct dispatch once values cross 2^31.make_decrypted fix
src/cryptonote_core/beldex_name_system.cpp:src/cryptonote_core/beldex_name_system.h, update declaration to match:mapping_typeis non-defaultable here becausedecryptrequires it. Since the function has no current callers this is a safe API correction.Why these two together
Both are in the same file, both are one-token-style correctness fixes, both touch BNS data handling rather than memory safety. Splitting them just doubles the review surface for two one-token fixes.
Tests
The first change is dispatch-only and produces the same column reads at current heights; the second has no callers to break. Happy to add a test that binds and reads back a
uint64_t > INT32_MAXto lock in the dispatch fix.