highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 22:50:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #15931: Remove GetDepthInMainChain dependency on locked chain interface

Browse Source 
36b68de5b2 Remove getBlockDepth method from Chain::interface (Antoine Riard)
b66c429c56 Remove locked_chain from GetDepthInMainChain and its callers (Antoine Riard)
0ff03871ad Use CWallet::m_last_block_processed_height in GetDepthInMainChain (Antoine Riard)
f77b1de16f Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (Antoine Riard)
769ff05e48 Refactor some importprunedfunds checks with guard clause (Antoine Riard)
5971d3848e Add block_height field in struct Confirmation (Antoine Riard)
9700fcb47f Replace CWalletTx::SetConf by Confirmation initialization list (Antoine Riard)
5aacc3eff1 Add m_last_block_processed_height field in CWallet (Antoine Riard)
10b4729e33 Pass block height in Chain::BlockConnected/Chain::BlockDisconnected (Antoine Riard)

Pull request description:

  Work starter to remove Chain::Lock interface by adding m_last_block_processed_height in CWallet and m_block_height in CMerkleTx to avoid GetDepthInMainChain having to keep a lock . Once this one done, it should ease work to wipe out more cs_main locks from wallet code.

  I think it's ready for a first round of review before to get further.

  - `BlockUntilSyncedToCurrent` : restrain isPotentialTip to isTip because we want to be sure that wallet see BlockDisconnected callbacks if its height differs from the Chain one. It means during a reorg, an RPC could return before the BlockDisconnected callback had been triggered. This could cause a tx that had been included in the disconnected block to be displayed as confirmed, for example.

  ~~- `AbandonTransaction` : in case of conflicted tx (nIndex = -1), we set its m_block_height to the one of conflicting blocks, but if this height is superior to CWallet::m_last_block_processed_height, that means tx isn't conflicted anymore so we return 0 as tx is again unconfirmed~~ After #16624, we instead rely on Confirmation.

  ~~- `AddToWalletIfInvolvingMe`: in case of block disconnected, transactions are added to mempool again, so we need to replace old txn in `mapWallet` with a height set to zero so we remove check on block_hash.IsNull~~ Already done in #16624

ACKs for top commit:
  jnewbery:
    @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch (36b68de5b2).
  jkczyz:
    > @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch ([36b68de](36b68de5b2)).
  meshcollider:
    utACK 36b68de5b2
  ryanofsky:
    Code review ACK 36b68de5b2. Changes since last review: new jkczyz refactor importprunedfunds commit, changed BlockUntilSyncedToCurrentChainChanges commit title and description, changed Confirmation struct field order and line-wrapped comment
  jnewbery:
    utACK 36b68de5b2
  promag:
    Code review ACK 36b68de5b2.

Tree-SHA512: 08b89a0bcc39f67c82a6cb6aee195e6a11697770c788ba737b90986b4893f44e90d1ab9ef87239ea3766508b7e24ea882b7199df41173ab27a3d000328c14644
This commit is contained in:
Samuel Dobson
2019-11-08 23:21:48 +13:00
parent 2fb6140d58 36b68de5b2
commit 99ab3a72c5
16 changed files with 255 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/wallet/rpcwallet.cpp
View File

@@ -135,7 +135,7 @@ LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet)
static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry)
{
int confirms = wtx.GetDepthInMainChain(locked_chain);
int confirms = wtx.GetDepthInMainChain();
entry.pushKV("confirmations", confirms);
if (wtx.IsCoinBase())
entry.pushKV("generated", true);
@@ -640,7 +640,7 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request)
for (const CTxOut& txout : wtx.tx->vout)
if (txout.scriptPubKey == scriptPubKey)
if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth)
if (wtx.GetDepthInMainChain() >= nMinDepth)
nAmount += txout.nValue;
}
@@ -706,7 +706,7 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request)
{
CTxDestination address;
if (ExtractDestination(txout.scriptPubKey, address) && pwallet->IsMine(address) && setAddress.count(address)) {
if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth)
if (wtx.GetDepthInMainChain() >= nMinDepth)
nAmount += txout.nValue;
}
}
@@ -1063,7 +1063,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co
continue;
}
int nDepth = wtx.GetDepthInMainChain(locked_chain);
int nDepth = wtx.GetDepthInMainChain();
if (nDepth < nMinDepth)
continue;
@@ -1320,8 +1320,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
}
// Received
if (listReceived.size() > 0 && wtx.GetDepthInMainChain(locked_chain) >= nMinDepth)
{
if (listReceived.size() > 0 && wtx.GetDepthInMainChain() >= nMinDepth) {
for (const COutputEntry& r : listReceived)
{
std::string label;
@@ -1338,9 +1337,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
MaybePushAddress(entry, r.destination);
if (wtx.IsCoinBase())
{
if (wtx.GetDepthInMainChain(locked_chain) < 1)
if (wtx.GetDepthInMainChain() < 1)
entry.pushKV("category", "orphan");
else if (wtx.IsImmatureCoinBase(locked_chain))
else if (wtx.IsImmatureCoinBase())
entry.pushKV("category", "immature");
else
entry.pushKV("category", "generate");
@@ -1604,7 +1603,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
for (const std::pair<const uint256, CWalletTx>& pairWtx : pwallet->mapWallet) {
CWalletTx tx = pairWtx.second;
if (depth == -1 || abs(tx.GetDepthInMainChain(*locked_chain)) < depth) {
if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) {
ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */);
}
}
@@ -1721,7 +1720,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
}
const CWalletTx& wtx = it->second;
CAmount nCredit = wtx.GetCredit(*locked_chain, filter);
CAmount nCredit = wtx.GetCredit(filter);
CAmount nDebit = wtx.GetDebit(filter);
CAmount nNet = nCredit - nDebit;
CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0);
@@ -1785,7 +1784,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request)
if (!pwallet->mapWallet.count(hash)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
}
if (!pwallet->AbandonTransaction(*locked_chain, hash)) {
if (!pwallet->AbandonTransaction(hash)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not eligible for abandonment");
}
@@ -2216,7 +2215,7 @@ static UniValue lockunspent(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout index out of bounds");
}
if (pwallet->IsSpent(*locked_chain, outpt.hash, outpt.n)) {
if (pwallet->IsSpent(outpt.hash, outpt.n)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected unspent output");
}