@laanwj shouldn't this be labeled with "mempool" too like #6405 ?
@jgarzik you didn't adapted the python tests, feel free to cherry pick it for #6405

By the way, I'm aware that we talked about merging #6470 first. But if it's takes much longer to merge it for whatever reason, I think we should be blocking other things like this or a dynamic min relay fee (which I would really want to do after #6068 ).

Oversimplifying default policy also makes policy customisation more difficult. So IMO concept NACK unless there's a good reason for this (missing from description).

In my opinion, policy abstraction (ability for customization) is something to plug in after the codebase in which it's plugged into is stable. That is currently not the case, the mempool will very likely significantly change for memory limiting, floating relay fee, indexes for block creation, ... much of that is much harder when trying to maintain a (IMO legacy and mostly broken) hardcoded policy.

So, what we should do, IMO, is get rid of the free relay policy and restrict to pure feerate, improve the mempool code to work based on that, and then plug in a sane policy configuration that can tweak what "cost" and "fee" we associate with transactions.

Concept ACK

Oversimplifying default policy also makes policy customisation more difficult. So IMO concept NACK unless there's a good reason for this (missing from description).

IMHO, it does the opposite, it makes it easier to customize. However, my experience here is probably not as extensive as yours, as it is based purely on editing the code as it stands not implementing policies specifically.

I agree with @dcousens. The default policy that is implemented now is very difficult to customize (it relies on 2 different ordering criteria, with associated specific code in block construction, and specific code in mempool acceptance to work). A policy that just introduces a "generalized cost" and "generalized fee" for transactions would be much more flexible and easier to work with.

@luke-jr the goal is simplifying this special-cased code. Once we have a CPolicy interface and a -policy parameter to select alternative policies we will make it easier for miners to implement their own policies. But changing the default policy is in my opinion mostly neutral to policy modularization.
If you could point to the parts of the removed code are you worried about concretely, that would be helpful.
It is also worth noting that it is noncompetitive/irrational for miners to include free transactions in their blocks (there's a block relay costs to them but no gain in exchange).

NACK
Free transactions are social aspect of the bitcoin (even 0.00001 is a lot of money somewhere). In the future, they should be made optional. Miner fee can be paid offline, so you can't say it is irrational for them to include such transaction. Maybe it is a miner's own transaction...

@paveljanik this doesn't stop miners from accepting free 0 fee transactions. This just makes the behaviour rational and based on the incentive, rather than something that is difficult to reason about in terms of miner priority.

It may be rational for miners to accept free txes if it decreases the UTXO set size.

E.G. https://www.blocktrail.com/BTC/tx/9b933f6f811dcc920d35bc48876181da4b29282ad452bdeb7563cff7c6c09bf9

NACK. NACK. I agree with @paveljanik et. al.

@paveljanik

Free transactions are social aspect of the bitcoin (even 0.00001 is a lot of money somewhere).

Free transactions create the illusion that the system (currently heavily subsidized by seigniorage) is cheap when it is actually very costly. They will eventually disappear when miners start using competitive policies and in the meantime are creating false expectations and confusing users and entrepreneurs. One satoshi is currently 0.000002 usd. I believe that if minimum mined fees rise from 0 to 1 satoshi we're still fine when it comes to attracting adoption (but it would still be a huge step forwards in miners implementing competitive policies).

In the future, they should be made optional

Fees are already optional. The policy minimum relay fee is also configurable at run time: you can set it to zero if you want to.

Miner fee can be paid offline, so you can't say it is irrational for them to include such transaction.

Miner's can still implement their own policy, but offline fees are useless when it comes to DoS network protections (which is what the minimum relay fee is about).
They could even receive a fee in-chain and give it back offline (on whatever payment method they're using to settle with their customers).

Maybe it is a miner's own transaction...

They can pay fees to themselves (which is better for their privacy) or, again, implement their own policy.
The current code is not addressing any of those cases either.

@chris-belcher That's a benefit for all miners, not just yourself (who are paying the cost). There should be an incentive to reduce/not-increase the utxo size built in the consensus rules (for example consensus_size(tx) = real_size(tx) + delta_to_the_utxo_size(tx)).

Anyway, since this seems to be controversial, I'm closing it. @jgarzik feel free to cherry pick from here if you still want to keep #6405 open (this one is rebased, passes travis tests and removes a little bit more code).

@sipa
If I create a PR that instead of removing the code at least encapsulates it in a function (hopefully a method assuming #6068 gets merged this year) in the policy folder, will it have to wait for #6470 (or its current replacement) too?
What if I remove the minRelayTxFee global (make it an attribute in CStandardPolicy) and implement a dynamic minRelayTxFee that uses the fee estimator at the same time?

I can have the code ready relatively fast if there's interest in reviewing it and it's not going to be blocked by #6470.

@jtimon Heh, I know we were just together, but I've been totally behind on PR's for the last couple weeks so didn't see this. I just wanted to give my two cents that I'm now happy to rebase any of my existing mempool limiting PR's on top of policy changes. During the rapid iteration to find a solution I felt like that was going to be a hindrance, but I think the work is far enough along now that it shouldn't hold up anything else.

I agree with @luke-jr @sipa and @paveljanik ~ I realize this is closed out, but in light of new comments, I'm just emphasizing my prior NACK. See in particular @sipa's comments. cc @morcos

@ABISprotocol: @sipa's comments were in favour of this change? Could you link directly to what you mean?

Correct, it is trivial to regenerate at any time.

@dcousens Please see here and also @sipa's more recent comment above (Sept. 16) here. My comments above may have seemed conflicting, to simplify, I think this puts the cart before the horse. Finish what was intended here and in #6557 then move on to other matters that are being suggested here in this issue and in 6405.

Yes, this is trivial to rebase. Also, I'm going to try another approach where the functionality is unaffected but encapsulated in policy and at the same time minTxRealyFee stops being a global and it's dynamic (using the fee estimator) instead. But I won't open a PR with that branch at least until #6068 is merged, but people can take a look at master...jtimon:policy-tip-0.12.99

Sorry, I meant this branch: master...jtimon:policy-minrelayfee-0.12.99