Wallet and daemon rpc fixes

src/cpp/daemon/py_monero_daemon_model.cpp

@@ -260,7 +260,7 @@ void PyMoneroTx::from_property_tree(const boost::property_tree::ptree& node, con
       for(auto it2 = node2.begin(); it2 != node2.end(); ++it2) {
         std::string _key = it2->first;
 
-        if (_key == std::string("txnfee")) {
+        if (_key == std::string("txnFee")) {
           tx->m_fee = it2->second.get_value<uint64_t>();
         }
       }

src/cpp/daemon/py_monero_daemon_rpc.cpp

@@ -370,7 +370,7 @@ std::vector<std::string> PyMoneroDaemonRpc::get_tx_hexes(const std::vector<std::
 }
 
 std::shared_ptr<PyMoneroMinerTxSum> PyMoneroDaemonRpc::get_miner_tx_sum(uint64_t height, uint64_t num_blocks) {
-  auto params = std::make_shared<PyMoneroGetMinerTxSumParams>();
+  auto params = std::make_shared<PyMoneroGetMinerTxSumParams>(height, num_blocks);
   PyMoneroJsonRequest request("get_coinbase_tx_sum", params);
   std::shared_ptr<PyMoneroJsonResponse> response = m_rpc->send_json_request(request);
   if (response->m_result == boost::none) throw std::runtime_error("Invalid Monero JSONRPC response");

src/cpp/py_monero.cpp

@@ -2106,9 +2106,9 @@ PYBIND11_MODULE(monero, m) {
 
   // monero_wallet_rpc
   py_monero_wallet_rpc
-    .def(py::init<std::shared_ptr<PyMoneroRpcConnection>>(), py::arg("rpc_connection"))
+    .def(py::init<const std::shared_ptr<PyMoneroRpcConnection>&>(), py::arg("rpc_connection"))
     .def(py::init<const std::string&, const std::string&, const std::string&>(), py::arg("uri") = "", py::arg("username") = "", py::arg("password") = "")
-    .def("create_wallet", [](PyMoneroWalletRpc& self, const std::shared_ptr<PyMoneroWalletConfig> config) {
+    .def("create_wallet", [](PyMoneroWalletRpc& self, const std::shared_ptr<PyMoneroWalletConfig>& config) {
       try {
         self.create_wallet(config);
         return &self;
@@ -2120,10 +2120,10 @@ PYBIND11_MODULE(monero, m) {
         throw;
       }
       catch(const std::exception& ex) {
-        throw py::value_error(ex.what());
+        throw PyMoneroError(ex.what());
       }
     }, py::arg("config"))
-    .def("open_wallet", [](PyMoneroWalletRpc& self, const std::shared_ptr<PyMoneroWalletConfig> config) {
+    .def("open_wallet", [](PyMoneroWalletRpc& self, const std::shared_ptr<PyMoneroWalletConfig>& config) {
       try {
         self.open_wallet(config);
         return &self;
@@ -2135,7 +2135,7 @@ PYBIND11_MODULE(monero, m) {
         throw;
       }
       catch(const std::exception& ex) {
-        throw py::value_error(ex.what());
+        throw PyMoneroError(ex.what());
       }
     }, py::arg("config"))
     .def("open_wallet", [](PyMoneroWalletRpc& self, const std::string& name, const std::string& password) {
@@ -2150,9 +2150,12 @@ PYBIND11_MODULE(monero, m) {
         throw;
       }
       catch(const std::exception& ex) {
-        throw py::value_error(ex.what());
+        throw PyMoneroError(ex.what());
       }
     }, py::arg("name"), py::arg("password"))
+    .def("is_closed", [](const PyMoneroWalletRpc& self) {
+      MONERO_CATCH_AND_RETHROW(self.is_closed());
+    })
     .def("get_seed_languages", [](PyMoneroWalletRpc& self) {
       MONERO_CATCH_AND_RETHROW(self.get_seed_languages());
     })

src/cpp/wallet/py_monero_wallet_model.cpp

@@ -395,6 +395,7 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
         outgoing_transfer->m_tx = tx;
       }
       auto node2 = it->second;
+      outgoing_transfer->m_destinations.clear();
 
       for(auto it2 = node2.begin(); it2 != node2.end(); ++it2) {
         auto node3 = it2->second;
@@ -413,10 +414,7 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
     }
     else if (key == std::string("amount_in")) tx->m_input_sum = it->second.get_value<uint64_t>();
     else if (key == std::string("amount_out")) tx->m_input_sum = it->second.get_value<uint64_t>();
-    else if (key == std::string("change_address")) {
-      std::string change_address = it->second.data();
-      if (change_address != std::string("")) tx->m_change_address = it->second.data();
-    }
+    else if (key == std::string("change_address") && !it->second.data().empty()) tx->m_change_address = it->second.data();
     else if (key == std::string("change_amount")) tx->m_change_amount = it->second.get_value<uint64_t>();
     else if (key == std::string("dummy_outputs")) tx->m_num_dummy_outputs = it->second.get_value<uint64_t>();
     //else if (key == std::string("extra")) tx->m_extra = it->second.data();
@@ -467,7 +465,7 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
       auto destinations = config.get_normalized_destinations();
       size_t num_destinations = destinations.size();
       if (num_destinations != amounts_by_dest.size()) throw std::runtime_error("Expected destinations size equal to amounts by dest size");
-
+      outgoing_transfer->m_destinations.clear();
       for(uint64_t i = 0; i < num_destinations; i++) {
         auto dest = std::make_shared<monero::monero_destination>();
         dest->m_address = destinations[i]->m_address;
@@ -493,7 +491,7 @@ void PyMoneroTxWallet::from_property_tree_with_transfer(const boost::property_tr
 
     if (tx->m_outgoing_transfer != boost::none) {
       // overwrite to avoid reconcile error TODO: remove after >18.3.1 when amounts_by_dest supported
-      if (tx->m_outgoing_transfer.get()->m_destinations.size() != 0) {
+      if (!outgoing_transfer->m_destinations.empty()) {
         tx->m_outgoing_transfer.get()->m_destinations.clear();
       }
       tx->m_outgoing_transfer.get()->merge(tx->m_outgoing_transfer.get(), outgoing_transfer);
@@ -826,6 +824,7 @@ void PyMoneroTxSet::from_sent_txs(const boost::property_tree::ptree& node, const
               if (conf == boost::none) throw std::runtime_error("Expected tx configuration");
               auto config = conf.get();
               if (config.m_destinations.size() == 1) {
+                // sweeping can create multiple withone address
                 auto dest = std::make_shared<monero::monero_destination>();
                 dest->m_address = config.m_destinations[0]->m_address;
                 dest->m_amount = amount;
@@ -1786,6 +1785,13 @@ void PyMoneroCheckTxProof::from_property_tree(const boost::property_tree::ptree&
     else if (key == std::string("confirmations")) check->m_num_confirmations = it->second.get_value<uint64_t>();
     else if (key == std::string("received")) check->m_received_amount = it->second.get_value<uint64_t>();
   }
+
+  if (!bool_equals_2(true, check->m_is_good)) {
+    // normalize invalid tx proof
+    check->m_in_tx_pool = boost::none;
+    check->m_num_confirmations = boost::none;
+    check->m_received_amount = boost::none;
+  }
 }
 
 std::string PyMoneroReserveProofSignature::from_property_tree(const boost::property_tree::ptree& node) {

src/cpp/wallet/py_monero_wallet_rpc.cpp

@@ -1,6 +1,11 @@
 #include "py_monero_wallet_rpc.h"
 #include "utils/monero_utils.h"
 
+PyMoneroWalletPoller::PyMoneroWalletPoller(PyMoneroWallet *wallet) {
+  m_wallet = wallet;
+  m_is_polling = false;
+  m_num_polling = 0;
+}
 
 PyMoneroWalletPoller::~PyMoneroWalletPoller() {
   set_is_polling(false);
@@ -214,7 +219,23 @@ bool PyMoneroWalletPoller::check_for_changed_balances() {
   return false;
 }
 
+PyMoneroWalletRpc::PyMoneroWalletRpc() {
+  m_rpc = std::make_shared<PyMoneroRpcConnection>();
+}
+
+PyMoneroWalletRpc::PyMoneroWalletRpc(const std::shared_ptr<PyMoneroRpcConnection>& rpc_connection) {
+  m_rpc = rpc_connection;
+  if (!m_rpc->is_online() && !m_rpc->m_uri->empty()) m_rpc->check_connection();
+}
+
+PyMoneroWalletRpc::PyMoneroWalletRpc(const std::string& uri, const std::string& username, const std::string& password) {
+  m_rpc = std::make_shared<PyMoneroRpcConnection>(uri, username, password);
+  if (!m_rpc->m_uri->empty()) m_rpc->check_connection();
+}
+
 PyMoneroWalletRpc::~PyMoneroWalletRpc() {
+  MTRACE("~PyMoneroWalletRpc()");
+  clear();
 }
 
 void PyMoneroWalletRpc::add_listener(monero_wallet_listener& listener) {
@@ -1091,6 +1112,7 @@ std::vector<std::shared_ptr<monero_tx_wallet>> PyMoneroWalletRpc::sweep_unlocked
     } else {
       std::vector<uint32_t> subaddress_indices;
       for (const monero_subaddress& subaddress : monero_wallet::get_subaddresses(config.m_account_index.get())) {
+        // TODO wallet rpc sweep_all now supports req.subaddr_indices_all
         if (subaddress.m_unlocked_balance.get() > 0) subaddress_indices.push_back(subaddress.m_index.get());
       }
       indices[config.m_account_index.get()] = subaddress_indices;
@@ -1141,7 +1163,6 @@ std::vector<std::shared_ptr<monero_tx_wallet>> PyMoneroWalletRpc::sweep_unlocked
   return txs;
 }
 
-
 std::shared_ptr<monero_tx_wallet> PyMoneroWalletRpc::sweep_output(const monero_tx_config& config) {
   // validate request
   std::vector<std::shared_ptr<monero_destination>> destinations = config.get_normalized_destinations();
@@ -1347,11 +1368,11 @@ std::shared_ptr<monero_check_tx> PyMoneroWalletRpc::check_tx_proof(const std::st
     PyMoneroCheckTxProof::from_property_tree(node, check);
     return check;
   } catch (const PyMoneroRpcError& ex) {
-    if (ex.code == -1 && ex.what() == std::string("basic_string")) {
+    // normalize error message
+    if (ex.code == -1 && std::string(ex.what()).find("basic_string") != std::string::npos) {
       throw PyMoneroRpcError(-1, "Must provide signature to check tx proof");
     }
     if (ex.code == -8 && ex.what() == std::string("TX ID has invalid format")) {
-      // normalize error message
       throw PyMoneroRpcError(-8, "TX hash has invalid format");
     }
     throw;
@@ -1709,7 +1730,18 @@ void PyMoneroWalletRpc::save() {
   m_rpc->send_json_request(request);
 }
 
+bool PyMoneroWalletRpc::is_closed() const {
+  try {
+    get_primary_address();
+  } catch (const PyMoneroRpcError& ex) {
+    return ex.code == -8 && ex.what() == std::string("No wallet file");
+  }
+
+  return false;
+}
+
 void PyMoneroWalletRpc::close(bool save) {
+  clear();
   auto params = std::make_shared<PyMoneroCloseWalletParams>(save);
   PyMoneroJsonRequest request("close_wallet", params);
   m_rpc->send_json_request(request);
@@ -1839,15 +1871,15 @@ std::string PyMoneroWalletRpc::query_key(const std::string& key_type) const {
 
 std::vector<std::shared_ptr<monero_tx_wallet>> PyMoneroWalletRpc::sweep_account(const monero_tx_config &conf) {
   auto config = conf.copy();
+  // validate config
   if (config.m_account_index == boost::none) throw std::runtime_error("Must specify an account index to sweep from");
   std::vector<std::shared_ptr<monero_destination>> destinations = config.get_normalized_destinations();
-  if (destinations.size() != 1) throw std::runtime_error("Must specify exactly one destination to sweep to");
+  if (destinations.size() != 1) throw std::runtime_error("Must provide exactly one destination address to sweep output to");
   if (destinations[0]->m_address == boost::none) throw std::runtime_error("Must specify destination address to sweep to");
-  if (destinations[0]->m_amount != boost::none) throw std::runtime_error("Cannot specify amount in sweep request");
-  if (config.m_key_image != boost::none) throw std::runtime_error("Key image defined; use sweepOutput() to sweep an output by its key image");
-  //if (config.m_subaddress_indices.size() == 0) throw std::runtime_error("Empty list given for subaddresses indices to sweep");
+  if (destinations[0]->m_amount != boost::none) throw std::runtime_error("Cannot specify destination amount to sweep");
+  if (config.m_key_image != boost::none) throw std::runtime_error("Cannot define key image in sweep_account(); use sweep_output() to sweep an output by its key image");
   if (bool_equals_2(true, config.m_sweep_each_subaddress)) throw std::runtime_error("Cannot sweep each subaddress with RPC `sweep_all`");
-  if (config.m_subtract_fee_from.size() > 0) throw std::runtime_error("Sweeping output does not support subtracting fees from destinations");
+  if (config.m_subtract_fee_from.size() > 0) throw std::runtime_error("Sweep transactions do not support subtracting fees from destinations");
 
   // sweep from all subaddresses if not otherwise defined
   if (config.m_subaddress_indices.empty()) {
@@ -1911,21 +1943,13 @@ void PyMoneroWalletRpc::clear_address_cache() {
 }
 
 void PyMoneroWalletRpc::refresh_listening() {
-  if (m_rpc->m_zmq_uri == boost::none || m_rpc->m_zmq_uri.get().empty()) {
-    if (m_poller == nullptr && m_listeners.size() > 0) {
-      m_poller = std::make_shared<PyMoneroWalletPoller>(this);
-      if (m_sync_period_in_ms != boost::none) m_poller->set_period_in_ms(m_sync_period_in_ms.get());
-    }
-    if (m_poller != nullptr) {
-      m_poller->set_is_polling(m_listeners.size() > 0);
-    }
+  if (m_poller == nullptr && !m_listeners.empty()) {
+    m_poller = std::make_unique<PyMoneroWalletPoller>(this);
+    if (m_sync_period_in_ms != boost::none) m_poller->set_period_in_ms(m_sync_period_in_ms.get());
   }
-  /*
-  else {
-    if (m_zmq_listener == nullptr && m_listeners.size() > 0) m_zmq_listener = std::make_shared<PyMoneroWalletRpcZmqListener>();
-    if (m_zmq_listener != nullptr) m_zmq_listener.set_is_polling(m_listeners.size() > 0);
+  if (m_poller != nullptr) {
+    m_poller->set_is_polling(!m_listeners.empty());
   }
-  */
 }
 
 void PyMoneroWalletRpc::poll() {
@@ -2178,7 +2202,7 @@ std::vector<std::shared_ptr<monero_transfer>> PyMoneroWalletRpc::get_transfers_a
   }
 
   if (_query->m_account_index == boost::none) {
-    if (_query->m_subaddress_index != boost::none) throw std::runtime_error("Filter specifies a subaddress index but not an account index");
+    if (_query->m_subaddress_index != boost::none || !_query->m_subaddress_indices.empty()) throw std::runtime_error("Filter specifies a subaddress index but not an account index");
     params->m_all_accounts = true;
   } else {
     params->m_account_index = _query->m_account_index;
@@ -2269,6 +2293,7 @@ std::vector<std::shared_ptr<monero_output_wallet>> PyMoneroWalletRpc::get_output
     for (const auto& subaddress_idx : _query->m_subaddress_indices) {
       subaddress_indices.push_back(subaddress_idx);
     }
+    // null will fetch from all subaddresses
     indices[_query->m_account_index.get()] = subaddress_indices;
   }
   else {
@@ -2295,6 +2320,7 @@ std::vector<std::shared_ptr<monero_output_wallet>> PyMoneroWalletRpc::get_output
     uint32_t account_idx = kv.first;
     params->m_account_index = account_idx;
     params->m_subaddr_indices = kv.second;
+    // send request
     PyMoneroJsonRequest request("incoming_transfers", params);
     auto response = m_rpc->send_json_request(request);
     if (response->m_result == boost::none) throw std::runtime_error("Invalid Monero JSONRPC response");

src/cpp/wallet/py_monero_wallet_rpc.h

@@ -5,13 +5,9 @@
 
 class PyMoneroWalletPoller {
 public:
-  explicit PyMoneroWalletPoller(PyMoneroWallet *wallet) {
-    m_wallet = wallet;
-    m_is_polling = false;
-    m_num_polling = 0;
-  }
 
   ~PyMoneroWalletPoller();
+  PyMoneroWalletPoller(PyMoneroWallet *wallet);
 
   bool is_polling() const { return m_is_polling; }
   void set_is_polling(bool is_polling);
@@ -42,21 +38,10 @@ class PyMoneroWalletPoller {
 class PyMoneroWalletRpc : public PyMoneroWallet {
 public:
 
-  PyMoneroWalletRpc() {
-    m_rpc = std::make_shared<PyMoneroRpcConnection>();
-  }
-
-  PyMoneroWalletRpc(std::shared_ptr<PyMoneroRpcConnection> rpc_connection) {
-    m_rpc = rpc_connection;
-    if (!m_rpc->is_online() && !m_rpc->m_uri->empty()) m_rpc->check_connection();
-  }
-
-  PyMoneroWalletRpc(const std::string& uri = "", const std::string& username = "", const std::string& password = "") {
-    m_rpc = std::make_shared<PyMoneroRpcConnection>(uri, username, password);
-    if (!m_rpc->m_uri->empty()) m_rpc->check_connection();
-  }
-
   ~PyMoneroWalletRpc();
+  PyMoneroWalletRpc();
+  PyMoneroWalletRpc(const std::shared_ptr<PyMoneroRpcConnection>& rpc_connection);
+  PyMoneroWalletRpc(const std::string& uri = "", const std::string& username = "", const std::string& password = "");
 
   PyMoneroWalletRpc* open_wallet(const std::shared_ptr<PyMoneroWalletConfig> &config);
   PyMoneroWalletRpc* open_wallet(const std::string& name, const std::string& password);
@@ -177,6 +162,7 @@ class PyMoneroWalletRpc : public PyMoneroWallet {
   std::vector<std::string> submit_multisig_tx_hex(const std::string& signed_multisig_tx_hex);
   void change_password(const std::string& old_password, const std::string& new_password) override;
   void save() override;
+  bool is_closed() const override;
   void close(bool save = false) override;
   std::shared_ptr<PyMoneroWalletBalance> get_balances(boost::optional<uint32_t> account_idx, boost::optional<uint32_t> subaddress_idx) const override;
 
@@ -186,7 +172,7 @@ class PyMoneroWalletRpc : public PyMoneroWallet {
   std::string m_path = "";
   std::shared_ptr<PyMoneroRpcConnection> m_rpc;
   std::shared_ptr<PyMoneroRpcConnection> m_daemon_connection;
-  std::shared_ptr<PyMoneroWalletPoller> m_poller;
+  std::unique_ptr<PyMoneroWalletPoller> m_poller;
 
   mutable boost::recursive_mutex m_sync_mutex;
   mutable std::unordered_map<uint32_t, std::unordered_map<uint32_t, std::string>> m_address_cache;

tests/config/config.ini

@@ -33,7 +33,7 @@ rpc_zmq_enabled=False
 rpc_zmq_port_start=48083
 rpc_zmq_bind_port_start=48083
 rpc_zmq_domain=127.0.0.1
-sync_period_in_ms=5000
+sync_period_in_ms=2500
 
 [mining_wallet]
 name=mining_wallet

tests/docker-compose.yml

@@ -86,6 +86,7 @@ services:
       "--daemon-address=node_2:18081",
       "--daemon-login=rpc_daemon_user:abc123",
       "--rpc-login=rpc_user:abc123",
+      "--rpc-max-connections-per-private-ip=100",
       "--wallet-dir=/wallet",
       "--rpc-access-control-origins=*",
       "--non-interactive"
@@ -112,6 +113,7 @@ services:
       "--daemon-address=node_2:18081",
       "--daemon-login=rpc_daemon_user:abc123",
       "--rpc-login=rpc_user:abc123",
+      "--rpc-max-connections-per-private-ip=100",
       "--wallet-dir=/wallet",
       "--rpc-access-control-origins=*",
       "--non-interactive"