Change default block priority size to 0

morcos commented at 9:10 pm on November 15, 2015: member

As discussed on the mailing list, the current notion of priority is no longer completely supported with the 0.12 release. (In particular limited mempools will not allow free transactions when they have recently hit their limit). It is likely that further support will be removed in future release and the current concept is now deprecated. As such it makes sense to make the mining default not include priority space. It is still supported to create a priority space if the command line argument is changed. (To what degree depends on other open PR’s)

luke-jr commented at 10:18 pm on November 15, 2015: member

NACK changing policy defaults as an attempt to influence miners. Also this change would likely be detrimental to Bitcoin in practice, since the priority space is mostly immune to spam attacks.

TheBlueMatt commented at 10:27 pm on November 15, 2015: member

Concept ACK

On November 15, 2015 1:10:23 PM PST, Alex Morcos notifications@github.com wrote:

As discussed on the mailing list, the current notion of priority is no longer completely supported with the 0.12 release. (In particular limited mempools will not allow free transactions when they have recently hit their limit). It is likely that further support will be removed in future release and the current concept is now deprecated. As such it makes sense to make the mining default not include priority space. It is still supported to create a priority space if the command line argument is changed. (To what degree depends on other open PR’s) You can view, comment on, or merge this pull request online at:

#7022

– Commit Summary –

Change default block priority size to 0

– File Changes –

M src/policy/policy.h (2)

– Patch Links –

#7022.patch #7022.diff

Reply to this email directly or view it on GitHub: #7022

morcos commented at 10:29 pm on November 15, 2015: member

I suppose I need to go fix up tests now…. I’ll wait to see whether we want to merge this.

paveljanik commented at 10:43 pm on November 15, 2015: contributor

NACK

Do we mix priority and low fee transactions here? Anyone seen sustaining attack of low fee high priority transactions?

dcousens commented at 11:08 pm on November 15, 2015: contributor

Concept ACK/utACK, this matches the development agenda of removing priority in general.

laanwj added the label Mining on Nov 16, 2015

sdaftuar commented at 5:38 pm on November 16, 2015: member

Concept ACK

TheBlueMatt commented at 10:21 pm on November 20, 2015: member

Can we also mark the option as “deprecated”. Though we may re-add a variation on it in the future, its current version will likely no longer be supported.

petertodd commented at 0:15 am on November 21, 2015: contributor

utACK

Also agree w/ the idea of marking it “deprecated”

Change default block priority size to 0

Make RPC tests have a default block priority size of 50000 (the old default) so we can still use free transactions in RPC tests.  When priority is eliminated, we will have to make a different change if we want to continue allowing free txs.

50947ef23f

morcos force-pushed on Nov 30, 2015

morcos commented at 9:28 pm on November 30, 2015: member

Updated to make the RPC tests still work by passing in the old -blockprioritysize

To be clear this just changes the default for mining code so that we are properly signaling the intention to remove the current concept of priority in future releases.

luke-jr commented at 10:48 pm on November 30, 2015: member

We do not intend to remove priority in future releases (for mining).

TheBlueMatt commented at 6:16 am on December 1, 2015: member

@luke-jr yes. I think everyone but you would very much to prefer to remove priority as it is used today. There is a bunch of discussion about adding a similar feature that, instead, effects transactions by modifying their “effective feerate” in its place, but block priority space will not be in future releases.

On 11/30/15 22:48, Luke-Jr wrote:

We do not intend to remove priority in future releases (for mining).

— Reply to this email directly or view it on GitHub #7022 (comment).

luke-jr commented at 10:23 am on December 1, 2015: member

@TheBlueMatt Then miners would be seriously harming Bitcoin to use future releases. NACK.

TheBlueMatt commented at 10:37 am on December 1, 2015: member

utACK

laanwj merged this on Dec 1, 2015

laanwj closed this on Dec 1, 2015

laanwj referenced this in commit 9afbd96919 on Dec 1, 2015

dcousens commented at 1:59 am on December 2, 2015: contributor

@luke-jr from what I can read from the last few IRC meetings, there was a some agreement [yourself excluded] that this would go ahead for 0.12. Obviously evident from the merge, that seems to be the consensus reached on this?

IRC meeting summaries: https://forum.bitcoin.com/bitcoin-discussion/bitcoin-dev-irc-meeting-in-layman-s-terms-2015-11-19-t2943.html https://forum.bitcoin.com/bitcoin-discussion/bitcoin-dev-irc-meeting-in-layman-s-terms-2015-11-12-t2682.html https://forum.bitcoin.com/bitcoin-discussion/bitcoin-dev-irc-meeting-in-layman-s-terms-2015-11-05-t2285.html

in qa/rpc-tests/test_framework/util.py: in 50947ef23f

216@@ -217,7 +217,8 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
217     datadir = os.path.join(dirname, "node"+str(i))
218     if binary is None:
219         binary = os.getenv("BITCOIND", "bitcoind")
220-    args = [ binary, "-datadir="+datadir, "-keypool=1", "-discover=0", "-rest" ]
221+    # RPC tests still depend on free transactions
222+    args = [ binary, "-datadir="+datadir, "-keypool=1", "-discover=0", "-rest", "-blockprioritysize=50000" ]

MarcoFalke commented at 8:04 am on December 9, 2015:

I don’t like how this changes the default for master but the unit tests run with different settings by default. See #7177

FinalHash commented at 9:28 am on December 14, 2015: none

As one of the worlds biggest miners I, and i think i speak for all my colleagues when i say, WE DO NOT SUPPORT THIS CHANGE. yes we really dont use it much but when tx’s get stuck we can make it work. This is horseshit. Some dude just trying to get a commit based on a conversation. Revert it

dcousens commented at 11:04 am on December 14, 2015: contributor

@FinalHash then don’t run it on your node?

sipa commented at 11:20 am on December 14, 2015: member

Marshall, you may be missing some context here.

This is a change that has been discussed many times among Core’s developers, and many (but not all) have wanted to make for a long time.

I’m a bit confused about your statement that “when tx’s get stuck we can make it work”. Are you sure you’re not talking about the prioritizetransaction RPC? That keeps working and will continue to do so, and allows manually bumping a transaction ahead of the queue for mining purposes.

All this change does is reduce the default size of the amount of block space dedicated to transactions that get included based on bitcoindays-destroyed-per-byte (instead of fee per byte) to zero. If there are problems that can be solved by having such a space, you can always turn it back on by setting a non-zero size manually.

Several people (myself included) would like the concept of sorting by bitcoindays-destroyed-per-byte go away entirely over time, however, and this is a step in that direction.

Maintaining this “priority area” complicates the mining-, mempool-, wallet-, and fee-estimation code significantly.
It works in a very non-optimal way. With a priority space, you may well kick out high fee transactions in favor of 0-fee transactions that just spend slightly older outputs. If bitcoin-days-destroyed-per-byte is something we value, I think we should account for it by having a single sort order that combines both, rather than having a dedicated space inside blocks.
At 50 kB it is hardly a useful measure against spam attacks. If it is, it would mean that 950 kB of data is hardly usable due to being filled with high-fee spam, and we get a 50 kB “usable” space in every block in which actual transactions can occur. The priority space can be set to be bigger (and still can be after this PR!), but at that point risks reducing fee income significantly. That may not be a worry now at the current fee levels, but it’s not a solution we should encourage long term, as it is incompatible with Bitcoin’s long term security (with fees paying for security).
Bitcoindays-destroyed-per-byte sorting unnecessarily benefits people who hold large amounts of old BTC.

FinalHash commented at 11:25 am on December 14, 2015: none

This is just a warning shot for when you guys make changes without discussing it with any miners.

Additionally, I think its a waste of time to do this. All miners have effectively unanimously agreed two days ago to go with seg wit. I¹ve only had a small amount of kick back from one dev in particular and the extent of the warning was, ³Some security issues.² Can you email me and lets get seg wit going.

From: Pieter Wuille notifications@github.com Reply-To: bitcoin/bitcoin <reply+00699eb269f54a2d00441fe9882e973444a98fb312160a3c92cf000000011286680d9 2a169ce06f98dc2@reply.github.com> Date: Monday, December 14, 2015 at 6:20 AM To: bitcoin/bitcoin bitcoin@noreply.github.com Cc: Marshall Long marshall.long@me.com Subject: Re: [bitcoin] Change default block priority size to 0 (#7022)

Marshall, you may be missing some context here.

This is a change that has been discussed many times among Core’s developers, and many (but not all) have wanted to make for a long time.

I’m a bit confused about your statement that “when tx’s get stuck we can make it work”. Are you sure you’re not talking about the prioritizetransaction RPC? That keeps working and will continue to do so, and allows manually bumping a transaction ahead of the queue for mining purposes.

All this change does is reduce the default size of the amount of block space dedicated to transactions that get included based on bitcoindays-destroyed-per-byte (instead of fee per byte) to zero. If there are problems that can be solved by having such a space, you can always turn it back on by setting a non-zero size manually.

Several people (myself included) would like the concept of sorting by bitcoindays-destroyed-per-byte go away entirely over time, however, and this is a step in that direction.

Maintaining this “priority area” complicates the mining-, mempool-, wallet-, and fee-estimation code significantly.
It works in a very non-optimal way. With a priority space, you may well kick out high fee transactions in favor of 0-fee transactions that just spend slightly older outputs. If bitcoin-days-destroyed-per-byte is something we value, I think we should account for it by having a single sort order that combines both, rather than having a dedicated space inside blocks.
At 50 kB it is hardly a useful measure against spam attacks. If it is, it would mean that 950 kB of data is hardly usable due to being filled with high-fee spam, and we get a 50 kB “usable” space in every block in which actual transactions can occur. The priority space can be set to be bigger (and still can be after this PR!), but at that point risks reducing fee income significantly. That may not be a worry now at the current fee levels, but it’s not a solution we should encourage long term, as it is incompatible with Bitcoin’s long term security (with fees paying for security).
Bitcoindays-destroyed-per-byte sorting unnecessarily benefits people who hold large amounts of old BTC.

Reply to this email directly or view it on GitHub #7022 (comment) .

sipa commented at 11:32 am on December 14, 2015: member

I’m very glad to hear you like segregated witness (FWIW, I proposed it), but this is not the place for making decisions about Bitcoin’s consensus rules.

This pull request is about changing a local policy, which every node can individually decide. It is not a scaling solution, has nothing to do with the consensus rules of the network, and is just a default.

Segregated witness is not a magic bullet that suddenly means we don’t have anything else to improve anymore, inside Bitcoin Core or elsewhere.

In particular, this change is needed to get the much-faster CreateNewBlock RPC that 0.12 is introducing.

luke-jr commented at 5:34 pm on December 14, 2015: member

In particular, this change is needed to get the much-faster CreateNewBlock RPC that 0.12 is introducing.

This is not true…

jyap808 commented at 8:28 pm on December 14, 2015: contributor

NACK. I have experienced to 2 very normal transactions take a very long time to confirm in the past few days. One took over 13 hours, another took over 24 hours.

dcousens commented at 11:23 pm on December 14, 2015: contributor

@jyap808 and that is related how?

jyap808 commented at 0:17 am on December 15, 2015: contributor

@dcousens isn’t it related to this change being committed?

gmaxwell commented at 0:19 am on December 15, 2015: contributor

no, jyap808, the things here are in software that won’t be released for months. And, I expect, will go along with other changes that haven’t been written yet.

sipa commented at 0:20 am on December 15, 2015: member

@jyap808 This is about a change that is merged, but is not in any release, nor anywhere deployed in production yet. If your transaction in the current environment take a long time to confirm, it’s certainly not due to this change, and they’re likely both too low in fee and too low in priority.

jyap808 commented at 0:21 am on December 15, 2015: contributor

Thanks, I’ll stop reading Reddit.

MarcoFalke locked this on Sep 8, 2021

Change default block priority size to 0 #7022