Remove priority estimation #7730
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
concept ACK |
|
So this essentially changes priority from "fully supported for usage" to "fallback only", at least from the perspective of Core's wallet. Concept ACK. |
|
utACK 2f3c306 |
|
Concept ACK, would be good to see this rebased. |
|
Concept ACK |
|
Needs rebase. |
|
trivial rebase |
|
utACK 9a936e3 |
| nBestSeenHeight = nFileBestSeenHeight; | ||
| if (nVersionRequired < 129900) { |
|
utACK 9a936e3 |
|
Rebase please? |
maflcko
reviewed
Sep 30, 2016
| tx.vin[0].prevout.n = 10000*blocknum+100*j+k; | ||
| uint256 hash = tx.GetHash(); | ||
| mpool.addUnchecked(hash, entry.Fee(feeV[k/4][j]).Time(GetTime()).Priority(priV[k/4][j]).Height(blocknum).FromTx(tx, &mpool)); | ||
| mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); | ||
| CTransaction btx; | ||
| if (mpool.lookup(hash, btx)) |
There was a problem hiding this comment.
Needs merge conflict solved here due to 288d85d
|
Do we want this for 0.14? |
|
Sorry for the delay.. I rebased including bumping the version numbers, and I just wanted to flag another minor change I made since the previous version had been acked. This seems like the correct way to handle reading old version files. |
|
utACK 9ea4ed9. Mind to reword or squash the "SQUASHME" commit? |
laanwj
added a commit
that referenced
this pull request
Nov 7, 2016
0bd581a add release notes for removal of priority estimation (Alex Morcos) b2322e0 Remove priority estimation (Alex Morcos)
|
Merged (and squashed) via 3c03dc2 |
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Jan 13, 2018
0bd581a add release notes for removal of priority estimation (Alex Morcos) b2322e0 Remove priority estimation (Alex Morcos)
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
0bd581a add release notes for removal of priority estimation (Alex Morcos) b2322e0 Remove priority estimation (Alex Morcos)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 15, 2019
0bd581a add release notes for removal of priority estimation (Alex Morcos) b2322e0 Remove priority estimation (Alex Morcos)
This was referenced Aug 4, 2020
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Aug 23, 2020
2da16f7 [Cleanup] Nuke estimate(smart)priority from the RPC interface (random-zebra) 041f62e [Core] Remove priority estimation (random-zebra) Pull request description: Based on top of - [x] #1787 This removes the functionality behind `estimatepriority` and `estimatesmartpriority` (from bitcoin#7730). Instead of just deprecating the relative RPC commands, I think we can outright remove them, as `estimatepriority` was added only in latest minor release (4.2), and `estimatesmartpriority` is introduced in a PR not even merged yet (#1787). ACKs for top commit: Fuzzbawls: utACK 2da16f7 furszy: utACK 2da16f7 Tree-SHA512: 7dc964c7627a08eae349df40f40c49bc85761171d97f3d7a828421f6dc46ca86dc7a410c75bd8025b7a684aa09d215a08ac6ae19b64617adac6365cfaee55afc
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the functionality behind
estimatepriorityandestimatesmartpriority. The rpc calls are now deprecated.estimateprioritywill return -1 always andestimatesmartprioritywill return 1e24 if the mempool is currently limited and -1 otherwise. The result of this behavior is that free transactions (if selected using the now debug option-sendfreetransactions) can be created and sent if the mempool is not currently limited and the transaction's priority is above the hard coded AllowFree threshold.This is effectively the behavior in place already as priority estimates do not appear until confirmation targets over 50 which aren't currently supported.
A side effect of this is that now all transactions (that aren't dependent on unformed inputs) are now considered data points for fee estimation. Even though some transactions may be mined due to their priority instead of fee, this is still safe because the threshold for fee estimation is very high (95%).