mempool: remove unused magic number from consistency check #16056

pull instagibbs wants to merge 1 commits into bitcoin:master from instagibbs:onemil_height changing 1 files +1 −1

instagibbs commented at 4:08 PM on May 20, 2019: member

Unexplained magic numbers are no good. Since the exact number does not matter, opt for a constant that is less peculiar.

Note that this could only possibly affect mempool consistency checks which is not active by default except on regtest.

see discussion: https://github.com/bitcoin/bitcoin/issues/15080
DrahtBot added the label Mempool on May 20, 2019
laanwj commented at 5:34 PM on May 20, 2019: member

If it's never used, can't we remove the argument completely?
instagibbs commented at 5:44 PM on May 20, 2019: member

@laanwj it's used in general(locktimes, etc), just not in this context, where things are being added to the mempool cache to check that all inputs have connections, not that they have the proper height.
sipa commented at 6:54 PM on May 20, 2019: member

I think using spendheight there is confusing; the transaction isn't actually included in a block at that height. I don't think it matters; but using MAX_INT or so is probably better.
instagibbs force-pushed on May 20, 2019

in src/txmempool.cpp:604 in ba59535784 outdated

 600 | @@ -601,7 +601,7 @@ static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& m
 601 |      CAmount txfee = 0;
 602 |      bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, state, mempoolDuplicate, spendheight, txfee);
 603 |      assert(fCheckResult);
 604 | -    UpdateCoins(tx, mempoolDuplicate, 1000000);
 605 | +    UpdateCoins(tx, mempoolDuplicate, INT_MAX);

practicalswift commented at 7:18 PM on May 20, 2019:

Nit: std::numeric_limits<int>::max() :-)

instagibbs commented at 7:18 PM on May 20, 2019:

lightning fast linting :joy:

practicalswift commented at 7:20 PM on May 20, 2019:

I do my best! :-)

mempool: remove unused magic number from consistency check fadbc5d895
instagibbs force-pushed on May 20, 2019
practicalswift commented at 7:20 PM on May 20, 2019: contributor

utACK fadbc5d89562df7e34379f9d01a757e30db7bbe2
instagibbs commented at 7:20 PM on May 20, 2019: member

@sipa right, UpdateCoins isn't a mempool thing at all, just being re-used used here for consistency checks. Updated.
Empact commented at 5:39 PM on May 28, 2019: member

/cc @TheBlueMatt b7b4318f3a
laanwj merged this on May 29, 2019
laanwj closed this on May 29, 2019
laanwj referenced this in commit de458da0c1 on May 29, 2019
sidhujag referenced this in commit 362d0777c7 on May 30, 2019
ptschip referenced this in commit a76b928ad0 on Oct 22, 2019
jonspock referenced this in commit 88f6719c23 on Dec 27, 2019
jonspock referenced this in commit d07c00deec on Dec 29, 2019
PastaPastaPasta referenced this in commit 758a470cb7 on Jun 27, 2021
PastaPastaPasta referenced this in commit 119c1f82b9 on Jun 28, 2021
PastaPastaPasta referenced this in commit 3cf5c685f5 on Jun 29, 2021
PastaPastaPasta referenced this in commit 79af4eaa30 on Jul 1, 2021
PastaPastaPasta referenced this in commit e9b5a6b4d6 on Jul 1, 2021
PastaPastaPasta referenced this in commit 7243a59952 on Jul 12, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15080 Unexplained 1 million block height in mempool code