From bb9094f1fe8184d3b227a74e03736d252aab159d Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 4 Oct 2016 23:11:19 +0300 Subject: [PATCH 1/8] libwallet_api: fixes for transaction history --- src/wallet/api/transaction_history.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 95aafb04f..86dff85a4 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -65,7 +65,11 @@ int TransactionHistoryImpl::count() const TransactionInfo *TransactionHistoryImpl::transaction(const std::string &id) const { - return nullptr; + auto itr = std::find_if(m_history.begin(), m_history.end(), + [&](const TransactionInfo * ti) { + return ti->hash() == id; + }); + return itr != m_history.end() ? *itr : nullptr; } std::vector TransactionHistoryImpl::getAll() const @@ -109,7 +113,7 @@ void TransactionHistoryImpl::refresh() ti->m_hash = string_tools::pod_to_hex(pd.m_tx_hash); ti->m_blockheight = pd.m_block_height; // TODO: - // ti->m_timestamp = pd.m_timestamp; + ti->m_timestamp = pd.m_timestamp; m_history.push_back(ti); /* output.insert(std::make_pair(pd.m_block_height, std::make_pair(true, (boost::format("%20.20s %s %s %s") @@ -151,6 +155,7 @@ void TransactionHistoryImpl::refresh() ti->m_direction = TransactionInfo::Direction_Out; ti->m_hash = string_tools::pod_to_hex(hash); ti->m_blockheight = pd.m_block_height; + ti->m_timestamp = pd.m_timestamp; // single output transaction might contain multiple transfers for (const auto &d: pd.m_dests) { @@ -180,6 +185,7 @@ void TransactionHistoryImpl::refresh() ti->m_failed = is_failed; ti->m_pending = true; ti->m_hash = string_tools::pod_to_hex(hash); + ti->m_timestamp = pd.m_timestamp; m_history.push_back(ti); } @@ -187,7 +193,11 @@ void TransactionHistoryImpl::refresh() TransactionInfo *TransactionHistoryImpl::transaction(int index) const { - return nullptr; + // sanity check + if (index < 0) + return nullptr; + unsigned index_ = static_cast(index); + return index_ < m_history.size() ? m_history[index_] : nullptr; } } From 85f5e73d9cb30b59c131fd4297e188fdf0225283 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 4 Oct 2016 23:11:19 +0300 Subject: [PATCH 2/8] libwallet_api: fixes for transaction history --- src/wallet/api/transaction_history.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 95aafb04f..86dff85a4 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -65,7 +65,11 @@ int TransactionHistoryImpl::count() const TransactionInfo *TransactionHistoryImpl::transaction(const std::string &id) const { - return nullptr; + auto itr = std::find_if(m_history.begin(), m_history.end(), + [&](const TransactionInfo * ti) { + return ti->hash() == id; + }); + return itr != m_history.end() ? *itr : nullptr; } std::vector TransactionHistoryImpl::getAll() const @@ -109,7 +113,7 @@ void TransactionHistoryImpl::refresh() ti->m_hash = string_tools::pod_to_hex(pd.m_tx_hash); ti->m_blockheight = pd.m_block_height; // TODO: - // ti->m_timestamp = pd.m_timestamp; + ti->m_timestamp = pd.m_timestamp; m_history.push_back(ti); /* output.insert(std::make_pair(pd.m_block_height, std::make_pair(true, (boost::format("%20.20s %s %s %s") @@ -151,6 +155,7 @@ void TransactionHistoryImpl::refresh() ti->m_direction = TransactionInfo::Direction_Out; ti->m_hash = string_tools::pod_to_hex(hash); ti->m_blockheight = pd.m_block_height; + ti->m_timestamp = pd.m_timestamp; // single output transaction might contain multiple transfers for (const auto &d: pd.m_dests) { @@ -180,6 +185,7 @@ void TransactionHistoryImpl::refresh() ti->m_failed = is_failed; ti->m_pending = true; ti->m_hash = string_tools::pod_to_hex(hash); + ti->m_timestamp = pd.m_timestamp; m_history.push_back(ti); } @@ -187,7 +193,11 @@ void TransactionHistoryImpl::refresh() TransactionInfo *TransactionHistoryImpl::transaction(int index) const { - return nullptr; + // sanity check + if (index < 0) + return nullptr; + unsigned index_ = static_cast(index); + return index_ < m_history.size() ? m_history[index_] : nullptr; } } From db3282cdf00aae69fdd38680b4f58999d8c5d34a Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Wed, 5 Oct 2016 19:01:26 +0300 Subject: [PATCH 3/8] Initialize transaction history if empty --- src/wallet/api/transaction_history.cpp | 5 +++-- src/wallet/api/transaction_history.h | 2 ++ src/wallet/api/wallet.cpp | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 86dff85a4..190aa67c5 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -79,6 +79,9 @@ std::vector TransactionHistoryImpl::getAll() const void TransactionHistoryImpl::refresh() { + // multithreaded access: + boost::lock_guard guarg(m_refreshMutex); + // TODO: configurable values; uint64_t min_height = 0; uint64_t max_height = (uint64_t)-1; @@ -88,8 +91,6 @@ void TransactionHistoryImpl::refresh() delete t; m_history.clear(); - - // transactions are stored in wallet2: // - confirmed_transfer_details - out transfers // - unconfirmed_transfer_details - pending out transfers diff --git a/src/wallet/api/transaction_history.h b/src/wallet/api/transaction_history.h index 171fd2210..0b7e079b9 100644 --- a/src/wallet/api/transaction_history.h +++ b/src/wallet/api/transaction_history.h @@ -29,6 +29,7 @@ // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers #include "wallet/wallet2_api.h" +#include namespace Bitmonero { @@ -51,6 +52,7 @@ private: // TransactionHistory is responsible of memory management std::vector m_history; WalletImpl *m_wallet; + boost::mutex m_refreshMutex; }; } diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 4d35bc404..eb6fe5db2 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -723,6 +723,12 @@ void WalletImpl::doRefresh() boost::lock_guard guarg(m_refreshMutex2); try { m_wallet->refresh(); + // assuming if we have empty history, it wasn't initialized yet + // for futher history changes client need to update history in + // "on_money_received" and "on_money_sent" callbacks + if (m_history->count() == 0) { + m_history->refresh(); + } } catch (const std::exception &e) { m_status = Status_Error; m_errorString = e.what(); From 8b0cb8caa44b9e985611b72ba956c170e6eea64a Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Thu, 6 Oct 2016 15:45:43 +0300 Subject: [PATCH 4/8] libwallet_api: some renamings --- src/wallet/api/transaction_history.cpp | 3 ++- src/wallet/api/transaction_history.h | 2 +- src/wallet/api/wallet.cpp | 1 - tests/libwallet_api_tests/main.cpp | 14 ++++++++------ 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 190aa67c5..5f99750d5 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -60,6 +60,7 @@ TransactionHistoryImpl::~TransactionHistoryImpl() int TransactionHistoryImpl::count() const { + boost::lock_guard guarg(m_historyMutex); return m_history.size(); } @@ -80,7 +81,7 @@ std::vector TransactionHistoryImpl::getAll() const void TransactionHistoryImpl::refresh() { // multithreaded access: - boost::lock_guard guarg(m_refreshMutex); + boost::lock_guard guarg(m_historyMutex); // TODO: configurable values; uint64_t min_height = 0; diff --git a/src/wallet/api/transaction_history.h b/src/wallet/api/transaction_history.h index 0b7e079b9..04db7e8e1 100644 --- a/src/wallet/api/transaction_history.h +++ b/src/wallet/api/transaction_history.h @@ -52,7 +52,7 @@ private: // TransactionHistory is responsible of memory management std::vector m_history; WalletImpl *m_wallet; - boost::mutex m_refreshMutex; + mutable boost::mutex m_historyMutex; }; } diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index eb6fe5db2..5150c3640 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -201,7 +201,6 @@ bool WalletImpl::create(const std::string &path, const std::string &password, co bool keys_file_exists; bool wallet_file_exists; tools::wallet2::wallet_exists(path, keys_file_exists, wallet_file_exists); - // TODO: figure out how to setup logger; LOG_PRINT_L3("wallet_path: " << path << ""); LOG_PRINT_L3("keys_file_exists: " << std::boolalpha << keys_file_exists << std::noboolalpha << " wallet_file_exists: " << std::boolalpha << wallet_file_exists << std::noboolalpha); diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index 45a94b4dd..0fc0dd028 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -539,8 +539,10 @@ TEST_F(WalletTest1, WalletReturnsDaemonBlockHeight) TEST_F(WalletTest1, WalletRefresh) { + std::cout << "Opening wallet: " << CURRENT_SRC_WALLET << std::endl; Bitmonero::Wallet * wallet1 = wmgr->openWallet(CURRENT_SRC_WALLET, TESTNET_WALLET_PASS, true); // make sure testnet daemon is running + std::cout << "connecting to daemon: " << TESTNET_DAEMON_ADDRESS << std::endl; ASSERT_TRUE(wallet1->init(TESTNET_DAEMON_ADDRESS, 0)); ASSERT_TRUE(wallet1->refresh()); ASSERT_TRUE(wmgr->closeWallet(wallet1)); @@ -570,13 +572,13 @@ TEST_F(WalletTest1, WalletTransaction) ASSERT_TRUE(wallet1->status() == Bitmonero::PendingTransaction::Status_Ok); std::string recepient_address = Utils::get_wallet_address(CURRENT_DST_WALLET, TESTNET_WALLET_PASS); - wallet1->setDefaultMixin(1); - ASSERT_TRUE(wallet1->defaultMixin() == 1); + wallet1->setDefaultMixin(10); + ASSERT_TRUE(wallet1->defaultMixin() == 10); Bitmonero::PendingTransaction * transaction = wallet1->createTransaction(recepient_address, PAYMENT_ID_EMPTY, AMOUNT_10XMR, - 1); + 2); ASSERT_TRUE(transaction->status() == Bitmonero::PendingTransaction::Status_Ok); wallet1->refresh(); @@ -1146,10 +1148,10 @@ int main(int argc, char** argv) TESTNET_WALLET5_NAME = WALLETS_ROOT_DIR + "/wallet_05.bin"; TESTNET_WALLET6_NAME = WALLETS_ROOT_DIR + "/wallet_06.bin"; - CURRENT_SRC_WALLET = TESTNET_WALLET6_NAME; - CURRENT_DST_WALLET = TESTNET_WALLET5_NAME; + CURRENT_SRC_WALLET = TESTNET_WALLET5_NAME; + CURRENT_DST_WALLET = TESTNET_WALLET1_NAME; ::testing::InitGoogleTest(&argc, argv); - // Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_Max); + Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_Max); return RUN_ALL_TESTS(); } From 559f37932749bded851cdd268a49fee3694d2cd4 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Thu, 6 Oct 2016 16:35:03 +0300 Subject: [PATCH 5/8] libwallet_api: test: adjusted mixin_count=4 as it's minumum allowed --- tests/libwallet_api_tests/main.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index 0fc0dd028..0468cf40a 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -572,13 +572,14 @@ TEST_F(WalletTest1, WalletTransaction) ASSERT_TRUE(wallet1->status() == Bitmonero::PendingTransaction::Status_Ok); std::string recepient_address = Utils::get_wallet_address(CURRENT_DST_WALLET, TESTNET_WALLET_PASS); - wallet1->setDefaultMixin(10); - ASSERT_TRUE(wallet1->defaultMixin() == 10); + const int MIXIN_COUNT = 4; + Bitmonero::PendingTransaction * transaction = wallet1->createTransaction(recepient_address, PAYMENT_ID_EMPTY, AMOUNT_10XMR, - 2); + MIXIN_COUNT, + Bitmonero::PendingTransaction::Priority_Medium); ASSERT_TRUE(transaction->status() == Bitmonero::PendingTransaction::Status_Ok); wallet1->refresh(); From 11fab41c3638e9368c5589c4092af0b0e29ff054 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Thu, 6 Oct 2016 23:25:43 +0300 Subject: [PATCH 6/8] libwallet_api: TransactionHistory: read/write syncchronization --- src/wallet/api/transaction_history.cpp | 31 ++++++++++++++++---------- src/wallet/api/transaction_history.h | 4 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 5f99750d5..df495591f 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -60,12 +60,24 @@ TransactionHistoryImpl::~TransactionHistoryImpl() int TransactionHistoryImpl::count() const { - boost::lock_guard guarg(m_historyMutex); - return m_history.size(); + boost::shared_lock lock(m_historyMutex); + int result = m_history.size(); + return result; +} + +TransactionInfo *TransactionHistoryImpl::transaction(int index) const +{ + boost::shared_lock lock(m_historyMutex); + // sanity check + if (index < 0) + return nullptr; + unsigned index_ = static_cast(index); + return index_ < m_history.size() ? m_history[index_] : nullptr; } TransactionInfo *TransactionHistoryImpl::transaction(const std::string &id) const { + boost::shared_lock lock(m_historyMutex); auto itr = std::find_if(m_history.begin(), m_history.end(), [&](const TransactionInfo * ti) { return ti->hash() == id; @@ -75,13 +87,16 @@ TransactionInfo *TransactionHistoryImpl::transaction(const std::string &id) cons std::vector TransactionHistoryImpl::getAll() const { + boost::shared_lock lock(m_historyMutex); return m_history; } void TransactionHistoryImpl::refresh() { // multithreaded access: - boost::lock_guard guarg(m_historyMutex); + // boost::lock_guard guarg(m_historyMutex); + // for "write" access, locking exclusively + boost::unique_lock lock(m_historyMutex); // TODO: configurable values; uint64_t min_height = 0; @@ -190,16 +205,8 @@ void TransactionHistoryImpl::refresh() ti->m_timestamp = pd.m_timestamp; m_history.push_back(ti); } - } -TransactionInfo *TransactionHistoryImpl::transaction(int index) const -{ - // sanity check - if (index < 0) - return nullptr; - unsigned index_ = static_cast(index); - return index_ < m_history.size() ? m_history[index_] : nullptr; -} + } diff --git a/src/wallet/api/transaction_history.h b/src/wallet/api/transaction_history.h index 04db7e8e1..1b6617ead 100644 --- a/src/wallet/api/transaction_history.h +++ b/src/wallet/api/transaction_history.h @@ -29,7 +29,7 @@ // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers #include "wallet/wallet2_api.h" -#include +#include namespace Bitmonero { @@ -52,7 +52,7 @@ private: // TransactionHistory is responsible of memory management std::vector m_history; WalletImpl *m_wallet; - mutable boost::mutex m_historyMutex; + mutable boost::shared_mutex m_historyMutex; }; } From 62b3708ea5b6a2ee45491f632d2c753eae8a00ee Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Fri, 7 Oct 2016 00:29:13 +0300 Subject: [PATCH 7/8] libwallet_api: do not signal on sent/received tx until wallet completely synchronized --- src/wallet/api/wallet.cpp | 28 ++++++++++++++++++++++------ src/wallet/api/wallet.h | 3 +++ src/wallet/wallet2_api.h | 6 ++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 5150c3640..a93e194b9 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -54,8 +54,9 @@ namespace { struct Wallet2CallbackImpl : public tools::i_wallet2_callback { - Wallet2CallbackImpl() + Wallet2CallbackImpl(WalletImpl * wallet) : m_listener(nullptr) + , m_wallet(wallet) { } @@ -93,7 +94,8 @@ struct Wallet2CallbackImpl : public tools::i_wallet2_callback LOG_PRINT_L3(__FUNCTION__ << ": money received. height: " << height << ", tx: " << tx_hash << ", amount: " << print_money(amount)); - if (m_listener) { + // do not signal on received tx if wallet is not syncronized completely + if (m_listener && m_wallet->synchronized()) { m_listener->moneyReceived(tx_hash, amount); m_listener->updated(); } @@ -107,7 +109,8 @@ struct Wallet2CallbackImpl : public tools::i_wallet2_callback LOG_PRINT_L3(__FUNCTION__ << ": money spent. height: " << height << ", tx: " << tx_hash << ", amount: " << print_money(amount)); - if (m_listener) { + // do not signal on sent tx if wallet is not syncronized completely + if (m_listener && m_wallet->synchronized()) { m_listener->moneySpent(tx_hash, amount); m_listener->updated(); } @@ -119,6 +122,7 @@ struct Wallet2CallbackImpl : public tools::i_wallet2_callback } WalletListener * m_listener; + WalletImpl * m_wallet; }; Wallet::~Wallet() {} @@ -166,12 +170,16 @@ uint64_t Wallet::maximumAllowedAmount() ///////////////////////// WalletImpl implementation //////////////////////// WalletImpl::WalletImpl(bool testnet) - :m_wallet(nullptr), m_status(Wallet::Status_Ok), m_trustedDaemon(false), - m_wallet2Callback(nullptr), m_recoveringFromSeed(false) + :m_wallet(nullptr) + , m_status(Wallet::Status_Ok) + , m_trustedDaemon(false) + , m_wallet2Callback(nullptr) + , m_recoveringFromSeed(false) + , m_synchronized(false) { m_wallet = new tools::wallet2(testnet); m_history = new TransactionHistoryImpl(this); - m_wallet2Callback = new Wallet2CallbackImpl; + m_wallet2Callback = new Wallet2CallbackImpl(this); m_wallet->callback(m_wallet2Callback); m_refreshThreadDone = false; m_refreshEnabled = false; @@ -454,6 +462,11 @@ uint64_t WalletImpl::daemonBlockChainTargetHeight() const return result; } +bool WalletImpl::synchronized() const +{ + return m_synchronized; +} + bool WalletImpl::refresh() { clearStatus(); @@ -722,6 +735,9 @@ void WalletImpl::doRefresh() boost::lock_guard guarg(m_refreshMutex2); try { m_wallet->refresh(); + if (!m_synchronized) { + m_synchronized = true; + } // assuming if we have empty history, it wasn't initialized yet // for futher history changes client need to update history in // "on_money_received" and "on_money_sent" callbacks diff --git a/src/wallet/api/wallet.h b/src/wallet/api/wallet.h index c399e3ab6..98a5f2d09 100644 --- a/src/wallet/api/wallet.h +++ b/src/wallet/api/wallet.h @@ -78,6 +78,7 @@ public: uint64_t blockChainHeight() const; uint64_t daemonBlockChainHeight() const; uint64_t daemonBlockChainTargetHeight() const; + bool synchronized() const; bool refresh(); void refreshAsync(); void setAutoRefreshInterval(int millis); @@ -108,6 +109,7 @@ private: private: friend class PendingTransactionImpl; friend class TransactionHistoryImpl; + friend class Wallet2CallbackImpl; tools::wallet2 * m_wallet; mutable std::atomic m_status; @@ -133,6 +135,7 @@ private: // so it shouldn't be considered as new and pull blocks (slow-refresh) // instead of pulling hashes (fast-refresh) bool m_recoveringFromSeed; + std::atomic m_synchronized; }; diff --git a/src/wallet/wallet2_api.h b/src/wallet/wallet2_api.h index 8d5223006..934cc6e38 100644 --- a/src/wallet/wallet2_api.h +++ b/src/wallet/wallet2_api.h @@ -255,6 +255,12 @@ struct Wallet */ virtual uint64_t daemonBlockChainTargetHeight() const = 0; + /** + * @brief synchronized - checks if wallet was ever synchronized + * @return + */ + virtual bool synchronized() const = 0; + static std::string displayAmount(uint64_t amount); static uint64_t amountFromString(const std::string &amount); static uint64_t amountFromDouble(double amount); From 697ce1d435d8751bd14333165c4c68f13c5c6d16 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Fri, 7 Oct 2016 00:43:45 +0300 Subject: [PATCH 8/8] libwallet_api: reverted deleted curly brace --- src/wallet/api/transaction_history.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index fa939c08c..e4a003b02 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/src/wallet/api/transaction_history.cpp @@ -207,3 +207,4 @@ void TransactionHistoryImpl::refresh() } } +} // namespace