Refactor CBlockPolicyEstimator#9942
Merged
laanwj merged 7 commits intobitcoin:masterfrom Apr 20, 2017
Merged
Conversation
morcos
force-pushed
the
moveTxConfirmStats
branch
from
c3b3de4 to
35e9c1a
Compare
TheBlueMatt
reviewed
Mar 7, 2017
src/policy/fees.cpp
Outdated
| LOCK(cs_feeEstimator); | ||
| int nVersionRequired, nVersionThatWrote, nFileBestSeenHeight; | ||
| filein >> nVersionRequired >> nVersionThatWrote; | ||
| filein >> nFileBestSeenHeight; |
Contributor
There was a problem hiding this comment.
Probably best to still check the nVersion before reading any further, no?
Contributor
Author
There was a problem hiding this comment.
oops. thats a pretty big oversight. will fix.
Member
|
utACK 40d31d0 |
Member
|
Should the fixup be squashed first before merging this? |
morcos
force-pushed
the
moveTxConfirmStats
branch
from
40d31d0 to
645946d
Compare
Contributor
Author
|
Squashed 40d31d0 (moveTxConfirmStats.tag1) -> 645946d |
Contributor
|
utAck 645946d. Later work needs to clean up the TxConfirmedStats pointer usage, but that is a complex refactor and outside the scope of this PR. |
and change to storing as a pointer.
morcos
force-pushed
the
moveTxConfirmStats
branch
from
645946d to
68af651
Compare
Contributor
Author
|
Rebased. No changes from 645946d |
Contributor
|
utACK 68af651. Nice cleanups, seems good to move this stuff out of the mempool. |
Merged
Member
|
utACK 68af651 |
theuni
approved these changes
Apr 19, 2017
| } | ||
| vfeelist.push_back(INF_FEERATE); | ||
| feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); | ||
| feeStats = new TxConfirmStats(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY); |
Contributor
Author
There was a problem hiding this comment.
heh... i agree, but i wanted to move code to .cpp file and then i was screwed by lack of a destructor. I'm sure there is a better way, but I went around in circles chasing my tail for a while.. Maybe we could save that for an actual programmer to clean up later.
laanwj
added a commit
that referenced
this pull request
Apr 20, 2017
68af651 MOVEONLY: move TxConfirmStats to cpp (Alex Morcos) 2332f19 Initialize TxConfirmStats in constructor (Alex Morcos) 5ba81e5 Read and Write fee estimate file directly from CBlockPolicyEstimator (Alex Morcos) 14e10aa Call estimate(Smart)Fee directly from CBlockPolicyEstimator (Alex Morcos) dbb9e36 Give CBlockPolicyEstimator it's own lock (Alex Morcos) f6187d6 Make processBlockTx private. (Alex Morcos) ae7327b Make feeEstimator its own global instance of CBlockPolicyEstimator (Alex Morcos) Tree-SHA512: dbf3bd2b30822e609a35f3da519b62d23f8a50e564750695ddebd08553b4c01874ae3e07d792c6cc78cc377d2db33b951ffedc46ac7edaf5793f9ebb931713af
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
May 27, 2019
68af651 MOVEONLY: move TxConfirmStats to cpp (Alex Morcos) 2332f19 Initialize TxConfirmStats in constructor (Alex Morcos) 5ba81e5 Read and Write fee estimate file directly from CBlockPolicyEstimator (Alex Morcos) 14e10aa Call estimate(Smart)Fee directly from CBlockPolicyEstimator (Alex Morcos) dbb9e36 Give CBlockPolicyEstimator it's own lock (Alex Morcos) f6187d6 Make processBlockTx private. (Alex Morcos) ae7327b Make feeEstimator its own global instance of CBlockPolicyEstimator (Alex Morcos) Tree-SHA512: dbf3bd2b30822e609a35f3da519b62d23f8a50e564750695ddebd08553b4c01874ae3e07d792c6cc78cc377d2db33b951ffedc46ac7edaf5793f9ebb931713af finish bitcoin#9942 by removing removed functions Signed-off-by: Pasta <[email protected]>
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
9 participants
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 moves towards disentangling CBlockPolicyEstimator from CTxMemPool by making it its own global object. At the end of this PR, the CTxMemPool still has to know about CBlockPolicyEstimator, but this lays the ground work so that in the future CBlockPolicyEstimator can just subscribe to transaction and block events and the mempool will know nothing about it.
The last two commits are preparation for future work where there will be multiple TxConfirmStats in the CBlockPolicyEstimator which all refer to the same set of buckets.
I've grouped the commits into this PR, because this is the of the commits which introduces no change into the fee estimates, so is possible to merge cleanly by itself. There are a set of major changes to fee estimation built on top of this that isn't finished yet.