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

Longer term workaround for chainstate corruption from negative versions. #2982

Merged
merged 1 commit into from
Sep 10, 2013
Merged

Longer term workaround for chainstate corruption from negative versions. #2982

merged 1 commit into from
Sep 10, 2013

Conversation

gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Sep 9, 2013

This also makes negative transaction versions non-standard.

This avoids an issue triggered in block 256818 where transactions with
negative version numbers were incorrectly serialized into the UTXO set.

On restart nodes detect the inconsistency and refuse to start so long as
a block with these transactions is inside the self-consistency check
window, logging "coin database inconsistencies found". The software
recommends reindexing, but reindexing does not correct the problem.

This should be fixed by changing the chainstate serialization, but
working around it seems harmless for now because the version is not
used by any network rule currently.

A patch free workaround is to start with -checklevel=2 which skips
the consistency checks, but the IsStandard change is important for
miners in order to protect unpatched nodes.

This also makes negative transaction versions non-standard.

This avoids an issue triggered in block 256818 where transactions with
negative version numbers were incorrectly serialized into the UTXO set.

On restart nodes detect the inconsistency and refuse to start so long as
a block with these transactions is inside the self-consistency check
window, logging "coin database inconsistencies found". The software
recommends reindexing, but reindexing does not correct the problem.

This should be fixed by changing the chainstate serialization, but
working around it seems harmless for now because the version is not
used by any network rule currently.

A patch free workaround is to start with -checklevel=2 which skips
the consistency checks, but the IsStandard change is important for
miners in order to protect unpatched nodes.
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f8b7aa862519ab2efd1ce327d2ed4bea1325dc11 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@gavinandresen
Copy link
Contributor

ACK: tested on OSX.

@laanwj
Copy link
Member

laanwj commented Sep 9, 2013

ACK: works for me on Ubuntu 12.04 64 bit

@wtogami
Copy link
Contributor

wtogami commented Sep 9, 2013

https://github.com/gmaxwell/bitcoin/commit/f8b7aa862519ab2efd1ce327d2ed4bea1325dc11
Testing backport to litecoin-0.8.4.1. Assuming I did it correctly, your bitcoin-0.8.x backport will be slightly different from this.

@jgarzik
Copy link
Contributor

jgarzik commented Sep 9, 2013

ACK

1 similar comment
@wtogami
Copy link
Contributor

wtogami commented Sep 9, 2013

ACK

@runeksvendsen
Copy link
Contributor

I can confirm that this patch fixes the issue for me, and tests run without errors.

@sipa
Copy link
Member

sipa commented Sep 9, 2013

ACK code changes; didn't test.

gmaxwell added a commit that referenced this pull request Sep 10, 2013
Longer term workaround for chainstate corruption from negative versions.
@gmaxwell gmaxwell merged commit 85311c1 into bitcoin:master Sep 10, 2013
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants