From 951f3b5d83d0c0c5a1ca2f2bd36cf5659600a44a Mon Sep 17 00:00:00 2001 From: Ilya Kitaev Date: Tue, 5 Apr 2016 15:24:44 +0300 Subject: [PATCH] Wallet::createTransaction API introduced Transaction API continued TODOs for Transaction/Transfer interface --- src/wallet/wallet2_api.cpp | 325 ++++++++++++++++++----------- src/wallet/wallet2_api.h | 21 +- tests/libwallet_api_tests/main.cpp | 14 +- 3 files changed, 225 insertions(+), 135 deletions(-) diff --git a/src/wallet/wallet2_api.cpp b/src/wallet/wallet2_api.cpp index 60e75802c..edf7d9931 100644 --- a/src/wallet/wallet2_api.cpp +++ b/src/wallet/wallet2_api.cpp @@ -66,67 +66,46 @@ using namespace cryptonote; Wallet::~Wallet() {} +PendingTransaction::~PendingTransaction() {} + + +class WalletImpl; + ///////////////////////// Transaction implementation /////////////////////////// -class TransactionImpl : public Transaction +class TransactionImpl : public PendingTransaction { public: - TransactionImpl(Wallet * wallet); + TransactionImpl(WalletImpl * wallet); ~TransactionImpl(); int status() const; std::string errorString() const; bool commit(); - - -private: - std::vector & transactions(); + uint64_t amount() const; + uint64_t dust() const; + uint64_t fee() const; + // TODO: continue with interface; private: friend class WalletImpl; - Wallet * m_wallet; + WalletImpl * m_wallet; + int m_status; std::string m_errorString; std::vector m_pending_tx; - - }; -TransactionImpl::TransactionImpl(Wallet *wallet) - : m_wallet(wallet) -{ -} -TransactionImpl::~TransactionImpl() +///////////////////////////////////////////////////////////////////////////////// +string Wallet::displayAmount(uint64_t amount) { - -} - -int TransactionImpl::status() const -{ - return m_status; -} - -string TransactionImpl::errorString() const -{ - return m_errorString; -} - -bool TransactionImpl::commit() -{ -// while (!m_pending_tx.empty()) { - -// } - return false; - - + return cryptonote::print_money(amount); } - - ///////////////////////// Wallet implementation //////////////////////////////// class WalletImpl : public Wallet { @@ -151,16 +130,15 @@ public: bool connectToDaemon(); uint64_t balance() const; uint64_t unlockedBalance() const; - std::string displayAmount(uint64_t amount) const; bool refresh(); - bool transfer(const std::string &dst_addr, uint64_t amount); - + PendingTransaction * createTransaction(const std::string &dst_addr, uint64_t amount); + virtual void disposeTransaction(PendingTransaction * t); private: void clearStatus(); private: - //std::unique_ptr m_wallet; + friend class TransactionImpl; tools::wallet2 * m_wallet; int m_status; std::string m_errorString; @@ -366,10 +344,6 @@ uint64_t WalletImpl::unlockedBalance() const return m_wallet->unlocked_balance(); } -std::string WalletImpl::displayAmount(uint64_t amount) const -{ - return cryptonote::print_money(amount); -} bool WalletImpl::refresh() { @@ -383,44 +357,178 @@ bool WalletImpl::refresh() return m_status == Status_Ok; } -bool WalletImpl::transfer(const std::string &dst_addr, uint64_t amount) +// TODO: +// 1 - properly handle payment id (add another menthod with explicit 'payment_id' param) +// 2 - check / design how "Transaction" can be single interface +// (instead of few different data structures within wallet2 implementation: +// - pending_tx; +// - transfer_details; +// - payment_details; +// - unconfirmed_transfer_details; +// - confirmed_transfer_details) +PendingTransaction *WalletImpl::createTransaction(const string &dst_addr, uint64_t amount) { clearStatus(); vector dsts; cryptonote::tx_destination_entry de; bool has_payment_id; - bool payment_id_seen = false; crypto::hash8 new_payment_id; + + // TODO: (https://bitcointalk.org/index.php?topic=753252.msg9985441#msg9985441) size_t fake_outs_count = m_wallet->default_mixin(); if (fake_outs_count == 0) fake_outs_count = DEFAULT_MIX; + TransactionImpl * transaction = new TransactionImpl(this); - if(!cryptonote::get_account_integrated_address_from_str(de.addr, has_payment_id, new_payment_id, m_wallet->testnet(), dst_addr)) { - // TODO: copy-paste 'if treating as an address fails, try as url' from simplewallet.cpp:1982 - m_status = Status_Error; - m_errorString = "Invalid destination address"; - return false; - } + do { - de.amount = amount; - if (de.amount <= 0) { - m_status = Status_Error; - m_errorString = "Invalid amount"; - return false; + if(!cryptonote::get_account_integrated_address_from_str(de.addr, has_payment_id, new_payment_id, m_wallet->testnet(), dst_addr)) { + // TODO: copy-paste 'if treating as an address fails, try as url' from simplewallet.cpp:1982 + m_status = Status_Error; + m_errorString = "Invalid destination address"; + break; + } + + de.amount = amount; + if (de.amount <= 0) { + m_status = Status_Error; + m_errorString = "Invalid amount"; + break; + } + + dsts.push_back(de); + //std::vector ptx_vector; + std::vector extra; + + + try { + transaction->m_pending_tx = m_wallet->create_transactions(dsts, fake_outs_count, 0 /* unlock_time */, 0 /* unused fee arg*/, extra); + + } catch (const tools::error::daemon_busy&) { + // TODO: make it translatable with "tr"? + m_errorString = tr("daemon is busy. Please try again later."); + m_status = Status_Error; + } catch (const tools::error::no_connection_to_daemon&) { + m_errorString = tr("no connection to daemon. Please make sure daemon is running."); + m_status = Status_Error; + } catch (const tools::error::wallet_rpc_error& e) { + m_errorString = tr("RPC error: ") + e.to_string(); + m_status = Status_Error; + } catch (const tools::error::get_random_outs_error&) { + m_errorString = tr("failed to get random outputs to mix"); + m_status = Status_Error; + + } catch (const tools::error::not_enough_money& e) { + m_status = Status_Error; + std::ostringstream writer(m_errorString); + + writer << boost::format(tr("not enough money to transfer, available only %s, transaction amount %s = %s + %s (fee)")) % + print_money(e.available()) % + print_money(e.tx_amount() + e.fee()) % + print_money(e.tx_amount()) % + print_money(e.fee()); + + } catch (const tools::error::not_enough_outs_to_mix& e) { + std::ostringstream writer(m_errorString); + writer << tr("not enough outputs for specified mixin_count") << " = " << e.mixin_count() << ":"; + for (const cryptonote::COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::outs_for_amount& outs_for_amount : e.scanty_outs()) { + writer << "\n" << tr("output amount") << " = " << print_money(outs_for_amount.amount) << ", " << tr("found outputs to mix") << " = " << outs_for_amount.outs.size(); + } + m_status = Status_Error; + } catch (const tools::error::tx_not_constructed&) { + m_errorString = tr("transaction was not constructed"); + m_status = Status_Error; + } catch (const tools::error::tx_rejected& e) { + std::ostringstream writer(m_errorString); + writer << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status(); + m_status = Status_Error; + } catch (const tools::error::tx_sum_overflow& e) { + m_errorString = e.what(); + m_status = Status_Error; + } catch (const tools::error::zero_destination&) { + m_errorString = tr("one of destinations is zero"); + m_status = Status_Error; + } catch (const tools::error::tx_too_big& e) { + m_errorString = tr("failed to find a suitable way to split transactions"); + m_status = Status_Error; + } catch (const tools::error::transfer_error& e) { + m_errorString = string(tr("unknown transfer error: ")) + e.what(); + m_status = Status_Error; + } catch (const tools::error::wallet_internal_error& e) { + m_errorString = string(tr("internal error: ")) + e.what(); + m_status = Status_Error; + } catch (const std::exception& e) { + m_errorString = string(tr("unexpected error: ")) + e.what(); + m_status = Status_Error; + } catch (...) { + m_errorString = tr("unknown error"); + m_status = Status_Error; + } + } while (false); + + transaction->m_status = m_status; + transaction->m_errorString = m_errorString; + return transaction; +} + +void WalletImpl::disposeTransaction(PendingTransaction *t) +{ + delete t; +} + +bool WalletImpl::connectToDaemon() +{ + bool result = m_wallet->check_connection(); + m_status = result ? Status_Ok : Status_Error; + if (!result) { + m_errorString = "Error connecting to daemon at " + m_wallet->get_daemon_address(); } - dsts.push_back(de); - std::vector ptx_vector; - std::vector extra; + return result; +} + +void WalletImpl::clearStatus() +{ + m_status = Status_Ok; + m_errorString.clear(); +} + + + + +TransactionImpl::TransactionImpl(WalletImpl *wallet) + : m_wallet(wallet) +{ + +} + +TransactionImpl::~TransactionImpl() +{ + +} + +int TransactionImpl::status() const +{ + return m_status; +} + +string TransactionImpl::errorString() const +{ + return m_errorString; +} + +bool TransactionImpl::commit() +{ + + LOG_PRINT_L0("m_pending_tx size: " << m_pending_tx.size()); + assert(m_pending_tx.size() == 1); try { - ptx_vector = m_wallet->create_transactions(dsts, fake_outs_count, 0 /* unlock_time */, 0 /* unused fee arg*/, extra); -// TODO: move it to transaction class - while (!ptx_vector.empty()) { - auto & ptx = ptx_vector.back(); - m_wallet->commit_tx(ptx); + while (!m_pending_tx.empty()) { + auto & ptx = m_pending_tx.back(); + m_wallet->m_wallet->commit_tx(ptx); // success_msg_writer(true) << tr("Money successfully sent, transaction ") << get_transaction_hash(ptx.tx); // if no exception, remove element from vector - ptx_vector.pop_back(); + m_pending_tx.pop_back(); } // TODO: extract method; } catch (const tools::error::daemon_busy&) { // TODO: make it translatable with "tr"? @@ -429,76 +537,49 @@ bool WalletImpl::transfer(const std::string &dst_addr, uint64_t amount) } catch (const tools::error::no_connection_to_daemon&) { m_errorString = tr("no connection to daemon. Please make sure daemon is running."); m_status = Status_Error; - } catch (const tools::error::wallet_rpc_error& e) { - m_errorString = tr("RPC error: ") + e.to_string(); - m_status = Status_Error; - } catch (const tools::error::get_random_outs_error&) { - m_errorString = tr("failed to get random outputs to mix"); - m_status = Status_Error; - - } catch (const tools::error::not_enough_money& e) { - m_status = Status_Error; - std::ostringstream writer(m_errorString); - - writer << boost::format(tr("not enough money to transfer, available only %s, transaction amount %s = %s + %s (fee)")) % - print_money(e.available()) % - print_money(e.tx_amount() + e.fee()) % - print_money(e.tx_amount()) % - print_money(e.fee()); - - } catch (const tools::error::not_enough_outs_to_mix& e) { - std::ostringstream writer(m_errorString); - writer << tr("not enough outputs for specified mixin_count") << " = " << e.mixin_count() << ":"; - for (const cryptonote::COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::outs_for_amount& outs_for_amount : e.scanty_outs()) { - writer << "\n" << tr("output amount") << " = " << print_money(outs_for_amount.amount) << ", " << tr("found outputs to mix") << " = " << outs_for_amount.outs.size(); - } - m_status = Status_Error; - } catch (const tools::error::tx_not_constructed&) { - m_errorString = tr("transaction was not constructed"); - m_status = Status_Error; } catch (const tools::error::tx_rejected& e) { std::ostringstream writer(m_errorString); writer << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status(); m_status = Status_Error; - } catch (const tools::error::tx_sum_overflow& e) { - m_errorString = e.what(); - m_status = Status_Error; - } catch (const tools::error::zero_destination&) { - m_errorString = tr("one of destinations is zero"); + } catch (std::exception &e) { + m_errorString = string(tr("Unknown exception: ")) + e.what(); m_status = Status_Error; - } catch (const tools::error::tx_too_big& e) { - m_errorString = tr("failed to find a suitable way to split transactions"); - m_status = Status_Error; - } catch (const tools::error::transfer_error& e) { - m_errorString = string(tr("unknown transfer error: ")) + e.what(); - m_status = Status_Error; - } catch (const tools::error::wallet_internal_error& e) { - m_errorString = string(tr("internal error: ")) + e.what(); - m_status = Status_Error; - } catch (const std::exception& e) { - m_errorString = string(tr("unexpected error: ")) + e.what(); - m_status = Status_Error; } catch (...) { - m_errorString = tr("unknown error"); - m_status = Status_Error; + m_errorString = tr("Unhandled exception"); + LOG_ERROR(m_errorString); + m_status = Status_Error; } + return m_status == Status_Ok; } -bool WalletImpl::connectToDaemon() +uint64_t TransactionImpl::amount() const { - bool result = m_wallet->check_connection(); - m_status = result ? Status_Ok : Status_Error; - if (!result) { - m_errorString = "Error connecting to daemon at " + m_wallet->get_daemon_address(); + uint64_t result = 0; + for (const auto &ptx : m_pending_tx) { + for (const auto &dest : ptx.dests) { + result += dest.amount; + } } return result; } -void WalletImpl::clearStatus() +uint64_t TransactionImpl::dust() const { - m_status = Status_Ok; - m_errorString.clear(); + uint32_t result = 0; + for (const auto & ptx : m_pending_tx) { + result += ptx.dust; + } + return result; +} + +uint64_t TransactionImpl::fee() const +{ + uint32_t result = 0; + for (const auto ptx : m_pending_tx) { + result += ptx.fee; + } + return result; } @@ -580,7 +661,7 @@ WalletManager *WalletManagerFactory::getWalletManager() { if (!g_walletManager) { - epee::log_space::log_singletone::add_logger(LOGGER_CONSOLE, NULL, NULL, LOG_LEVEL_0); + epee::log_space::log_singletone::add_logger(LOGGER_CONSOLE, NULL, NULL, LOG_LEVEL_MAX); g_walletManager = new WalletManagerImpl(); } diff --git a/src/wallet/wallet2_api.h b/src/wallet/wallet2_api.h index 662264f50..56a91bfbb 100644 --- a/src/wallet/wallet2_api.h +++ b/src/wallet/wallet2_api.h @@ -39,16 +39,19 @@ namespace Bitmonero { /** * @brief Transaction interface */ -struct Transaction +struct PendingTransaction { enum Status { Status_Ok, Status_Error }; - + virtual ~PendingTransaction() = 0; virtual int status() const = 0; virtual std::string errorString() const = 0; virtual bool commit() = 0; + virtual uint64_t amount() const = 0; + virtual uint64_t dust() const = 0; + virtual uint64_t fee() const = 0; }; /** @@ -57,17 +60,12 @@ struct Transaction */ struct Wallet { - // TODO define wallet interface (decide what needed from wallet2) enum Status { Status_Ok, Status_Error }; - struct Listener - { - // TODO - }; virtual ~Wallet() = 0; virtual std::string seed() const = 0; @@ -85,12 +83,15 @@ struct Wallet virtual bool connectToDaemon() = 0; virtual uint64_t balance() const = 0; virtual uint64_t unlockedBalance() const = 0; - virtual std::string displayAmount(uint64_t amount) const = 0; + static std::string displayAmount(uint64_t amount); // TODO? // virtual uint64_t unlockedDustBalance() const = 0; virtual bool refresh() = 0; - // TODO transfer - virtual bool transfer(const std::string &dst_addr, uint64_t amount) = 0; + virtual PendingTransaction * createTransaction(const std::string &dst_addr, uint64_t amount) = 0; + virtual void disposeTransaction(PendingTransaction * t) = 0; + // TODO + virtual void getPayments() const; + }; diff --git a/tests/libwallet_api_tests/main.cpp b/tests/libwallet_api_tests/main.cpp index 970d9a74e..dee874cd6 100644 --- a/tests/libwallet_api_tests/main.cpp +++ b/tests/libwallet_api_tests/main.cpp @@ -264,14 +264,22 @@ TEST_F(WalletManagerTest, WalletRefresh) ASSERT_TRUE(wmgr->closeWallet(wallet1)); } -TEST_F(WalletManagerTest, WalletTransfer) +TEST_F(WalletManagerTest, WalletTransaction) { Bitmonero::Wallet * wallet1 = wmgr->openWallet(TESTNET_WALLET_NAME, TESTNET_WALLET_PASS, true); // make sure testnet daemon is running ASSERT_TRUE(wallet1->init(TESTNET_DAEMON_ADDRESS, 0)); ASSERT_TRUE(wallet1->refresh()); uint64_t balance = wallet1->balance(); - ASSERT_TRUE(wallet1->transfer(RECIPIENT_WALLET_ADDRESS, AMOUNT_10XMR)); + ASSERT_TRUE(wallet1->status() == Bitmonero::PendingTransaction::Status_Ok); + + Bitmonero::PendingTransaction * transaction = wallet1->createTransaction( + RECIPIENT_WALLET_ADDRESS, AMOUNT_10XMR); + ASSERT_TRUE(transaction->status() == Bitmonero::PendingTransaction::Status_Ok); + + ASSERT_TRUE(wallet1->balance() == balance); + ASSERT_TRUE(transaction->amount() == AMOUNT_10XMR); + ASSERT_TRUE(transaction->commit()); ASSERT_FALSE(wallet1->balance() == balance); ASSERT_TRUE(wmgr->closeWallet(wallet1)); } @@ -280,7 +288,7 @@ TEST_F(WalletManagerTest, WalletTransfer) int main(int argc, char** argv) { - //epee::debug::get_set_enable_assert(true, false); + ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); }