Do not save temporary keys in vault; JSON-RPC fix for getTransactions#160
Merged
Conversation
ITBear
approved these changes
May 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent ephemeral ADNL handshake keys from being persisted in Secrets Vault (by generating an in-memory Ed25519 key instead). It also includes additional (seemingly unrelated) behavioral changes in the get_transactions RPC handler around handling inconsistent lookup_block_by_lt results and mapping “not found” to a structured ApiError::NotFound.
Changes:
- Generate a non-vault-backed Ed25519 key for ADNL handshake packets to avoid persisting temporary keys.
- Add LT-range/account-presence sanity checks in
get_transactions, logging warnings and returning partial results where possible. - Change the “no transactions found” case in
get_transactionsto returnApiError::NotFound(404) instead of a generic internal error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/node/src/rpc_server/handlers.rs |
Adds additional validation/logging around block lookup by LT and changes “not found” error mapping for get_transactions. |
src/adnl/src/adnl/node.rs |
Switches handshake key generation to Ed25519KeyOption::<ZeroizingBytes>::generate() to avoid Vault persistence. |
Comments suppressed due to low confidence (2)
src/node/src/rpc_server/handlers.rs:219
- This
breakhappens without any warning/logging, while other "lookup returned inconsistent block" branches log a warning before breaking. Adding alog::warn!here (similar to thelt > end_ltcase) would make diagnosing inconsistentlookup_block_by_ltresults much easier in production.
let block_info = block_stuff.block()?.read_info()?;
if lt < block_info.start_lt() {
break;
}
src/node/src/rpc_server/handlers.rs:230
- When
lookup_block_by_ltreturns an inconsistent block (outside LT range / missing account), the code logs a warning and then exits the loop, which can later surface to the client as a genericNotFoundif no transactions were exported. That response can be misleading for a backend/index inconsistency; consider returning an explicit internal error (or a more specificNotFoundmessage) whenexportis still empty at the point you detect the inconsistency.
if lt > block_info.end_lt() {
log::warn!(
"getTransactions: lookup_block_by_lt returned block outside LT range: \
address={}, target_lt={lt}, block={block_id}, block_start_lt={}, block_end_lt={}",
p.address,
block_info.start_lt(),
block_info.end_lt(),
);
break;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
No description provided.