Skip to content

Conversation

amitiuttarwar
Copy link
Contributor

In 0.21, we added unbroadcast txids to mempool.dat (#18038). When users upgraded from 0.21 to 0.22, this would throw a misleading "failed to deserialize mempool data" error even though everything actually loaded properly. So, commit 9c8a55d added a try-block to prevent throwing the error. After upgrading to 0.22, this exception handling is no longer useful, so now we can remove it.

In 0.21, we added unbroadcast txids to mempool.dat. Commit 9c8a55d
added a try-block to prevent throwing a "failed to deserialize mempool data"
error when a user upgrades from 0.21 to 0.22. This exception handling is no
longer useful, so now we can remove it.
@amitiuttarwar
Copy link
Contributor Author

from a quick look, the test causing CI to be red looks like a failure in interface_zmq.py around the sequence number of the coinbase tx in a block.

seems unrelated to these changes.

@maflcko
Copy link
Member

maflcko commented Jan 5, 2021

review ACK 7ff0535

nice cleanup. ci failure is known and can be ignored

@maflcko maflcko added Mempool and removed Validation labels Jan 5, 2021
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 7ff0535

@maflcko maflcko merged commit c376007 into bitcoin:master Jan 5, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 5, 2021
7ff0535 [mempool] Remove error suppression on upgrade (Amiti Uttarwar)

Pull request description:

  In 0.21, we added unbroadcast txids to mempool.dat (bitcoin#18038). When users upgraded from 0.21 to 0.22, this would throw a misleading "failed to deserialize mempool data" error even though everything actually loaded properly. So, commit 9c8a55d added a try-block to prevent throwing the error. After upgrading to 0.22, this exception handling is no longer useful, so now we can remove it.

ACKs for top commit:
  MarcoFalke:
    review ACK 7ff0535
  theStack:
    Code review ACK 7ff0535

Tree-SHA512: 0444eea2b1326904f9855fd0af6669a4990f0427cf7c9293252a5b7049cdcc785bdf9398fd08ed8dedacfdd78e75039ddf1087b3654c558ff52498df15f05daf
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 20, 2022
Summary:
This is a backport of [[bitcoin/bitcoin#20854 | core#20854]]

Depends on D11764

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D11765
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants