diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 65ec6fd7c..e9f59cd8c 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1610,39 +1610,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); } } //---------------------------------------------------------------------------------------------------- @@ -1964,8 +1985,17 @@ 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"); + + 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_subaddresses({0, 0}); } //---------------------------------------------------------------------------------------------------- /*! diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index f4b5fa636..187f6ccc3 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1123,6 +1123,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/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 32017c5c1..020b16fb8 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -702,6 +702,31 @@ 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); + 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; + } + } + else + { + er.code = WALLET_RPC_ERROR_CODE_INVALID_PASSWORD; + er.message = "Invalid password."; + 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 bc203b23a..7619b5588 100644 --- a/src/wallet/wallet_rpc_server.h +++ b/src/wallet/wallet_rpc_server.h @@ -72,6 +72,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 d3503264b..ebcd1135c 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,29 @@ namespace wallet_rpc typedef epee::misc_utils::struct_init response; }; + struct COMMAND_RPC_SET_SUBADDR_LOOKAHEAD + { + 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() + }; + 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/transfer.py b/tests/functional_tests/transfer.py index 60eb09a10..78d16f802 100755 --- a/tests/functional_tests/transfer.py +++ b/tests/functional_tests/transfer.py @@ -77,6 +77,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') @@ -1515,5 +1516,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() diff --git a/tests/functional_tests/wallet.py b/tests/functional_tests/wallet.py index 8182cecb2..1cbe4ee68 100755 --- a/tests/functional_tests/wallet.py +++ b/tests/functional_tests/wallet.py @@ -47,6 +47,7 @@ class WalletTest(): self.check_keys() self.create_subaddresses() self.tags() + self.update_lookahead() self.attributes() self.open_close() self.languages() @@ -164,6 +165,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/tests/unit_tests/subaddress.cpp b/tests/unit_tests/subaddress.cpp index 17fc0fe78..71cbdbee6 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,114 @@ TEST_F(WalletSubaddress, OutOfBoundsIndexes) EXPECT_STREQ("index.minor is out of bound", e.what()); } } + +// Helper function to check max subaddrs allocated +static void check_expected_max(const tools::wallet2 &w1, const cryptonote::subaddress_index exp_max) +{ + 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); + 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); + 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); + 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 010f4dd08..d95c3b2b5 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -333,6 +333,19 @@ class Wallet(object): } return self.rpc.send_json_rpc_request(generate_from_keys) + 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 + }, + 'id': '0' + } + return self.rpc.send_json_rpc_request(lookahead) + def open_wallet(self, filename, password='', autosave_current = True): open_wallet = { 'method': 'open_wallet',