-
Notifications
You must be signed in to change notification settings - Fork 2.2k
rpcserver: add wallet_synced to GetInfoResponse
#10507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @hieblmi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
walled_is_synced to ListUnspentResponse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a wallet_is_synced field to the ListUnspent RPC response, which is a useful addition for clients to know if the wallet's view of UTXOs is up-to-date. The implementation is clean and duplicated correctly across both the main rpcserver and the walletrpc sub-server. The new integration test covers the intended functionality well by simulating a node restart and sync process. The changes to protobuf and swagger definitions are also correct. I have one minor suggestion for the release notes.
walled_is_synced to ListUnspentResponseWalletSynced to GetInfo
WalletSynced to GetInfowallet_synced to GetInfoResponse
In this commit, we add a new `wallet_synced` boolean field to the GetInfoResponse message. This field exposes the wallet's internal sync state with the backing chain source, providing visibility into whether the wallet has caught up to the current chain tip. This is distinct from the existing `synced_to_chain` field, which represents a composite sync state that also considers the router and blockbeat dispatcher. The new field allows callers to distinguish between wallet sync delays and other subsystem sync states.
Generated files for the wallet_synced field addition to GetInfoResponse.
In this commit, we extend the chainSyncInfo struct with a new isWalletSynced field that tracks the wallet's sync state independently from the composite isSynced field. The GetInfo RPC handler now populates the WalletSynced response field from this new struct field. A debug log line is added to GetInfo to help diagnose sync state issues, showing both the composite sync status and the wallet-specific sync status. Currently isWalletSynced mirrors isSynced since both ultimately derive from the same underlying wallet sync check. This prepares the plumbing for future differentiation where wallet sync state could be tracked separately from router and blockbeat dispatcher states.
In this commit, we add an integration test that verifies the wallet_synced field in GetInfoResponse correctly reflects the wallet's sync state. The test creates a node, verifies wallet_synced becomes true after initial sync, then stops the node and mines blocks while it's offline. After restart, the test polls GetInfo to observe the wallet catching up, ideally capturing the transition from wallet_synced=false to true. The test is registered in the "wallet sync" test case group.
|
|
||
| // Whether the wallet is fully synced to the best chain. This indicates the | ||
| // wallet's internal sync state with the backing chain source. | ||
| bool wallet_synced = 23; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we expose the block at which the wallet is?
uint32 wallet_block_height = 24;
string wallet_block_hash = 25;
Then we can track this field to reach the block we're handling deposits for:
blockSub := RegisterBlockEpochNtfn()
block := <-blockSub
info := GetInfo()
for info.WalletBlockHeight < block.Height {
sleep
info = GetInfo()
}
ListUnspent()
Change Description
This PR adds a new field
wallet_syncedto theGetInfoResponseto inform the caller if the wallet is still syncing to the best chain tip.Steps to Test
See the attached itest.