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

Jump to bottom

Remove utxo db upgrade code #24236

Merged
merged 1 commit into from Apr 5, 2022
Merged

Remove utxo db upgrade code #24236

merged 1 commit into from Apr 5, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Feb 2, 2022 •
edited

It is not possible to upgrade Bitcoin Core pre-segwit (pre-0.13.1) to a recent version without a full IBD from scratch after commit 19a56d1 (released in version 22.0).

Any Bitcoin Core version with the new database format after commit 1088b02 (released in version 0.15), can upgrade to any version that is supported as of today.

This leaves the versions 0.13.1-0.14.x. Even though those versions are unsupported, some users with an existing datadir may want to upgrade to a recent version. However, it seems reasonable to simply ask them to -reindex to run a full IBD from scratch. This allows us to remove the utxo db upgrade code.

laanwj
laanwj reviewed Feb 2, 2022
View reviewed changes
src/init.cpp Outdated Show resolved Hide resolved
@laanwj
Copy link
Member

laanwj commented Feb 2, 2022

Concept ACK

1 similar comment
@gruve-p
Copy link
Contributor

gruve-p commented Feb 2, 2022

Concept ACK

@maflcko maflcko force-pushed the 2202-txdbNoUpgrade branch 4 times, most recently from fafd3a6 to faf989d Compare February 3, 2022 07:48
Sjors
Sjors approved these changes Feb 3, 2022
View reviewed changes
Copy link
Member

@Sjors Sjors left a comment •
edited

Choose a reason for hiding this comment

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

Concept ACK. Prefer to merge this after the v23 branch-off, unless anything builds on it.

tACK faf989d

In the release notes we could suggest that users who are still on v0.13.1-0.14.x can, as an alternative to reindexing, first upgrade to v22.0.

src/node/chainstate.cpp Show resolved Hide resolved
src/txdb.cpp Show resolved Hide resolved
@maflcko
Copy link
Member Author

maflcko commented Feb 3, 2022

Concept ACK. Prefer to merge this after the v23 branch-off, unless anything builds on it.

There is a bunch of stuff that builds on it, but no rush. Assigned 24.0 for now.

@maflcko maflcko added this to the 24.0 milestone Feb 3, 2022
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 4, 2022 •
edited

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot mentioned this pull request Feb 4, 2022
Merged
@DrahtBot DrahtBot mentioned this pull request Feb 20, 2022
Merged
otech47
otech47 approved these changes Mar 9, 2022
View reviewed changes
Copy link

@otech47 otech47 left a comment

Choose a reason for hiding this comment

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

LGTM

any rationale for testing against v0.14.3 versus v0.13.1? or both?

@maflcko
Copy link
Member Author

maflcko commented Mar 10, 2022

any rationale for testing against v0.14.3 versus v0.13.1? or both?

Thanks for the review. 13.x can not be tested by our current test framework, as there were changes in 14.x that were backward compatible, but not forward compatible. See https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.14.0.md#support-for-json-rpc-named-arguments

It is possible to adjust the test framework for this to test 13.x, but I don't think this is worth it. The automated test with 14.x should be sufficient. Also, anyone can run a test with 13.x locally for a one-off.

@maflcko
Copy link
Member Author

maflcko commented Mar 10, 2022

Addressed test nit. Should be trivial to re-ACK with:

git range-diff bitcoin-core/master fabd801fc0 fa9112aac0

@Sjors
Copy link
Member

Sjors commented Mar 16, 2022

re-ACK fa9112a

@laanwj
Copy link
Member

laanwj commented Apr 5, 2022

Code review ACK fa9112a

@laanwj laanwj merged commit f421de5 into bitcoin:master Apr 5, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 5, 2022
@laanwj
Merge bitcoin#24236: Remove utxo db upgrade code
a49b3fc
@bitcoin bitcoin locked and limited conversation to collaborators Apr 5, 2023
@maflcko maflcko deleted the 2202-txdbNoUpgrade branch August 22, 2023 08:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laanwj laanwj laanwj left review comments

@otech47 otech47 otech47 approved these changes

@Sjors Sjors Sjors approved these changes

Assignees
No one assigned
Labels
UTXO Db and Indexes
Projects
None yet
Milestone
24.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@maflcko @laanwj @gruve-p @DrahtBot @Sjors @otech47