Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dead feeest-file read code for old versions #11951

Merged
merged 1 commit into from Dec 20, 2017

Conversation

TheBlueMatt
Copy link
Contributor

0.15.0 introduced a new feeest file format, and support for parsing
old versions was never fully added. We now simply fail to read the
old format, so remove the dead partial-implementation.

Follow up to #11273.

0.15.0 introduced a new feeest file format, and support for parsing
old versions was never fully added. We now simply fail to read the
old format, so remove the dead partial-implementation.
@murchandamus
Copy link
Contributor

utACK 62e7c04

@jnewbery
Copy link
Contributor

Why not remove nFileVersion from the TxConfirmStats::Read() function signature entirely?

@TheBlueMatt
Copy link
Contributor Author

I figured it may be useful to have in a future change, so might as well leave it instead of creating more diff to change the function signature.

@jnewbery
Copy link
Contributor

I'm able to hit the new check in #11273 by manually editing the nVersionRequired bytes in the fee_estimate.dat file:

...
2017-12-19 18:20:23 Read: incompatible old fee estimation data (non-fatal). Version: 140000

which means that TxConfirmStats::Read() doesn't get hit at all.

It'd be good to add a check in CBlockPolicyEstimator::Read() that nVersionThatWrote <= nVersionRequired, although that doesn't necessarily have to be done in this PR. That would ensure that we couldn't possibly enter TxConfirmStats::Read() if nVersionThatWrote < 149900.

it may be useful to have in a future change, so might as well leave it

My default preference is the other way. If code is unused it should be removed at the earliest opportunity.

Tested ACK 62e7c04

@meshcollider
Copy link
Contributor

utACK 62e7c04

@laanwj
Copy link
Member

laanwj commented Dec 20, 2017

utACK 62e7c04

@laanwj laanwj merged commit 62e7c04 into bitcoin:master Dec 20, 2017
laanwj added a commit that referenced this pull request Dec 20, 2017
62e7c04 Remove dead feeest-file read code for old versions (Matt Corallo)

Pull request description:

  0.15.0 introduced a new feeest file format, and support for parsing
  old versions was never fully added. We now simply fail to read the
  old format, so remove the dead partial-implementation.

  Follow up to #11273.

Tree-SHA512: c291ce51b7cb0c93479c18a1885dd646cbe03e4c7d12df03c0e99c0634e1bf9f9e41facf54a645227154bab58b9988f21b928cf3fc0520087c4eede4258c8861
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
62e7c04 Remove dead feeest-file read code for old versions (Matt Corallo)

Pull request description:

  0.15.0 introduced a new feeest file format, and support for parsing
  old versions was never fully added. We now simply fail to read the
  old format, so remove the dead partial-implementation.

  Follow up to bitcoin#11273.

Tree-SHA512: c291ce51b7cb0c93479c18a1885dd646cbe03e4c7d12df03c0e99c0634e1bf9f9e41facf54a645227154bab58b9988f21b928cf3fc0520087c4eede4258c8861
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
62e7c04 Remove dead feeest-file read code for old versions (Matt Corallo)

Pull request description:

  0.15.0 introduced a new feeest file format, and support for parsing
  old versions was never fully added. We now simply fail to read the
  old format, so remove the dead partial-implementation.

  Follow up to bitcoin#11273.

Tree-SHA512: c291ce51b7cb0c93479c18a1885dd646cbe03e4c7d12df03c0e99c0634e1bf9f9e41facf54a645227154bab58b9988f21b928cf3fc0520087c4eede4258c8861
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
62e7c04 Remove dead feeest-file read code for old versions (Matt Corallo)

Pull request description:

  0.15.0 introduced a new feeest file format, and support for parsing
  old versions was never fully added. We now simply fail to read the
  old format, so remove the dead partial-implementation.

  Follow up to bitcoin#11273.

Tree-SHA512: c291ce51b7cb0c93479c18a1885dd646cbe03e4c7d12df03c0e99c0634e1bf9f9e41facf54a645227154bab58b9988f21b928cf3fc0520087c4eede4258c8861
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
62e7c04 Remove dead feeest-file read code for old versions (Matt Corallo)

Pull request description:

  0.15.0 introduced a new feeest file format, and support for parsing
  old versions was never fully added. We now simply fail to read the
  old format, so remove the dead partial-implementation.

  Follow up to bitcoin#11273.

Tree-SHA512: c291ce51b7cb0c93479c18a1885dd646cbe03e4c7d12df03c0e99c0634e1bf9f9e41facf54a645227154bab58b9988f21b928cf3fc0520087c4eede4258c8861
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants