Minor cleanups for AcceptToMemoryPool #11309

pull morcos wants to merge 3 commits into bitcoin:master from morcos:ATMP_cleanup changing 6 files +34 −27

morcos commented at 5:08 PM on September 12, 2017: member

First commit just removes default arguments from AcceptToMemoryPool and consolidates two arguments, it does not change behavior.

Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned here

Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303.

Change AcceptToMemoryPool function signature

Combine fLimitFree and fOverrideMempoolLimit into a single boolean:
bypass_limits.  This is used to indicate that mempool limiting based on feerate
should be bypassed.  It is used when readding transactions from a reorg and then
the mempool is trimmed to size after all transactions are added and they can be
evaluated in the context of their descendants. No changes to behavior.

fd849e1b03

Do not reject based on mempool min fee when bypass_limits is set.

This should have always been the case, but we will correctly trim to size after
a reorg which is when bypass_limits is set.

04f78ab5b9

Ignore transactions added to mempool during a reorg for fee estimation purposes. bf64c3cb34
fanquake added the label Mempool on Sep 12, 2017
in src/net_processing.cpp:1792 in bf64c3cb34
```
1787 | @@ -1788,7 +1788,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
1788 |  
1789 |          std::list<CTransactionRef> lRemovedTxn;
1790 |  
1791 | -        if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs, &lRemovedTxn)) {
1792 | +        if (!AlreadyHave(inv) &&
1793 | +            AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
```
promag commented at 2:29 PM on September 13, 2017:

Nit, feeling uncomfortable with these comments 😄.

sipa commented at 11:43 PM on September 15, 2017:

@promag Why? I think that the function calls are effectively unreadable without them (though we should change ATMP to take an options object instead of a gazillion arguments instead).

ryanofsky commented at 7:07 AM on September 16, 2017:

I think he's just joking about the existence of "bypass limits" and "absurd fee" options.

promag commented at 7:59 PM on September 18, 2017:

No joking, just what @sipa said. One struct with the arguments and fill them, resembles named arguments.
promag commented at 2:31 PM on September 13, 2017: member

Remove periods from commit messages?

utACK bf64c3c.
practicalswift commented at 12:49 PM on September 15, 2017: contributor

utACK bf64c3cb34571cfcd2ec4cce29bbf9c49ec3700c
TheBlueMatt commented at 8:12 PM on September 15, 2017: member

utACK bf64c3cb34571cfcd2ec4cce29bbf9c49ec3700c. I also have a branch to delete plTxnReplaced from ATMP in the future as well.
sipa commented at 12:12 AM on September 16, 2017: member

utACK bf64c3cb34571cfcd2ec4cce29bbf9c49ec3700c
instagibbs commented at 7:48 PM on September 18, 2017: member

utACK https://github.com/bitcoin/bitcoin/pull/11309/commits/bf64c3cb34571cfcd2ec4cce29bbf9c49ec3700c
MarcoFalke added the label Refactoring on Sep 29, 2017
MarcoFalke removed the label Refactoring on Sep 29, 2017
MarcoFalke commented at 1:06 PM on September 29, 2017: member

utACK bf64c3cb34571cfcd2ec4cce29bbf9c49ec3700c
MarcoFalke merged this on Sep 29, 2017
MarcoFalke closed this on Sep 29, 2017
MarcoFalke referenced this in commit 93d20a734d on Sep 29, 2017
PastaPastaPasta referenced this in commit 2208b6320b on Mar 14, 2020
PastaPastaPasta referenced this in commit df3e0e6ebc on Mar 15, 2020
PastaPastaPasta referenced this in commit 0aa6508e31 on Mar 16, 2020
PastaPastaPasta referenced this in commit 75f32fb50f on Mar 16, 2020
ckti referenced this in commit 9bb144fee3 on Mar 28, 2021
gades referenced this in commit 1733872eda on Jun 30, 2021
DrahtBot locked this on Sep 8, 2021