From b987676d63976a2e3ce4ad0a5cf3f7ccab8f2923 Mon Sep 17 00:00:00 2001 From: benevanoff Date: Fri, 3 Nov 2023 13:07:17 -0500 Subject: [PATCH 1/4] wallet: ensure subaddress keys table is at least size of requested lookahead --- src/wallet/wallet2.cpp | 19 +++++++++++++++++++ tests/unit_tests/subaddress.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index dd0ed39bfc..8bfec919f1 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1947,6 +1947,25 @@ void wallet2::set_subaddress_lookahead(size_t major, size_t minor) THROW_WALLET_EXCEPTION_IF(major > 0xffffffff, error::wallet_internal_error, "Subaddress major lookahead is too large"); THROW_WALLET_EXCEPTION_IF(minor == 0, error::wallet_internal_error, "Subaddress minor lookahead may not be zero"); THROW_WALLET_EXCEPTION_IF(minor > 0xffffffff, error::wallet_internal_error, "Subaddress minor lookahead is too large"); + + if (major > m_subaddress_lookahead_major) { // if increasing the lookahead + // then generate new subaddress pubkeys and add them to m_subaddresses table + for (uint32_t i = m_subaddress_labels.size()+m_subaddress_lookahead_major; i < m_subaddress_labels.size()+major; i++) { // m_subaddress_labels are the accounts the user is conciously keeping track of. We want that number plus the lookahead major accounts in our key table + for (uint32_t j = 0; j < minor; j++) { // these are newly made accounts, minor index will start from zero + cryptonote::subaddress_index idx = {i,j}; + create_one_off_subaddress(idx); // then generate the key and add it to the table + } + } + } + if (minor > m_subaddress_lookahead_minor) { // if increasing the minor lookahead we need to also go back and expand the existing accounts + for (uint32_t i = 0; i < m_subaddress_labels.size()+m_subaddress_lookahead_major; i++) { + uint32_t minor_idx_start = i < m_subaddress_labels.size() ? m_subaddress_labels[i].size()+m_subaddress_lookahead_minor : m_subaddress_lookahead_minor; // if there are existing minor indices being tracked under this account we need to account for that + for (uint32_t j = minor_idx_start; j < minor; j++) { + cryptonote::subaddress_index idx = {i,j}; + create_one_off_subaddress(idx); + } + } + } m_subaddress_lookahead_major = major; m_subaddress_lookahead_minor = minor; } diff --git a/tests/unit_tests/subaddress.cpp b/tests/unit_tests/subaddress.cpp index b7b09a851c..03302747c5 100644 --- a/tests/unit_tests/subaddress.cpp +++ b/tests/unit_tests/subaddress.cpp @@ -28,6 +28,7 @@ // // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers #include +#include #include "gtest/gtest.h" #include "include_base_utils.h" @@ -101,3 +102,29 @@ TEST_F(WalletSubaddress, OutOfBoundsIndexes) EXPECT_STREQ("index.minor is out of bound", e.what()); } } + +TEST_F(WalletSubaddress, ExpandPubkeyTable) +{ + // these test assume we are starting with the default setup state + EXPECT_EQ(2, w1.get_num_subaddress_accounts()); + EXPECT_EQ(50, w1.get_subaddress_lookahead().first); + EXPECT_EQ(200, w1.get_subaddress_lookahead().second); + // get_subaddress_index looks up keys in the private m_subaddresses dictionary so we will use it to test if a key is properly being scanned for + cryptonote::subaddress_index test_idx = {50, 199}; + auto subaddr = w1.get_subaddress(test_idx); + EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); + // fist test expanding the major lookahead + w1.set_subaddress_lookahead(100, 200); + EXPECT_EQ(100, w1.get_subaddress_lookahead().first); + EXPECT_EQ(200, w1.get_subaddress_lookahead().second); + test_idx = {100, 199}; + subaddr = w1.get_subaddress(test_idx); + EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); + // next test expanding the minor lookahead + w1.set_subaddress_lookahead(100, 300); + EXPECT_EQ(100, w1.get_subaddress_lookahead().first); + EXPECT_EQ(300, w1.get_subaddress_lookahead().second); + test_idx = {100, 299}; + subaddr = w1.get_subaddress(test_idx); + EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); +} From 6f3603711692fcbfd321aff071d49502a212517a Mon Sep 17 00:00:00 2001 From: benevanoff Date: Fri, 3 Nov 2023 18:40:57 -0500 Subject: [PATCH 2/4] wallet: create set_subaddress_lookahead wallet rpc endpoint --- src/wallet/wallet_rpc_server.cpp | 12 ++++++++++ src/wallet/wallet_rpc_server.h | 2 ++ src/wallet/wallet_rpc_server_commands_defs.h | 23 +++++++++++++++++++- tests/functional_tests/wallet.py | 15 +++++++++++++ utils/python-rpc/framework/wallet.py | 12 ++++++++++ 5 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index c1d4b1476b..1ed8eb9fcc 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -704,6 +704,18 @@ namespace tools res.index = *index; return true; } + bool wallet_rpc_server::on_set_subaddr_lookahead(const wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::request& req, wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::response& res, epee::json_rpc::error& er, const connection_context *ctx) + { + if (!m_wallet) return not_open(er); + try { + m_wallet->set_subaddress_lookahead(req.major_idx, req.minor_idx); + } + catch (const std::exception& e) { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } + return true; + } //------------------------------------------------------------------------------------------------------------------------------ bool wallet_rpc_server::on_create_address(const wallet_rpc::COMMAND_RPC_CREATE_ADDRESS::request& req, wallet_rpc::COMMAND_RPC_CREATE_ADDRESS::response& res, epee::json_rpc::error& er, const connection_context *ctx) { diff --git a/src/wallet/wallet_rpc_server.h b/src/wallet/wallet_rpc_server.h index ac386ce111..3f92ffcff4 100644 --- a/src/wallet/wallet_rpc_server.h +++ b/src/wallet/wallet_rpc_server.h @@ -71,6 +71,7 @@ namespace tools MAP_JON_RPC_WE("get_balance", on_getbalance, wallet_rpc::COMMAND_RPC_GET_BALANCE) MAP_JON_RPC_WE("get_address", on_getaddress, wallet_rpc::COMMAND_RPC_GET_ADDRESS) MAP_JON_RPC_WE("get_address_index", on_getaddress_index, wallet_rpc::COMMAND_RPC_GET_ADDRESS_INDEX) + MAP_JON_RPC_WE("set_subaddress_lookahead", on_set_subaddr_lookahead, wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD) MAP_JON_RPC_WE("getbalance", on_getbalance, wallet_rpc::COMMAND_RPC_GET_BALANCE) MAP_JON_RPC_WE("getaddress", on_getaddress, wallet_rpc::COMMAND_RPC_GET_ADDRESS) MAP_JON_RPC_WE("create_address", on_create_address, wallet_rpc::COMMAND_RPC_CREATE_ADDRESS) @@ -172,6 +173,7 @@ namespace tools bool on_getbalance(const wallet_rpc::COMMAND_RPC_GET_BALANCE::request& req, wallet_rpc::COMMAND_RPC_GET_BALANCE::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); bool on_getaddress(const wallet_rpc::COMMAND_RPC_GET_ADDRESS::request& req, wallet_rpc::COMMAND_RPC_GET_ADDRESS::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); bool on_getaddress_index(const wallet_rpc::COMMAND_RPC_GET_ADDRESS_INDEX::request& req, wallet_rpc::COMMAND_RPC_GET_ADDRESS_INDEX::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); + bool on_set_subaddr_lookahead(const wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::request& req, wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); bool on_create_address(const wallet_rpc::COMMAND_RPC_CREATE_ADDRESS::request& req, wallet_rpc::COMMAND_RPC_CREATE_ADDRESS::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); bool on_label_address(const wallet_rpc::COMMAND_RPC_LABEL_ADDRESS::request& req, wallet_rpc::COMMAND_RPC_LABEL_ADDRESS::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); bool on_get_accounts(const wallet_rpc::COMMAND_RPC_GET_ACCOUNTS::request& req, wallet_rpc::COMMAND_RPC_GET_ACCOUNTS::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL); diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index 907c5c29d4..9a99b15227 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -47,7 +47,7 @@ // advance which version they will stop working with // Don't go over 32767 for any of these #define WALLET_RPC_VERSION_MAJOR 1 -#define WALLET_RPC_VERSION_MINOR 28 +#define WALLET_RPC_VERSION_MINOR 29 #define MAKE_WALLET_RPC_VERSION(major,minor) (((major)<<16)|(minor)) #define WALLET_RPC_VERSION MAKE_WALLET_RPC_VERSION(WALLET_RPC_VERSION_MAJOR, WALLET_RPC_VERSION_MINOR) namespace tools @@ -182,6 +182,27 @@ namespace wallet_rpc typedef epee::misc_utils::struct_init response; }; + struct COMMAND_RPC_SET_SUBADDR_LOOKAHEAD + { + struct request_t + { + uint32_t major_idx; + uint32_t minor_idx; + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(major_idx) + KV_SERIALIZE(minor_idx) + END_KV_SERIALIZE_MAP() + }; + typedef epee::misc_utils::struct_init request; + + struct response_t + { + BEGIN_KV_SERIALIZE_MAP() + END_KV_SERIALIZE_MAP() + }; + typedef epee::misc_utils::struct_init response; + }; + struct COMMAND_RPC_CREATE_ADDRESS { struct request_t diff --git a/tests/functional_tests/wallet.py b/tests/functional_tests/wallet.py index 68ff4ebf89..e38cf707ca 100755 --- a/tests/functional_tests/wallet.py +++ b/tests/functional_tests/wallet.py @@ -46,6 +46,7 @@ class WalletTest(): self.check_keys() self.create_subaddresses() self.tags() + self.update_lookahead() self.attributes() self.open_close() self.languages() @@ -163,6 +164,20 @@ class WalletTest(): res = wallet.label_account(0, "main") + def update_lookahead(self): + print('Updating subaddress lookahead') + wallet = Wallet() + address_0_999 = '8BQKgTSSqJjP14AKnZUBwnXWj46MuNmLvHfPTpmry52DbfNjjHVvHUk4mczU8nj8yZ57zBhksTJ8kM5xKeJXw55kCMVqyG7' # this is the address for address 999 of the main account in the test wallet + try: # assert address_1_999 is not in the current pubkey table + wallet.get_address_index(address_0_999) + except Exception as e: + assert str(e) == "{'error': {'code': -2, 'message': \"Address doesn't belong to the wallet\"}, 'id': '0', 'jsonrpc': '2.0'}" + # update the lookahead and assert the high index address is now in the table + wallet.set_subaddress_lookahead(50, 1000) + r = wallet.get_address_index(address_0_999) + assert r['index']['major'] == 0 + assert r['index']['minor'] == 999 + def tags(self): print('Testing tags') wallet = Wallet() diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py index 9159c4a9a9..88cece2bcc 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -334,6 +334,18 @@ class Wallet(object): } return self.rpc.send_json_rpc_request(generate_from_keys) + def set_subaddress_lookahead(self, major_idx: int, minor_idx: int): + lookahead = { + 'method': 'set_subaddress_lookahead', + 'jsonrpc': '2.0', + 'params' : { + 'major_idx': major_idx, + 'minor_idx': minor_idx + }, + 'id': '0' + } + return self.rpc.send_json_rpc_request(lookahead) + def open_wallet(self, filename, password='', autosave_current = True): open_wallet = { 'method': 'open_wallet', From 6e26e4477ef2f273d220e40620e6ff08487d60c1 Mon Sep 17 00:00:00 2001 From: Justin Berman Date: Thu, 26 Jun 2025 17:32:12 +0000 Subject: [PATCH 3/4] wallet: improve lookahead logic & make rpc persistent --- src/wallet/wallet2.cpp | 51 ++++++---- src/wallet/wallet_rpc_server.cpp | 21 +++- src/wallet/wallet_rpc_server_commands_defs.h | 2 + tests/unit_tests/subaddress.cpp | 101 +++++++++++++++++-- utils/python-rpc/framework/wallet.py | 3 +- 5 files changed, 147 insertions(+), 31 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 8bfec919f1..23a0880e69 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1948,26 +1948,41 @@ void wallet2::set_subaddress_lookahead(size_t major, size_t minor) THROW_WALLET_EXCEPTION_IF(minor == 0, error::wallet_internal_error, "Subaddress minor lookahead may not be zero"); THROW_WALLET_EXCEPTION_IF(minor > 0xffffffff, error::wallet_internal_error, "Subaddress minor lookahead is too large"); - if (major > m_subaddress_lookahead_major) { // if increasing the lookahead - // then generate new subaddress pubkeys and add them to m_subaddresses table - for (uint32_t i = m_subaddress_labels.size()+m_subaddress_lookahead_major; i < m_subaddress_labels.size()+major; i++) { // m_subaddress_labels are the accounts the user is conciously keeping track of. We want that number plus the lookahead major accounts in our key table - for (uint32_t j = 0; j < minor; j++) { // these are newly made accounts, minor index will start from zero - cryptonote::subaddress_index idx = {i,j}; - create_one_off_subaddress(idx); // then generate the key and add it to the table - } - } - } - if (minor > m_subaddress_lookahead_minor) { // if increasing the minor lookahead we need to also go back and expand the existing accounts - for (uint32_t i = 0; i < m_subaddress_labels.size()+m_subaddress_lookahead_major; i++) { - uint32_t minor_idx_start = i < m_subaddress_labels.size() ? m_subaddress_labels[i].size()+m_subaddress_lookahead_minor : m_subaddress_lookahead_minor; // if there are existing minor indices being tracked under this account we need to account for that - for (uint32_t j = minor_idx_start; j < minor; j++) { - cryptonote::subaddress_index idx = {i,j}; - create_one_off_subaddress(idx); - } - } - } + const uint32_t old_major_lookahead = m_subaddress_lookahead_major; + const uint32_t old_minor_lookahead = m_subaddress_lookahead_minor; + m_subaddress_lookahead_major = major; m_subaddress_lookahead_minor = minor; + + if (old_major_lookahead >= major && old_minor_lookahead >= minor) + return; + + // Expand the subaddresses map so that outputs received to the higher lookaheads will be identified in the scan loop + hw::device &hwdev = m_account.get_device(); + cryptonote::subaddress_index index2; + const uint32_t max_major_idx = this->get_num_subaddress_accounts() > 0 ? (this->get_num_subaddress_accounts() - 1) : 0; + const uint32_t major_end = get_subaddress_clamped_sum(max_major_idx, major); + for (index2.major = 0; index2.major < major_end; ++index2.major) + { + // The existing minor addresses already set for this account + const uint32_t n_minor_subaddrs = this->get_num_subaddresses(index2.major); + + // The subaddress lookahead is expected to expand from the max index in expand_subaddresses + const uint32_t max_minor_idx = n_minor_subaddrs > 0 ? (n_minor_subaddrs - 1) : 0; + const uint32_t begin = (n_minor_subaddrs || index2.major < old_major_lookahead) ? get_subaddress_clamped_sum(max_minor_idx, old_minor_lookahead) : 0; + // The expected new n minor subaddresses allocated for this account + const uint32_t end = get_subaddress_clamped_sum(max_minor_idx, minor); + + if (begin >= end) + continue; + + const std::vector pkeys = hwdev.get_subaddress_spend_public_keys(m_account.get_keys(), index2.major, begin, end); + for (index2.minor = begin; index2.minor < end; ++index2.minor) + { + const crypto::public_key &D = pkeys.at(index2.minor - begin); + m_subaddresses[D] = index2; + } + } } //---------------------------------------------------------------------------------------------------- /*! diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 1ed8eb9fcc..4f349e4bdc 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -707,11 +707,24 @@ namespace tools bool wallet_rpc_server::on_set_subaddr_lookahead(const wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::request& req, wallet_rpc::COMMAND_RPC_SET_SUBADDR_LOOKAHEAD::response& res, epee::json_rpc::error& er, const connection_context *ctx) { if (!m_wallet) return not_open(er); - try { - m_wallet->set_subaddress_lookahead(req.major_idx, req.minor_idx); + CHECK_IF_BACKGROUND_SYNCING(); + const std::string wallet_file = m_wallet->get_wallet_file(); + if (wallet_file == "" || m_wallet->verify_password(req.password)) + { + try + { + m_wallet->set_subaddress_lookahead(req.major_idx, req.minor_idx); + m_wallet->rewrite(wallet_file, req.password); + } + catch (const std::exception& e) { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } } - catch (const std::exception& e) { - handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + else + { + er.code = WALLET_RPC_ERROR_CODE_INVALID_PASSWORD; + er.message = "Invalid password."; return false; } return true; diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index 9a99b15227..317add94bb 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -186,9 +186,11 @@ namespace wallet_rpc { struct request_t { + std::string password; uint32_t major_idx; uint32_t minor_idx; BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(password) KV_SERIALIZE(major_idx) KV_SERIALIZE(minor_idx) END_KV_SERIALIZE_MAP() diff --git a/tests/unit_tests/subaddress.cpp b/tests/unit_tests/subaddress.cpp index 03302747c5..4152b665e1 100644 --- a/tests/unit_tests/subaddress.cpp +++ b/tests/unit_tests/subaddress.cpp @@ -103,12 +103,36 @@ TEST_F(WalletSubaddress, OutOfBoundsIndexes) } } -TEST_F(WalletSubaddress, ExpandPubkeyTable) +// Helper function to check max subaddrs allocated +static void check_expected_max(const tools::wallet2 &w1, const cryptonote::subaddress_index exp_max) { - // these test assume we are starting with the default setup state + for (uint32_t i = 0; i <= exp_max.minor; ++i) + { + auto subaddr = w1.get_subaddress({exp_max.major, i}); + EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); + } + auto subaddr = w1.get_subaddress({exp_max.major, exp_max.minor + 1}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(subaddr)); +}; + +static void expect_default_wallet_state(const tools::wallet2 &w1) +{ + // these tests assume we are starting with the default setup state EXPECT_EQ(2, w1.get_num_subaddress_accounts()); EXPECT_EQ(50, w1.get_subaddress_lookahead().first); EXPECT_EQ(200, w1.get_subaddress_lookahead().second); + + // We assume we start with subaddrs for minor indexes 0 to 199 + check_expected_max(w1, {0,199}); + check_expected_max(w1, {1,199}); + check_expected_max(w1, {49,199}); + check_expected_max(w1, {50,199}); // 50 because the test starts with accounts 0 and 1 already allocated + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({51,0}))); +} + +TEST_F(WalletSubaddress, SetLookahead) +{ + expect_default_wallet_state(w1); // get_subaddress_index looks up keys in the private m_subaddresses dictionary so we will use it to test if a key is properly being scanned for cryptonote::subaddress_index test_idx = {50, 199}; auto subaddr = w1.get_subaddress(test_idx); @@ -117,14 +141,75 @@ TEST_F(WalletSubaddress, ExpandPubkeyTable) w1.set_subaddress_lookahead(100, 200); EXPECT_EQ(100, w1.get_subaddress_lookahead().first); EXPECT_EQ(200, w1.get_subaddress_lookahead().second); - test_idx = {100, 199}; - subaddr = w1.get_subaddress(test_idx); - EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); + check_expected_max(w1, {100, 199}); // next test expanding the minor lookahead w1.set_subaddress_lookahead(100, 300); EXPECT_EQ(100, w1.get_subaddress_lookahead().first); EXPECT_EQ(300, w1.get_subaddress_lookahead().second); - test_idx = {100, 299}; - subaddr = w1.get_subaddress(test_idx); - EXPECT_NE(boost::none, w1.get_subaddress_index(subaddr)); + check_expected_max(w1, {100, 299}); +} + +TEST_F(WalletSubaddress, ExpandThenSetMinorIncreaseOnly) +{ + expect_default_wallet_state(w1); + + // Mock receive to {0,150}, so expand from there + w1.expand_subaddresses({0,150}); + // We should now have subaddresses for minor indexes 0 to 349 + check_expected_max(w1, {0,349}); + check_expected_max(w1, {1,199}); + check_expected_max(w1, {49,199}); + check_expected_max(w1, {50,199}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({51,0}))); + + // Now set the minor lookahead 100 higher + w1.set_subaddress_lookahead(50, 200+100); + // We should have subaddresses for minor indexes 0 to 449 + check_expected_max(w1, {0,449}); + check_expected_max(w1, {1,299}); + check_expected_max(w1, {49,299}); + check_expected_max(w1, {50,299}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({51,0}))); +} + +TEST_F(WalletSubaddress, ExpandThenSetMajorIncreaseOnly) +{ + expect_default_wallet_state(w1); + + // Mock receive to {40,0}, so expand from there + w1.expand_subaddresses({40,0}); + check_expected_max(w1, {0,199}); + check_expected_max(w1, {1,199}); + check_expected_max(w1, {40,199}); + check_expected_max(w1, {89,199}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({90,0}))); + + // Now set the major lookahead 10 higher + w1.set_subaddress_lookahead(50+10, 200); + check_expected_max(w1, {0,199}); + check_expected_max(w1, {1,199}); + check_expected_max(w1, {40,199}); + check_expected_max(w1, {99,199}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({100,0}))); +} + +TEST_F(WalletSubaddress, ExpandThenSetIncreaseBoth) +{ + expect_default_wallet_state(w1); + + // Mock receive to {40,150}, so expand from there + w1.expand_subaddresses({40,150}); + check_expected_max(w1, {0,199}); + check_expected_max(w1, {1,199}); + check_expected_max(w1, {40,349}); + check_expected_max(w1, {89,199}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({90,0}))); + + // Now set the major lookahead 10 higher and minor 100 higher + w1.set_subaddress_lookahead(50+10, 200+100); + check_expected_max(w1, {0,299}); + check_expected_max(w1, {1,299}); + check_expected_max(w1, {40,449}); + check_expected_max(w1, {99,299}); + EXPECT_EQ(boost::none, w1.get_subaddress_index(w1.get_subaddress({100,0}))); } diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py index 88cece2bcc..d0a516c1fd 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -334,11 +334,12 @@ class Wallet(object): } return self.rpc.send_json_rpc_request(generate_from_keys) - def set_subaddress_lookahead(self, major_idx: int, minor_idx: int): + def set_subaddress_lookahead(self, major_idx: int, minor_idx: int, password = ""): lookahead = { 'method': 'set_subaddress_lookahead', 'jsonrpc': '2.0', 'params' : { + 'password': password, 'major_idx': major_idx, 'minor_idx': minor_idx }, From 6d06de74b24f42cdbb06c95531a134c8eb5e3172 Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Mon, 7 Jul 2025 20:07:24 +0000 Subject: [PATCH 4/4] wallet: refactor subaddress expansion & add to transfer test --- src/wallet/wallet2.cpp | 104 ++++++++++++++--------------- src/wallet/wallet2.h | 7 ++ tests/functional_tests/transfer.py | 43 ++++++++++++ 3 files changed, 100 insertions(+), 54 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 23a0880e69..94f198e36d 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1597,39 +1597,60 @@ bool wallet2::should_expand(const cryptonote::subaddress_index &index) const //---------------------------------------------------------------------------------------------------- void wallet2::expand_subaddresses(const cryptonote::subaddress_index& index) { - hw::device &hwdev = m_account.get_device(); - if (m_subaddress_labels.size() <= index.major) + // check if index will overflow container (usually only applicable on 32-bit systems) + if constexpr (sizeof(std::size_t) <= sizeof(std::uint32_t)) { - // add new accounts - cryptonote::subaddress_index index2; - const uint32_t major_end = get_subaddress_clamped_sum(index.major, m_subaddress_lookahead_major); - for (index2.major = m_subaddress_labels.size(); index2.major < major_end; ++index2.major) - { - const uint32_t end = get_subaddress_clamped_sum((index2.major == index.major ? index.minor : 0), m_subaddress_lookahead_minor); - const std::vector pkeys = hwdev.get_subaddress_spend_public_keys(m_account.get_keys(), index2.major, 0, end); - for (index2.minor = 0; index2.minor < end; ++index2.minor) - { - const crypto::public_key &D = pkeys[index2.minor]; - m_subaddresses[D] = index2; - } - } - m_subaddress_labels.resize(index.major + 1, {"Untitled account"}); - m_subaddress_labels[index.major].resize(index.minor + 1); - get_account_tags(); + static constexpr std::uint32_t max_idx = static_cast(std::numeric_limits::max()); + const bool cannot_label_index = index.major == max_idx || index.minor == max_idx; + THROW_WALLET_EXCEPTION_IF(cannot_label_index, error::wallet_internal_error, "subaddress index out of range"); } - else if (m_subaddress_labels[index.major].size() <= index.minor) + + // resize subaddress labels just big enough that `m_subaddress_labels[index.major][index.minor]` is present + if (m_subaddress_labels.size() <= index.major) + m_subaddress_labels.resize(index.major + 1, {"Untitled account"}); + auto &subaddr_labels_in_account = m_subaddress_labels[index.major]; + if (subaddr_labels_in_account.size() <= index.minor) + subaddr_labels_in_account.resize(index.minor + 1); + get_account_tags(); //trigger m_account_tags integrity checks + + // compile all indices present in subaddress scanning map, as well as every major index + std::unordered_set all_indices; + std::unordered_map lowest_missing_minor; + for (const auto &p : m_subaddresses) { - // add new subaddresses - const uint32_t end = get_subaddress_clamped_sum(index.minor, m_subaddress_lookahead_minor); - const uint32_t begin = m_subaddress_labels[index.major].size(); - cryptonote::subaddress_index index2 = {index.major, begin}; - const std::vector pkeys = hwdev.get_subaddress_spend_public_keys(m_account.get_keys(), index2.major, index2.minor, end); - for (; index2.minor < end; ++index2.minor) + all_indices.insert(p.second); + lowest_missing_minor[p.second.major]; + } + + // find lowest "missing" minor index in map, for all major indices + // this is an optimization which allows us to skip re-generating pubkeys that we already have + for (auto &p : lowest_missing_minor) + { + const std::uint32_t major = p.first; + std::uint32_t &minor = p.second; + while (all_indices.count({major, minor}) && minor < std::numeric_limits::max()) + ++minor; + } + + // resize subaddress scanning map to highest historical received subaddress index plus lookahead + hw::device &hwdev = m_account.get_device(); + const std::uint32_t major_base = std::max(m_subaddress_labels.size(), 1) - 1; + const std::uint32_t major_end = get_subaddress_clamped_sum(major_base, m_subaddress_lookahead_major); + for (std::uint32_t major = 0; major < major_end; ++major) + { + const std::size_t n_minor_labels = (major < m_subaddress_labels.size()) ? m_subaddress_labels.at(major).size() : 0; + const std::uint32_t minor_base = std::max(n_minor_labels, 1) - 1; + const std::uint32_t minor_end = get_subaddress_clamped_sum(minor_base, m_subaddress_lookahead_minor); + const std::uint32_t minor_begin = lowest_missing_minor.count(major) ? lowest_missing_minor.at(major) : 0; + if (minor_begin >= minor_end) + continue; + const std::vector pkeys + = hwdev.get_subaddress_spend_public_keys(m_account.get_keys(), major, minor_begin, minor_end); + for (std::uint32_t minor = minor_begin; minor < minor_end; ++minor) { - const crypto::public_key &D = pkeys[index2.minor - begin]; - m_subaddresses[D] = index2; + const crypto::public_key &D = pkeys.at(minor - minor_begin); + m_subaddresses[D] = {major, minor}; } - m_subaddress_labels[index.major].resize(index.minor + 1); } } //---------------------------------------------------------------------------------------------------- @@ -1957,32 +1978,7 @@ void wallet2::set_subaddress_lookahead(size_t major, size_t minor) if (old_major_lookahead >= major && old_minor_lookahead >= minor) return; - // Expand the subaddresses map so that outputs received to the higher lookaheads will be identified in the scan loop - hw::device &hwdev = m_account.get_device(); - cryptonote::subaddress_index index2; - const uint32_t max_major_idx = this->get_num_subaddress_accounts() > 0 ? (this->get_num_subaddress_accounts() - 1) : 0; - const uint32_t major_end = get_subaddress_clamped_sum(max_major_idx, major); - for (index2.major = 0; index2.major < major_end; ++index2.major) - { - // The existing minor addresses already set for this account - const uint32_t n_minor_subaddrs = this->get_num_subaddresses(index2.major); - - // The subaddress lookahead is expected to expand from the max index in expand_subaddresses - const uint32_t max_minor_idx = n_minor_subaddrs > 0 ? (n_minor_subaddrs - 1) : 0; - const uint32_t begin = (n_minor_subaddrs || index2.major < old_major_lookahead) ? get_subaddress_clamped_sum(max_minor_idx, old_minor_lookahead) : 0; - // The expected new n minor subaddresses allocated for this account - const uint32_t end = get_subaddress_clamped_sum(max_minor_idx, minor); - - if (begin >= end) - continue; - - const std::vector pkeys = hwdev.get_subaddress_spend_public_keys(m_account.get_keys(), index2.major, begin, end); - for (index2.minor = begin; index2.minor < end; ++index2.minor) - { - const crypto::public_key &D = pkeys.at(index2.minor - begin); - m_subaddresses[D] = index2; - } - } + expand_subaddresses({0, 0}); } //---------------------------------------------------------------------------------------------------- /*! diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 47b1753d76..41f1dffaa4 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1124,6 +1124,13 @@ private: size_t get_num_subaddress_accounts() const { return m_subaddress_labels.size(); } size_t get_num_subaddresses(uint32_t index_major) const { return index_major < m_subaddress_labels.size() ? m_subaddress_labels[index_major].size() : 0; } void add_subaddress(uint32_t index_major, const std::string& label); // throws when index is out of bound + /** + * brief: expand subaddress labels up to `index`, and scanning map to highest labeled index plus current lookahead + * param: index - + * + * All calls to `expand_subaddresses()` will *always* expand the subaddress map if the lookahead + * values have been increased since the last call. + */ void expand_subaddresses(const cryptonote::subaddress_index& index); void create_one_off_subaddress(const cryptonote::subaddress_index& index); std::string get_subaddress_label(const cryptonote::subaddress_index& index) const; diff --git a/tests/functional_tests/transfer.py b/tests/functional_tests/transfer.py index 0df870e9c4..f1fb6e51d4 100755 --- a/tests/functional_tests/transfer.py +++ b/tests/functional_tests/transfer.py @@ -86,6 +86,7 @@ class TransferTest(): self.check_subtract_fee_from_outputs() self.check_background_sync() self.check_background_sync_reorg_recovery() + self.check_subaddress_lookahead() def reset(self): print('Resetting blockchain') @@ -1531,5 +1532,47 @@ class TransferTest(): self.wallet[0].close_wallet() self.wallet[0].restore_deterministic_wallet(seed = seeds[0]) + def check_subaddress_lookahead(self): + daemon = Daemon() + + print('Testing transfers to subaddresses with large lookahead') + + # From wallet 1 to wallet 0 at subaddress (0, 999) + + address_0_999 = '8BQKgTSSqJjP14AKnZUBwnXWj46MuNmLvHfPTpmry52DbfNjjHVvHUk4mczU8nj8yZ57zBhksTJ8kM5xKeJXw55kCMVqyG7' # this is the address for address 999 of the main account in the test wallet + try: # assert address_1_999 is not in the current pubkey table + self.wallet[0].get_address_index(address_0_999) + assert False # address should not already be loaded + except Exception as e: + assert str(e) == "{'error': {'code': -2, 'message': \"Address doesn't belong to the wallet\"}, 'id': '0', 'jsonrpc': '2.0'}" + # update the lookahead and assert the high index address is now in the table + self.wallet[0].set_subaddress_lookahead(50, 1000) + res = self.wallet[0].get_address_index(address_0_999) + assert res['index']['major'] == 0 + assert res['index']['minor'] == 999 + + dst = {'address': address_0_999, 'amount': 454545454545} + + self.wallet[1].refresh() + assert self.wallet[1].get_balance().balance > dst['amount'] + self.wallet[1].transfer([dst]) + daemon.generateblocks('46r4nYSevkfBUMhuykdK3gQ98XDqDTYW1hNLaXNvjpsJaSbNtdXh1sKMsdVgqkaihChAzEy29zEDPMR3NHQvGoZCLGwTerK', 1) + self.wallet[0].refresh() + + res = self.wallet[0].get_balance() + balance_info_0_999 = None + for balance_info in res['per_subaddress']: + if balance_info['account_index'] == 0 and balance_info['address_index'] == 999: + balance_info_0_999 = balance_info + break + assert balance_info_0_999 is not None, "balance info for address (0, 999) not found" + assert balance_info_0_999['address'] == address_0_999 + assert balance_info_0_999['balance'] == dst['amount'] + assert balance_info_0_999['unlocked_balance'] == 0 + assert balance_info_0_999['label'] == '' + assert balance_info_0_999['num_unspent_outputs'] == 1 + assert balance_info_0_999['blocks_to_unlock'] == 9 + assert balance_info_0_999['time_to_unlock'] == 0 + if __name__ == '__main__': TransferTest().run_test()