From 0de1571abd2a8b169e0a3c67cc5769e804b08957 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 7 Aug 2022 14:56:59 +0000 Subject: [PATCH 1/7] wallet2: fix missing subaddress indices in "light" exported outputs --- src/wallet/wallet2.cpp | 4 +++- src/wallet/wallet2.h | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 195763949..d87aa0e33 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -13241,7 +13241,7 @@ size_t wallet2::import_outputs(const std::pair m_additional_tx_keys; + uint32_t m_subaddr_index_major; + uint32_t m_subaddr_index_minor; BEGIN_SERIALIZE_OBJECT() - VERSION_FIELD(0) + VERSION_FIELD(1) FIELD(m_pubkey) VARINT_FIELD(m_internal_output_index) VARINT_FIELD(m_global_output_index) @@ -411,6 +413,8 @@ private: FIELD(m_flags.flags) VARINT_FIELD(m_amount) FIELD(m_additional_tx_keys) + VARINT_FIELD(m_subaddr_index_major) + VARINT_FIELD(m_subaddr_index_minor) END_SERIALIZE() }; From 5b98bebad146854cabf11b51bbd6ff5fafccbdcc Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 7 Aug 2022 15:00:16 +0000 Subject: [PATCH 2/7] wallet2: prevent importing outputs in a hot wallet --- src/wallet/wallet2.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index d87aa0e33..aba929855 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -13162,6 +13162,8 @@ size_t wallet2::import_outputs(const std::pair m_transfers.size(), error::wallet_internal_error, "Imported outputs omit more outputs that we know of"); From 4b7eb573b2925306fe84a6fff8e25bd0062b1082 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 7 Aug 2022 15:19:32 +0000 Subject: [PATCH 3/7] wallet2: do not assume imported outputs must be non empty --- src/wallet/wallet2.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index aba929855..9caf43cbb 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -6606,7 +6606,7 @@ bool wallet2::sign_tx(unsigned_tx_set &exported_txs, std::vector Date: Mon, 15 Aug 2022 19:50:34 -0700 Subject: [PATCH 4/7] wallet2: fixes for export/import output flow - only allow offline wallets to import outputs - don't import empty outputs - export subaddress indexes when exporting outputs --- src/wallet/wallet2.cpp | 8 +- src/wallet/wallet2.h | 2 + tests/functional_tests/cold_signing.py | 79 +++++++++++++++---- .../functional_tests/functional_tests_rpc.py | 6 +- 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 9caf43cbb..536460396 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -6606,7 +6606,7 @@ bool wallet2::sign_tx(unsigned_tx_set &exported_txs, std::vector> wall etd.m_flags.m_key_image_partial = td.m_key_image_partial; etd.m_amount = td.m_amount; etd.m_additional_tx_keys = get_additional_tx_pub_keys_from_extra(td.m_tx); + etd.m_subaddr_index_major = td.m_subaddr_index.major; + etd.m_subaddr_index_minor = td.m_subaddr_index.minor; outs.push_back(etd); } @@ -13162,7 +13164,7 @@ size_t wallet2::import_outputs(const std::pair m_transfers.size(), error::wallet_internal_error, "Imported outputs omit more outputs that we know of"); @@ -13230,6 +13232,8 @@ size_t wallet2::import_outputs(const std::pair m_transfers.size(), error::wallet_internal_error, "Imported outputs omit more outputs that we know of. Try using export_outputs all."); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 6598dea10..06d4e636e 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -406,6 +406,8 @@ private: BEGIN_SERIALIZE_OBJECT() VERSION_FIELD(1) + if (version < 1) + return false; FIELD(m_pubkey) VARINT_FIELD(m_internal_output_index) VARINT_FIELD(m_global_output_index) diff --git a/tests/functional_tests/cold_signing.py b/tests/functional_tests/cold_signing.py index 31d5780bb..9f8143a8c 100755 --- a/tests/functional_tests/cold_signing.py +++ b/tests/functional_tests/cold_signing.py @@ -35,12 +35,18 @@ from __future__ import print_function from framework.daemon import Daemon from framework.wallet import Wallet +SEED = 'velvet lymph giddy number token physics poetry unquoted nibs useful sabotage limits benches lifestyle eden nitrogen anvil fewest avoid batch vials washing fences goat unquoted' +STANDARD_ADDRESS = '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm' +SUBADDRESS = '84QRUYawRNrU3NN1VpFRndSukeyEb3Xpv8qZjjsoJZnTYpDYceuUTpog13D7qPxpviS7J29bSgSkR11hFFoXWk2yNdsR9WF' + class ColdSigningTest(): def run_test(self): self.reset() self.create(0) self.mine() self.transfer() + self.self_transfer_to_subaddress() + self.transfer_after_empty_export_import() def reset(self): print('Resetting blockchain') @@ -57,17 +63,15 @@ class ColdSigningTest(): try: self.hot_wallet.close_wallet() except: pass - self.cold_wallet = Wallet(idx = 1) + self.cold_wallet = Wallet(idx = 5) # close the wallet if any, will throw if none is loaded try: self.cold_wallet.close_wallet() except: pass - seed = 'velvet lymph giddy number token physics poetry unquoted nibs useful sabotage limits benches lifestyle eden nitrogen anvil fewest avoid batch vials washing fences goat unquoted' - res = self.cold_wallet.restore_deterministic_wallet(seed = seed) - self.cold_wallet.set_daemon('127.0.0.1:11111', ssl_support = "disabled") + res = self.cold_wallet.restore_deterministic_wallet(seed = SEED) spend_key = self.cold_wallet.query_key("spend_key").key view_key = self.cold_wallet.query_key("view_key").key - res = self.hot_wallet.generate_from_keys(viewkey = view_key, address = '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm') + res = self.hot_wallet.generate_from_keys(viewkey = view_key, address = STANDARD_ADDRESS) ok = False try: res = self.hot_wallet.query_key("spend_key") @@ -79,28 +83,30 @@ class ColdSigningTest(): assert ok assert self.cold_wallet.query_key("view_key").key == view_key assert self.cold_wallet.get_address().address == self.hot_wallet.get_address().address + assert self.cold_wallet.get_address().address == STANDARD_ADDRESS def mine(self): print("Mining some blocks") daemon = Daemon() wallet = Wallet() - daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 80) + daemon.generateblocks(STANDARD_ADDRESS, 80) wallet.refresh() - def transfer(self): - daemon = Daemon() - - print("Creating transaction in hot wallet") - - dst = {'address': '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 'amount': 1000000000000} - + def export_import(self): self.hot_wallet.refresh() res = self.hot_wallet.export_outputs() self.cold_wallet.import_outputs(res.outputs_data_hex) res = self.cold_wallet.export_key_images(True) self.hot_wallet.import_key_images(res.signed_key_images, offset = res.offset) + def create_tx(self, destination_addr): + daemon = Daemon() + + dst = {'address': destination_addr, 'amount': 1000000000000} + + self.export_import() + res = self.hot_wallet.transfer([dst], ring_size = 16, get_tx_key = False) assert len(res.tx_hash) == 32*2 txid = res.tx_hash @@ -125,11 +131,11 @@ class ColdSigningTest(): assert desc.unlock_time == 0 assert desc.payment_id in ['', '0000000000000000'] assert desc.change_amount == desc.amount_in - 1000000000000 - fee - assert desc.change_address == '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm' + assert desc.change_address == STANDARD_ADDRESS assert desc.fee == fee assert len(desc.recipients) == 1 rec = desc.recipients[0] - assert rec.address == '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm' + assert rec.address == destination_addr assert rec.amount == 1000000000000 res = self.cold_wallet.sign_transfer(unsigned_txset) @@ -148,7 +154,7 @@ class ColdSigningTest(): assert len([x for x in (res['pending'] if 'pending' in res else []) if x.txid == txid]) == 1 assert len([x for x in (res['out'] if 'out' in res else []) if x.txid == txid]) == 0 - daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) + daemon.generateblocks(STANDARD_ADDRESS, 1) self.hot_wallet.refresh() res = self.hot_wallet.get_transfers() @@ -160,6 +166,47 @@ class ColdSigningTest(): res = self.cold_wallet.get_tx_key(txid) assert len(res.tx_key) == 64 + self.export_import() + + def transfer(self): + print("Creating transaction in hot wallet") + self.create_tx(STANDARD_ADDRESS) + + res = self.cold_wallet.get_address() + assert len(res['addresses']) == 1 + assert res['addresses'][0].address == STANDARD_ADDRESS + assert res['addresses'][0].used + + res = self.hot_wallet.get_address() + assert len(res['addresses']) == 1 + assert res['addresses'][0].address == STANDARD_ADDRESS + assert res['addresses'][0].used + + def self_transfer_to_subaddress(self): + print("Self-spending to subaddress in hot wallet") + self.create_tx(SUBADDRESS) + + res = self.cold_wallet.get_address() + assert len(res['addresses']) == 2 + assert res['addresses'][0].address == STANDARD_ADDRESS + assert res['addresses'][0].used + assert res['addresses'][1].address == SUBADDRESS + assert res['addresses'][1].used + + res = self.hot_wallet.get_address() + assert len(res['addresses']) == 2 + assert res['addresses'][0].address == STANDARD_ADDRESS + assert res['addresses'][0].used + assert res['addresses'][1].address == SUBADDRESS + assert res['addresses'][1].used + + def transfer_after_empty_export_import(self): + print("Creating transaction in hot wallet after empty export & import") + start_len = len(self.hot_wallet.get_transfers()['in']) + self.export_import() + assert start_len == len(self.hot_wallet.get_transfers()['in']) + self.create_tx(STANDARD_ADDRESS) + assert start_len == len(self.hot_wallet.get_transfers()['in']) - 1 class Guard: def __enter__(self): diff --git a/tests/functional_tests/functional_tests_rpc.py b/tests/functional_tests/functional_tests_rpc.py index eb8d51f08..d020bf2f6 100755 --- a/tests/functional_tests/functional_tests_rpc.py +++ b/tests/functional_tests/functional_tests_rpc.py @@ -40,8 +40,9 @@ except: N_MONERODS = 4 # 4 wallets connected to the main offline monerod -# a wallet connected to the first local online monerod -N_WALLETS = 5 +# 1 wallet connected to the first local online monerod +# 1 offline wallet +N_WALLETS = 6 WALLET_DIRECTORY = builddir + "/functional-tests-directory" FUNCTIONAL_TESTS_DIRECTORY = builddir + "/tests/functional_tests" @@ -61,6 +62,7 @@ wallet_extra = [ ["--daemon-port", "18180"], ["--daemon-port", "18180"], ["--daemon-port", "18182"], + ["--offline"], ] command_lines = [] From 0cbf5571d3ccd07c81d33b05dd23b2ac9c777c3b Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 16 Aug 2022 20:20:38 +0000 Subject: [PATCH 5/7] allow exporting outputs in chunks this will make it easier huge wallets to do so without hitting random limits (eg, max string size in node). --- src/device_trezor/device_trezor.cpp | 12 +- src/device_trezor/trezor/protocol.hpp | 4 +- src/serialization/tuple.h | 169 +++++++++++++++++++ src/simplewallet/simplewallet.cpp | 6 +- src/wallet/api/wallet.cpp | 4 +- src/wallet/wallet2.cpp | 94 +++++++---- src/wallet/wallet2.h | 64 +++++-- src/wallet/wallet_rpc_server.cpp | 2 +- src/wallet/wallet_rpc_server_commands_defs.h | 4 + tests/data/fuzz/cold-outputs/out-all-6 | Bin 2607 -> 394 bytes tests/functional_tests/cold_signing.py | 54 ++++-- tests/fuzz/cold-outputs.cpp | 2 +- utils/python-rpc/framework/wallet.py | 5 +- 13 files changed, 348 insertions(+), 72 deletions(-) create mode 100644 src/serialization/tuple.h diff --git a/src/device_trezor/device_trezor.cpp b/src/device_trezor/device_trezor.cpp index 58c36f2c9..3f7b10be4 100644 --- a/src/device_trezor/device_trezor.cpp +++ b/src/device_trezor/device_trezor.cpp @@ -511,7 +511,7 @@ namespace trezor { tools::wallet2::signed_tx_set & signed_tx, hw::tx_aux_data & aux_data) { - CHECK_AND_ASSERT_THROW_MES(unsigned_tx.transfers.first == 0, "Unsuported non zero offset"); + CHECK_AND_ASSERT_THROW_MES(std::get<0>(unsigned_tx.transfers) == 0, "Unsuported non zero offset"); TREZOR_AUTO_LOCK_CMD(); require_connected(); @@ -522,7 +522,7 @@ namespace trezor { const size_t num_tx = unsigned_tx.txes.size(); m_num_transations_to_sign = num_tx; signed_tx.key_images.clear(); - signed_tx.key_images.resize(unsigned_tx.transfers.second.size()); + signed_tx.key_images.resize(std::get<2>(unsigned_tx.transfers).size()); for(size_t tx_idx = 0; tx_idx < num_tx; ++tx_idx) { std::shared_ptr signer; @@ -566,8 +566,8 @@ namespace trezor { cpend.key_images = key_images; // KI sync - for(size_t cidx=0, trans_max=unsigned_tx.transfers.second.size(); cidx < trans_max; ++cidx){ - signed_tx.key_images[cidx] = unsigned_tx.transfers.second[cidx].m_key_image; + for(size_t cidx=0, trans_max=std::get<2>(unsigned_tx.transfers).size(); cidx < trans_max; ++cidx){ + signed_tx.key_images[cidx] = std::get<2>(unsigned_tx.transfers)[cidx].m_key_image; } size_t num_sources = cdata.tx_data.sources.size(); @@ -579,9 +579,9 @@ namespace trezor { CHECK_AND_ASSERT_THROW_MES(src_idx < cdata.tx.vin.size(), "Invalid idx_mapped"); size_t idx_map_src = cdata.tx_data.selected_transfers[idx_mapped]; - CHECK_AND_ASSERT_THROW_MES(idx_map_src >= unsigned_tx.transfers.first, "Invalid offset"); + CHECK_AND_ASSERT_THROW_MES(idx_map_src >= std::get<0>(unsigned_tx.transfers), "Invalid offset"); - idx_map_src -= unsigned_tx.transfers.first; + idx_map_src -= std::get<0>(unsigned_tx.transfers); CHECK_AND_ASSERT_THROW_MES(idx_map_src < signed_tx.key_images.size(), "Invalid key image index"); const auto vini = boost::get(cdata.tx.vin[src_idx]); diff --git a/src/device_trezor/trezor/protocol.hpp b/src/device_trezor/trezor/protocol.hpp index fa8355200..7ffadd9aa 100644 --- a/src/device_trezor/trezor/protocol.hpp +++ b/src/device_trezor/trezor/protocol.hpp @@ -230,8 +230,8 @@ namespace tx { } const tools::wallet2::transfer_details & get_transfer(size_t idx) const { - CHECK_AND_ASSERT_THROW_MES(idx < m_unsigned_tx->transfers.second.size() + m_unsigned_tx->transfers.first && idx >= m_unsigned_tx->transfers.first, "Invalid transfer index"); - return m_unsigned_tx->transfers.second[idx - m_unsigned_tx->transfers.first]; + CHECK_AND_ASSERT_THROW_MES(idx < std::get<2>(m_unsigned_tx->transfers).size() + std::get<0>(m_unsigned_tx->transfers) && idx >= std::get<0>(m_unsigned_tx->transfers), "Invalid transfer index"); + return std::get<2>(m_unsigned_tx->transfers)[idx - std::get<0>(m_unsigned_tx->transfers)]; } const tools::wallet2::transfer_details & get_source_transfer(size_t idx) const { diff --git a/src/serialization/tuple.h b/src/serialization/tuple.h new file mode 100644 index 000000000..6d98e05b0 --- /dev/null +++ b/src/serialization/tuple.h @@ -0,0 +1,169 @@ +// Copyright (c) 2014-2020, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers + +#pragma once +#include +#include "serialization.h" + +namespace serialization +{ + namespace detail + { + template + bool serialize_tuple_element(Archive& ar, T& e) + { + return ::do_serialize(ar, e); + } + + template + bool serialize_tuple_element(Archive& ar, uint64_t& e) + { + ar.serialize_varint(e); + return true; + } + } +} + +template