Merge pull request #10123

4e9d199 daemon: fix print_cn command when 0 connections (jeffro256)

src/cryptonote_core/blockchain.cpp

@@ -3591,22 +3591,6 @@ bool Blockchain::check_tx_inputs(transaction& tx, tx_verification_context &tvc,
       MERROR_VER("Unsupported rct type: " << rv.type);
       return false;
     }
-
-    // for bulletproofs, check they're only multi-output after v8
-    if (rct::is_rct_bulletproof(rv.type))
-    {
-      if (hf_version < 8)
-      {
-        for (const rct::Bulletproof &proof: rv.p.bulletproofs)
-        {
-          if (proof.V.size() > 1)
-          {
-            MERROR_VER("Multi output bulletproofs are invalid before v8");
-            return false;
-          }
-        }
-      }
-    }
   }
   return true;
 }

src/daemon/rpc_command_executor.cpp

@@ -644,12 +644,9 @@ bool t_rpc_command_executor::print_connections() {
     }
   }
 
-  auto longest_host = *std::max_element(res.connections.begin(), res.connections.end(),
-                                        [](const auto &info1, const auto &info2)
-                                        {
-                                          return info1.address.length() < info2.address.length();
-                                        });
-  int host_field_width = std::max(15, 8 + (int) longest_host.address.length());
+  int host_field_width = 15;
+  for (const auto &conn : res.connections)
+    host_field_width = std::max(host_field_width, 8 + (int) conn.address.length());
 
   tools::msg_writer() << std::setw(host_field_width) << std::left << "Remote Host"
       << std::setw(8) << "Type"

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<decltype(tx_money_got_in_outs.begin()), uint64_t>(tx_money_got_in_outs.begin(), tx_money_got_in_outs.end(), 0,
       [&subaddr_account] (uint64_t acc, const std::pair<cryptonote::subaddress_index, uint64_t>& 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_field> 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<size_t> un
   return found_money;
 }
 //----------------------------------------------------------------------------------------------------
-void wallet2::add_unconfirmed_tx(const cryptonote::transaction& tx, uint64_t amount_in, const std::vector<cryptonote::tx_destination_entry> &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set<uint32_t>& subaddr_indices)
+void wallet2::add_unconfirmed_tx(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount_in, const std::vector<cryptonote::tx_destination_entry> &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set<uint32_t>& 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;
@@ -8978,7 +9003,9 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
       return;
 
     const auto unique = outs_unique(outs);
-    if (tx_sanity_check(unique.first, unique.second, rct_offsets.empty() ? 0 : rct_offsets.back()))
+    const uint64_t rct_outs_available = rct_offsets.size() >= CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE
+      ? rct_offsets.at(rct_offsets.size() - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE) : 0;
+    if (tx_sanity_check(unique.first, unique.second, rct_outs_available))
     {
       return;
     }

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<uint32_t>& subaddr_indices);
-    void add_unconfirmed_tx(const cryptonote::transaction& tx, uint64_t amount_in, const std::vector<cryptonote::tx_destination_entry> &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set<uint32_t>& subaddr_indices);
+    void add_unconfirmed_tx(const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount_in, const std::vector<cryptonote::tx_destination_entry> &dests, const crypto::hash &payment_id, uint64_t change_amount, uint32_t subaddr_account, const std::set<uint32_t>& 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;

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)

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