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()