Daemon RPC: add a new method to get height from a timestamp #8088
Conversation
|
Is it worth replacing the wallet2 code with this RPC call then? |
| @@ -88,7 +88,7 @@ namespace cryptonote | |||
| // advance which version they will stop working with | |||
| // Don't go over 32767 for any of these | |||
| #define CORE_RPC_VERSION_MAJOR 3 | |||
| #define CORE_RPC_VERSION_MINOR 8 | |||
| #define CORE_RPC_VERSION_MINOR 9 | |||
There was a problem hiding this comment.
This will conflict, we're already at 9 in master.
There was a problem hiding this comment.
OK, rebased and bumped the version to 10.
tevador
force-pushed
the
getheightbytime
branch
from
9233afb
to
271781f
Compare
Yes and I'm assuming the GUI and 3rd party wallets will use it too. Also services like MyMonero that scan on behalf of users. |
This is for wallets that need to convert a user-provided date to a blockchain height.
tevador
force-pushed
the
getheightbytime
branch
from
271781f
to
82b8888
Compare
One thing to consider is that at least the GUI doesn't have a daemon connection yet during wallet restore. We use this code currently which is a port from wallet2: https://github.com/monero-project/monero-gui/blob/master/js/Wizard.js#L134 |
|
At some point the GUI has to connect to a daemon to start scanning outputs, so the request could be deferred until then. I know it is possible to make a reasonably good estimate without a daemon connection (by hardcoding some timestamps and heights), but it's a much less maintainable solution. I tested |
| KV_SERIALIZE(timestamp) | ||
| END_KV_SERIALIZE_MAP() | ||
| }; | ||
| typedef epee::misc_utils::struct_init<response_t> response; |
There was a problem hiding this comment.
Prefer using to typedef. For example: using response = epee::misc_utils::struct_init<response_t>;
There was a problem hiding this comment.
That is not my decision. I'm simply following the established coding practice for this header file.
| CHECK_PAYMENT(req, res, COST_PER_HEIGHT_BY_TIME); | ||
|
|
||
| constexpr uint64_t buffer_time = 1 * 60 * 60; //1 hour | ||
| constexpr uint64_t buffer_height = 1 * 30; //1 hour |
There was a problem hiding this comment.
Is this 30 from 60 minutes in an hour divided by a block every two minutes? In other words, what is buffer_height?
There was a problem hiding this comment.
Refer to lines 3638-3642. The binary search ends when the boundary block heights are closer than buffer_height. The search uses both a time-based and a height-based condition to ensure a good behavior even with irregular block times.
| @@ -3570,6 +3570,93 @@ namespace cryptonote | |||
| return true; | |||
| } | |||
| //------------------------------------------------------------------------------------------------------------------------------ | |||
| bool core_rpc_server::on_get_height_by_time(const COMMAND_RPC_GET_HEIGHT_BY_TIME::request& req, COMMAND_RPC_GET_HEIGHT_BY_TIME::response& res, epee::json_rpc::error& error_resp, const connection_context *ctx) | |||
There was a problem hiding this comment.
Might want to also follow up with a request to add this method to the online developer documentation if it gets merged. See this.
There was a problem hiding this comment.
Yes, this should be done if this PR ever gets merged.
|
|
||
| auto& db = m_core.get_blockchain_storage().get_db(); | ||
|
|
||
| for (;;) |
There was a problem hiding this comment.
I don't like to be fussy or pedantic, but I'm not too sure about these endless cycles. Would be possible to implement the same functionality without the for(;;) and break? It's very tricky to manage all the hypothetical branches imho.
|
Any chance this PR can be revived and move forward? Without this, there is currently no way for RPC clients to restore a wallet from a date, since there is no way to translate from the date to the restore height. |
This is for wallets that need to convert a user-provided date to a blockchain height.
Similar function is currently implemented in wallet2::get_blockchain_height_by_date, but it's done quite inefficiently by pulling whole block blobs from the daemon. The code is ported from there with some optimizations. The precision is increased from 2 days to 1 hour.
This RPC method will be especially useful if #6639 is implemented and the mnemonic seed encodes the wallet birthday timestamp.