Wallet-related improvements #1199
Merged
Conversation
This file contains 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
Currently all the variables in here need to be added into every compilation unit; C++17 inline constexpr avoids that by throwing away the duplicates at link time, plus lets the compiler optimize things away. Also eliminates a side effect in `get_config()` that mutated the variable (so that one `get_config()` would end up changing the data of a previous `get_config()`). Instead `get_config()` is now always constant and doesn't care about the hard fork version; instead there is a convenience method to get the governance wallet_address which takes the hf version.
The parameters for estimating the mainnet height have drifted slightly over the past two years -- there is now only about 2 days grace time (of the 7 days added). Testnet height estimates were already invalid (producing estimates a little more than 3 weeks into the future because of some testnet stalls and rollbacks), which means a newly generated testnet wallet created without a daemon connection wouldn't work for ~3 weeks. Updated them to values from the current mainnet and testnet chains. This also moves the height and timestamp estimate parameters to constexpr's in cryptonote_config as that seems a much better place for them than line 13120 of wallet2.cpp.
Also stop using the deprecated name in wallet2.
Replace invoke_http_{bin,json,json_rpc} with a invoke_http<T>, where T
is the RPC type from which we can figure out binary or JSON or JSON RPC
or light wallet RPC, and thus figure out the URL.
Also remove superfluous arguments: neither timeout or http_method
arguments ever actually differ, so just eliminate them.
|
The cryptonote_config.h one isn't wallet-specific, but the height estimate PR depends on it. |
3 minutes and 30 seconds is an insane amount of time to wait on an RPC request. Reduce it to 30 seconds (which is still high, but not quite so insane).
The two static functions here are already in an anonymous namespace. The static local constexpr is not an optimization (and possibly worse -- a static local variable incurs an atomic initialization penalty).
wallet2 does nearly identical tx fetching in several places; this DRYs out the code a bit.
jagerman
force-pushed
the
wallet-rpc-fixes
branch
from
800ba64
to
9931230
Compare
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.
Cherry picked some commits out of the Ledger update PR (#1195) that aren't Ledger specific.