-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Refresh improvements to wallet_rpc_server #8355
Conversation
} catch (const std::exception& ex) { | ||
LOG_ERROR("Exception at while refreshing, what=" << ex.what()); | ||
} | ||
m_last_auto_refresh_time = boost::posix_time::microsec_clock::universal_time(); | ||
// if we got the max amount of blocks, do not set the last refresh time, we did only part of the refresh and will | ||
// continue asap, and only set the last refresh time once the refresh is actually finished |
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.
What if it, by chance, fetches 256 blocks, but it's the actual 100% refresh (no blocks left)? It will try to refresh again and fetch 0 blocks next time, so it should be safe, but I'm not sure.
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.
That is executed all the time, whether there are blocks left to fetch or not (because it can never know as the daemon can add blocks at any time). A lot of existing calls will fetch no blocks. So all it'll do with the patch is call once asap for, most likely, nothing.
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.
Also, this may be a bit confusing, but since the wallet pulls blocks in chunks of either 1000 or... some max byte size, it'll refresh more than 256 at once in practice, until blocks get large enough often enough.
src/wallet/wallet_rpc_server.cpp
Outdated
try { | ||
if (m_wallet) m_wallet->refresh(m_wallet->is_trusted_daemon()); | ||
bool received_money = false; | ||
if (m_wallet) m_wallet->refresh(m_wallet->is_trusted_daemon(), 0, blocks_fetched, received_money, true, 256); |
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.
Could you make this 256
value a variable / macro to clarify its usage / make it easier to adjust please? Especially since this raw value is used later on in the code, for a new reader the connection may not be clear, and perhaps in the future one instance of the value will get changed and not the other, leading to some weird behavior.
No description provided.