Wallet::createTransaction API introduced

Transaction API continued

TODOs for Transaction/Transfer interface

src/wallet/wallet2_api.cpp

@@ -66,65 +66,44 @@ 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();
-
+    uint64_t amount() const;
-
+    uint64_t dust() const;
-private:
+    uint64_t fee() const;
-    std::vector<tools::wallet2::pending_tx> & transactions();
+    // TODO: continue with interface;
 
 private:
     friend class WalletImpl;
-    Wallet * m_wallet;
+    WalletImpl * m_wallet;
+
     int  m_status;
     std::string m_errorString;
     std::vector<tools::wallet2::pending_tx> m_pending_tx;
-
-
 };
 
 
-TransactionImpl::TransactionImpl(Wallet *wallet)
+
-    : m_wallet(wallet)
+
+/////////////////////////////////////////////////////////////////////////////////
+string Wallet::displayAmount(uint64_t amount)
 {
-
+    return cryptonote::print_money(amount);
 }
 
-TransactionImpl::~TransactionImpl()
-{
-
-}
-
-int TransactionImpl::status() const
-{
-    return m_status;
-}
-
-string TransactionImpl::errorString() const
-{
-    return m_errorString;
-}
-
-bool TransactionImpl::commit()
-{
-//    while (!m_pending_tx.empty()) {
-
-//    }
-    return false;
-
-
-}
-
-
-
 
 
 ///////////////////////// Wallet implementation ////////////////////////////////
@@ -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<tools::wallet2> 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,106 +357,124 @@ 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<cryptonote::tx_destination_entry> 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)) {
+    do {
-        // 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;
-    }
 
-    de.amount = amount;
+        if(!cryptonote::get_account_integrated_address_from_str(de.addr, has_payment_id, new_payment_id, m_wallet->testnet(), dst_addr)) {
-    if (de.amount <= 0) {
+            // TODO: copy-paste 'if treating as an address fails, try as url' from simplewallet.cpp:1982
-        m_status = Status_Error;
+            m_status = Status_Error;
-        m_errorString = "Invalid amount";
+            m_errorString = "Invalid destination address";
-        return false;
+            break;
-    }
-    dsts.push_back(de);
-    std::vector<tools::wallet2::pending_tx> ptx_vector;
-    std::vector<uint8_t> extra;
-    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);
-            // 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();
-        } // TODO: extract method;
-    } 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&) {
+        de.amount = amount;
-        m_errorString = tr("transaction was not constructed");
+        if (de.amount <= 0) {
-        m_status = Status_Error;
+            m_status = Status_Error;
-    } catch (const tools::error::tx_rejected& e) {
+            m_errorString = "Invalid amount";
-        std::ostringstream writer(m_errorString);
+            break;
-        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) {
+        dsts.push_back(de);
-        m_errorString = e.what();
+        //std::vector<tools::wallet2::pending_tx> ptx_vector;
-        m_status = Status_Error;
+        std::vector<uint8_t> extra;
-    } catch (const tools::error::zero_destination&) {
+
-        m_errorString =  tr("one of destinations is zero");
+
-        m_status = Status_Error;
+        try {
-    } catch (const tools::error::tx_too_big& e) {
+            transaction->m_pending_tx = m_wallet->create_transactions(dsts, fake_outs_count, 0 /* unlock_time */, 0 /* unused fee arg*/, extra);
-        m_errorString =  tr("failed to find a suitable way to split transactions");
+
-        m_status = Status_Error;
+        } catch (const tools::error::daemon_busy&) {
-    } catch (const tools::error::transfer_error& e) {
+            // TODO: make it translatable with "tr"?
-       m_errorString = string(tr("unknown transfer error: ")) + e.what();
+            m_errorString = tr("daemon is busy. Please try again later.");
-       m_status = Status_Error;
+            m_status = Status_Error;
-    } catch (const tools::error::wallet_internal_error& e) {
+        } catch (const tools::error::no_connection_to_daemon&) {
-       m_errorString =  string(tr("internal error: ")) + e.what();
+            m_errorString = tr("no connection to daemon. Please make sure daemon is running.");
-       m_status = Status_Error;
+            m_status = Status_Error;
-    } catch (const std::exception& e) {
+        } catch (const tools::error::wallet_rpc_error& e) {
-      m_errorString =  string(tr("unexpected error: ")) + e.what();
+            m_errorString = tr("RPC error: ") +  e.to_string();
-      m_status = Status_Error;
+            m_status = Status_Error;
-    } catch (...) {
+        } catch (const tools::error::get_random_outs_error&) {
-      m_errorString = tr("unknown error");
+            m_errorString = tr("failed to get random outputs to mix");
-      m_status = Status_Error;
+            m_status = Status_Error;
-    }
+
-    return m_status == Status_Ok;
+        } 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()
@@ -503,6 +495,95 @@ void WalletImpl::clearStatus()
 
 
 
+
+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 {
+        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
+            m_pending_tx.pop_back();
+        } // TODO: extract method;
+    } 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::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 (std::exception &e) {
+        m_errorString = string(tr("Unknown exception: ")) + e.what();
+        m_status = Status_Error;
+    } catch (...) {
+        m_errorString = tr("Unhandled exception");
+        LOG_ERROR(m_errorString);
+        m_status = Status_Error;
+    }
+
+    return m_status == Status_Ok;
+}
+
+uint64_t TransactionImpl::amount() const
+{
+    uint64_t result = 0;
+    for (const auto &ptx : m_pending_tx)   {
+        for (const auto &dest : ptx.dests) {
+            result += dest.amount;
+        }
+    }
+    return result;
+}
+
+uint64_t TransactionImpl::dust() const
+{
+    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;
+}
+
+
+
 ///////////////////////// WalletManager implementation /////////////////////////
 class WalletManagerImpl : public WalletManager
 {
@@ -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();
     }
 

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 PendingTransaction * createTransaction(const std::string &dst_addr, uint64_t amount) = 0;
-    virtual bool transfer(const std::string &dst_addr, uint64_t amount) = 0;
+    virtual void disposeTransaction(PendingTransaction * t) = 0;
+    // TODO
+    virtual void getPayments() const;
+
 
 };
 

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();
 }