From b1453ba6e259570b87bb82b4c070a1ad60627fa0 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Tue, 1 Apr 2025 16:18:53 -0400 Subject: [PATCH] Fixed several bugs --- src/backend.cpp | 45 +++++++++++++++++------- src/backend.h | 10 ++++-- src/ledger_main.cpp | 6 +++- src/lwsf_config.h | 1 + src/transaction_info.h | 2 +- src/wallet.cpp | 77 ++++++++++++++++++++++++++++-------------- src/wallet.h | 3 +- 7 files changed, 100 insertions(+), 44 deletions(-) diff --git a/src/backend.cpp b/src/backend.cpp index 507478a..604f61e 100644 --- a/src/backend.cpp +++ b/src/backend.cpp @@ -189,7 +189,7 @@ namespace lwsf { namespace internal { namespace backend read_bytes(source, *dest.second); dest.first = dest.second->id; } - static void write_bytes(wire::writer& dest, const std::pair>& source) + static void write_bytes(wire::writer& dest, const std::pair>& source) { if (!source.second) WIRE_DLOG_THROW(wire::error::schema::object, "Unexpected nullptr"); @@ -364,11 +364,13 @@ namespace lwsf { namespace internal { namespace backend `refresh()` method; the wallet is never in a partial-state. Swapping the transactions at the end helps with this guarantee. */ - boost::container::flat_map, memory> updated_txes; + std::unordered_map> updated_txes; updated_txes.reserve( std::max(self.primary.txes.size(), source.transactions.size()) ); + std::unordered_multimap> images; + /* The frontend will know about the spend first, iff the frontend was used to perform the spend. We copy _all_ transactions that have a spend secret, even if the backend doesn't acknowledge it, otherwise the @@ -382,10 +384,17 @@ namespace lwsf { namespace internal { namespace backend { if (spend.second.secret) { - updated_txes.emplace_hint( - updated_txes.end(), tx.first, std::make_shared(*tx.second) + const auto iter = updated_txes.emplace_hint( + updated_txes.end(), tx.first, nullptr ); - break; // inner loop + if (!iter->second) + { + iter->second = std::make_shared(*tx.second); + iter->second->height = std::nullopt; + iter->second->timestamp = std::nullopt; + iter->second->failed = false; + } + images.emplace(spend.first, iter->second); } } } @@ -405,7 +414,15 @@ namespace lwsf { namespace internal { namespace backend if (existing == self.primary.txes.end()) out.push_back(inserted.first->second); } - if (!update_tx(self.primary, *inserted.first->second, tx)) + if (update_tx(self.primary, *inserted.first->second, tx)) + { + for (const auto& spend : inserted.first->second->spends) + { + for (auto range = images.equal_range(spend.first); range.first != range.second; ++range.first) + range.first->second->failed = true; + } + } + else updated_txes.erase(tx.hash); } @@ -425,7 +442,6 @@ namespace lwsf { namespace internal { namespace backend self.per_byte_fee = unspents.per_byte_fee; self.fee_mask = unspents.fee_mask; - return out; } @@ -492,8 +508,11 @@ namespace lwsf { namespace internal { namespace backend primary{}, last_sync(0), blockchain_height(0), + per_byte_fee(0), fee_mask(0), - sync() + sync(), + sync_listener(), + sync_refresh() { primary.subaccounts[0].minor[0]; } @@ -592,12 +611,13 @@ namespace lwsf { namespace internal { namespace backend const std::chrono::steady_clock::time_point start{ std::chrono::steady_clock::time_point::duration{last_sync.load()} }; - if (now - start < config::refresh_interval) + if (now - start < config::refresh_interval_min) return {}; } expect outs_response{common_error::kInvalidArgument}; + boost::unique_lock lock_refresh{sync_refresh}; boost::unique_lock lock{sync}; const auto now = std::chrono::steady_clock::now(); if (!mandatory) // double-check @@ -605,10 +625,9 @@ namespace lwsf { namespace internal { namespace backend const std::chrono::steady_clock::time_point start{ std::chrono::steady_clock::time_point::duration{last_sync.load()} }; - if (now - start < config::refresh_interval) + if (now - start < config::refresh_interval_min) return {}; } - last_sync = now.time_since_epoch().count(); const rpc::login login{primary.address, primary.view.sec}; lock.unlock(); @@ -625,10 +644,11 @@ namespace lwsf { namespace internal { namespace backend return outs_response.error(); lock.lock(); + last_sync = now.time_since_epoch().count(); const std::uint64_t orig_scan_height = primary.scan_height; const auto new_txes = merge_response(*this, *txs_response, *outs_response); - const boost::lock_guard lock2{sync_listener}; + const boost::lock_guard lock_listener{sync_listener}; if (!listener) return {}; @@ -637,6 +657,7 @@ namespace lwsf { namespace internal { namespace backend const std::uint64_t new_scan_height = std::max(orig_scan_height, primary.scan_height); lock.unlock(); + lock_refresh.unlock(); listener->refreshed(); if (!new_txes.empty() || new_scan_height - orig_scan_height) diff --git a/src/backend.h b/src/backend.h index 435d7c8..38c5030 100644 --- a/src/backend.h +++ b/src/backend.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -135,7 +136,8 @@ namespace lwsf { namespace internal { namespace backend payment_id(), id{}, prefix{}, - coinbase(false) + coinbase(false), + failed(false) {} transaction(transaction&&) = default; @@ -161,6 +163,7 @@ namespace lwsf { namespace internal { namespace backend crypto::hash id; crypto::hash prefix; bool coinbase; + bool failed; }; WIRE_DECLARE_OBJECT(transaction); @@ -180,8 +183,8 @@ namespace lwsf { namespace internal { namespace backend std::string language; std::vector addressbook; std::map subaccounts; - boost::container::flat_map, memory> txes; - std::map attributes; + std::unordered_map> txes; + std::unordered_map attributes; std::uint64_t scan_height; std::uint64_t restore_height; Monero::NetworkType type; @@ -202,6 +205,7 @@ namespace lwsf { namespace internal { namespace backend std::uint64_t fee_mask; mutable boost::mutex sync; boost::mutex sync_listener; + boost::mutex sync_refresh; wallet(); diff --git a/src/ledger_main.cpp b/src/ledger_main.cpp index 75939ed..86c1b03 100644 --- a/src/ledger_main.cpp +++ b/src/ledger_main.cpp @@ -190,6 +190,7 @@ namespace if (!prog.lws_url.empty()) { + wallet->setAutoRefreshInterval(0); // do not start background thread if (!wallet->init(prog.lws_url, 0, "", "", (prog.lws_url.substr(0, 8) == "https://"), true) || !wallet->refresh()) throw std::runtime_error{"Failed to refresh wallet: " + wallet->errorString()}; } @@ -204,7 +205,7 @@ namespace std::cout << "; Blockchain Height: " << wallet->daemonBlockChainHeight() << std::endl; std::cout << "; Start Height: " << wallet->getRefreshFromBlockHeight() << std::endl; std::cout << "; Scan Height: " << wallet->blockChainHeight() << std::endl; - std::cout << "; Accounts: " << wallet->numSubaddressAccounts() << std::endl << std::endl; + std::cout << "; Accounts: " << wallet->numSubaddressAccounts() << std::endl; Monero::TransactionHistory* history = wallet->history(); if (!history) @@ -212,6 +213,9 @@ namespace history->refresh(); const std::vector& info_list = history->getAll(); + + std::cout << "; Transactions: " << info_list.size() << std::endl << std::endl; + for (const Monero::TransactionInfo* info : info_list) { if (!info) diff --git a/src/lwsf_config.h b/src/lwsf_config.h index c1655b6..7020881 100644 --- a/src/lwsf_config.h +++ b/src/lwsf_config.h @@ -36,6 +36,7 @@ namespace lwsf { namespace config constexpr const std::size_t max_txes_in_rpc = 2048; constexpr const std::uint32_t mixin_default = 16; constexpr const std::chrono::seconds refresh_interval{30}; + constexpr const std::chrono::seconds refresh_interval_min{5}; constexpr const std::chrono::seconds rpc_timeout{5}; constexpr const unsigned subaddr_retry{10}; }} // lwsf // config diff --git a/src/transaction_info.h b/src/transaction_info.h index 7fa5681..74901f1 100644 --- a/src/transaction_info.h +++ b/src/transaction_info.h @@ -60,7 +60,7 @@ namespace lwsf { namespace internal virtual int direction() const override { return data_->direction; } virtual bool isPending() const override { return !data_->height; } - virtual bool isFailed() const override { return false; } + virtual bool isFailed() const override { return data_->failed; } virtual bool isCoinbase() const override { return data_->coinbase; } virtual std::uint64_t amount() const override { return data_->amount; } virtual std::uint64_t fee() const override { return data_->fee; } diff --git a/src/wallet.cpp b/src/wallet.cpp index 3a8a6a3..cd213e5 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -359,8 +359,9 @@ namespace lwsf { namespace internal void wallet::stop_refresh_loop() { + const boost::lock_guard lock{thread_sync_}; { - const boost::lock_guard lock{refresh_sync_}; + const boost::lock_guard lock2{refresh_sync_}; thread_state_ = state::stop; } refresh_notify_.notify_all(); @@ -370,10 +371,19 @@ namespace lwsf { namespace internal void wallet::refresh_loop() { + struct set_stop_ + { + void operator()(state* val) const noexcept + { + if (val) + *val = state::stop; + } + }; try { boost::unique_lock lock{refresh_sync_}; - while (thread_state_ != state::stop) + const std::unique_ptr set_stop{std::addressof(thread_state_)}; + while (mandatory_refresh_ || thread_state_ != state::stop) { const bool mandatory_refresh = mandatory_refresh_; mandatory_refresh_ = false; @@ -464,6 +474,7 @@ namespace lwsf { namespace internal refresh_notify_(), error_sync_(), refresh_sync_(), + thread_sync_(), thread_state_(state::stop), mandatory_refresh_(false) { @@ -494,6 +505,7 @@ namespace lwsf { namespace internal refresh_notify_(), error_sync_(), refresh_sync_(), + thread_sync_(), thread_state_(state::stop), mandatory_refresh_(false) { @@ -552,6 +564,7 @@ namespace lwsf { namespace internal refresh_notify_(), error_sync_(), refresh_sync_(), + thread_sync_(), thread_state_(state::stop), mandatory_refresh_(false) { @@ -711,16 +724,12 @@ namespace lwsf { namespace internal const std::filesystem::path directory = std::filesystem::path{real_path}.remove_filename(); - if (std::filesystem::exists(new_file, status)) - std::filesystem::remove(file, status); - std::filesystem::rename(new_file, file, status); - + if (std::filesystem::exists(new_file)) + std::filesystem::rename(new_file, file, status); + // blocks until file and directory contents are synced if (atomic_file_write(new_file, directory, epee::byte_slice{std::move(buffer)})) - { - std::filesystem::remove(file, status); std::filesystem::rename(new_file, file, status); - } else status = error::write_failure; } @@ -786,8 +795,7 @@ namespace lwsf { namespace internal if (!connectToDaemon()) return false; - thread_state_ = state::run; - thread_ = boost::thread{[this] () { this->refresh_loop(); }}; + startRefresh(); rollback.release(); } catch (const std::exception& e) @@ -930,9 +938,26 @@ namespace lwsf { namespace internal void wallet::startRefresh() { + const boost::lock_guard lock{thread_sync_}; { - const boost::lock_guard lock{refresh_sync_}; - thread_state_ = state::run; + const boost::lock_guard lock2{refresh_sync_}; + const bool no_refresh = + refresh_interval_ <= std::chrono::milliseconds{0}; + if (no_refresh) + { + thread_state_ = state::stop; + if (!mandatory_refresh_) + return; + } + + const state old_state = thread_state_; + thread_state_ = no_refresh ? state::stop : state::run; + if (old_state == state::stop) + { + if (thread_.joinable()) + thread_.join(); + thread_ = boost::thread{[this] () { this->refresh_loop(); }}; + } } refresh_notify_.notify_all(); } @@ -940,12 +965,15 @@ namespace lwsf { namespace internal void wallet::pauseRefresh() { const boost::lock_guard lock{refresh_sync_}; - thread_state_ = state::paused; + if (thread_state_ != state::stop) + thread_state_ = state::paused; } bool wallet::refresh() { - return set_error(data_->refresh(false)); + try { return set_error(data_->refresh(true)); } + catch (const std::exception& e) { set_critical(e); } + return false; } void wallet::refreshAsync() @@ -954,25 +982,22 @@ namespace lwsf { namespace internal const boost::lock_guard lock{refresh_sync_}; mandatory_refresh_ = true; } - refresh_notify_.notify_all(); + startRefresh(); } void wallet::setAutoRefreshInterval(int millis) { using rep_type = std::chrono::milliseconds::rep; static_assert(std::numeric_limits::max() <= std::numeric_limits::max()); - if (0 < millis) + + boost::unique_lock lock{refresh_sync_}; + refresh_interval_ = std::chrono::milliseconds{millis}; + if (millis <= 0) { - const boost::lock_guard lock{refresh_sync_}; - refresh_interval_ = std::chrono::milliseconds{millis}; - if (thread_state_ == state::stop) - { - thread_state_ = state::run; - thread_ = boost::thread{[this] () { this->refresh_loop(); }}; - } + thread_state_ = state::stop; + lock.unlock(); + stop_refresh_loop(); } - else - return stop_refresh_loop(); } int wallet::autoRefreshInterval() const diff --git a/src/wallet.h b/src/wallet.h index 15d93ba..78ef259 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -66,7 +66,8 @@ namespace internal std::uint32_t mixin_; boost::condition_variable refresh_notify_; mutable boost::mutex error_sync_; - boost::mutex refresh_sync_; + boost::mutex refresh_sync_; //!< Synchronizes with requests from other threads + boost::mutex thread_sync_; //!< Syncrhonizes thread destruction+creation state thread_state_; bool mandatory_refresh_;