Page MenuHomePhabricator

remove priority free transactions mechanism (currently off by default)
ClosedPublic

Authored by markblundeberg on Dec 17 2019, 07:54.

Details

Summary

Currently we have a dual mechanism where:

  • free transactions can be admitted into mempool, with some limit per time. (by default, limitfreerelay=0 so no free transactions are allowed, except in some special circumstances)
  • restrict free-ness to only high-priority transactions. (on by default: relaypriority=1)

The default behaviour is that transactions below 1 sat/byte will be
rejected when transactions come in over the p2p layer, but in some other
cases they will be admitted. Although this mechanism was "disabled" by default,
it caused an unfortunate bug where high priority, low-fee transactions could
be accepted if submitted via RPC command sendrawtransaction. This is a
problem as they can't be successfully relayed since peers do not relay low
fee, and many light wallets rely on sendrawtransaction.

In order to reduce maintenance burden, let's just remove these mechanisms
altogether.

Note, in core the corresponding pr was PR9602, which was much larger.
(and some of this has already been done in D1360)
This Diff performs only *part* of the removal that was done in PR9602.
After this diff, we still have priority transactions for mining, which
affects which txs are mined first (during congestion). It is probably
worth removing that too but that can be a separate Diff.

Also note, this includes the bugfix from PR11309 so reorgs will put
low-fee transactions back into mempool (but all other checks are still
performed). We currently roughly have this functionality
(fLimitFree=false for reorg) and it's good behaviour to keep (and
abc-p2p-compactblocks relies on this, for instance).

Depends on D4744

Test Plan
make check
test_runner.py --extended

Diff Detail

Repository
rABC Bitcoin ABC
Branch
rem-highprio
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8612
Build 15211: Default Diff Build & Tests
Build 15210: arc lint + arc unit

Event Timeline

There are more cleanups that can be done in the mempool code, but that's a good start.

This revision is now accepted and ready to land.Dec 17 2019, 23:27

(oops, missed removing a comment that is no longer valid)

Future note: if it is desired to revert this, see the fixes in D4730 and consider some of my comments there, regarding subjectivity of free transactions. Also, try to preserve the behaviour of seen here of putting back *all* free transactions into mempool after a reorg.