From 7d4a64b8ced52cbc1b1675e9fbd5527882cd23b2 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Jun 2021 11:49:03 -0300
Subject: [PATCH 1/6] Refactor: Create generic CallResult and move functions
 using PairResult to use it.

---
 src/operationresult.h                         | 13 +++++++++++
 src/qt/addresstablemodel.cpp                  |  4 ++--
 src/qt/pivx/masternodewizarddialog.cpp        |  6 ++---
 src/qt/pivx/receivewidget.cpp                 |  6 ++---
 src/qt/pivx/requestdialog.cpp                 |  4 ++--
 src/qt/pivx/send.cpp                          |  2 +-
 src/qt/walletmodel.cpp                        | 22 +++++++++----------
 src/qt/walletmodel.h                          |  6 ++---
 src/wallet/rpcwallet.cpp                      |  6 ++---
 ...sapling_transactions_validations_tests.cpp |  2 +-
 src/wallet/test/wallet_tests.cpp              |  2 +-
 src/wallet/wallet.cpp                         | 10 ++++-----
 src/wallet/wallet.h                           |  8 +++----
 13 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/src/operationresult.h b/src/operationresult.h
index 549e5e2b5b89..31bc1f44bd52 100644
--- a/src/operationresult.h
+++ b/src/operationresult.h
@@ -27,4 +27,17 @@ inline OperationResult errorOut(const std::string& errorStr)
     return OperationResult(false, errorStr);
 }
 
+
+template <class T>
+class CallResult : public OperationResult
+{
+private:
+    Optional<T> m_obj_res{nullopt};
+public:
+    CallResult(bool _res, const std::string& _error) : OperationResult(_res, _error) { }
+    CallResult(bool _res, T _obj) : OperationResult(_res), m_obj_res(_obj) { }
+    Optional<T>& getObjResult() const { return m_obj_res; }
+};
+
+
 #endif //OPERATIONRESULT_H
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index 01472ba72e14..957d4dffed28 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -631,12 +631,12 @@ QString AddressTableModel::getAddressToShow(bool isShielded) const
     }
 
     // For some reason we don't have any address in our address book, let's create one
-    PairResult res(false);
+    CallResult<Destination> res(false, "");
     QString addressStr;
     if (!isShielded) {
         Destination newAddress;
         res = walletModel->getNewAddress(newAddress, "Default");
-        if (res.result) {
+        if (res) {
             addressStr = QString::fromStdString(newAddress.ToString());
         }
     } else {
diff --git a/src/qt/pivx/masternodewizarddialog.cpp b/src/qt/pivx/masternodewizarddialog.cpp
index 8d5cb6297efd..9d2e80c519ae 100644
--- a/src/qt/pivx/masternodewizarddialog.cpp
+++ b/src/qt/pivx/masternodewizarddialog.cpp
@@ -222,11 +222,11 @@ bool MasterNodeWizardDialog::createMN()
     if (!walletModel->getMNCollateralCandidate(collateralOut)) {
         // New receive address
         Destination dest;
-        PairResult r = walletModel->getNewAddress(dest, alias);
+        auto r = walletModel->getNewAddress(dest, alias);
 
-        if (!r.result) {
+        if (!r) {
             // generate address fail
-            inform(tr(r.status->c_str()));
+            inform(tr(r.getError().c_str()));
             return false;
         }
 
diff --git a/src/qt/pivx/receivewidget.cpp b/src/qt/pivx/receivewidget.cpp
index c17dbe9725b1..008c377c0b17 100644
--- a/src/qt/pivx/receivewidget.cpp
+++ b/src/qt/pivx/receivewidget.cpp
@@ -252,7 +252,7 @@ void ReceiveWidget::onNewAddressClicked()
         }
 
         QString strAddress;
-        PairResult r(false);
+        CallResult<Destination> r(false, "");
         if (!shieldedMode) {
             Destination address;
             r = walletModel->getNewAddress(address, "");
@@ -262,8 +262,8 @@ void ReceiveWidget::onNewAddressClicked()
         }
 
         // Check validity
-        if (!r.result) {
-            inform(r.status->c_str());
+        if (!r) {
+            inform(r.getError().c_str());
             return;
         }
 
diff --git a/src/qt/pivx/requestdialog.cpp b/src/qt/pivx/requestdialog.cpp
index cb0e685f2b1b..e953578b3847 100644
--- a/src/qt/pivx/requestdialog.cpp
+++ b/src/qt/pivx/requestdialog.cpp
@@ -115,7 +115,7 @@ void RequestDialog::accept()
         QString title;
 
         Destination address;
-        PairResult r(false);
+        CallResult<Destination> r(false, "");
         if (this->isPaymentRequest) {
             r = walletModel->getNewAddress(address, label);
             title = tr("Request for ") + BitcoinUnits::format(displayUnit, value, false, BitcoinUnits::separatorAlways) + " " + QString(CURRENCY_UNIT.c_str());
@@ -124,7 +124,7 @@ void RequestDialog::accept()
             title = tr("Cold Staking Address Generated");
         }
 
-        if (!r.result) {
+        if (!r) {
             // TODO: notify user about this error
             close();
             return;
diff --git a/src/qt/pivx/send.cpp b/src/qt/pivx/send.cpp
index f9e4c401a802..f0dedc4a20b8 100644
--- a/src/qt/pivx/send.cpp
+++ b/src/qt/pivx/send.cpp
@@ -746,7 +746,7 @@ void SendWidget::onShieldCoinsClicked()
             QString strAddress;
             auto res = walletModel->getNewShieldedAddress(strAddress, "");
             // Check for generation errors
-            if (!res.result) {
+            if (!res) {
                 inform(tr("Error generating address to shield PIVs"));
                 return false;
             }
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index cee21970c4cb..0dc4eadf4e99 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -475,7 +475,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
                 Destination ownerAdd;
                 if (rcp.ownerAddress.isEmpty()) {
                     // Create new internal owner address
-                    if (!getNewAddress(ownerAdd).result)
+                    if (!getNewAddress(ownerAdd))
                         return CannotCreateInternalAddress;
                 } else {
                     ownerAdd = Destination(DecodeDestination(rcp.ownerAddress.toStdString()), false);
@@ -926,27 +926,27 @@ int64_t WalletModel::getKeyCreationTime(const libzcash::SaplingPaymentAddress& a
     return 0;
 }
 
-PairResult WalletModel::getNewAddress(Destination& ret, std::string label) const
+CallResult<Destination> WalletModel::getNewAddress(Destination& ret, std::string label) const
 {
     CTxDestination dest;
-    PairResult res = wallet->getNewAddress(dest, label);
-    if (res.result) ret = Destination(dest, false);
-    return res;
+    auto res = wallet->getNewAddress(dest, label);
+    if (res) ret = Destination(dest, false);
+    return CallResult<Destination>(res.getResult(), ret);
 }
 
-PairResult WalletModel::getNewStakingAddress(Destination& ret,std::string label) const
+CallResult<Destination> WalletModel::getNewStakingAddress(Destination& ret, std::string label) const
 {
     CTxDestination dest;
-    PairResult res = wallet->getNewStakingAddress(dest, label);
-    if (res.result) ret = Destination(dest, true);
-    return res;
+    auto res = wallet->getNewStakingAddress(dest, label);
+    if (res) ret = Destination(dest, true);
+    return CallResult<Destination>(res.getResult(), ret);
 }
 
-PairResult WalletModel::getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel)
+CallResult<Destination> WalletModel::getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel)
 {
     shieldedAddrRet = QString::fromStdString(
             KeyIO::EncodePaymentAddress(wallet->GenerateNewSaplingZKey(strLabel)));
-    return PairResult(true);
+    return CallResult<Destination>(true, "");
 }
 
 bool WalletModel::whitelistAddressFromColdStaking(const QString &addressStr)
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index 0ad630a6ec72..e7e24191f025 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -273,14 +273,14 @@ class WalletModel : public QObject
     int64_t getKeyCreationTime(const CTxDestination& address);
     int64_t getKeyCreationTime(const std::string& address);
     int64_t getKeyCreationTime(const libzcash::SaplingPaymentAddress& address);
-    PairResult getNewAddress(Destination& ret, std::string label = "") const;
+    CallResult<Destination> getNewAddress(Destination& ret, std::string label = "") const;
     /**
      * Return a new address used to receive for delegated cold stake purpose.
      */
-    PairResult getNewStakingAddress(Destination& ret, std::string label = "") const;
+    CallResult<Destination> getNewStakingAddress(Destination& ret, std::string label = "") const;
 
     //! Return a new shielded address.
-    PairResult getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel = "");
+    CallResult<Destination> getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel = "");
 
     //! Return new wallet rescan reserver
     WalletRescanReserver getRescanReserver() const { return WalletRescanReserver(wallet); }
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index ae7101e19733..bc34876ff4c5 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -118,9 +118,9 @@ static CTxDestination GetNewAddressFromLabel(CWallet* const pwallet, const std::
         label = LabelFromValue(params[0]);
 
     CTxDestination address;
-    PairResult r = pwallet->getNewAddress(address, label, purpose, addrType);
-    if(!r.result)
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, *r.status);
+    auto r = pwallet->getNewAddress(address, label, purpose, addrType);
+    if(!r)
+        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, r.getError());
     return address;
 }
 
diff --git a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
index f7fd88daeea4..ade9a1078d2a 100644
--- a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
+++ b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
@@ -79,7 +79,7 @@ BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
 {
     CTxDestination coinbaseDest;
     auto ret = pwalletMain->getNewAddress(coinbaseDest, "coinbase");
-    BOOST_ASSERT_MSG(ret.result, "cannot create address");
+    BOOST_ASSERT_MSG(ret, "cannot create address");
     BOOST_ASSERT_MSG(IsValidDestination(coinbaseDest), "invalid destination");
     BOOST_ASSERT_MSG(IsMine(*pwalletMain, coinbaseDest), "destination not from wallet");
 
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 7fa369848239..08ebf899ddc5 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -578,7 +578,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests)
 
     // Receive balance from an external source
     CTxDestination receivingAddr;
-    BOOST_ASSERT(wallet.getNewAddress(receivingAddr, "receiving_address").result);
+    BOOST_ASSERT(wallet.getNewAddress(receivingAddr, "receiving_address"));
     CTxOut creditOut(nCredit/2, GetScriptForDestination(receivingAddr));
     CWalletTx& wtxCredit = ReceiveBalanceWith({creditOut, creditOut},wallet);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index ea7df0aa74b8..eac30bef3544 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -176,15 +176,15 @@ std::vector<CWalletTx> CWallet::getWalletTxs()
     return result;
 }
 
-PairResult CWallet::getNewAddress(CTxDestination& ret, std::string label){
+CallResult<CTxDestination> CWallet::getNewAddress(CTxDestination& ret, std::string label){
     return getNewAddress(ret, label, AddressBook::AddressBookPurpose::RECEIVE);
 }
 
-PairResult CWallet::getNewStakingAddress(CTxDestination& ret, std::string label){
+CallResult<CTxDestination> CWallet::getNewStakingAddress(CTxDestination& ret, std::string label){
     return getNewAddress(ret, label, AddressBook::AddressBookPurpose::COLD_STAKING, CChainParams::Base58Type::STAKING_ADDRESS);
 }
 
-PairResult CWallet::getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
+CallResult<CTxDestination> CWallet::getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
                                          const CChainParams::Base58Type addrType)
 {
     LOCK(cs_wallet);
@@ -198,7 +198,7 @@ PairResult CWallet::getNewAddress(CTxDestination& ret, const std::string address
     // Get a key
     if (!GetKeyFromPool(newKey, type)) {
         // inform the user to top-up the keypool or unlock the wallet
-        return PairResult(false, new std::string(
+        return CallResult<CTxDestination>(false, std::string(
                         _("Keypool ran out, please call keypoolrefill first, or unlock the wallet.")));
     }
     CKeyID keyID = newKey.GetID();
@@ -207,7 +207,7 @@ PairResult CWallet::getNewAddress(CTxDestination& ret, const std::string address
         throw std::runtime_error("CWallet::getNewAddress() : SetAddressBook failed");
 
     ret = CTxDestination(keyID);
-    return PairResult(true);
+    return CallResult<CTxDestination>(true, ret);
 }
 
 int64_t CWallet::GetKeyCreationTime(const CWDestination& dest)
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 9ebc998ed09c..1a49043502bc 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -18,7 +18,7 @@
 #include "key.h"
 #include "key_io.h"
 #include "keystore.h"
-#include "pairresult.h"
+#include "operationresult.h"
 #include "policy/feerate.h"
 #include "primitives/block.h"
 #include "primitives/transaction.h"
@@ -886,10 +886,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     void LockIfMyCollateral(const CTransactionRef& ptx);
 
     //  keystore implementation
-    PairResult getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
+    CallResult<CTxDestination> getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
                                            const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS);
-    PairResult getNewAddress(CTxDestination& ret, std::string label);
-    PairResult getNewStakingAddress(CTxDestination& ret, std::string label);
+    CallResult<CTxDestination> getNewAddress(CTxDestination& ret, std::string label);
+    CallResult<CTxDestination> getNewStakingAddress(CTxDestination& ret, std::string label);
     int64_t GetKeyCreationTime(const CWDestination& dest);
     int64_t GetKeyCreationTime(CPubKey pubkey);
     int64_t GetKeyCreationTime(const CTxDestination& address);

From 3eae9373c936ec55d83a21b8993157a16326e44c Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Jun 2021 11:50:53 -0300
Subject: [PATCH 2/6] Refactor: remove now unused pairresult.h file

---
 src/Makefile.am                        |  1 -
 src/pairresult.h                       | 20 --------------------
 src/qt/pivx/masternodewizarddialog.cpp |  1 -
 src/qt/pivx/receivewidget.cpp          |  1 -
 src/qt/pivx/requestdialog.cpp          |  1 -
 src/qt/walletmodel.h                   |  1 -
 6 files changed, 25 deletions(-)
 delete mode 100644 src/pairresult.h

diff --git a/src/Makefile.am b/src/Makefile.am
index c873941d9586..64897dec01cb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -187,7 +187,6 @@ BITCOIN_CORE_H = \
   evo/evonotificationinterface.h \
   evo/providertx.h \
   evo/specialtx.h \
-  pairresult.h \
   addressbook.h \
   wallet/db.h \
   flatfile.h \
diff --git a/src/pairresult.h b/src/pairresult.h
deleted file mode 100644
index 9cb6b3fe2d6f..000000000000
--- a/src/pairresult.h
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright (c) 2019 The PIVX developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef PIVX_PAIRRESULT_H
-#define PIVX_PAIRRESULT_H
-
-
-class PairResult {
-
-public:
-    PairResult(bool res):result(res){}
-    PairResult(bool res, std::string* statusStr):result(res), status(statusStr){}
-
-    bool result;
-    std::string* status = nullptr;
-};
-
-
-#endif //PIVX_PAIRRESULT_H
diff --git a/src/qt/pivx/masternodewizarddialog.cpp b/src/qt/pivx/masternodewizarddialog.cpp
index 9d2e80c519ae..663d466a0a36 100644
--- a/src/qt/pivx/masternodewizarddialog.cpp
+++ b/src/qt/pivx/masternodewizarddialog.cpp
@@ -9,7 +9,6 @@
 #include "clientmodel.h"
 #include "key_io.h"
 #include "optionsmodel.h"
-#include "pairresult.h"
 #include "qt/pivx/mnmodel.h"
 #include "qt/pivx/guitransactionsutils.h"
 #include "qt/pivx/qtutils.h"
diff --git a/src/qt/pivx/receivewidget.cpp b/src/qt/pivx/receivewidget.cpp
index 008c377c0b17..4fc8c9347b55 100644
--- a/src/qt/pivx/receivewidget.cpp
+++ b/src/qt/pivx/receivewidget.cpp
@@ -12,7 +12,6 @@
 #include "qt/pivx/addressholder.h"
 #include "walletmodel.h"
 #include "guiutil.h"
-#include "pairresult.h"
 
 #include <QModelIndex>
 #include <QColor>
diff --git a/src/qt/pivx/requestdialog.cpp b/src/qt/pivx/requestdialog.cpp
index e953578b3847..caf67ce5f4dd 100644
--- a/src/qt/pivx/requestdialog.cpp
+++ b/src/qt/pivx/requestdialog.cpp
@@ -9,7 +9,6 @@
 #include "qt/pivx/qtutils.h"
 #include "guiutil.h"
 #include "amount.h"
-#include "pairresult.h"
 #include "optionsmodel.h"
 
 RequestDialog::RequestDialog(QWidget *parent) :
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index e7e24191f025..e00f93858fc2 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -17,7 +17,6 @@
 #include "key_io.h"
 #include "operationresult.h"
 #include "support/allocators/zeroafterfree.h"
-#include "pairresult.h"
 
 #include <map>
 #include <vector>

From fe7580e6f2c19b0bd5cdd1a1eb030aa6422d322f Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Jun 2021 12:21:29 -0300
Subject: [PATCH 3/6] Refactor: remove now unneeded CTxDestination ref argument
  in `CWallet::getNewAddress` and `CWallet::getNewStakingAddress` methods.

---
 src/operationresult.h                            |  2 +-
 src/qt/walletmodel.cpp                           | 14 ++++++--------
 src/qt/walletmodel.h                             |  4 ++--
 src/test/librust/sapling_rpc_wallet_tests.cpp    |  5 +++--
 src/wallet/rpcwallet.cpp                         |  7 +++----
 ...et_sapling_transactions_validations_tests.cpp | 15 +++++++++------
 src/wallet/test/wallet_tests.cpp                 |  5 +++--
 src/wallet/wallet.cpp                            | 16 ++++++++--------
 src/wallet/wallet.h                              |  8 ++++----
 9 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/src/operationresult.h b/src/operationresult.h
index 31bc1f44bd52..93a7b9d062ff 100644
--- a/src/operationresult.h
+++ b/src/operationresult.h
@@ -36,7 +36,7 @@ class CallResult : public OperationResult
 public:
     CallResult(bool _res, const std::string& _error) : OperationResult(_res, _error) { }
     CallResult(bool _res, T _obj) : OperationResult(_res), m_obj_res(_obj) { }
-    Optional<T>& getObjResult() const { return m_obj_res; }
+    const Optional<T>& getObjResult() const { return m_obj_res; }
 };
 
 
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 0dc4eadf4e99..1ba365a7dad5 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -926,19 +926,17 @@ int64_t WalletModel::getKeyCreationTime(const libzcash::SaplingPaymentAddress& a
     return 0;
 }
 
-CallResult<Destination> WalletModel::getNewAddress(Destination& ret, std::string label) const
+CallResult<Destination> WalletModel::getNewAddress(Destination& ret, const std::string& label) const
 {
-    CTxDestination dest;
-    auto res = wallet->getNewAddress(dest, label);
-    if (res) ret = Destination(dest, false);
+    auto res = wallet->getNewAddress(label);
+    if (res) ret = Destination(*res.getObjResult(), false);
     return CallResult<Destination>(res.getResult(), ret);
 }
 
-CallResult<Destination> WalletModel::getNewStakingAddress(Destination& ret, std::string label) const
+CallResult<Destination> WalletModel::getNewStakingAddress(Destination& ret, const std::string& label) const
 {
-    CTxDestination dest;
-    auto res = wallet->getNewStakingAddress(dest, label);
-    if (res) ret = Destination(dest, true);
+    auto res = wallet->getNewStakingAddress(label);
+    if (res) ret = Destination(*res.getObjResult(), true);
     return CallResult<Destination>(res.getResult(), ret);
 }
 
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index e00f93858fc2..646a16c130dc 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -272,11 +272,11 @@ class WalletModel : public QObject
     int64_t getKeyCreationTime(const CTxDestination& address);
     int64_t getKeyCreationTime(const std::string& address);
     int64_t getKeyCreationTime(const libzcash::SaplingPaymentAddress& address);
-    CallResult<Destination> getNewAddress(Destination& ret, std::string label = "") const;
+    CallResult<Destination> getNewAddress(Destination& ret, const std::string& label = "") const;
     /**
      * Return a new address used to receive for delegated cold stake purpose.
      */
-    CallResult<Destination> getNewStakingAddress(Destination& ret, std::string label = "") const;
+    CallResult<Destination> getNewStakingAddress(Destination& ret, const std::string& label = "") const;
 
     //! Return a new shielded address.
     CallResult<Destination> getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel = "");
diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp
index 05cb6178a4b9..8615582036a4 100644
--- a/src/test/librust/sapling_rpc_wallet_tests.cpp
+++ b/src/test/librust/sapling_rpc_wallet_tests.cpp
@@ -421,8 +421,9 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
     UniValue retValue;
 
     // add keys manually
-    CTxDestination taddr;
-    m_wallet.getNewAddress(taddr, "");
+    auto res = m_wallet.getNewAddress("");
+    BOOST_CHECK(res);
+    CTxDestination taddr = *res.getObjResult();
     std::string taddr1 = EncodeDestination(taddr);
     auto zaddr1 = m_wallet.GenerateNewSaplingZKey();
 
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index bc34876ff4c5..32cb187dba03 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -117,11 +117,10 @@ static CTxDestination GetNewAddressFromLabel(CWallet* const pwallet, const std::
     if (!params.isNull() && params.size() > 0)
         label = LabelFromValue(params[0]);
 
-    CTxDestination address;
-    auto r = pwallet->getNewAddress(address, label, purpose, addrType);
+    auto r = pwallet->getNewAddress(label, purpose, addrType);
     if(!r)
         throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, r.getError());
-    return address;
+    return *r.getObjResult();
 }
 
 /** Convert CAddressBookData to JSON record.  */
@@ -2437,7 +2436,7 @@ UniValue sendmany(const JSONRPCRequest& request)
     bool fShieldSend = false;
     for (const std::string& key : sendTo.getKeys()) {
         bool isStaking = false, isShielded = false;
-        const CWDestination& dest = Standard::DecodeDestination(key, isStaking, isShielded);
+        Standard::DecodeDestination(key, isStaking, isShielded);
         if (isShielded) {
             fShieldSend = true;
             break;
diff --git a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
index ade9a1078d2a..a4d5a4b2cf95 100644
--- a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
+++ b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
@@ -77,8 +77,9 @@ static SaplingOperation createOperationAndBuildTx(std::unique_ptr<CWallet>& pwal
 // Test double spend notes in the mempool and in blocks.
 BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
 {
-    CTxDestination coinbaseDest;
-    auto ret = pwalletMain->getNewAddress(coinbaseDest, "coinbase");
+    auto ret = pwalletMain->getNewAddress("coinbase");
+    BOOST_CHECK(ret);
+    CTxDestination coinbaseDest = *ret.getObjResult();
     BOOST_ASSERT_MSG(ret, "cannot create address");
     BOOST_ASSERT_MSG(IsValidDestination(coinbaseDest), "invalid destination");
     BOOST_ASSERT_MSG(IsMine(*pwalletMain, coinbaseDest), "destination not from wallet");
@@ -126,15 +127,17 @@ BOOST_AUTO_TEST_CASE(test_in_block_and_mempool_notes_double_spend)
 
     // first generate a valid tx spending only one note
     // Create the operation and build the transaction
-    CTxDestination tDest2;
-    pwalletMain->getNewAddress(tDest2, "receiveValid");
+    auto res = pwalletMain->getNewAddress("receiveValid");
+    BOOST_CHECK(res);
+    CTxDestination tDest2 = *res.getObjResult();
     std::vector<SendManyRecipient> recipients2;
     recipients2.emplace_back(tDest2, CAmount(90 * COIN), false);
     SaplingOperation operation2 = createOperationAndBuildTx(pwalletMain, recipients2, tipHeight + 1, false);
 
     // Create a second transaction that spends the same note with a different output now
-    CTxDestination tDest3;
-    pwalletMain->getNewAddress(tDest3, "receiveInvalid");
+    res = pwalletMain->getNewAddress("receiveInvalid");
+    BOOST_CHECK(res);
+    CTxDestination tDest3 = *res.getObjResult();;
     std::vector<SendManyRecipient> recipients3;
     recipients3.emplace_back(tDest3, CAmount(5 * COIN), false);
     SaplingOperation operation3 = createOperationAndBuildTx(pwalletMain, recipients3, tipHeight + 1, false);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 08ebf899ddc5..645a7f1341e0 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -577,8 +577,9 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests)
     wallet.SetLastBlockProcessed(chainActive.Tip());
 
     // Receive balance from an external source
-    CTxDestination receivingAddr;
-    BOOST_ASSERT(wallet.getNewAddress(receivingAddr, "receiving_address"));
+    auto res = wallet.getNewAddress("receiving_address");
+    BOOST_ASSERT(res);
+    CTxDestination receivingAddr = *res.getObjResult();
     CTxOut creditOut(nCredit/2, GetScriptForDestination(receivingAddr));
     CWalletTx& wtxCredit = ReceiveBalanceWith({creditOut, creditOut},wallet);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index eac30bef3544..0781a20ec4e2 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -12,7 +12,6 @@
 #include "evo/deterministicmns.h"
 #include "guiinterfaceutil.h"
 #include "masternode.h"
-#include "masternode-payments.h"
 #include "policy/policy.h"
 #include "sapling/key_io_sapling.h"
 #include "script/sign.h"
@@ -176,15 +175,17 @@ std::vector<CWalletTx> CWallet::getWalletTxs()
     return result;
 }
 
-CallResult<CTxDestination> CWallet::getNewAddress(CTxDestination& ret, std::string label){
-    return getNewAddress(ret, label, AddressBook::AddressBookPurpose::RECEIVE);
+CallResult<CTxDestination> CWallet::getNewAddress(const std::string& label)
+{
+    return getNewAddress(label, AddressBook::AddressBookPurpose::RECEIVE);
 }
 
-CallResult<CTxDestination> CWallet::getNewStakingAddress(CTxDestination& ret, std::string label){
-    return getNewAddress(ret, label, AddressBook::AddressBookPurpose::COLD_STAKING, CChainParams::Base58Type::STAKING_ADDRESS);
+CallResult<CTxDestination> CWallet::getNewStakingAddress(const std::string& label)
+{
+    return getNewAddress(label, AddressBook::AddressBookPurpose::COLD_STAKING, CChainParams::Base58Type::STAKING_ADDRESS);
 }
 
-CallResult<CTxDestination> CWallet::getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
+CallResult<CTxDestination> CWallet::getNewAddress(const std::string& addressLabel, const std::string purpose,
                                          const CChainParams::Base58Type addrType)
 {
     LOCK(cs_wallet);
@@ -206,8 +207,7 @@ CallResult<CTxDestination> CWallet::getNewAddress(CTxDestination& ret, const std
     if (!SetAddressBook(keyID, addressLabel, purpose))
         throw std::runtime_error("CWallet::getNewAddress() : SetAddressBook failed");
 
-    ret = CTxDestination(keyID);
-    return CallResult<CTxDestination>(true, ret);
+    return CallResult<CTxDestination>(true, CTxDestination(keyID));
 }
 
 int64_t CWallet::GetKeyCreationTime(const CWDestination& dest)
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 1a49043502bc..250e5851b7a1 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -886,10 +886,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     void LockIfMyCollateral(const CTransactionRef& ptx);
 
     //  keystore implementation
-    CallResult<CTxDestination> getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose,
-                                           const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS);
-    CallResult<CTxDestination> getNewAddress(CTxDestination& ret, std::string label);
-    CallResult<CTxDestination> getNewStakingAddress(CTxDestination& ret, std::string label);
+    CallResult<CTxDestination> getNewAddress(const std::string& addressLabel, const std::string purpose,
+                                             const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS);
+    CallResult<CTxDestination> getNewAddress(const std::string& label);
+    CallResult<CTxDestination> getNewStakingAddress(const std::string& label);
     int64_t GetKeyCreationTime(const CWDestination& dest);
     int64_t GetKeyCreationTime(CPubKey pubkey);
     int64_t GetKeyCreationTime(const CTxDestination& address);

From 215644ad267db8b70f2f4699677a0a95511c3266 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Jun 2021 12:47:24 -0300
Subject: [PATCH 4/6] Refactor: remove now unneeded Destination ref argument 
 in `WalletModel::getNewAddress` and `WalletModel::getNewStakingAddress`
 methods.

---
 src/key_io.h                           |  2 +-
 src/operationresult.h                  |  5 +++--
 src/qt/addresstablemodel.cpp           |  7 ++-----
 src/qt/pivx/masternodewizarddialog.cpp |  6 ++----
 src/qt/pivx/receivewidget.cpp          |  5 ++---
 src/qt/pivx/requestdialog.cpp          |  9 ++++-----
 src/qt/walletmodel.cpp                 | 17 +++++++++--------
 src/qt/walletmodel.h                   |  4 ++--
 src/wallet/wallet.cpp                  |  4 ++--
 9 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/src/key_io.h b/src/key_io.h
index 5ded92522a7c..78404bc59ea2 100644
--- a/src/key_io.h
+++ b/src/key_io.h
@@ -56,7 +56,7 @@ struct Destination {
         return *this;
     }
 
-    std::string ToString()
+    std::string ToString() const
     {
         if (!IsValidDestination(dest)) {
             // Invalid address
diff --git a/src/operationresult.h b/src/operationresult.h
index 93a7b9d062ff..087ed5e0a0e4 100644
--- a/src/operationresult.h
+++ b/src/operationresult.h
@@ -34,8 +34,9 @@ class CallResult : public OperationResult
 private:
     Optional<T> m_obj_res{nullopt};
 public:
-    CallResult(bool _res, const std::string& _error) : OperationResult(_res, _error) { }
-    CallResult(bool _res, T _obj) : OperationResult(_res), m_obj_res(_obj) { }
+    CallResult() : OperationResult(false) {}
+    CallResult(T _obj) : OperationResult(true), m_obj_res(_obj) { }
+    CallResult(const std::string& error) : OperationResult(false, error) { }
     const Optional<T>& getObjResult() const { return m_obj_res; }
 };
 
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index 957d4dffed28..a14ae2de179a 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -634,11 +634,8 @@ QString AddressTableModel::getAddressToShow(bool isShielded) const
     CallResult<Destination> res(false, "");
     QString addressStr;
     if (!isShielded) {
-        Destination newAddress;
-        res = walletModel->getNewAddress(newAddress, "Default");
-        if (res) {
-            addressStr = QString::fromStdString(newAddress.ToString());
-        }
+        res = walletModel->getNewAddress("Default");
+        if (res) { addressStr = QString::fromStdString(res.getObjResult()->ToString()); }
     } else {
         res = walletModel->getNewShieldedAddress(addressStr, "default shielded");
     }
diff --git a/src/qt/pivx/masternodewizarddialog.cpp b/src/qt/pivx/masternodewizarddialog.cpp
index 663d466a0a36..25c4850b6c04 100644
--- a/src/qt/pivx/masternodewizarddialog.cpp
+++ b/src/qt/pivx/masternodewizarddialog.cpp
@@ -220,9 +220,7 @@ bool MasterNodeWizardDialog::createMN()
     // If not found create a new collateral tx
     if (!walletModel->getMNCollateralCandidate(collateralOut)) {
         // New receive address
-        Destination dest;
-        auto r = walletModel->getNewAddress(dest, alias);
-
+        auto r = walletModel->getNewAddress(alias);
         if (!r) {
             // generate address fail
             inform(tr(r.getError().c_str()));
@@ -231,7 +229,7 @@ bool MasterNodeWizardDialog::createMN()
 
         // const QString& addr, const QString& label, const CAmount& amount, const QString& message
         SendCoinsRecipient sendCoinsRecipient(
-                QString::fromStdString(dest.ToString()),
+                QString::fromStdString(r.getObjResult()->ToString()),
                 QString::fromStdString(alias),
                 clientModel->getMNCollateralRequiredAmount(),
                 "");
diff --git a/src/qt/pivx/receivewidget.cpp b/src/qt/pivx/receivewidget.cpp
index 4fc8c9347b55..ecfb66857968 100644
--- a/src/qt/pivx/receivewidget.cpp
+++ b/src/qt/pivx/receivewidget.cpp
@@ -253,9 +253,8 @@ void ReceiveWidget::onNewAddressClicked()
         QString strAddress;
         CallResult<Destination> r(false, "");
         if (!shieldedMode) {
-            Destination address;
-            r = walletModel->getNewAddress(address, "");
-            strAddress = QString::fromStdString(address.ToString());
+            r = walletModel->getNewAddress("");
+            if (r) strAddress = QString::fromStdString(r.getObjResult()->ToString());
         } else {
             r = walletModel->getNewShieldedAddress(strAddress, "");
         }
diff --git a/src/qt/pivx/requestdialog.cpp b/src/qt/pivx/requestdialog.cpp
index caf67ce5f4dd..31196111824d 100644
--- a/src/qt/pivx/requestdialog.cpp
+++ b/src/qt/pivx/requestdialog.cpp
@@ -113,13 +113,12 @@ void RequestDialog::accept()
         std::string label = info->label.isEmpty() ? "" : info->label.toStdString();
         QString title;
 
-        Destination address;
-        CallResult<Destination> r(false, "");
+        CallResult<Destination> r;
         if (this->isPaymentRequest) {
-            r = walletModel->getNewAddress(address, label);
+            r = walletModel->getNewAddress(label);
             title = tr("Request for ") + BitcoinUnits::format(displayUnit, value, false, BitcoinUnits::separatorAlways) + " " + QString(CURRENCY_UNIT.c_str());
         } else {
-            r = walletModel->getNewStakingAddress(address, label);
+            r = walletModel->getNewStakingAddress(label);
             title = tr("Cold Staking Address Generated");
         }
 
@@ -129,7 +128,7 @@ void RequestDialog::accept()
             return;
         }
 
-        info->address = QString::fromStdString(address.ToString());
+        info->address = QString::fromStdString(r.getObjResult()->ToString());
         ui->labelTitle->setText(title);
 
         updateQr(info->address);
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 1ba365a7dad5..8e3ac563c59d 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -475,8 +475,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
                 Destination ownerAdd;
                 if (rcp.ownerAddress.isEmpty()) {
                     // Create new internal owner address
-                    if (!getNewAddress(ownerAdd))
-                        return CannotCreateInternalAddress;
+                    auto res = getNewAddress();
+                    if (!res) return CannotCreateInternalAddress;
+                    ownerAdd = *res.getObjResult();
                 } else {
                     ownerAdd = Destination(DecodeDestination(rcp.ownerAddress.toStdString()), false);
                 }
@@ -926,18 +927,18 @@ int64_t WalletModel::getKeyCreationTime(const libzcash::SaplingPaymentAddress& a
     return 0;
 }
 
-CallResult<Destination> WalletModel::getNewAddress(Destination& ret, const std::string& label) const
+CallResult<Destination> WalletModel::getNewAddress(const std::string& label) const
 {
     auto res = wallet->getNewAddress(label);
-    if (res) ret = Destination(*res.getObjResult(), false);
-    return CallResult<Destination>(res.getResult(), ret);
+    return res ? CallResult<Destination>(Destination(*res.getObjResult(), false)) :
+           CallResult<Destination>(res.getError());
 }
 
-CallResult<Destination> WalletModel::getNewStakingAddress(Destination& ret, const std::string& label) const
+CallResult<Destination> WalletModel::getNewStakingAddress(const std::string& label) const
 {
     auto res = wallet->getNewStakingAddress(label);
-    if (res) ret = Destination(*res.getObjResult(), true);
-    return CallResult<Destination>(res.getResult(), ret);
+    return res ? CallResult<Destination>(Destination(*res.getObjResult(), true)) :
+           CallResult<Destination>(res.getError());
 }
 
 CallResult<Destination> WalletModel::getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel)
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index 646a16c130dc..c50022dc9c4c 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -272,11 +272,11 @@ class WalletModel : public QObject
     int64_t getKeyCreationTime(const CTxDestination& address);
     int64_t getKeyCreationTime(const std::string& address);
     int64_t getKeyCreationTime(const libzcash::SaplingPaymentAddress& address);
-    CallResult<Destination> getNewAddress(Destination& ret, const std::string& label = "") const;
+    CallResult<Destination> getNewAddress(const std::string& label = "") const;
     /**
      * Return a new address used to receive for delegated cold stake purpose.
      */
-    CallResult<Destination> getNewStakingAddress(Destination& ret, const std::string& label = "") const;
+    CallResult<Destination> getNewStakingAddress(const std::string& label = "") const;
 
     //! Return a new shielded address.
     CallResult<Destination> getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel = "");
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 0781a20ec4e2..55b47e48f320 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -199,7 +199,7 @@ CallResult<CTxDestination> CWallet::getNewAddress(const std::string& addressLabe
     // Get a key
     if (!GetKeyFromPool(newKey, type)) {
         // inform the user to top-up the keypool or unlock the wallet
-        return CallResult<CTxDestination>(false, std::string(
+        return CallResult<CTxDestination>(std::string(
                         _("Keypool ran out, please call keypoolrefill first, or unlock the wallet.")));
     }
     CKeyID keyID = newKey.GetID();
@@ -207,7 +207,7 @@ CallResult<CTxDestination> CWallet::getNewAddress(const std::string& addressLabe
     if (!SetAddressBook(keyID, addressLabel, purpose))
         throw std::runtime_error("CWallet::getNewAddress() : SetAddressBook failed");
 
-    return CallResult<CTxDestination>(true, CTxDestination(keyID));
+    return CallResult<CTxDestination>(CTxDestination(keyID));
 }
 
 int64_t CWallet::GetKeyCreationTime(const CWDestination& dest)

From a8a74ec26d9f222da2a9b526ddd69ef7d08d2748 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Sun, 25 Jul 2021 12:27:31 -0300
Subject: [PATCH 5/6] Refactor: move Destination to destination_io.h and add
 CWDestination variant support.

---
 src/destination_io.h   | 35 +++++++++++++++++++++++++++++++++++
 src/key_io.h           | 28 ----------------------------
 src/qt/walletmodel.cpp |  2 +-
 src/qt/walletmodel.h   |  1 -
 4 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/src/destination_io.h b/src/destination_io.h
index 938e693b57e6..6efdfc6580f5 100644
--- a/src/destination_io.h
+++ b/src/destination_io.h
@@ -26,4 +26,39 @@ namespace Standard {
 
 } // End Standard namespace
 
+/**
+ * Wrapper class for every supported address
+ */
+struct Destination {
+public:
+    explicit Destination() {}
+    explicit Destination(const CTxDestination& _dest, bool _isP2CS) : dest(_dest), isP2CS(_isP2CS) {}
+
+    CWDestination dest{CNoDestination()};
+    bool isP2CS{false};
+
+    Destination& operator=(const Destination& from)
+    {
+        this->dest = from.dest;
+        this->isP2CS = from.isP2CS;
+        return *this;
+    }
+
+    // Returns the key ID if Destination is a transparent "regular" destination
+    const CKeyID* getKeyID()
+    {
+        const CTxDestination* regDest = Standard::GetTransparentDestination(dest);
+        return (regDest) ? boost::get<CKeyID>(regDest) : nullptr;
+    }
+
+    std::string ToString() const
+    {
+        if (!Standard::IsValidDestination(dest)) {
+            // Invalid address
+            return "";
+        }
+        return Standard::EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS);
+    }
+};
+
 #endif //DESTINATION_IO_H
diff --git a/src/key_io.h b/src/key_io.h
index 78404bc59ea2..b6cfb58621cc 100644
--- a/src/key_io.h
+++ b/src/key_io.h
@@ -38,32 +38,4 @@ namespace KeyIO {
 
 }
 
-/**
- * Wrapper class for every supported address
- */
-struct Destination {
-public:
-    explicit Destination() {}
-    explicit Destination(const CTxDestination& _dest, bool _isP2CS) : dest(_dest), isP2CS(_isP2CS) {}
-
-    CTxDestination dest{CNoDestination()};
-    bool isP2CS{false};
-
-    Destination& operator=(const Destination& from)
-    {
-        this->dest = from.dest;
-        this->isP2CS = from.isP2CS;
-        return *this;
-    }
-
-    std::string ToString() const
-    {
-        if (!IsValidDestination(dest)) {
-            // Invalid address
-            return "";
-        }
-        return EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS);
-    }
-};
-
 #endif //PIVX_KEY_IO_H
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 8e3ac563c59d..db4a08843503 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -483,7 +483,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
                 }
 
                 const CKeyID* stakerId = boost::get<CKeyID>(&out);
-                const CKeyID* ownerId = boost::get<CKeyID>(&ownerAdd.dest);
+                const CKeyID* ownerId = ownerAdd.getKeyID();
                 if (!stakerId || !ownerId) {
                     return InvalidAddress;
                 }
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index c50022dc9c4c..ea8b5ed76d9f 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -14,7 +14,6 @@
 #include "interfaces/wallet.h"
 
 #include "key.h"
-#include "key_io.h"
 #include "operationresult.h"
 #include "support/allocators/zeroafterfree.h"
 

From c4fb14da56d318801079ac17e896c91abd1f70f4 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Sun, 25 Jul 2021 13:33:05 -0300
Subject: [PATCH 6/6] Refactor: WalletModel::getNewShieldedAddress fully
 aligned to the CallResult<Destination> abstraction. No more ref args.

Plus moved Destination functions implementation to cpp file.
---
 src/destination_io.cpp        | 23 +++++++++++++++++++++++
 src/destination_io.h          | 28 ++++++----------------------
 src/qt/addresstablemodel.cpp  | 12 +++---------
 src/qt/pivx/receivewidget.cpp | 12 +++---------
 src/qt/pivx/send.cpp          |  6 ++----
 src/qt/walletmodel.cpp        |  7 +++----
 src/qt/walletmodel.h          |  2 +-
 7 files changed, 41 insertions(+), 49 deletions(-)

diff --git a/src/destination_io.cpp b/src/destination_io.cpp
index f4816f52c59f..5917263f481f 100644
--- a/src/destination_io.cpp
+++ b/src/destination_io.cpp
@@ -66,3 +66,26 @@ namespace Standard {
 
 } // End Standard namespace
 
+Destination& Destination::operator=(const Destination& from)
+{
+    this->dest = from.dest;
+    this->isP2CS = from.isP2CS;
+    return *this;
+}
+
+// Returns the key ID if Destination is a transparent "regular" destination
+const CKeyID* Destination::getKeyID()
+{
+    const CTxDestination* regDest = Standard::GetTransparentDestination(dest);
+    return (regDest) ? boost::get<CKeyID>(regDest) : nullptr;
+}
+
+std::string Destination::ToString() const
+{
+    if (!Standard::IsValidDestination(dest)) {
+        // Invalid address
+        return "";
+    }
+    return Standard::EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS);
+}
+
diff --git a/src/destination_io.h b/src/destination_io.h
index 6efdfc6580f5..91730bae0298 100644
--- a/src/destination_io.h
+++ b/src/destination_io.h
@@ -29,36 +29,20 @@ namespace Standard {
 /**
  * Wrapper class for every supported address
  */
-struct Destination {
+class Destination {
 public:
     explicit Destination() {}
     explicit Destination(const CTxDestination& _dest, bool _isP2CS) : dest(_dest), isP2CS(_isP2CS) {}
+    explicit Destination(const libzcash::SaplingPaymentAddress& _dest) : dest(_dest) {}
 
     CWDestination dest{CNoDestination()};
     bool isP2CS{false};
 
-    Destination& operator=(const Destination& from)
-    {
-        this->dest = from.dest;
-        this->isP2CS = from.isP2CS;
-        return *this;
-    }
-
+    Destination& operator=(const Destination& from);
     // Returns the key ID if Destination is a transparent "regular" destination
-    const CKeyID* getKeyID()
-    {
-        const CTxDestination* regDest = Standard::GetTransparentDestination(dest);
-        return (regDest) ? boost::get<CKeyID>(regDest) : nullptr;
-    }
-
-    std::string ToString() const
-    {
-        if (!Standard::IsValidDestination(dest)) {
-            // Invalid address
-            return "";
-        }
-        return Standard::EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS);
-    }
+    const CKeyID* getKeyID();
+    // Returns the encoded string address
+    std::string ToString() const;
 };
 
 #endif //DESTINATION_IO_H
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index a14ae2de179a..8bc3830807e8 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -631,15 +631,9 @@ QString AddressTableModel::getAddressToShow(bool isShielded) const
     }
 
     // For some reason we don't have any address in our address book, let's create one
-    CallResult<Destination> res(false, "");
-    QString addressStr;
-    if (!isShielded) {
-        res = walletModel->getNewAddress("Default");
-        if (res) { addressStr = QString::fromStdString(res.getObjResult()->ToString()); }
-    } else {
-        res = walletModel->getNewShieldedAddress(addressStr, "default shielded");
-    }
-    return addressStr;
+    CallResult<Destination> res = !isShielded ? walletModel->getNewAddress("Default") :
+            walletModel->getNewShieldedAddress("default shielded");
+    return (res) ? QString::fromStdString(res.getObjResult()->ToString()) : "";;
 }
 
 void AddressTableModel::emitDataChanged(int idx)
diff --git a/src/qt/pivx/receivewidget.cpp b/src/qt/pivx/receivewidget.cpp
index ecfb66857968..220c0eb2497c 100644
--- a/src/qt/pivx/receivewidget.cpp
+++ b/src/qt/pivx/receivewidget.cpp
@@ -250,14 +250,8 @@ void ReceiveWidget::onNewAddressClicked()
             return;
         }
 
-        QString strAddress;
-        CallResult<Destination> r(false, "");
-        if (!shieldedMode) {
-            r = walletModel->getNewAddress("");
-            if (r) strAddress = QString::fromStdString(r.getObjResult()->ToString());
-        } else {
-            r = walletModel->getNewShieldedAddress(strAddress, "");
-        }
+        CallResult<Destination> r = !shieldedMode ? walletModel->getNewAddress("") :
+                walletModel->getNewShieldedAddress("");
 
         // Check validity
         if (!r) {
@@ -265,7 +259,7 @@ void ReceiveWidget::onNewAddressClicked()
             return;
         }
 
-        refreshView(strAddress);
+        refreshView(QString::fromStdString(r.getObjResult()->ToString()));
         inform(tr("New address created"));
     } catch (const std::runtime_error& error) {
         // Error generating address
diff --git a/src/qt/pivx/send.cpp b/src/qt/pivx/send.cpp
index f0dedc4a20b8..9826a4a6e198 100644
--- a/src/qt/pivx/send.cpp
+++ b/src/qt/pivx/send.cpp
@@ -743,14 +743,12 @@ void SendWidget::onShieldCoinsClicked()
 
         // Process spending
         ProcessSend(recipients, true, [this](QList<SendCoinsRecipient>& recipients) {
-            QString strAddress;
-            auto res = walletModel->getNewShieldedAddress(strAddress, "");
-            // Check for generation errors
+            auto res = walletModel->getNewShieldedAddress("");
             if (!res) {
                 inform(tr("Error generating address to shield PIVs"));
                 return false;
             }
-            recipients.back().address = strAddress;
+            recipients.back().address = QString::fromStdString(res.getObjResult()->ToString());
             resetCoinControl();
             return true;
         });
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index db4a08843503..6a7cacc16f70 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -27,6 +27,7 @@
 #include <QtConcurrent/QtConcurrent>
 #include <QSet>
 #include <QTimer>
+#include <utility>
 
 
 WalletModel::WalletModel(CWallet* wallet, OptionsModel* optionsModel, QObject* parent) : QObject(parent), wallet(wallet), walletWrapper(*wallet),
@@ -941,11 +942,9 @@ CallResult<Destination> WalletModel::getNewStakingAddress(const std::string& lab
            CallResult<Destination>(res.getError());
 }
 
-CallResult<Destination> WalletModel::getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel)
+CallResult<Destination> WalletModel::getNewShieldedAddress(std::string strLabel)
 {
-    shieldedAddrRet = QString::fromStdString(
-            KeyIO::EncodePaymentAddress(wallet->GenerateNewSaplingZKey(strLabel)));
-    return CallResult<Destination>(true, "");
+    return CallResult<Destination>(Destination(wallet->GenerateNewSaplingZKey(std::move(strLabel))));
 }
 
 bool WalletModel::whitelistAddressFromColdStaking(const QString &addressStr)
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index ea8b5ed76d9f..1b14ca6ae69a 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -278,7 +278,7 @@ class WalletModel : public QObject
     CallResult<Destination> getNewStakingAddress(const std::string& label = "") const;
 
     //! Return a new shielded address.
-    CallResult<Destination> getNewShieldedAddress(QString& shieldedAddrRet, std::string strLabel = "");
+    CallResult<Destination> getNewShieldedAddress(std::string strLabel = "");
 
     //! Return new wallet rescan reserver
     WalletRescanReserver getRescanReserver() const { return WalletRescanReserver(wallet); }