Skip to content
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

Wallet refresh improvements #8941

Merged
merged 2 commits into from
Sep 15, 2023

Conversation

moneromooo-monero
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@selsta selsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release branch equivalent of #8355

@plowsof
Copy link
Contributor

plowsof commented Jul 8, 2023

+1 , the --no-initial-sync is a small but imo a huge ux++ for anyone working with monero-wallet-rpc. I can now simply ask the wallet its current sync height and receive an answer instead of hanging until its fully synced. I have tested this PR and confirm it works, (also run a github action that syncs a wallet once a day which has been benefiting from it) Thanks!

@@ -60,6 +60,7 @@ using namespace epee;
#define MONERO_DEFAULT_LOG_CATEGORY "wallet.rpc"

#define DEFAULT_AUTO_REFRESH_PERIOD 20 // seconds
#define REFRESH_INFICATIVE_BLOCK_CHUNK_SIZE 256 // just to split refresh in separate calls to play nicer with other threads

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was inficative (not a word) meant to be indicative? Even indicative seems like an odd name choice, it's more like a default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was. IIRC that value is only respected by monerod if the blocks aren't too big, so it's only indicative. Maybe "max" would have been better.
I won't bother changing it here since it's a copy of the master patch, which is already merged.

@luigi1111 luigi1111 merged commit 031d318 into monero-project:release-v0.18 Sep 15, 2023
16 of 17 checks passed
@binarybaron
Copy link

binarybaron commented Dec 10, 2023

I'm using the monero-wallet-rpc in my application and I'm attempting to:

  1. Refresh wallet by calling refresh
  2. While refresh is pending I want to periodically check how far we've synced by calling get_height

Problem is that the rpc does not respond to get_height while it's refreshing? Is this expected behaviour. I thought the change made here to sync blocks in batches should have fixed that issue.
Do you have any pointers for me what the issue could be?

binarybaron pushed a commit to comit-network/xmr-btc-swap that referenced this pull request Jun 10, 2024
This PR changes the following behaviour in the refresh functionality of the monero wallet
- Allows for multiple retries because in some cases users have experienced an issue where the wallet rpc returns `no connection to daemon` even though the daemon is available. I'm not 100% sure why this happens but retrying often fixes the issue
- Print the current sync height after each failed attempt at syncing to see how far we've come
- The `monero-wallet-rpc` is started with the `--no-initial-sync` flag which ensures that as soon as it's started, it's ready to respond to requests
- The `monero-wallet-rpc` was upgraded to `v0.18.3.1` because this PR monero-project/monero#8941 has improved some of the issues mentioned above


This PR is part of a larger effort to fix this issue #1432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants