Remove utxo db upgrade code

MarcoFalke commented at 11:14 am on February 2, 2022: member

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 19a56d1519fb493c3e1bd5cad55360b6b80fa52b (released in version 22.0).

Any Bitcoin Core version with the new database format after commit 1088b02f0ccd7358d2b7076bb9e122d59d502d02 (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.

MarcoFalke force-pushed on Feb 2, 2022

DrahtBot added the label UTXO Db and Indexes on Feb 2, 2022

in src/init.cpp:1463 in fa9698279e outdated

1458@@ -1459,8 +1459,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1459                 strLoadError = _("Error initializing block database");
1460                 break;
1461             case ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED:
1462-                strLoadError = _("Error upgrading chainstate database");
1463-                break;
1464+                return InitError(_("Unsupported chainstate database format found. "
1465+                                   "Please restart with -reindex. This will redownload "

laanwj commented at 5:56 pm on February 2, 2022:

Isn’t -reindex-chainstate enough? Also, -reindex will not redownload the blockchain. It only reindexes the one on disk.

MarcoFalke commented at 8:09 pm on February 2, 2022:

No idea. For me Bitcoin Core just hangs itself up when using -reindex-chainstate and -prune, but that’s probably an unrelated bug.

MarcoFalke commented at 8:14 pm on February 2, 2022:

Reported as https://github.com/bitcoin/bitcoin/issues/24242

MarcoFalke commented at 8:14 pm on February 2, 2022:

Picked your suggestion, thx.

laanwj commented at 10:57 am on February 3, 2022:

It wasn’t clear to me that the context here was about pruning. But that definitely sounds like a bug. With pruning, -reindex-chainstate and -reindex should be the same.

laanwj commented at 5:57 pm on February 2, 2022: member

Concept ACK

gruve-p commented at 7:35 pm on February 2, 2022: contributor

Concept ACK

MarcoFalke force-pushed on Feb 2, 2022

MarcoFalke force-pushed on Feb 3, 2022

in src/node/chainstate.cpp:93 in faf989d08a outdated

89@@ -90,9 +90,9 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
90             chainstate->CoinsErrorCatcher().AddReadErrCallback(coins_error_cb);
91         }
92 
93-        // If necessary, upgrade from older database format.
94+        // Refuse to load unsupported database format.

Sjors commented at 10:44 am on February 3, 2022:

0(v0.8 - v0.14.x)

MarcoFalke commented at 1:08 pm on February 3, 2022:

Thanks, added a comment (somewhere else)

in src/txdb.cpp:30 in faf989d08a outdated

26@@ -29,6 +27,7 @@ static constexpr uint8_t DB_REINDEX_FLAG{'R'};
27 static constexpr uint8_t DB_LAST_BLOCK{'l'};
28 
29 // Keys used in previous version that might still be found in the DB:
30+static constexpr uint8_t DB_COINS{'c'};

Sjors commented at 10:57 am on February 3, 2022:

0// v0.8 - 0.14.x : deprecated in 1088b02f0ccd7358d2b7076bb9e122d59d502d02

MarcoFalke commented at 1:08 pm on February 3, 2022:

Thanks, added a comment (somewhere else)

Sjors approved

Sjors commented at 11:09 am on February 3, 2022: member

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

tACK faf989d08a5ae914cb8510a4edd49dd9ed076086

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.

MarcoFalke commented at 12:32 pm on February 3, 2022: member

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.

MarcoFalke added this to the milestone 24.0 on Feb 3, 2022

MarcoFalke force-pushed on Feb 3, 2022

DrahtBot commented at 4:17 am on February 4, 2022: contributor

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

Conflicts

No conflicts as of last run.

DrahtBot added the label Needs rebase on Feb 24, 2022

MarcoFalke force-pushed on Feb 24, 2022

DrahtBot removed the label Needs rebase on Feb 24, 2022

MarcoFalke force-pushed on Mar 7, 2022

otech47 approved

otech47 commented at 3:05 pm on March 9, 2022: none

LGTM

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

in test/functional/feature_unsupported_utxo_db.py:34 in fabd801fc0 outdated

29+                None,  # For MiniWallet, without migration code
30+            ],
31+        )
32+
33+    def run_test(self):
34+        self.log.info("Create previous version utxo db")

otech47 commented at 3:06 pm on March 9, 2022:

Consider logging the version tested Create previous version (v0.14.3) utxo db

MarcoFalke commented at 12:06 pm on March 10, 2022:

Thanks, fixed

MarcoFalke commented at 12:03 pm on March 10, 2022: member

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.

Remove utxo db upgrade code fa9112aac0

MarcoFalke force-pushed on Mar 10, 2022

MarcoFalke commented at 12:07 pm on March 10, 2022: member

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

0git range-diff bitcoin-core/master fabd801fc0 fa9112aac0

Sjors commented at 10:23 am on March 16, 2022: member

re-ACK fa9112aac07dc371bfda437d40eb1b841f36f392

laanwj commented at 1:33 pm on April 5, 2022: member

Code review ACK fa9112aac07dc371bfda437d40eb1b841f36f392

laanwj merged this on Apr 5, 2022

laanwj closed this on Apr 5, 2022

sidhujag referenced this in commit a49b3fcdcd on Apr 5, 2022

bitcoin locked this on Apr 5, 2023

MarcoFalke deleted the branch on Aug 22, 2023

Remove utxo db upgrade code #24236

Conflicts