Merge #16415: Get rid of PendingWalletTx class

4d94916f0d Get rid of PendingWalletTx class. (Russell Yanofsky)

Pull request description:

  No reason for this class to exist if it doesn't have any code to run in the destructor. e10e1e8db0 from https://github.com/bitcoin/bitcoin/pull/16208 recently removed the destructor code that would return an unused key if the transaction wasn't committed.

  This is just cleanup, there's no change in behavior.

ACKs for top commit:
  ariard:
    utACK 4d94916. Successfully built both `bitcoind` and `bitcoin-qt`. `PendingWalletTx` was only a wrapper to enforce call to `ReturnDestination` if `CommitTransaction` doesn't `KeepDestination` before.
  promag:
    ACK 4d94916f0d, refactor looks good to me.
  meshcollider:
    utACK 4d94916f0d

Tree-SHA512: f3f93d2f2f5d8f1e7810d609d881c1b1cbbaa8629f483f4293e20b3210292605e947bc4903fde9d2d8736277ca3bd6de182f7eac1e13515d5a327f2ebc130839
This commit is contained in:
MeshCollider 2019-07-27 22:35:13 +12:00
commit 1139e3cb76
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
6 changed files with 33 additions and 55 deletions

View file

@ -33,31 +33,6 @@
namespace interfaces {
namespace {
class PendingWalletTxImpl : public PendingWalletTx
{
public:
explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet) {}
const CTransaction& get() override { return *m_tx; }
bool commit(WalletValueMap value_map,
WalletOrderForm order_form,
std::string& reject_reason) override
{
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
CValidationState state;
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), state)) {
reject_reason = state.GetRejectReason();
return false;
}
return true;
}
CTransactionRef m_tx;
CWallet& m_wallet;
};
//! Construct wallet tx struct.
WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx)
{
@ -227,7 +202,7 @@ public:
LOCK(m_wallet->cs_wallet);
return m_wallet->ListLockedCoins(outputs);
}
std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
const CCoinControl& coin_control,
bool sign,
int& change_pos,
@ -236,12 +211,26 @@ public:
{
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet);
auto pending = MakeUnique<PendingWalletTxImpl>(*m_wallet);
if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, fee, change_pos,
CTransactionRef tx;
if (!m_wallet->CreateTransaction(*locked_chain, recipients, tx, fee, change_pos,
fail_reason, coin_control, sign)) {
return {};
}
return std::move(pending);
return tx;
}
bool commitTransaction(CTransactionRef tx,
WalletValueMap value_map,
WalletOrderForm order_form,
std::string& reject_reason) override
{
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet);
CValidationState state;
if (!m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form), state)) {
reject_reason = state.GetRejectReason();
return false;
}
return true;
}
bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
bool abandonTransaction(const uint256& txid) override

View file

@ -34,7 +34,6 @@ struct CRecipient;
namespace interfaces {
class Handler;
class PendingWalletTx;
struct WalletAddress;
struct WalletBalances;
struct WalletTx;
@ -134,13 +133,19 @@ public:
virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0;
//! Create transaction.
virtual std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients,
virtual CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
const CCoinControl& coin_control,
bool sign,
int& change_pos,
CAmount& fee,
std::string& fail_reason) = 0;
//! Commit transaction.
virtual bool commitTransaction(CTransactionRef tx,
WalletValueMap value_map,
WalletOrderForm order_form,
std::string& reject_reason) = 0;
//! Return whether transaction can be abandoned.
virtual bool transactionCanBeAbandoned(const uint256& txid) = 0;
@ -288,21 +293,6 @@ public:
virtual std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) = 0;
};
//! Tracking object returned by CreateTransaction and passed to CommitTransaction.
class PendingWalletTx
{
public:
virtual ~PendingWalletTx() {}
//! Get transaction data.
virtual const CTransaction& get() = 0;
//! Send pending transaction and commit to wallet.
virtual bool commit(WalletValueMap value_map,
WalletOrderForm order_form,
std::string& reject_reason) = 0;
};
//! Information about one wallet address.
struct WalletAddress
{

View file

@ -392,7 +392,7 @@ void SendCoinsDialog::on_sendButton_clicked()
accept();
CoinControlDialog::coinControl()->UnSelectAll();
coinControlUpdateLabels();
Q_EMIT coinsSent(currentTransaction.getWtx()->get().GetHash());
Q_EMIT coinsSent(currentTransaction.getWtx()->GetHash());
}
fNewRecipientAllowed = true;
}

View file

@ -261,11 +261,11 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran
auto& newTx = transaction.getWtx();
std::string rejectReason;
if (!newTx->commit({} /* mapValue */, std::move(vOrderForm), rejectReason))
if (!wallet().commitTransaction(newTx, {} /* mapValue */, std::move(vOrderForm), rejectReason))
return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(rejectReason));
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << newTx->get();
ssTx << *newTx;
transaction_array.append(&(ssTx[0]), ssTx.size());
}

View file

@ -21,14 +21,14 @@ QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const
return recipients;
}
std::unique_ptr<interfaces::PendingWalletTx>& WalletModelTransaction::getWtx()
CTransactionRef& WalletModelTransaction::getWtx()
{
return wtx;
}
unsigned int WalletModelTransaction::getTransactionSize()
{
return wtx ? GetVirtualTransactionSize(wtx->get()) : 0;
return wtx ? GetVirtualTransactionSize(*wtx) : 0;
}
CAmount WalletModelTransaction::getTransactionFee() const
@ -43,7 +43,7 @@ void WalletModelTransaction::setTransactionFee(const CAmount& newFee)
void WalletModelTransaction::reassignAmounts(int nChangePosRet)
{
const CTransaction* walletTransaction = &wtx->get();
const CTransaction* walletTransaction = wtx.get();
int i = 0;
for (QList<SendCoinsRecipient>::iterator it = recipients.begin(); it != recipients.end(); ++it)
{

View file

@ -16,7 +16,6 @@ class SendCoinsRecipient;
namespace interfaces {
class Node;
class PendingWalletTx;
}
/** Data model for a walletmodel transaction. */
@ -27,7 +26,7 @@ public:
QList<SendCoinsRecipient> getRecipients() const;
std::unique_ptr<interfaces::PendingWalletTx>& getWtx();
CTransactionRef& getWtx();
unsigned int getTransactionSize();
void setTransactionFee(const CAmount& newFee);
@ -39,7 +38,7 @@ public:
private:
QList<SendCoinsRecipient> recipients;
std::unique_ptr<interfaces::PendingWalletTx> wtx;
CTransactionRef wtx;
CAmount fee;
};