select_txs_for_account_spendability_check tests added

main.cpp

@@ -141,6 +141,11 @@ if (!xmreg::get_blockchain_path(blockchain_path, nettype))
 bc_setup.blockchain_path         =  blockchain_path.string();
 bc_setup.deamon_url              = deamon_url;
 
+if (!bc_setup.parse_addr_and_viewkey())
+{
+    return EXIT_FAILURE;
+}
+
 cout << "Using blockchain path: " << blockchain_path.string() << endl;
 
 

src/BlockchainSetup.cpp

@@ -3,3 +3,38 @@
 //
 
 #include "BlockchainSetup.h"
+
+namespace xmreg
+{
+
+bool
+BlockchainSetup::parse_addr_and_viewkey()
+{
+    if (import_payment_address_str.empty() || import_payment_viewkey_str.empty())
+        return false;
+
+    if (!parse_str_address(
+            import_payment_address_str,
+            import_payment_address,
+            net_type))
+    {
+        cerr << "Cant parse address_str: "
+             << import_payment_address_str
+             << endl;
+        return false;
+    }
+
+    if (!xmreg::parse_str_secret_key(
+            import_payment_viewkey_str,
+            import_payment_viewkey))
+    {
+        cerr << "Cant parse the viewkey_str: "
+             << import_payment_viewkey_str
+             << endl;
+        return false;
+    }
+
+    return true;
+}
+
+}

src/BlockchainSetup.h

@@ -6,6 +6,7 @@
 #define OPENMONERO_BLOCKCHAINSETUP_H
 
 #include "monero_headers.h"
+#include "tools.h"
 
 #include <string>
 
@@ -46,6 +47,9 @@ public:
 
     address_parse_info import_payment_address;
     secret_key         import_payment_viewkey;
+
+    bool
+    parse_addr_and_viewkey();
 };
 
 }

src/CMakeLists.txt

@@ -15,7 +15,8 @@ set(SOURCE_FILES
 		rpccalls.cpp
 		OutputInputIdentification.cpp
 		version.h.in
-		BlockchainSetup.cpp)
+		BlockchainSetup.cpp
+		ThreadRAII.cpp)
 
 # make static library called libmyxrm
 # that we are going to link to

src/CurrentBlockchainStatus.cpp

@@ -31,30 +31,6 @@ CurrentBlockchainStatus::start_monitor_blockchain_thread()
 
     TxSearch::set_search_thread_life(bc_setup.search_thread_life_in_seconds);
 
-    if (!bc_setup.import_payment_address_str.empty() && !bc_setup.import_payment_viewkey_str.empty())
-    {
-        if (!xmreg::parse_str_address(
-                bc_setup.import_payment_address_str,
-                bc_setup.import_payment_address,
-                net_type))
-        {
-            cerr << "Cant parse address_str: "
-                 << bc_setup.import_payment_address_str
-                 << endl;
-            return;
-        }
-
-        if (!xmreg::parse_str_secret_key(
-                bc_setup.import_payment_viewkey_str,
-                bc_setup.import_payment_viewkey))
-        {
-            cerr << "Cant parse the viewkey_str: "
-                 << bc_setup.import_payment_viewkey_str
-                 << endl;
-            return;
-        }
-    }
-
     if (!is_running)
     {
         m_thread = std::thread{[this]()

src/MySqlAccounts.cpp

@@ -471,12 +471,8 @@ bool
 MySqlAccounts::select_txs_for_account_spendability_check(
         const uint64_t& account_id, vector<XmrTransaction>& txs)
 {
-    vector<XmrTransaction> txs_tmp;
 
-    if (!select(account_id, txs_tmp))
-        return false;
-
-    for (XmrTransaction& tx: txs_tmp)
+    for (auto it = txs.begin(); it != txs.end(); )
     {
         // first we check if txs stored in db are already spendable
         // it means if they are older than 10 blocks. If  yes,
@@ -484,6 +480,8 @@ MySqlAccounts::select_txs_for_account_spendability_check(
         // older than 10 blocks are permanent, i.e, they wont get
         // orphaned.
 
+        XmrTransaction& tx = *it;
+
         if (bool {tx.spendable} == false)
         {
 
@@ -491,16 +489,12 @@ MySqlAccounts::select_txs_for_account_spendability_check(
             {
                 // this tx was before marked as unspendable, but now
                 // it is spendable. Meaning, that its older than 10 blocks.
-                // so mark it as spendable, so that its permanet.
+                // so mark it as spendable in mysql, so that its permanet.
 
                 uint64_t no_row_updated = mark_tx_spendable(tx.id.data);
 
                 if (no_row_updated != 1)
                 {
-//                    throw runtime_error("no_row_updated != 1 "
-//                                                "due to "
-//                                                "xmr_accounts->mark_tx_spendable(tx.id)");
-
                     cerr << "no_row_updated != 1 due to  xmr_accounts->mark_tx_spendable(tx.id)\n";
                     return false;
                 }
@@ -528,9 +522,6 @@ MySqlAccounts::select_txs_for_account_spendability_check(
 
                     if (no_row_updated != 1)
                     {
-//                        throw runtime_error("no_row_updated != 1 "
-//                                                    "due to "
-//                                                    "xmr_accounts->delete_tx(tx.id)");
                         cerr << "no_row_updated != 1 due to  xmr_accounts->delete_tx(tx.id)\n";
                         return false;
                     }
@@ -539,11 +530,11 @@ MySqlAccounts::select_txs_for_account_spendability_check(
                     // we assume its back to mempool, and it will be rescanned
                     // by tx search thread once added again to some block.
 
+                    // so we remove it from txs vector
+                    it = txs.erase(it);
                     continue;
-
                 }
 
-
                 // set unlock_time field so that frontend displies it
                 // as a locked tx, if unlock_time is zero.
                 // coinbtase txs have this set already. regular tx
@@ -557,9 +548,9 @@ MySqlAccounts::select_txs_for_account_spendability_check(
 
         } // if (bool {tx.spendable} == false)
 
-        txs.push_back(tx);
+        ++it;
 
-    } //for (XmrTransaction& tx: txs_tmp)
+    } // for (auto it = txs.begin(); it != txs.end(); )
 
     return true;
 }
@@ -668,6 +659,13 @@ MySqlAccounts::get_connection()
     return conn;
 }
 
+
+void
+MySqlAccounts::set_bc_status_provider(shared_ptr<CurrentBlockchainStatus> bc_status_provider)
+{
+    current_bc_status = bc_status_provider;
+}
+
 void
 MySqlAccounts::_init()
 {

src/MySqlAccounts.h

@@ -235,6 +235,11 @@ public:
         return 0;
     }
 
+    // this is useful in unit tests, as we can inject mock CurrentBlockchainStatus
+    // after an instance of MySqlAccounts has been created.
+    void
+    set_bc_status_provider(shared_ptr<CurrentBlockchainStatus> bc_status_provider);
+
 private:
     void _init();
 };

src/ThreadRAII.cpp

@@ -0,0 +1,25 @@
+//
+// Created by mwo on 11/07/18.
+//
+
+#include "ThreadRAII.h"
+
+namespace xmreg
+{
+
+ThreadRAII::ThreadRAII(std::thread&& _t, DtorAction _action)
+: t {std::move(_t)}, action {_action}
+{}
+
+ThreadRAII::~ThreadRAII()
+{
+    if (t.joinable())
+    {
+        if (action == DtorAction::join)
+            t.join();
+        else
+            t.detach();
+    }
+}
+
+}

src/ThreadRAII.h

@@ -0,0 +1,35 @@
+//
+// Created by mwo on 11/07/18.
+//
+
+#ifndef OPENMONERO_PINGTHREAD_H
+#define OPENMONERO_PINGTHREAD_H
+
+#include <thread>
+
+namespace xmreg
+{
+
+// based on Mayer's ThreadRAII class (item 37)
+class ThreadRAII
+{
+public:
+    enum class DtorAction { join, detach};
+
+    ThreadRAII(std::thread&& _t, DtorAction _action);
+
+    ThreadRAII(ThreadRAII&&) = default;
+    ThreadRAII& operator=(ThreadRAII&&) = default;
+
+    std::thread& get() {return t;}
+
+    ~ThreadRAII();
+
+private:
+    std::thread t;
+    DtorAction action;
+};
+
+}
+
+#endif //OPENMONERO_PINGTHREAD_H

src/YourMoneroRequests.cpp

@@ -441,6 +441,12 @@ YourMoneroRequests::get_address_info(const shared_ptr< Session > session, const
 
         vector<XmrTransaction> txs;
 
+        // get all txs of for the account
+        xmr_accounts->select(acc.id.data, txs);
+
+        // now, filter out or updated transactions from txs vector that no
+        // longer exisit in the recent blocks. Update is done to check for their
+        // spendability status.
         if (xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs))
         {
             json j_spent_outputs = json::array();

tests/mysql_tests.cpp

@@ -1158,18 +1158,6 @@ TEST_F(MYSQL_TEST, UpdatePayment)
     EXPECT_EQ(payments2.at(0).tx_hash, updated_payment.tx_hash);
 }
 
-//
-//class MockCurrentBlockchainStatus : public xmreg::CurrentBlockchainStatus
-//{
-//public:
-//    MockCurrentBlockchainStatus()
-//            : xmreg::CurrentBlockchainStatus(xmreg::BlockchainSetup()) {}
-//
-//    MOCK_METHOD2(is_tx_unlocked, bool(uint64_t unlock_time, uint64_t block_height));
-//    MOCK_METHOD2(tx_exist, bool(crypto::hash const&, uint64_t& tx_index));
-//};
-//
-
 /*
  * Dont want to use real blockchain data, so we are going to
  * mock xmreg::CurrentBlockchainStatus
@@ -1180,92 +1168,297 @@ public:
     MockCurrentBlockchainStatus1()
             : xmreg::CurrentBlockchainStatus(xmreg::BlockchainSetup()) {}
 
+    bool tx_unlock_state {true};
+    bool tx_exist_state {true};
+
+    std::map<string, uint64_t> tx_exist_mock_data;
+
+    // all txs in the blockchain are unlocked
     virtual bool
     is_tx_unlocked(uint64_t unlock_time, uint64_t block_height) override
     {
-        return true;
+        return tx_unlock_state;
     }
 
+    // all ts in the blockchain exists
     virtual bool
     tx_exist(const string& tx_hash_str, uint64_t& tx_index) override
     {
+        if (tx_exist_mock_data.empty())
+            return tx_exist_state;
+
+        tx_index = tx_exist_mock_data[tx_hash_str];
+
         return true;
     }
 };
 
-// now mock when tx is locked
-class MockCurrentBlockchainStatus2 : public MockCurrentBlockchainStatus1
+class MYSQL_TEST2 : public MYSQL_TEST
 {
-public:
 
-    virtual bool
-    is_tx_unlocked(uint64_t unlock_time, uint64_t block_height) override
-    {
-        return false;
-    }
 };
 
+TEST_F(MYSQL_TEST2, SelectTxsIfAllAreSpendableAndExist)
+{
+    // if all txs selected for the given account are spendable
+    // the select_txs_for_account_spendability_check method
+    // should not change anything
+
+    // use mock CurrentBlockchainStatus instead of real object
+    // which would access the real blockchain.
+    auto mock_bc_status = make_shared<MockCurrentBlockchainStatus1>();
 
-// use MockCurrentBlockchainStatus
-template <typename T>
-class MYSQL_TEST2 : public MYSQL_TEST
+    xmr_accounts->set_bc_status_provider(mock_bc_status);
+
+    ACC_FROM_HEX(owner_addr_5Ajfk);
+
+    vector<xmreg::XmrTransaction> txs;
+
+    // we mark all txs for this account as spendable
+    // so that we have something to work with in this test
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    auto no_of_original_txs = txs.size();
+
+    for (size_t i = 0; i < txs.size(); ++i)
+        this->xmr_accounts->mark_tx_spendable(txs[i].id.data);
+
+    // reselect tx after they were marked as spendable
+    txs.clear();
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    EXPECT_TRUE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
+    // we check if non of the input txs got filtere out
+    EXPECT_EQ(txs.size(), no_of_original_txs);
+
+    // and we check if all of remained spendable
+    for (auto const& tx: txs)
+        EXPECT_TRUE(bool {tx.spendable});
+}
+
+
+
+TEST_F(MYSQL_TEST2, SelectTxsIfAllAreNonspendableButUnlockedAndExist)
 {
+    // if all txs selected for the given account are non-spendable
+    // the select_txs_for_account_spendability_check method
+    // will check their unlock time, and marked them spendable in
+    // they are unlocked.
+    // We are going to mock that all are unlocked
 
-    virtual void SetUp() override
-    {
-        current_bc_status = make_shared<T>();
-        connect(current_bc_status);
-        initDatabase();
-    }
+    // use mock CurrentBlockchainStatus instead of real object
+    // which would access the real blockchain.
+    auto mock_bc_status = make_shared<MockCurrentBlockchainStatus1>();
 
-};
+    xmr_accounts->set_bc_status_provider(mock_bc_status);
 
-typedef testing::Types<MockCurrentBlockchainStatus1, MockCurrentBlockchainStatus2> Implementations;
+    ACC_FROM_HEX(owner_addr_5Ajfk);
 
-TYPED_TEST_CASE(MYSQL_TEST2, Implementations);
+    vector<xmreg::XmrTransaction> txs;
 
-// Execute tests for two cases where CurrentBlockchainStatus::is_tx_unlocked
-// returns true or false
-//
-TYPED_TEST(MYSQL_TEST2, SelectTxsForAccountSpendabilityCheck)
+    // we mark all txs for this account as spendable
+    // so that we have something to work with in this test
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    auto no_of_original_txs = txs.size();
+
+    for (size_t i = 0; i < txs.size(); ++i)
+        this->xmr_accounts->mark_tx_nonspendable(txs[i].id.data);
+
+    // reselect tx after they were marked as spendable
+    txs.clear();
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    // and we check if all non-spendable before we fetch them into
+    // select_txs_for_account_spendability_check
+    for (auto const& tx: txs)
+        ASSERT_FALSE(bool {tx.spendable});
+
+    EXPECT_TRUE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
+    // we check if non of the input txs got filtere out
+    EXPECT_EQ(txs.size(), no_of_original_txs);
+
+
+    // reselect tx after they were marked as spendable
+    txs.clear();
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    // and we check if all of remained spendable
+    for (auto const& tx: txs)
+        ASSERT_TRUE(bool {tx.spendable});
+
+    // and we check if all of remained spendable
+    for (auto const& tx: txs)
+        EXPECT_TRUE(bool {tx.spendable});
+}
+
+
+
+TEST_F(MYSQL_TEST2, SelectTxsIfAllAreNonspendableLockedButExist)
 {
+    // if all txs selected for the given account are non-spendable
+    // the select_txs_for_account_spendability_check method
+    // will check if they are unlocked. In this thest all will be locked
+    // so new unlock_time for them is going to be set for them.
+    // all txs are set to exisit in the blockchain
+
+    ACC_FROM_HEX(owner_addr_5Ajfk);
 
     vector<xmreg::XmrTransaction> txs;
 
-    // first check if we select for account with any txs
-    EXPECT_FALSE(this->xmr_accounts->select_txs_for_account_spendability_check(55555, txs));
+    // we mark all txs for this account as spendable
+    // so that we have something to work with in this test
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    // use mock CurrentBlockchainStatus instead of real object
+    // which would access the real blockchain.
+    auto mock_bc_status = make_shared<MockCurrentBlockchainStatus1>();
+
+    // we mock that all txs are still locked and cant be spent
+    mock_bc_status->tx_unlock_state = false;
+
+    // now we need to populate mock_bc_status->tx_exist_mock_data map
+    // so that tx_exist mock works as if all txs existed
+    // and it returns correct/expected blockchain_tx_id
+    for(auto const& tx: txs)
+        mock_bc_status->tx_exist_mock_data[tx.hash] = tx.blockchain_tx_id;
+
+    // now set the mock_bc_status to be used by xmr_accounts later on
+    xmr_accounts->set_bc_status_provider(mock_bc_status);
 
+    auto no_of_original_txs = txs.size();
+
+    for (size_t i = 0; i < txs.size(); ++i)
+        this->xmr_accounts->mark_tx_nonspendable(txs[i].id.data);
+
+    // reselect tx after they were marked as spendable
     txs.clear();
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    // and we check if all non-spendable before we fetch them into
+    // select_txs_for_account_spendability_check
+    for (auto const& tx: txs)
+    {
+        ASSERT_FALSE(bool {tx.spendable});
+        if (!bool {tx.coinbase})
+            ASSERT_EQ(tx.unlock_time, 0);
+    }
+
+    EXPECT_TRUE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
+    // we check if non of the input txs got filtere out
+    EXPECT_EQ(txs.size(), no_of_original_txs);
+
+    // and we check if all of remained non-spendable and
+    // if their unlock time was modified as needed
+    for (auto const& tx: txs)
+    {
+        ASSERT_FALSE(bool {tx.spendable});
+        if (!bool {tx.coinbase})
+            ASSERT_EQ(tx.unlock_time, tx.height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE);
+    }
+}
+
+
+TEST_F(MYSQL_TEST2, SelectTxsIfAllAreNonspendableUnlockedAndDontExist)
+{
+    // if all txs selected for the given account are non-spendable,
+    // locked and dont exist in blockchain
+    // they should be filtered and removed from the mysql
 
-    // so now check for account that has some txs
+    // use mock CurrentBlockchainStatus instead of real object
+    // which would access the real blockchain.
+    auto mock_bc_status = make_shared<MockCurrentBlockchainStatus1>();
+
+    // all txs are locked and dont exisit in blockchain
+    mock_bc_status->tx_unlock_state = false;
+    mock_bc_status->tx_exist_state = false;
+
+    xmr_accounts->set_bc_status_provider(mock_bc_status);
 
     ACC_FROM_HEX(owner_addr_5Ajfk);
 
-    // we mark have of txs for this account as non_spendable
+    vector<xmreg::XmrTransaction> txs;
+
+    // we mark all txs for this account as nonspendable
     // so that we have something to work with in this test
     ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
 
-    for (size_t i = 0; i < txs.size() / 2; ++i)
+    auto no_of_original_txs = txs.size();
+
+    for (size_t i = 0; i < txs.size(); ++i)
         this->xmr_accounts->mark_tx_nonspendable(txs[i].id.data);
 
+    // reselect tx after they were marked as spendable
+    txs.clear();
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    // and we check if all non-spendable before we fetch them into
+    // select_txs_for_account_spendability_check
+    for (auto const& tx: txs)
+        ASSERT_FALSE(bool {tx.spendable});
+
     EXPECT_TRUE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
+    // after the call to select_txs_for_account_spendability_check
+    // all txs should be filted out
+    EXPECT_EQ(txs.size(), 0);
+
+    // all these txs should also be deleted from the mysql
+    txs.clear();
+    ASSERT_FALSE(this->xmr_accounts->select(acc.id.data, txs));
+    EXPECT_EQ(txs.size(), 0);
+}
+
+
+
+TEST_F(MYSQL_TEST2, SelectTxsFailureDueToSpendabilityAndTxDeletion)
+{
+    // if all txs selected for the given account are non-spendable,
+    // locked and dont exist in blockchain
+    // they should be filtered and removed from the mysql
+    // however, the mark_tx_spendable and delete_tx can
+    // fail. We simulate the failure by providing tx.id which does not
+    // exisit in the mysql
+
+    // use mock CurrentBlockchainStatus instead of real object
+    // which would access the real blockchain.
+    auto mock_bc_status = make_shared<MockCurrentBlockchainStatus1>();
+
+    // all txs are unlocked and dont exisit in blockchain
+    mock_bc_status->tx_unlock_state = true;
+    mock_bc_status->tx_exist_state = false;
+
+    xmr_accounts->set_bc_status_provider(mock_bc_status);
+
+    ACC_FROM_HEX(owner_addr_5Ajfk);
+
+    vector<xmreg::XmrTransaction> txs;
+
+    // we mark all txs for this account as nonspendable
+    // so that we have something to work with in this test
+    ASSERT_TRUE(this->xmr_accounts->select(acc.id.data, txs));
+
+    auto no_of_original_txs = txs.size();
+
+    for (size_t i = 0; i < txs.size(); ++i)
+    {
+        txs[i].id = 5555555555 /*some non-existing id*/;
+        txs[i].spendable = false;
+    }
+
+    // the non-exisiting ids should result in failure
+    EXPECT_FALSE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
+    // now repeat if all txs are locked and dont exisit in blockchain
+    mock_bc_status->tx_unlock_state = false;
+
+    // also should lead to false
+    EXPECT_FALSE(this->xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
+
 }
-//
-//TEST_F(MYSQL_TEST2, SelectTxsForAccountSpendabilityCheck)
-//{
-//    ACC_FROM_HEX(owner_addr_5Ajfk);
-//
-//    vector<xmreg::XmrTransaction> txs;
-//
-//    // we mark have of txs for this account as non_spendable
-//    // so that we have something to work with in this test
-//    ASSERT_TRUE(xmr_accounts->select(acc.id.data, txs));
-//
-//    for (size_t i = 0; i < txs.size() / 2; ++i)
-//        xmr_accounts->mark_tx_nonspendable(txs[i].id.data);
-//
-//    EXPECT_TRUE(xmr_accounts->select_txs_for_account_spendability_check(acc.id.data, txs));
-//}
 
 
 //TEST(TEST_CHAIN, GenerateTestChain)