Merge pull request #1195

697ce1d libwallet_api: reverted deleted curly brace (Ilya Kitaev)
bb9094f libwallet_api: fixes for transaction history (Ilya Kitaev)
62b3708 libwallet_api: do not signal on sent/received tx until wallet completely synchronized (Ilya Kitaev)
11fab41 libwallet_api: TransactionHistory: read/write syncchronization (Ilya Kitaev)
559f379 libwallet_api: test: adjusted mixin_count=4 as it's minumum allowed (Ilya Kitaev)
8b0cb8c libwallet_api: some renamings (Ilya Kitaev)
db3282c Initialize transaction history if empty (Ilya Kitaev)
85f5e73 libwallet_api: fixes for transaction history (Ilya Kitaev)

src/wallet/api/transaction_history.cpp

@@ -60,21 +60,44 @@ TransactionHistoryImpl::~TransactionHistoryImpl()
 
 int TransactionHistoryImpl::count() const
 {
-    return m_history.size();
+    boost::shared_lock<boost::shared_mutex> lock(m_historyMutex);
+    int result = m_history.size();
+    return result;
+}
+
+TransactionInfo *TransactionHistoryImpl::transaction(int index) const
+{
+    boost::shared_lock<boost::shared_mutex> lock(m_historyMutex);
+    // sanity check
+    if (index < 0)
+        return nullptr;
+    unsigned index_ = static_cast<unsigned>(index);
+    return index_ < m_history.size() ? m_history[index_] : nullptr;
 }
 
 TransactionInfo *TransactionHistoryImpl::transaction(const std::string &id) const
 {
-    return nullptr;
+    boost::shared_lock<boost::shared_mutex> lock(m_historyMutex);
+    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<TransactionInfo *> TransactionHistoryImpl::getAll() const
 {
+    boost::shared_lock<boost::shared_mutex> lock(m_historyMutex);
     return m_history;
 }
 
 void TransactionHistoryImpl::refresh()
 {
+    // multithreaded access:
+    // boost::lock_guard<boost::mutex> guarg(m_historyMutex);
+    // for "write" access, locking exclusively
+    boost::unique_lock<boost::shared_mutex> lock(m_historyMutex);
+
     // TODO: configurable values;
     uint64_t min_height = 0;
     uint64_t max_height = (uint64_t)-1;
@@ -84,8 +107,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
@@ -109,7 +130,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 +172,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,14 +202,9 @@ 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);
     }
-
-}
-
-TransactionInfo *TransactionHistoryImpl::transaction(int index) const
-{
-    return nullptr;
 }
 
-}
+} // namespace

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 <boost/thread/shared_mutex.hpp>
 
 namespace Bitmonero {
 
@@ -51,6 +52,7 @@ private:
     // TransactionHistory is responsible of memory management
     std::vector<TransactionInfo*> m_history;
     WalletImpl *m_wallet;
+    mutable boost::shared_mutex   m_historyMutex;
 };
 
 }

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_wallet(nullptr)
-      m_wallet2Callback(nullptr), m_recoveringFromSeed(false)
+    , 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;
@@ -201,7 +209,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);
@@ -459,6 +466,11 @@ uint64_t WalletImpl::daemonBlockChainTargetHeight() const
     return result;
 }
 
+bool WalletImpl::synchronized() const
+{
+    return m_synchronized;
+}
+
 bool WalletImpl::refresh()
 {
     clearStatus();
@@ -727,6 +739,15 @@ void WalletImpl::doRefresh()
     boost::lock_guard<boost::mutex> 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
+        if (m_history->count() == 0) {
+            m_history->refresh();
+        }
     } catch (const std::exception &e) {
         m_status = Status_Error;
         m_errorString = e.what();

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);
@@ -109,6 +110,7 @@ private:
 private:
     friend class PendingTransactionImpl;
     friend class TransactionHistoryImpl;
+    friend class Wallet2CallbackImpl;
 
     tools::wallet2 * m_wallet;
     mutable std::atomic<int>  m_status;
@@ -134,6 +136,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<bool>   m_synchronized;
 };
 
 

src/wallet/wallet2_api.h

@@ -262,6 +262,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);

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,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(1);
+    const int MIXIN_COUNT = 4;
-    ASSERT_TRUE(wallet1->defaultMixin() == 1);
+
 
     Bitmonero::PendingTransaction * transaction = wallet1->createTransaction(recepient_address,
                                                                              PAYMENT_ID_EMPTY,
                                                                              AMOUNT_10XMR,
-                                                                             1);
+                                                                             MIXIN_COUNT,
+                                                                             Bitmonero::PendingTransaction::Priority_Medium);
     ASSERT_TRUE(transaction->status() == Bitmonero::PendingTransaction::Status_Ok);
     wallet1->refresh();
 
@@ -1146,10 +1149,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_SRC_WALLET = TESTNET_WALLET5_NAME;
-    CURRENT_DST_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();
 }