From dc24312bc3c8c9d865e4576631832c81d6dd212d Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Mon, 12 Jun 2023 01:30:38 -0500 Subject: [PATCH] wallet: respect frozen key images in multisig wallets [RELEASE] Before this change, if a multisig peer asked you to sign a transaction with a frozen enote, the wallet will do it without any error or warning. This change makes it so that wallets will refuse to sign multisig transactions with frozen enotes. Disclaimer: This PR was generously funded by @LocalMonero. --- src/wallet/wallet2.cpp | 41 +++++++++++- src/wallet/wallet2.h | 1 + tests/functional_tests/multisig.py | 101 +++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 63e87e52e..05efd8dc8 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1748,6 +1748,36 @@ bool wallet2::frozen(size_t idx) const return td.m_frozen; } //---------------------------------------------------------------------------------------------------- +bool wallet2::frozen(const multisig_tx_set& txs) const +{ + // Each call to frozen(const key_image&) is O(N), so if we didn't use batching like we did here, + // this op would be O(M * N) instead of O(M + N). N = # wallet transfers, M = # key images in set. + // Step 1. Collect all key images from all pending txs into set + std::unordered_set kis_to_sign; + for (const auto& ptx : txs.m_ptx) + { + const tools::wallet2::tx_construction_data& cd = ptx.construction_data; + CHECK_AND_ASSERT_THROW_MES(cd.sources.size() == ptx.tx.vin.size(), "mismatched multisg tx set source sizes"); + for (size_t src_idx = 0; src_idx < cd.sources.size(); ++src_idx) + { + // Check that the key images are consistent between tx vin and construction data + const crypto::key_image multisig_ki = rct::rct2ki(cd.sources[src_idx].multisig_kLRki.ki); + CHECK_AND_ASSERT_THROW_MES(ptx.tx.vin[src_idx].type() == typeid(cryptonote::txin_to_key), "multisig tx cannot be miner"); + const crypto::key_image vin_ki = boost::get(ptx.tx.vin[src_idx]).k_image; + CHECK_AND_ASSERT_THROW_MES(multisig_ki == vin_ki, "Mismatched key image b/t vin and construction data"); + + // Add key image to set + kis_to_sign.insert(multisig_ki); + } + } + // Step 2. Scan all transfers for frozen key images + for (const auto& td : m_transfers) + if (td.m_frozen && kis_to_sign.count(td.m_key_image)) + return true; + + return false; +} +//---------------------------------------------------------------------------------------------------- void wallet2::freeze(const crypto::key_image &ki) { freeze(get_transfer_details(ki)); @@ -1768,8 +1798,13 @@ size_t wallet2::get_transfer_details(const crypto::key_image &ki) const for (size_t idx = 0; idx < m_transfers.size(); ++idx) { const transfer_details &td = m_transfers[idx]; - if (td.m_key_image_known && td.m_key_image == ki) - return idx; + if (td.m_key_image == ki) + { + if (td.m_key_image_known) + return idx; + else if (td.m_key_image_partial) + CHECK_AND_ASSERT_THROW_MES(false, "Transfer detail lookups are not allowed for multisig partial key images"); + } } CHECK_AND_ASSERT_THROW_MES(false, "Key image not found"); } @@ -7265,6 +7300,8 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector= 2 + + daemon = Daemon() + + print("Creating multisig transaction from wallet " + str(signers[0])) + + dst = {'address': '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 'amount': 1000000000000} + res = self.wallet[signers[0]].transfer([dst]) + assert len(res.tx_hash) == 0 # not known yet + txid = res.tx_hash + assert len(res.tx_key) == 32*2 + assert res.amount > 0 + amount = res.amount + assert res.fee > 0 + fee = res.fee + assert len(res.tx_blob) == 0 + assert len(res.tx_metadata) == 0 + assert len(res.multisig_txset) > 0 + assert len(res.unsigned_txset) == 0 + spent_key_images = res.spent_key_images.key_images + multisig_txset = res.multisig_txset + + daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) + for i in range(len(self.wallet)): + self.wallet[i].refresh() + + for i in range(len(signers[1:])): + # Check that each signer wallet has key image and it is not frozen + for ki in spent_key_images: + frozen = self.wallet[signers[i+1]].frozen(ki).frozen + assert not frozen + + # Freeze key image involved with initiated transfer + assert len(spent_key_images) + ki0 = spent_key_images[0] + print("Freezing involved key image:", ki0) + self.wallet[signers[1]].freeze(ki0) + frozen = self.wallet[signers[1]].frozen(ki).frozen + assert frozen + + # Try signing multisig (this operation should fail b/c of the frozen key image) + print("Attemping to sign with frozen key image. This should fail") + try: + res = self.wallet[signers[1]].sign_multisig(multisig_txset) + raise ValueError('sign_multisig should not have succeeded w/ fronzen enotes') + except AssertionError: + pass + + # Thaw key image and continue transfer as normal + print("Thawing key image and continuing transfer as normal") + self.wallet[signers[1]].thaw(ki0) + frozen = self.wallet[signers[1]].frozen(ki).frozen + assert not frozen + + for i in range(len(signers[1:])): + print('Signing multisig transaction with wallet ' + str(signers[i+1])) + res = self.wallet[signers[i+1]].describe_transfer(multisig_txset = multisig_txset) + assert len(res.desc) == 1 + desc = res.desc[0] + assert desc.amount_in >= amount + fee + assert desc.amount_out == desc.amount_in - fee + assert desc.ring_size == 16 + assert desc.unlock_time == 0 + assert not 'payment_id' in desc or desc.payment_id in ['', '0000000000000000'] + assert desc.change_amount == desc.amount_in - 1000000000000 - fee + assert desc.change_address == self.wallet_address + assert desc.fee == fee + assert len(desc.recipients) == 1 + rec = desc.recipients[0] + assert rec.address == '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm' + assert rec.amount == 1000000000000 + + res = self.wallet[signers[i+1]].sign_multisig(multisig_txset) + multisig_txset = res.tx_data_hex + assert len(res.tx_hash_list if 'tx_hash_list' in res else []) == (i == len(signers[1:]) - 1) + + if i < len(signers[1:]) - 1: + print('Submitting multisig transaction prematurely with wallet ' + str(signers[-1])) + ok = False + try: self.wallet[signers[-1]].submit_multisig(multisig_txset) + except: ok = True + assert ok + + print('Submitting multisig transaction with wallet ' + str(signers[-1])) + res = self.wallet[signers[-1]].submit_multisig(multisig_txset) + assert len(res.tx_hash_list) == 1 + txid = res.tx_hash_list[0] + + for i in range(len(self.wallet)): + 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) + assert len([x for x in (res['out'] if 'out' in res else []) if x.txid == txid]) == 0 + + daemon.generateblocks('42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm', 1) + return txid + def check_transaction(self, txid): for i in range(len(self.wallet)): self.wallet[i].refresh()