Add block_height field in struct Confirmation
At wallet loading, we rely on chain state querying to retrieve height of txn, to do so we ensure that lock order is respected between cs_main and cs_wallet. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as confirmation height is not used by wallet-tool. Reorder arguments and document Confirmation calls to avoid ambiguity. Fixes nits left from #16624
This commit is contained in:
parent
9700fcb47f
commit
5971d3848e
4 changed files with 43 additions and 26 deletions
|
@ -364,10 +364,12 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
|
|||
std::vector<uint256> vMatch;
|
||||
std::vector<unsigned int> vIndex;
|
||||
unsigned int txnIndex = 0;
|
||||
Optional<int> height;
|
||||
if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) {
|
||||
|
||||
auto locked_chain = pwallet->chain().lock();
|
||||
if (locked_chain->getBlockHeight(merkleBlock.header.GetHash()) == nullopt) {
|
||||
height = locked_chain->getBlockHeight(merkleBlock.header.GetHash());
|
||||
if (height == nullopt) {
|
||||
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain");
|
||||
}
|
||||
|
||||
|
@ -382,7 +384,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
|
|||
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock");
|
||||
}
|
||||
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex);
|
||||
wtx.m_confirm = confirm;
|
||||
|
||||
auto locked_chain = pwallet->chain().lock();
|
||||
|
|
|
@ -276,7 +276,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
|
|||
AssertLockHeld(spk_man->cs_wallet);
|
||||
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
|
||||
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 0);
|
||||
wtx.m_confirm = confirm;
|
||||
|
||||
// Call GetImmatureCredit() once before adding the key to the wallet to
|
||||
|
@ -318,7 +318,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
|
|||
wallet.AddToWallet(wtx);
|
||||
}
|
||||
if (block) {
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->nHeight, block->GetBlockHash(), 0);
|
||||
wtx.m_confirm = confirm;
|
||||
}
|
||||
wallet.AddToWallet(wtx);
|
||||
|
@ -499,7 +499,7 @@ public:
|
|||
wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash());
|
||||
auto it = wallet->mapWallet.find(tx->GetHash());
|
||||
BOOST_CHECK(it != wallet->mapWallet.end());
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 1);
|
||||
it->second.m_confirm = confirm;
|
||||
return it->second;
|
||||
}
|
||||
|
|
|
@ -766,10 +766,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
|
|||
wtx.m_confirm.status = wtxIn.m_confirm.status;
|
||||
wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex;
|
||||
wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock;
|
||||
wtx.m_confirm.block_height = wtxIn.m_confirm.block_height;
|
||||
fUpdated = true;
|
||||
} else {
|
||||
assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex);
|
||||
assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock);
|
||||
assert(wtx.m_confirm.block_height == wtxIn.m_confirm.block_height);
|
||||
}
|
||||
if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe)
|
||||
{
|
||||
|
@ -820,12 +822,22 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn)
|
|||
{
|
||||
// If wallet doesn't have a chain (e.g wallet-tool), lock can't be taken.
|
||||
auto locked_chain = LockChain();
|
||||
// If tx hasn't been reorged out of chain while wallet being shutdown
|
||||
// change tx status to UNCONFIRMED and reset hashBlock/nIndex.
|
||||
if (!wtxIn.m_confirm.hashBlock.IsNull()) {
|
||||
if (locked_chain && !locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock)) {
|
||||
if (locked_chain) {
|
||||
Optional<int> block_height = locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock);
|
||||
if (block_height) {
|
||||
// Update cached block height variable since it not stored in the
|
||||
// serialized transaction.
|
||||
wtxIn.m_confirm.block_height = *block_height;
|
||||
} else if (wtxIn.isConflicted() || wtxIn.isConfirmed()) {
|
||||
// If tx block (or conflicting block) was reorged out of chain
|
||||
// while the wallet was shutdown, change tx status to UNCONFIRMED
|
||||
// and reset block height, hash, and index. ABANDONED tx don't have
|
||||
// associated blocks and don't need to be updated. The case where a
|
||||
// transaction was reorged out while online and then reconfirmed
|
||||
// while offline is covered by the rescan logic.
|
||||
wtxIn.setUnconfirmed();
|
||||
wtxIn.m_confirm.hashBlock = uint256();
|
||||
wtxIn.m_confirm.block_height = 0;
|
||||
wtxIn.m_confirm.nIndex = 0;
|
||||
}
|
||||
}
|
||||
|
@ -842,7 +854,7 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn)
|
|||
if (it != mapWallet.end()) {
|
||||
CWalletTx& prevtx = it->second;
|
||||
if (prevtx.isConflicted()) {
|
||||
MarkConflicted(prevtx.m_confirm.hashBlock, wtx.GetHash());
|
||||
MarkConflicted(prevtx.m_confirm.hashBlock, prevtx.m_confirm.block_height, wtx.GetHash());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -860,7 +872,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co
|
|||
while (range.first != range.second) {
|
||||
if (range.first->second != tx.GetHash()) {
|
||||
WalletLogPrintf("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);
|
||||
MarkConflicted(confirm.hashBlock, confirm.block_height, range.first->second);
|
||||
}
|
||||
range.first++;
|
||||
}
|
||||
|
@ -948,7 +960,6 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui
|
|||
if (currentconfirm == 0 && !wtx.isAbandoned()) {
|
||||
// If the orig tx was not in block/mempool, none of its spends can be in mempool
|
||||
assert(!wtx.InMempool());
|
||||
wtx.m_confirm.nIndex = 0;
|
||||
wtx.setAbandoned();
|
||||
wtx.MarkDirty();
|
||||
batch.WriteTx(wtx);
|
||||
|
@ -970,7 +981,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui
|
|||
return true;
|
||||
}
|
||||
|
||||
void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
|
||||
void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx)
|
||||
{
|
||||
auto locked_chain = chain().lock();
|
||||
LOCK(cs_wallet);
|
||||
|
@ -1004,6 +1015,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
|
|||
// Mark transaction as conflicted with this block.
|
||||
wtx.m_confirm.nIndex = 0;
|
||||
wtx.m_confirm.hashBlock = hashBlock;
|
||||
wtx.m_confirm.block_height = conflicting_height;
|
||||
wtx.setConflicted();
|
||||
wtx.MarkDirty();
|
||||
batch.WriteTx(wtx);
|
||||
|
@ -1036,7 +1048,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
|
|||
void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) {
|
||||
auto locked_chain = chain().lock();
|
||||
LOCK(cs_wallet);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
|
||||
SyncTransaction(ptx, confirm);
|
||||
|
||||
auto it = mapWallet.find(ptx->GetHash());
|
||||
|
@ -1061,10 +1073,10 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector<CTransaction
|
|||
|
||||
m_last_block_processed_height = height;
|
||||
m_last_block_processed = block_hash;
|
||||
for (size_t i = 0; i < block.vtx.size(); i++) {
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_last_block_processed, i);
|
||||
SyncTransaction(block.vtx[i], confirm);
|
||||
TransactionRemovedFromMempool(block.vtx[i]);
|
||||
for (size_t index = 0; index < block.vtx.size(); index++) {
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
|
||||
SyncTransaction(block.vtx[index], confirm);
|
||||
TransactionRemovedFromMempool(block.vtx[index]);
|
||||
}
|
||||
for (const CTransactionRef& ptx : vtxConflicted) {
|
||||
TransactionRemovedFromMempool(ptx);
|
||||
|
@ -1083,7 +1095,7 @@ void CWallet::BlockDisconnected(const CBlock& block, int height)
|
|||
m_last_block_processed_height = height - 1;
|
||||
m_last_block_processed = block.hashPrevBlock;
|
||||
for (const CTransactionRef& ptx : block.vtx) {
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
|
||||
SyncTransaction(ptx, confirm);
|
||||
}
|
||||
}
|
||||
|
@ -1630,7 +1642,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
|
|||
break;
|
||||
}
|
||||
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block_hash, posInBlock);
|
||||
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *block_height, block_hash, posInBlock);
|
||||
SyncTransaction(block.vtx[posInBlock], confirm, fUpdate);
|
||||
}
|
||||
// scan succeeded, record block as most recent successfully scanned
|
||||
|
|
|
@ -356,15 +356,17 @@ public:
|
|||
ABANDONED
|
||||
};
|
||||
|
||||
/* Confirmation includes tx status and a pair of {block hash/tx index in block} at which tx has been confirmed.
|
||||
* This pair is both 0 if tx hasn't confirmed yet. Meaning of these fields changes with CONFLICTED state
|
||||
* where they instead point to block hash and index of the deepest conflicting tx.
|
||||
/* Confirmation includes tx status and a triplet of {block height/block hash/tx index in block}
|
||||
* at which tx has been confirmed. All three are set to 0 if tx is unconfirmed or abandoned.
|
||||
* Meaning of these fields changes with CONFLICTED state where they instead point to block hash
|
||||
* and block height of the deepest conflicting tx.
|
||||
*/
|
||||
struct Confirmation {
|
||||
Status status;
|
||||
int block_height;
|
||||
uint256 hashBlock;
|
||||
int nIndex;
|
||||
Confirmation(Status s = UNCONFIRMED, uint256 h = uint256(), int i = 0) : status(s), hashBlock(h), nIndex(i) {}
|
||||
Confirmation(Status s = UNCONFIRMED, int b = 0, uint256 h = uint256(), int i = 0) : status(s), block_height(b), hashBlock(h), nIndex(i) {}
|
||||
};
|
||||
|
||||
Confirmation m_confirm;
|
||||
|
@ -407,7 +409,6 @@ public:
|
|||
* compatibility (pre-commit 9ac63d6).
|
||||
*/
|
||||
if (serializedIndex == -1 && m_confirm.hashBlock == ABANDON_HASH) {
|
||||
m_confirm.hashBlock = uint256();
|
||||
setAbandoned();
|
||||
} else if (serializedIndex == -1) {
|
||||
setConflicted();
|
||||
|
@ -512,12 +513,14 @@ public:
|
|||
{
|
||||
m_confirm.status = CWalletTx::ABANDONED;
|
||||
m_confirm.hashBlock = uint256();
|
||||
m_confirm.block_height = 0;
|
||||
m_confirm.nIndex = 0;
|
||||
}
|
||||
bool isConflicted() const { return m_confirm.status == CWalletTx::CONFLICTED; }
|
||||
void setConflicted() { m_confirm.status = CWalletTx::CONFLICTED; }
|
||||
bool isUnconfirmed() const { return m_confirm.status == CWalletTx::UNCONFIRMED; }
|
||||
void setUnconfirmed() { m_confirm.status = CWalletTx::UNCONFIRMED; }
|
||||
bool isConfirmed() const { return m_confirm.status == CWalletTx::CONFIRMED; }
|
||||
void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; }
|
||||
const uint256& GetHash() const { return tx->GetHash(); }
|
||||
bool IsCoinBase() const { return tx->IsCoinBase(); }
|
||||
|
@ -644,7 +647,7 @@ private:
|
|||
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
|
||||
|
||||
/* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */
|
||||
void MarkConflicted(const uint256& hashBlock, const uint256& hashTx);
|
||||
void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx);
|
||||
|
||||
/* Mark a transaction's inputs dirty, thus forcing the outputs to be recomputed */
|
||||
void MarkInputsDirty(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
|
||||
|
|
Loading…
Reference in a new issue