From b1a5a937ff6177723c729f94624e282351de27bb Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Mon, 22 Aug 2016 15:53:07 +0300 Subject: [PATCH 1/5] libwallet_api: do not store wallet on close if status is not ok --- src/wallet/api/wallet.cpp | 8 ++-- tests/libwallet_api_tests/main.cpp | 69 +++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 4b97e2f1d..ff74ec6c1 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -267,16 +267,18 @@ bool WalletImpl::recover(const std::string &path, const std::string &seed) bool WalletImpl::close() { - clearStatus(); + bool result = false; try { - // LOG_PRINT_L0("Calling wallet::store..."); - m_wallet->store(); + // do not store wallet with invalid status + if (status() == Status_Ok) + m_wallet->store(); // LOG_PRINT_L0("wallet::store done"); // LOG_PRINT_L0("Calling wallet::stop..."); m_wallet->stop(); // LOG_PRINT_L0("wallet::stop done"); result = true; + clearStatus(); } catch (const std::exception &e) { m_status = Status_Error; m_errorString = e.what(); diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index d642534b0..590096505 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -31,6 +31,7 @@ #include "gtest/gtest.h" #include "wallet/wallet2_api.h" +#include "wallet/wallet2.h" #include "include_base_utils.h" #include @@ -41,6 +42,7 @@ #include #include #include +#include #include @@ -140,7 +142,7 @@ struct WalletManagerTest : public testing::Test { std::cout << __FUNCTION__ << std::endl; wmgr = Bitmonero::WalletManagerFactory::getWalletManager(); - // Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_4); + Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_4); Utils::deleteWallet(WALLET_NAME); Utils::deleteDir(boost::filesystem::path(WALLET_NAME_WITH_DIR).parent_path().string()); } @@ -210,6 +212,69 @@ TEST_F(WalletManagerTest, WalletManagerOpensWallet) std::cout << "** seed: " << wallet2->seed() << std::endl; } + +void open_wallet(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, const std::string &pass, std::mutex *mutex) +{ + if (mutex) + mutex->lock(); + LOG_PRINT_L3("opening wallet with password '" << pass << "' in thread: " << std::this_thread::get_id()); + *wallet = wmgr->openWallet(WALLET_NAME, pass, true); + LOG_PRINT_L3("wallet address: " << (*wallet)->address()); + LOG_PRINT_L3("wallet status: " << (*wallet)->status()); + LOG_PRINT_L3("closing wallet with password '" << pass << "' in thread: " << std::this_thread::get_id()); + if (mutex) + mutex->unlock(); +} + +//TEST_F(WalletManagerTest, WalletManagerOpensWalletWithPasswordAndReopenMultiThreaded) +//{ +// // create password protected wallet +// std::string wallet_pass = "password"; +// std::string wrong_wallet_pass = "1111"; +// Bitmonero::Wallet * wallet1 = wmgr->createWallet(WALLET_NAME, wallet_pass, WALLET_LANG, true); +// std::string seed1 = wallet1->seed(); +// ASSERT_TRUE(wmgr->closeWallet(wallet1)); + +// Bitmonero::Wallet *wallet2 = nullptr; +// Bitmonero::Wallet *wallet3 = nullptr; + +// std::mutex mutex; +// std::thread thread1(open_wallet, wmgr, &wallet2, wrong_wallet_pass, &mutex); +// thread1.join(); +// ASSERT_TRUE(wallet2->status() != Bitmonero::Wallet::Status_Ok); +// ASSERT_TRUE(wmgr->closeWallet(wallet2)); + +// std::thread thread2(open_wallet, wmgr, &wallet3, wallet_pass, &mutex); +// thread2.join(); + +// ASSERT_TRUE(wallet3->status() == Bitmonero::Wallet::Status_Ok); +// ASSERT_TRUE(wmgr->closeWallet(wallet3)); +//} + + +TEST_F(WalletManagerTest, WalletManagerOpensWalletWithPasswordAndReopen) +{ + // create password protected wallet + std::string wallet_pass = "password"; + std::string wrong_wallet_pass = "1111"; + Bitmonero::Wallet * wallet1 = wmgr->createWallet(WALLET_NAME, wallet_pass, WALLET_LANG, true); + std::string seed1 = wallet1->seed(); + ASSERT_TRUE(wmgr->closeWallet(wallet1)); + + Bitmonero::Wallet *wallet2 = nullptr; + Bitmonero::Wallet *wallet3 = nullptr; + std::mutex mutex; + + open_wallet(wmgr, &wallet2, wrong_wallet_pass, nullptr); + ASSERT_TRUE(wallet2->status() != Bitmonero::Wallet::Status_Ok); + ASSERT_TRUE(wmgr->closeWallet(wallet2)); + + open_wallet(wmgr, &wallet3, wallet_pass, nullptr); + ASSERT_TRUE(wallet3->status() == Bitmonero::Wallet::Status_Ok); + ASSERT_TRUE(wmgr->closeWallet(wallet3)); +} + + TEST_F(WalletManagerTest, WalletManagerStoresWallet) { @@ -239,7 +304,6 @@ TEST_F(WalletManagerTest, WalletManagerMovesWallet) } - TEST_F(WalletManagerTest, WalletManagerChangesPassword) { Bitmonero::Wallet * wallet1 = wmgr->createWallet(WALLET_NAME, WALLET_PASS, WALLET_LANG); @@ -819,6 +883,7 @@ TEST_F(WalletTest2, WalletCallbackReceived) } + int main(int argc, char** argv) { From cbe534db1a3dcca07f60ce487d116f57cf90f385 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Mon, 22 Aug 2016 23:14:58 +0300 Subject: [PATCH 2/5] libwallet_api: tests: removed logged passwords --- tests/libwallet_api_tests/main.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index 590096505..f4ef5dbe1 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -217,11 +217,11 @@ void open_wallet(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, con { if (mutex) mutex->lock(); - LOG_PRINT_L3("opening wallet with password '" << pass << "' in thread: " << std::this_thread::get_id()); + LOG_PRINT_L3("opening wallet in thread: " << std::this_thread::get_id()); *wallet = wmgr->openWallet(WALLET_NAME, pass, true); LOG_PRINT_L3("wallet address: " << (*wallet)->address()); LOG_PRINT_L3("wallet status: " << (*wallet)->status()); - LOG_PRINT_L3("closing wallet with password '" << pass << "' in thread: " << std::this_thread::get_id()); + LOG_PRINT_L3("closing wallet in thread: " << std::this_thread::get_id()); if (mutex) mutex->unlock(); } From 32bc7b41c0193297cc3b67a1b7a941813a407679 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 23 Aug 2016 12:35:01 +0300 Subject: [PATCH 3/5] libwallet_api: helper method to return maximumAllowedAmount --- src/wallet/api/wallet.cpp | 5 +++++ src/wallet/wallet2_api.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index ff74ec6c1..a2066e62e 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -154,6 +154,11 @@ bool Wallet::paymentIdValid(const string &paiment_id) return tools::wallet2::parse_short_payment_id(paiment_id, pid); } +uint64_t Wallet::maximumAllowedAmount() +{ + return std::numeric_limits::max(); +} + ///////////////////////// WalletImpl implementation //////////////////////// WalletImpl::WalletImpl(bool testnet) diff --git a/src/wallet/wallet2_api.h b/src/wallet/wallet2_api.h index 2c5836573..e880b1c68 100644 --- a/src/wallet/wallet2_api.h +++ b/src/wallet/wallet2_api.h @@ -216,6 +216,7 @@ struct Wallet static uint64_t amountFromDouble(double amount); static std::string genPaymentId(); static bool paymentIdValid(const std::string &paiment_id); + static uint64_t maximumAllowedAmount(); /** * @brief refresh - refreshes the wallet, updating transactions from daemon From bcf7b67cd6f85cdf8bf7799aa3890a56ea58ddc8 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 23 Aug 2016 13:46:51 +0300 Subject: [PATCH 4/5] libwallet_api: Wallet::amountFromString fixed --- src/wallet/api/wallet.cpp | 2 +- tests/libwallet_api_tests/main.cpp | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index a2066e62e..b3ee4112b 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -129,7 +129,7 @@ string Wallet::displayAmount(uint64_t amount) uint64_t Wallet::amountFromString(const string &amount) { - uint64_t result; + uint64_t result = 0; cryptonote::parse_amount(result, amount); return result; } diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index f4ef5dbe1..4d09ffb1b 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -213,6 +213,26 @@ TEST_F(WalletManagerTest, WalletManagerOpensWallet) } +TEST_F(WalletManagerTest, WalletMaxAmountAsString) +{ + LOG_PRINT_L3("max amount: " << Bitmonero::Wallet::displayAmount( + Bitmonero::Wallet::maximumAllowedAmount())); + +} + +TEST_F(WalletManagerTest, WalletAmountFromString) +{ + uint64_t amount = Bitmonero::Wallet::amountFromString("18446740"); + ASSERT_TRUE(amount > 0); + amount = Bitmonero::Wallet::amountFromString("11000000000000"); + ASSERT_FALSE(amount > 0); + amount = Bitmonero::Wallet::amountFromString("0.0"); + ASSERT_FALSE(amount > 0); + amount = Bitmonero::Wallet::amountFromString("10.1"); + ASSERT_TRUE(amount > 0); + +} + void open_wallet(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, const std::string &pass, std::mutex *mutex) { if (mutex) @@ -226,6 +246,9 @@ void open_wallet(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, con mutex->unlock(); } + + + //TEST_F(WalletManagerTest, WalletManagerOpensWalletWithPasswordAndReopenMultiThreaded) //{ // // create password protected wallet From 99dd57258f8c8b0869d572cf4c728d7771499827 Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 23 Aug 2016 14:13:30 +0300 Subject: [PATCH 5/5] libwallet_api: tests: checking for result while opening wallet --- tests/libwallet_api_tests/main.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index 4d09ffb1b..b4bc86f91 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -142,7 +142,7 @@ struct WalletManagerTest : public testing::Test { std::cout << __FUNCTION__ << std::endl; wmgr = Bitmonero::WalletManagerFactory::getWalletManager(); - Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_4); + // Bitmonero::WalletManagerFactory::setLogLevel(Bitmonero::WalletManagerFactory::LogLevel_4); Utils::deleteWallet(WALLET_NAME); Utils::deleteDir(boost::filesystem::path(WALLET_NAME_WITH_DIR).parent_path().string()); } @@ -233,7 +233,7 @@ TEST_F(WalletManagerTest, WalletAmountFromString) } -void open_wallet(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, const std::string &pass, std::mutex *mutex) +void open_wallet_helper(Bitmonero::WalletManager *wmgr, Bitmonero::Wallet **wallet, const std::string &pass, std::mutex *mutex) { if (mutex) mutex->lock(); @@ -288,11 +288,13 @@ TEST_F(WalletManagerTest, WalletManagerOpensWalletWithPasswordAndReopen) Bitmonero::Wallet *wallet3 = nullptr; std::mutex mutex; - open_wallet(wmgr, &wallet2, wrong_wallet_pass, nullptr); + open_wallet_helper(wmgr, &wallet2, wrong_wallet_pass, nullptr); + ASSERT_TRUE(wallet2 != nullptr); ASSERT_TRUE(wallet2->status() != Bitmonero::Wallet::Status_Ok); ASSERT_TRUE(wmgr->closeWallet(wallet2)); - open_wallet(wmgr, &wallet3, wallet_pass, nullptr); + open_wallet_helper(wmgr, &wallet3, wallet_pass, nullptr); + ASSERT_TRUE(wallet3 != nullptr); ASSERT_TRUE(wallet3->status() == Bitmonero::Wallet::Status_Ok); ASSERT_TRUE(wmgr->closeWallet(wallet3)); }