Skip to content

Commit

Permalink
Merge #10368: [wallet] Remove helper conversion operator from wallet
Browse files Browse the repository at this point in the history 
5a5e4e9 [wallet] Remove CTransaction&() helper conversion operator from wallet implementation. (Karl-Johan Alm)

Pull request description:

  The `CTransaction&()` operator in `CMerkleTx` makes conversion into `CTransaction`s transparent, but was marked as to-be-removed in favor of explicitly getting the `tx` ivar, presumably as the operator can lead to ambiguous behavior and makes the code harder to follow.

  This PR removes the operator and adapts callers. This includes some cases of `static_cast<CTransaction>(wtx)` → `*wtx.tx`, which is definitely an improvement.

Tree-SHA512: 95856fec7194d6a79615ea1c322abfcd6bcedf6ffd0cfa89bbdd332ce13035fa52dd4b828d20df673072dde1be64b79c513529a6f422dd5f0961ce722a32d56a
  • Loading branch information
@laanwj
laanwj committed Nov 9, 2017
2 parents ef3758d + 5a5e4e9 commit 77ba4bf
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/qt/transactiondesc.cpp
View file
Expand Up @@ -24,7 +24,7 @@
QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx)
{
AssertLockHeld(cs_main);
if (!CheckFinalTx(wtx))
if (!CheckFinalTx(*wtx.tx))
{
if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD)
return tr("Open for %n more block(s)", "", wtx.tx->nLockTime - chainActive.Height());
Expand Down
2 changes: 1 addition & 1 deletion src/qt/transactionrecord.cpp
View file
Expand Up @@ -182,7 +182,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx)
status.depth = wtx.GetDepthInMainChain();
status.cur_num_blocks = chainActive.Height();

if (!CheckFinalTx(wtx))
if (!CheckFinalTx(*wtx.tx))
{
if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD)
{
Expand Down
2 changes: 1 addition & 1 deletion src/qt/transactiontablemodel.cpp
View file
Expand Up @@ -230,7 +230,7 @@ class TransactionTablePriv
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
if(mi != wallet->mapWallet.end())
{
std::string strHex = EncodeHexTx(static_cast<CTransaction>(mi->second));
std::string strHex = EncodeHexTx(*mi->second.tx);
return QString::fromStdString(strHex);
}
return QString();
Expand Down
2 changes: 1 addition & 1 deletion src/qt/walletmodeltransaction.cpp
View file
Expand Up @@ -34,7 +34,7 @@ CWalletTx *WalletModelTransaction::getTransaction() const

unsigned int WalletModelTransaction::getTransactionSize()
{
return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction));
return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction->tx));
}

CAmount WalletModelTransaction::getTransactionFee() const
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/feebumper.cpp
View file
Expand Up @@ -89,7 +89,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin
return;
}

if (!SignalsOptInRBF(wtx)) {
if (!SignalsOptInRBF(*wtx.tx)) {
vErrors.push_back("Transaction is not BIP 125 replaceable");
currentResult = BumpFeeResult::WALLET_ERROR;
return;
Expand All @@ -103,7 +103,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin

// check that original tx consists entirely of our inputs
// if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
if (!pWallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) {
if (!pWallet->IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
vErrors.push_back("Transaction contains inputs that don't belong to this wallet");
currentResult = BumpFeeResult::WALLET_ERROR;
return;
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/rpcdump.cpp
View file
Expand Up @@ -340,7 +340,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request)

LOCK2(cs_main, pwallet->cs_wallet);

if (pwallet->IsMine(wtx)) {
if (pwallet->IsMine(*wtx.tx)) {
pwallet->AddToWallet(wtx, false);
return NullUniValue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/rpcwallet.cpp
View file
Expand Up @@ -108,7 +108,7 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry)
std::string rbfStatus = "no";
if (confirms <= 0) {
LOCK(mempool.cs);
RBFTransactionState rbfState = IsRBFOptIn(wtx, mempool);
RBFTransactionState rbfState = IsRBFOptIn(*wtx.tx, mempool);
if (rbfState == RBF_TRANSACTIONSTATE_UNKNOWN)
rbfStatus = "unknown";
else if (rbfState == RBF_TRANSACTIONSTATE_REPLACEABLE_BIP125)
Expand Down Expand Up @@ -2051,7 +2051,7 @@ UniValue gettransaction(const JSONRPCRequest& request)
ListTransactions(pwallet, wtx, "*", 0, false, details, filter);
entry.push_back(Pair("details", details));

std::string strHex = EncodeHexTx(static_cast<CTransaction>(wtx), RPCSerializationFlags());
std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags());
entry.push_back(Pair("hex", strHex));

return entry;
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/test/accounting_tests.cpp
View file
Expand Up @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)

wtx.mapValue["comment"] = "y";
{
CMutableTransaction tx(wtx);
CMutableTransaction tx(*wtx.tx);
--tx.nLockTime; // Just to change the hash :)
wtx.SetTx(MakeTransactionRef(std::move(tx)));
}
Expand All @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)

wtx.mapValue["comment"] = "x";
{
CMutableTransaction tx(wtx);
CMutableTransaction tx(*wtx.tx);
--tx.nLockTime; // Just to change the hash :)
wtx.SetTx(MakeTransactionRef(std::move(tx)));
}
Expand Down
20 changes: 10 additions & 10 deletions src/wallet/wallet.cpp
View file
Expand Up @@ -1718,7 +1718,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
debit += nDebitCached;
else
{
nDebitCached = pwallet->GetDebit(*this, ISMINE_SPENDABLE);
nDebitCached = pwallet->GetDebit(*tx, ISMINE_SPENDABLE);
fDebitCached = true;
debit += nDebitCached;
}
Expand All @@ -1729,7 +1729,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
debit += nWatchDebitCached;
else
{
nWatchDebitCached = pwallet->GetDebit(*this, ISMINE_WATCH_ONLY);
nWatchDebitCached = pwallet->GetDebit(*tx, ISMINE_WATCH_ONLY);
fWatchDebitCached = true;
debit += nWatchDebitCached;
}
Expand All @@ -1751,7 +1751,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const
credit += nCreditCached;
else
{
nCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE);
nCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE);
fCreditCached = true;
credit += nCreditCached;
}
Expand All @@ -1762,7 +1762,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const
credit += nWatchCreditCached;
else
{
nWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY);
nWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY);
fWatchCreditCached = true;
credit += nWatchCreditCached;
}
Expand All @@ -1776,7 +1776,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const
{
if (fUseCache && fImmatureCreditCached)
return nImmatureCreditCached;
nImmatureCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE);
nImmatureCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE);
fImmatureCreditCached = true;
return nImmatureCreditCached;
}
Expand Down Expand Up @@ -1820,7 +1820,7 @@ CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool& fUseCache) const
{
if (fUseCache && fImmatureWatchCreditCached)
return nImmatureWatchCreditCached;
nImmatureWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY);
nImmatureWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY);
fImmatureWatchCreditCached = true;
return nImmatureWatchCreditCached;
}
Expand Down Expand Up @@ -1861,7 +1861,7 @@ CAmount CWalletTx::GetChange() const
{
if (fChangeCached)
return nChangeCached;
nChangeCached = pwallet->GetChange(*this);
nChangeCached = pwallet->GetChange(*tx);
fChangeCached = true;
return nChangeCached;
}
Expand All @@ -1875,7 +1875,7 @@ bool CWalletTx::InMempool() const
bool CWalletTx::IsTrusted() const
{
// Quick answer in most cases
if (!CheckFinalTx(*this))
if (!CheckFinalTx(*tx))
return false;
int nDepth = GetDepthInMainChain();
if (nDepth >= 1)
Expand Down Expand Up @@ -2133,7 +2133,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
const uint256& wtxid = it->first;
const CWalletTx* pcoin = &(*it).second;

if (!CheckFinalTx(*pcoin))
if (!CheckFinalTx(*pcoin->tx))
continue;

if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0)
Expand Down Expand Up @@ -2915,7 +2915,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
wtxNew.SetTx(MakeTransactionRef(std::move(txNew)));

// Limit size
if (GetTransactionWeight(wtxNew) >= MAX_STANDARD_TX_WEIGHT)
if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT)
{
strFailReason = _("Transaction too large");
return false;
Expand Down
4 changes: 0 additions & 4 deletions src/wallet/wallet.h
View file
Expand Up @@ -214,10 +214,6 @@ class CMerkleTx
Init();
}

/** Helper conversion operator to allow passing CMerkleTx where CTransaction is expected.
* TODO: adapt callers and remove this operator. */
operator const CTransaction&() const { return *tx; }

void Init()
{
hashBlock = uint256();
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/walletdb.cpp
View file
Expand Up @@ -268,7 +268,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
CWalletTx wtx;
ssValue >> wtx;
CValidationState state;
if (!(CheckTransaction(wtx, state) && (wtx.GetHash() == hash) && state.IsValid()))
if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid()))
return false;

// Undo serialize changes in 31600
Expand Down

0 comments on commit 77ba4bf

Please sign in to comment.