diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index dea1cf250..e149a910c 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -923,6 +923,11 @@ bool get_short_payment_id(crypto::hash8 &payment_id8, const tools::wallet2::pend return false; } +uint64_t get_outgoing_amount(const cryptonote::transaction &tx, const uint64_t amount_spent) +{ + return tx.version == 1 ? get_outs_money_amount(tx) : (amount_spent - tx.rct_signatures.txnFee); +} + tools::wallet2::tx_construction_data get_construction_data_with_decrypted_short_payment_id(const tools::wallet2::pending_tx &ptx, hw::device &hwdev) { tools::wallet2::tx_construction_data construction_data = ptx.construction_data; @@ -2692,10 +2697,10 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote LOG_ERROR("spent funds are from different subaddress accounts; count of incoming/outgoing payments will be incorrect"); subaddr_account = td.m_subaddr_index.major; subaddr_indices.insert(td.m_subaddr_index.minor); + LOG_PRINT_L0("Spent money: " << print_money(amount) << ", with tx: " << txid); + set_spent(it->second, height); if (!pool) { - LOG_PRINT_L0("Spent money: " << print_money(amount) << ", with tx: " << txid); - set_spent(it->second, height); if (!ignore_callbacks && 0 != m_callback) m_callback->on_money_spent(height, txid, tx, amount, tx, td.m_subaddr_index); @@ -2784,21 +2789,33 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote uint64_t fee = miner_tx ? 0 : tx.version == 1 ? tx_money_spent_in_ins - get_outs_money_amount(tx) : tx.rct_signatures.txnFee; - if (tx_money_spent_in_ins > 0 && !pool) + if (tx_money_spent_in_ins > 0) { uint64_t self_received = std::accumulate(tx_money_got_in_outs.begin(), tx_money_got_in_outs.end(), 0, [&subaddr_account] (uint64_t acc, const std::pair& p) { return acc + (p.first.major == *subaddr_account ? p.second : 0); }); - process_outgoing(txid, tx, height, ts, tx_money_spent_in_ins, self_received, *subaddr_account, subaddr_indices); - // if sending to yourself at the same subaddress account, set the outgoing payment amount to 0 so that it's less confusing - if (tx_money_spent_in_ins == self_received + fee) + if (!pool) { - auto i = m_confirmed_txs.find(txid); - THROW_WALLET_EXCEPTION_IF(i == m_confirmed_txs.end(), error::wallet_internal_error, - "confirmed tx wasn't found: " + string_tools::pod_to_hex(txid)); - i->second.m_change = self_received; + process_outgoing(txid, tx, height, ts, tx_money_spent_in_ins, self_received, *subaddr_account, subaddr_indices); + // if sending to yourself at the same subaddress account, set the outgoing payment amount to 0 so that it's less confusing + if (tx_money_spent_in_ins == self_received + fee) + { + auto i = m_confirmed_txs.find(txid); + THROW_WALLET_EXCEPTION_IF(i == m_confirmed_txs.end(), error::wallet_internal_error, + "confirmed tx wasn't found: " + string_tools::pod_to_hex(txid)); + i->second.m_change = self_received; + } + } + else if (!m_unconfirmed_txs.count(txid)) + { + // Add to unconfirmed txs if not already there (e.g. restoring wallet, or running the wallet in parallel to the sending wallet w/same seed) + add_unconfirmed_tx(txid, tx, tx_money_spent_in_ins, {}/*don't know dests*/, crypto::null_hash/*don't know payment_id*/, self_received, *subaddr_account, subaddr_indices); + auto i = m_unconfirmed_txs.find(txid); + THROW_WALLET_EXCEPTION_IF(i == m_unconfirmed_txs.end(), error::wallet_internal_error, + "unconfirmed tx wasn't found: " + string_tools::pod_to_hex(txid)); + i->second.m_amount_out = get_outgoing_amount(tx, tx_money_spent_in_ins); } } @@ -2947,10 +2964,7 @@ void wallet2::process_outgoing(const crypto::hash &txid, const cryptonote::trans // wallet (eg, we're a cold wallet and the hot wallet sent it). For RCT transactions, // we only see 0 input amounts, so have to deduce amount out from other parameters. entry.first->second.m_amount_in = spent; - if (tx.version == 1) - entry.first->second.m_amount_out = get_outs_money_amount(tx); - else - entry.first->second.m_amount_out = spent - tx.rct_signatures.txnFee; + entry.first->second.m_amount_out = get_outgoing_amount(tx, spent); entry.first->second.m_change = received; std::vector tx_extra_fields; @@ -4038,6 +4052,17 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo return; } + if (!m_first_refresh_done) + { + // We want to process the whole pool again, in case we identify received outputs in the chain we might have spent in the pool + m_pool_info_query_time = 0; + m_scanned_pool_txs[0].clear(); + m_scanned_pool_txs[1].clear(); + // Clear unconfirmed (received) payments because the data is 100% recovered when scanning + m_unconfirmed_payments.clear(); + // Don't clear unconfirmed (sent) txs because some data is not recover-able when scanning (dests) + } + received_money = false; blocks_fetched = 0; uint64_t added_blocks = 0; @@ -7464,9 +7489,9 @@ uint64_t wallet2::select_transfers(uint64_t needed_money, std::vector un return found_money; } //---------------------------------------------------------------------------------------------------- -void wallet2::add_unconfirmed_tx(const cryptonote::transaction& tx, uint64_t amount_in, const std::vector &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set& subaddr_indices) +void wallet2::add_unconfirmed_tx(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount_in, const std::vector &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set& subaddr_indices) { - unconfirmed_transfer_details& utd = m_unconfirmed_txs[cryptonote::get_transaction_hash(tx)]; + unconfirmed_transfer_details& utd = m_unconfirmed_txs[txid]; utd.m_amount_in = amount_in; utd.m_amount_out = 0; for (const auto &d: dests) @@ -7581,7 +7606,7 @@ void wallet2::commit_tx(pending_tx& ptx) for(size_t idx: ptx.selected_transfers) amount_in += m_transfers[idx].amount(); } - add_unconfirmed_tx(ptx.tx, amount_in, dests, payment_id, ptx.change_dts.amount, ptx.construction_data.subaddr_account, ptx.construction_data.subaddr_indices); + add_unconfirmed_tx(txid, ptx.tx, amount_in, dests, payment_id, ptx.change_dts.amount, ptx.construction_data.subaddr_account, ptx.construction_data.subaddr_indices); if (store_tx_info() && ptx.tx_key != crypto::null_skey) { m_tx_keys[txid] = ptx.tx_key; diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index f6271bd36..ff6111f95 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1835,7 +1835,7 @@ private: bool prepare_file_names(const std::string& file_path); void process_unconfirmed(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t height); void process_outgoing(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t height, uint64_t ts, uint64_t spent, uint64_t received, uint32_t subaddr_account, const std::set& subaddr_indices); - void add_unconfirmed_tx(const cryptonote::transaction& tx, uint64_t amount_in, const std::vector &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set& subaddr_indices); + void add_unconfirmed_tx(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount_in, const std::vector &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set& subaddr_indices); void generate_genesis(cryptonote::block& b) const; void check_genesis(const crypto::hash& genesis_hash) const; //throws bool generate_chacha_key_from_secret_keys(crypto::chacha_key &key) const; diff --git a/tests/functional_tests/multisig.py b/tests/functional_tests/multisig.py index 1a59e1779..446037c67 100755 --- a/tests/functional_tests/multisig.py +++ b/tests/functional_tests/multisig.py @@ -28,6 +28,7 @@ # 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. +import json import random import time @@ -417,10 +418,37 @@ class MultisigTest(): assert len(res.tx_hash_list) == 1 txid = res.tx_hash_list[0] + # Retrieve spent key images from daemon + res = daemon.get_transactions([txid], decode_as_json = True) + assert len(res.txs) == 1 + tx = res.txs[0] + assert tx.tx_hash == txid + assert len(tx.as_json) > 0 + try: + j = json.loads(tx.as_json) + except: + j = None + assert j + assert len(j['vin']) >= 1 + spent_key_images = [vin['key']['k_image'] for vin in j['vin']] + assert len(spent_key_images) == len(j['vin']) + for i in range(len(self.wallet)): + # Check if the wallet knows about any spent key images (all signers *should*, non-signers *might*) + is_a_signer = len([x for x in signers if x == i]) > 0 + knows_key_image = False + for ki in spent_key_images: + try: + res = self.wallet[i].frozen(ki) + knows_key_image = True + except AssertionError: + if is_a_signer: + raise ValueError('Signer should know about spent key image') + pass self.wallet[i].refresh() res = self.wallet[i].get_transfers() - assert len([x for x in (res['pending'] if 'pending' in res else []) if x.txid == txid]) == (1 if i == signers[-1] else 0) + # Any wallet that knows about any spent key images should be able to detect the spend in the pool + assert len([x for x in (res['pending'] if 'pending' in res else []) if x.txid == txid]) == (1 if knows_key_image else 0) assert len([x for x in (res['out'] if 'out' in res else []) if x.txid == txid]) == 0 daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) @@ -516,9 +544,13 @@ class MultisigTest(): txid = res.tx_hash_list[0] for i in range(len(self.wallet)): + # Make sure wallet knows about the key image + frozen = self.wallet[i].frozen(ki).frozen + assert not frozen self.wallet[i].refresh() res = self.wallet[i].get_transfers() - assert len([x for x in (res['pending'] if 'pending' in res else []) if x.txid == txid]) == (1 if i == signers[-1] else 0) + # Since all wallets should have key image, all wallets should be able to detect the spend in the pool + 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) diff --git a/tests/functional_tests/transfer.py b/tests/functional_tests/transfer.py index f1fb6e51d..823285644 100755 --- a/tests/functional_tests/transfer.py +++ b/tests/functional_tests/transfer.py @@ -64,7 +64,9 @@ def restore_wallet(wallet, seed, restore_height = 0, filename = '', password = ' util_resources.remove_wallet_files(filename) wallet.auto_refresh(enable = False) wallet.restore_deterministic_wallet(seed = seed, restore_height = restore_height, filename = filename, password = password) - assert wallet.get_transfers() == {} + res = wallet.get_transfers() + assert not 'in' in res or len(res['in']) == 0 + assert not 'out' in res or len(res.out) == 0 class TransferTest(): def run_test(self): @@ -87,6 +89,7 @@ class TransferTest(): self.check_background_sync() self.check_background_sync_reorg_recovery() self.check_subaddress_lookahead() + self.check_pool_scanner() def reset(self): print('Resetting blockchain') @@ -274,6 +277,7 @@ class TransferTest(): assert len(res.multisig_txset) == 0 assert len(res.unsigned_txset) == 0 tx_blob = res.tx_blob + running_balances[0] -= 1000000000000 + fee res = daemon.send_raw_transaction(tx_blob) assert res.not_relayed == False @@ -315,7 +319,6 @@ class TransferTest(): daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) res = daemon.getlastblockheader() - running_balances[0] -= 1000000000000 + fee running_balances[0] += res.block_header.reward self.wallet[1].refresh() running_balances[1] += 1000000000000 @@ -1574,5 +1577,62 @@ class TransferTest(): assert balance_info_0_999['blocks_to_unlock'] == 9 assert balance_info_0_999['time_to_unlock'] == 0 + def check_pool_scanner(self): + daemon = Daemon() + + print('Checking pool scanner') + + # Sync first wallet + daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) + self.wallet[0].refresh() + + # Open second wallet with same seed as first + restore_wallet(self.wallet[1], seeds[0]) + assert self.wallet[0].get_address().address == self.wallet[1].get_address().address + + # Send to another wallet, spending from first wallet + dst = {'address': '44Kbx4sJ7JDRDV5aAhLJzQCjDz2ViLRduE3ijDZu3osWKBjMGkV1XPk4pfDUMqt1Aiezvephdqm6YD19GKFD9ZcXVUTp6BW', 'amount': 1000000000000} + res = self.wallet[0].transfer([dst]) + assert len(res.tx_hash) == 32*2 + txid = res.tx_hash + assert res.fee > 0 + fee = res.fee + + # Sync both wallets + self.wallet[0].refresh() + self.wallet[1].refresh() + + # Both wallets should be able to detect the spend tx in the pool + res_wallet0 = self.wallet[0].get_transfers() + res_wallet1 = self.wallet[1].get_transfers() + + # After restoring, should still be able to detect the spend in the pool + restore_wallet(self.wallet[1], seed = seeds[0]) + self.wallet[1].refresh() + res_wallet1_after_restore = self.wallet[1].get_transfers() + + for res in [res_wallet0, res_wallet1, res_wallet1_after_restore]: + assert len(res.pending) == 1 + assert not 'pool' in res or len(res.pool) == 0 + assert not 'failed' in res or len(res.failed) == 0 + e = res.pending[0] + assert e.txid == txid + assert e.payment_id in ['', '0000000000000000'] + assert e.type == 'pending' + assert e.unlock_time == 0 + assert e.subaddr_index.major == 0 + assert e.subaddr_indices == [{'major': 0, 'minor': 0}] + assert e.address == '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm' + assert e.double_spend_seen == False + assert not 'confirmations' in e or e.confirmations == 0 + assert e.amount == dst['amount'] + assert e.fee == fee + + # Mine a block to mine the tx and reset 2nd wallet + daemon.generateblocks('46r4nYSevkfBUMhuykdK3gQ98XDqDTYW1hNLaXNvjpsJaSbNtdXh1sKMsdVgqkaihChAzEy29zEDPMR3NHQvGoZCLGwTerK', 1) + restore_wallet(self.wallet[1], seeds[1]) + self.wallet[1].refresh() + self.wallet[0].refresh() + if __name__ == '__main__': TransferTest().run_test()