Remove TX priority and free transaction area from mempool, block creator.

[jgarzik]

It was noted on #bitcoin-dev that many simplifications arise from removing the complex, dual-policy aspects of priority-based low fee transactions on policy estimation, block template creation and mempool code. This change updates Bitcoin Core to relay purely based on fee/kb rate, removing the concepts of priority and free transactions from the codebase.

fSendFreeTransactions, fLimitFree and similar controls have been operating in the field defaulting to fee-based choices already.

This change should make other changes such as @pstratem 's work to cap the mempool size dynamically much easier.

Notes:

• Status: Watching travis. A large change, largely untested, so not ready for merging without lots of feedback and testing.
• Thus, requesting feedback and testing...
• Transaction fee deltas are preserved - thus TX "priority" remains to some extent
• prioritisetransaction RPC call continues to work, by adjusting the fee.
• needs focused review in the area of wallet, to ensure IsMine() transactions work fine through reorgs

[luke-jr]

NACK. This makes a number of policies harder to implement for the sake of a "simpler" (and less competent) default policy. I'm working on a mempool size cap that doesn't break policy freedom so completely.

[jgarzik]

Can you be more specific about which policies become harder to implement?

[luke-jr]

Priority-sorted area followed by fee/kB-sorted area (the previous default policy) would be the most obvious one. The policy before that (gradually higher min fee/kB as blocks are filled) as well.

[jgarzik]

@luke-jr Read the code. You can implement such a policy with fee deltas.

[petertodd]

Concept ACK, will review code and test later.

I've looked into policy stuff myself, and fee-delta methods make the most sense to me.

[domob1812]

I actually like the concept of priority, which means that coinage can be used to "pay" for fees as a spam prevention. The "production rate" of coinage is bounded, which means that one can set a minimum priority for transactions and a certain reserved size in each block and be ensured that, on average, all free transactions fit in this reserved block space forever. I think that's a nice feature of the current fee policy, because it allows free transactions (psychologically appealing to users) for everyone who does not overuse their coins. No spamming or DoS possible.

But of course, I agree that the change makes the code much simpler.

[dgenr8]

Design ACK

[rubensayshi]

Concept ACK, I think the illusion that miners will mine TXs with 0 fee and high priority should be burned to the ground.

no matter what the blocksize debat does we'll eventually end up with fees being a significant part of miner revenue and I don't see miners maintaining the high priority policy at that point, might as well get rid of the code for it and since you're keeping some of the priority stuff left intact a miner who truely believe they wants to keep doing that could still do so.

[jtimon]

If GetMinRelayFee returns 0 this error only triggers when new coins are being created, which is checked later in Consensus::CheckTxInputs later (which should be checked before doing any policy checks about inputs, see #6445 ).

If it returns minRelayTxFee, you're just repeating the check that is done a few lines later (if (nFees < ::minRelayTxFee.GetFee(nSize))).

Therefore this check and GetMinRelayFee() can be completely removed.

[dcousens]

concept ACK, needs rebase

[jtimon]

#6584 solves my nit here (and is rebased). It also includes #6445.
That of course means I utACK this one, I just prefer my version (where I remove GetMinRelayFee()).

[jgarzik]

Closing

[dcousens]

@jgarzik for the same reasons as #6584?

[jgarzik]

The effort of frequent rebasing isn't worth keeping this open.

[dcousens]

@jgarzik is there any consensus on the change? It seems wrong to close this unless the concept has been rejected?
@laanwj if I were to pick up this PR, would it be worth keeping open in light of #6584?

[jgarzik]

As a general principle, we probably don't want big pull requests to linger for a long time. This is straightforward to regenerate.

[pstratem]

@jgarzik Is there an issue for this?

[jgarzik]

@pstratem Good point - an issue would probably be appropriate, linking to this outdated branch as an example.

[wallclockbuilder]

Concept ACK. Simplicity FTW!