Add from_height to /import_wallet_request (#194)
What changed, and why it matters
This commit adds a new optional 'from_height' field to the /import_wallet_request endpoint in the Monero Light Wallet Server. Previously, import requests always started scanning from block 0. Now users can request a later starting block height. The change appears to be a feature addition rather than a security fix, though it touches authentication-related code paths.
Review the database layer's import_request function and any downstream scan logic to ensure db::block_id(req.from_height) cannot cause out-of-bounds behavior, integer truncation, or bypass of account state checks. Consider adding server-side validation for from_height relative to current blockchain height.
Security signals we found
Authentication-related code modified (open_account call now uses req.creds sub-field)
New user-controlled integer parameter (from_height) accepted and passed to database layer
No visible bounds/overflow validation on from_height in the diff
No explicit security context in commit message or title
Evidence from the diff
The patch refactors the import request RPC type from account_credentials to a new import_request struct containing account_credentials plus a from_height field (defaulting to 0). The REST handler now uses req.creds for login and req.creds.address for the import request, while passing db::block_id(req.from_height) instead of hardcoded block_id(0). A unit test verifies the new from_height behavior. No security-relevant validation changes are evident in the diff.
Changed components
src/rest_server.cppsrc/rpc/light_wallet.cppsrc/rpc/light_wallet.htests/unit/rest.test.cppInspect captured patch +74 / −11
diff --git a/src/rest_server.cpp b/src/rest_server.cpp
index a712750..cef36c7 100644
--- a/src/rest_server.cpp
+++ b/src/rest_server.cpp
@@ -1303,7 +1303,7 @@ namespace lws
struct import_request
{
- using request = rpc::account_credentials;
+ using request = rpc::import_request;
using response = rpc::import_response;
static expect<response> handle(request req, connection_data& data, std::function<async_complete>&&)
@@ -1311,17 +1311,17 @@ namespace lws
bool new_request = false;
bool fulfilled = false;
{
- auto user = open_account(req, data.global->disk.clone());
+ auto user = open_account(req.creds, data.global->disk.clone());
if (!user)
return user.error();
data.passed_login = true;
- if (user->first.start_height == db::block_id(0))
+ if (user->first.start_height <= db::block_id(req.from_height))
fulfilled = true;
else
{
const expect<db::request_info> info =
- user->second.get_request(db::request::import_scan, req.address);
+ user->second.get_request(db::request::import_scan, req.creds.address);
if (!info)
{
@@ -1334,7 +1334,7 @@ namespace lws
} // close reader
if (new_request)
- MONERO_CHECK(data.global->disk.clone().import_request(req.address, db::block_id(0)));
+ MONERO_CHECK(data.global->disk.clone().import_request(req.creds.address, db::block_id(req.from_height)));
const char* status = new_request ?
"Accepted, waiting for approval" : (fulfilled ? "Approved" : "Waiting for Approval");
diff --git a/src/rpc/light_wallet.cpp b/src/rpc/light_wallet.cpp
index 78d2686..adc83bc 100644
--- a/src/rpc/light_wallet.cpp
+++ b/src/rpc/light_wallet.cpp
@@ -48,6 +48,7 @@
#include "wire/traits.h"
#include "wire/vector.h"
#include "wire/wrapper/array.h"
+#include "wire/wrapper/defaulted.h"
#include "wire/wrappers_impl.h"
namespace
@@ -361,6 +362,17 @@ namespace lws
);
}
+ void rpc::read_bytes(wire::json_reader& source, import_request& self)
+ {
+ std::string address;
+ wire::object(source,
+ wire::field("address", std::ref(address)),
+ wire::field("view_key", std::ref(unwrap(unwrap(self.creds.key)))),
+ WIRE_FIELD_DEFAULTED(from_height, unsigned(0))
+ );
+ convert_address(address, self.creds.address);
+ }
+
void rpc::write_bytes(wire::json_writer& dest, const import_response& self)
{
wire::object(dest,
diff --git a/src/rpc/light_wallet.h b/src/rpc/light_wallet.h
index 10a01a4..dc78199 100644
--- a/src/rpc/light_wallet.h
+++ b/src/rpc/light_wallet.h
@@ -224,6 +224,14 @@ namespace rpc
void write_bytes(wire::json_writer&, const get_subaddrs_response&);
+ struct import_request
+ {
+ import_request() = delete;
+ account_credentials creds;
+ std::uint64_t from_height;
+ };
+ void read_bytes(wire::json_reader&, import_request&);
+
struct import_response
{
import_response() = delete;
diff --git a/tests/unit/rest.test.cpp b/tests/unit/rest.test.cpp
index ee036c9..281c418 100644
--- a/tests/unit/rest.test.cpp
+++ b/tests/unit/rest.test.cpp
@@ -108,11 +108,11 @@ namespace
LWS_CASE("rest_server")
{
- lws::db::account_address account{};
+ lws::db::account_address account_address{};
crypto::secret_key view{};
- crypto::generate_keys(account.spend_public, view);
- crypto::generate_keys(account.view_public, view);
- const std::string address = lws::db::address_string(account);
+ crypto::generate_keys(account_address.spend_public, view);
+ crypto::generate_keys(account_address.view_public, view);
+ const std::string address = lws::db::address_string(account_address);
const std::string viewkey = epee::to_hex::string(epee::as_byte_span(unwrap(unwrap(view))));
SETUP("Database and login")
@@ -139,9 +139,9 @@ LWS_CASE("rest_server")
const lws::db::block_info last_block =
MONERO_UNWRAP(MONERO_UNWRAP(db.start_read()).get_last_block());
- const auto get_account = [&db, &account] () -> lws::db::account
+ const auto get_account = [&db, &account_address] () -> lws::db::account
{
- return MONERO_UNWRAP(MONERO_UNWRAP(db.start_read()).get_account(account)).second;
+ return MONERO_UNWRAP(MONERO_UNWRAP(db.start_read()).get_account(account_address)).second;
};
enet::http::http_simple_client client{};
@@ -193,6 +193,49 @@ LWS_CASE("rest_server")
EXPECT(response == "{\"per_byte_fee\":39,\"fee_mask\":1000,\"amount\":\"0\",\"fees\":[40,41]}");
}
+ SECTION("Import from height")
+ {
+ EXPECT(account.start_height != lws::db::block_id(0));
+
+ const std::string scan_height = std::to_string(std::uint64_t(account.scan_height));
+ const std::string start_height = std::to_string(std::uint64_t(account.start_height));
+ const std::string import_height = std::to_string(std::uint64_t(account.start_height) - 1);
+ message = "{\"address\":\"" + address + "\",\"view_key\":\"" + viewkey + "\"}";
+ response = invoke(client, "/get_address_info", message);
+ EXPECT(response ==
+ "{\"locked_funds\":\"0\","
+ "\"total_received\":\"0\","
+ "\"total_sent\":\"0\","
+ "\"scanned_height\":" + scan_height + "," +
+ "\"scanned_block_height\":" + scan_height + ","
+ "\"start_height\":" + start_height + ","
+ "\"transaction_height\":" + scan_height + ","
+ "\"blockchain_height\":" + scan_height + "}"
+ );
+
+ message = "{\"address\":\"" + address + "\",\"view_key\":\"" + viewkey + "\", \"from_height\":" + import_height + "}";
+ response = invoke(client, "/import_wallet_request", message);
+ EXPECT(response ==
+ "{\"import_fee\":\"0\","
+ "\"status\":\"Accepted, waiting for approval\","
+ "\"new_request\":true,"
+ "\"request_fulfilled\":false}"
+ );
+
+ EXPECT(db.accept_requests(lws::db::request::import_scan, {std::addressof(account_address), 1}));
+ response = invoke(client, "/get_address_info", message);
+ EXPECT(response ==
+ "{\"locked_funds\":\"0\","
+ "\"total_received\":\"0\","
+ "\"total_sent\":\"0\","
+ "\"scanned_height\":" + import_height + "," +
+ "\"scanned_block_height\":" + import_height + ","
+ "\"start_height\":" + import_height + ","
+ "\"transaction_height\":" + scan_height + ","
+ "\"blockchain_height\":" + scan_height + "}"
+ );
+ }
+
SECTION("One Receive, Zero Spends")
{
const std::string scan_height = std::to_string(std::uint64_t(account.scan_height) + 5);
Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.