Skip to content

Commit

Permalink
Replace CWalletTx::SetConf by Confirmation initialization list
Browse files Browse the repository at this point in the history 
Adaptation of btc@9700fcb47feca9d78e005b8d18b41148c8f6b25f
  • Loading branch information
@furszy
furszy committed Mar 10, 2021
1 parent 6320efb commit 4405ac0
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/test/librust/sapling_rpc_wallet_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
chainActive.SetTip(&fakeIndex);
BOOST_CHECK(chainActive.Contains(&fakeIndex));
BOOST_CHECK_EQUAL(1, chainActive.Height());
wtx.SetConf(CWalletTx::Status::CONFIRMED, blockHash, 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, blockHash, 0);
pwalletMain->LoadToWallet(wtx);
BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed");

Expand Down
14 changes: 7 additions & 7 deletions src/test/librust/sapling_wallet_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(GetConflictedSaplingNotes) {
auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first;
BOOST_CHECK(saplingNoteData.size() > 0);
wtx.SetSaplingNoteData(saplingNoteData);
wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
BOOST_CHECK(wallet.LoadToWallet(wtx));

// Simulate receiving new block and ChainTip signal
Expand Down Expand Up @@ -351,7 +351,7 @@ BOOST_AUTO_TEST_CASE(SaplingNullifierIsSpent) {
BOOST_CHECK(chainActive.Contains(&fakeIndex));
BOOST_CHECK_EQUAL(0, chainActive.Height());

wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wallet.LoadToWallet(wtx);

// Verify note has been spent
Expand Down Expand Up @@ -414,7 +414,7 @@ BOOST_AUTO_TEST_CASE(NavigateFromSaplingNullifierToNote) {
BOOST_CHECK_EQUAL(0, chainActive.Height());

// Simulate SyncTransaction which calls AddToWalletIfInvolvingMe
wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first;
BOOST_CHECK(saplingNoteData.size() > 0);
wtx.SetSaplingNoteData(saplingNoteData);
Expand Down Expand Up @@ -512,7 +512,7 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) {
auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first;
BOOST_CHECK(saplingNoteData.size() > 0);
wtx.SetSaplingNoteData(saplingNoteData);
wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wallet.LoadToWallet(wtx);

// Simulate receiving new block and ChainTip signal.
Expand Down Expand Up @@ -588,7 +588,7 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) {
auto saplingNoteData2 = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx2.tx).first;
BOOST_CHECK(saplingNoteData2.size() > 0);
wtx2.SetSaplingNoteData(saplingNoteData2);
wtx2.SetConf(CWalletTx::Status::CONFIRMED, block2.GetHash(), 0);
wtx2.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block2.GetHash(), 0);
wallet.LoadToWallet(wtx2);

// Verify note B is spent. AddToWallet invokes AddToSpends which updates mapTxSaplingNullifiers
Expand Down Expand Up @@ -967,7 +967,7 @@ BOOST_AUTO_TEST_CASE(UpdatedSaplingNoteData) {
auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first;
BOOST_CHECK(saplingNoteData.size() == 1); // wallet only has key for change output
wtx.SetSaplingNoteData(saplingNoteData);
wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wallet.LoadToWallet(wtx);

// Simulate receiving new block and ChainTip signal
Expand Down Expand Up @@ -1083,7 +1083,7 @@ BOOST_AUTO_TEST_CASE(MarkAffectedSaplingTransactionsDirty) {
auto saplingNoteData = wallet.GetSaplingScriptPubKeyMan()->FindMySaplingNotes(*wtx.tx).first;
BOOST_CHECK(saplingNoteData.size() > 0);
wtx.SetSaplingNoteData(saplingNoteData);
wtx.SetConf(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, block.GetHash(), 0);
wallet.LoadToWallet(wtx);

// Simulate receiving new block and ChainTip signal
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/wallet_shielded_balances_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ FakeBlock SimpleFakeMine(CWalletTx& wtx, SaplingMerkleTree& currentTree, CWallet
chainActive.SetTip(fakeBlock.pindex);
BOOST_CHECK(chainActive.Contains(fakeBlock.pindex));
WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(fakeBlock.pindex));
wtx.SetConf(CWalletTx::Status::CONFIRMED, fakeBlock.pindex->GetBlockHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeBlock.pindex->GetBlockHash(), 0);
return fakeBlock;
}

Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/wallet_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CWallet &wallet, CBlockIndex* pprev
chainActive.SetTip(fakeIndex);
BOOST_CHECK(chainActive.Contains(fakeIndex));
WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(fakeIndex));
wtx.SetConf(CWalletTx::Status::CONFIRMED, fakeIndex->GetBlockHash(), 0);
wtx.m_confirm = CWalletTx::Confirmation(CWalletTx::Status::CONFIRMED, fakeIndex->GetBlockHash(), 0);
removeTxFromMempool(wtx);
wtx.fInMempool = false;
return fakeIndex;
Expand Down
42 changes: 15 additions & 27 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1055,19 +1055,19 @@ void CWallet::AddExternalNotesDataToTx(CWalletTx& wtx) const
* Abandoned state should probably be more carefully tracked via different
* posInBlock signals or by checking mempool presence when necessary.
*/
bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Status status, const uint256& blockHash, int posInBlock, bool fUpdate)
bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CWalletTx::Confirmation& confirm, bool fUpdate)
{
const CTransaction& tx = *ptx;
{
AssertLockHeld(cs_wallet);

if (!blockHash.IsNull() && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) {
if (!confirm.hashBlock.IsNull() && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) {
for (const CTxIn& txin : tx.vin) {
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(txin.prevout);
while (range.first != range.second) {
if (range.first->second != tx.GetHash()) {
LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), blockHash.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n);
MarkConflicted(blockHash, range.first->second);
LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n);
MarkConflicted(confirm.hashBlock, range.first->second);
}
range.first++;
}
Expand Down Expand Up @@ -1111,7 +1111,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::St

// Block disconnection override an abandoned tx as unconfirmed
// which means user may have to call abandontransaction again
wtx.SetConf(status, blockHash, posInBlock);
wtx.m_confirm = confirm;

return AddToWallet(wtx, false);
}
Expand Down Expand Up @@ -1242,13 +1242,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
}
}

void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Status status, const CBlockIndex *pindexBlockConnected, int posInBlock)
void CWallet::SyncTransaction(const CTransactionRef& ptx, const CWalletTx::Confirmation& confirm)
{
if (!AddToWalletIfInvolvingMe(ptx,
status,
(pindexBlockConnected) ? pindexBlockConnected->GetBlockHash() : uint256(),
posInBlock,
true)) {
if (!AddToWalletIfInvolvingMe(ptx, confirm, true)) {
return; // Not one of ours
}

Expand All @@ -1258,7 +1254,8 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Status stat
void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx)
{
LOCK2(cs_main, cs_wallet);
SyncTransaction(ptx, CWalletTx::Status::UNCONFIRMED, nullptr, 0);
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0);
SyncTransaction(ptx, confirm);

auto it = mapWallet.find(ptx->GetHash());
if (it != mapWallet.end()) {
Expand All @@ -1282,7 +1279,8 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
m_last_block_processed_time = pindex->GetBlockTime();
m_last_block_processed_height = pindex->nHeight;
for (size_t i = 0; i < pblock->vtx.size(); i++) {
SyncTransaction(pblock->vtx[i], CWalletTx::Status::CONFIRMED, pindex, i);
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_last_block_processed->GetBlockHash(), i);
SyncTransaction(pblock->vtx[i], confirm);
TransactionRemovedFromMempool(pblock->vtx[i]);
}
for (const CTransactionRef& ptx : vtxConflicted) {
Expand Down Expand Up @@ -1318,7 +1316,8 @@ void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, con
m_last_block_processed_time = blockTime;
m_last_block_processed = blockHash;
for (const CTransactionRef& ptx : pblock->vtx) {
SyncTransaction(ptx, CWalletTx::Status::UNCONFIRMED, nullptr, 0);
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0);
SyncTransaction(ptx, confirm);
}

if (Params().GetConsensus().NetworkUpgradeActive(nBlockHeight, Consensus::UPGRADE_V5_0)) {
Expand Down Expand Up @@ -1849,7 +1848,8 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b
}
for (int posInBlock = 0; posInBlock < (int) block.vtx.size(); posInBlock++) {
const auto& tx = block.vtx[posInBlock];
if (AddToWalletIfInvolvingMe(tx, CWalletTx::Status::CONFIRMED, pindex->GetBlockHash(), posInBlock, fUpdate)) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, pindex->GetBlockHash(), posInBlock);
if (AddToWalletIfInvolvingMe(tx, confirm, fUpdate)) {
myTxHashes.push_back(tx->GetHash());
ret++;
}
Expand Down Expand Up @@ -4299,18 +4299,6 @@ CWalletKey::CWalletKey(int64_t nExpires)
nTimeExpires = nExpires;
}

void CWalletTx::SetConf(Status status, const uint256& blockHash, int posInBlock)
{
// Update tx status
m_confirm.status = status;

// Update the tx's hashBlock
m_confirm.hashBlock = blockHash;

// set the position of the transaction in the block
m_confirm.nIndex = posInBlock;
}

int CWalletTx::GetDepthInMainChain() const
{
if (isUnconfirmed() || isAbandoned()) return 0;
Expand Down
13 changes: 6 additions & 7 deletions src/wallet/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,10 @@ class CWalletTx
* where they instead point to block hash and index of the deepest conflicting tx.
*/
struct Confirmation {
Status status = UNCONFIRMED;
uint256 hashBlock = uint256();
int nIndex = 0;
Status status;
uint256 hashBlock;
int nIndex;
Confirmation(Status s = UNCONFIRMED, uint256 h = uint256(), int i = 0) : status(s), hashBlock(h), nIndex(i) {}
};

Confirmation m_confirm;
Expand Down Expand Up @@ -512,8 +513,6 @@ class CWalletTx
void RelayWalletTransaction(CConnman* connman);
std::set<uint256> GetConflicts() const;

void SetConf(Status status, const uint256& blockHash, int posInBlock);

/**
* Return depth of transaction in blockchain:
* <0 : conflicts with a transaction this deep in the blockchain
Expand Down Expand Up @@ -610,7 +609,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
void ChainTipAdded(const CBlockIndex *pindex, const CBlock *pblock, SaplingMerkleTree saplingTree);

/* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected */
void SyncTransaction(const CTransactionRef& tx, CWalletTx::Status status, const CBlockIndex *pindexBlockConnected, int posInBlock);
void SyncTransaction(const CTransactionRef& tx, const CWalletTx::Confirmation& confirm);

bool IsKeyUsed(const CPubKey& vchPubKey);

Expand Down Expand Up @@ -928,7 +927,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
void TransactionAddedToMempool(const CTransactionRef& tx) override;
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) override;
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) override;
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Status status, const uint256& blockHash, int posInBlock, bool fUpdate);
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CWalletTx::Confirmation& confirm, bool fUpdate);
void EraseFromWallet(const uint256& hash);

/**
Expand Down

0 comments on commit 4405ac0

Please sign in to comment.