From 3f8ef564463ea2da3b7afd056af6920d4ec51fe0 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Wed, 28 Jun 2023 16:48:47 +1000 Subject: [PATCH] Release 23.4.4 (#5651) * Fail fast in case of downgrade done without reverting variables storage (#5637) Signed-off-by: Fabio Di Fabio * 23.4.4 release Signed-off-by: Simon Dudley --------- Signed-off-by: Fabio Di Fabio Signed-off-by: Simon Dudley Co-authored-by: Fabio Di Fabio --- ...ueStoragePrefixedKeyBlockchainStorage.java | 94 +++++++++++++++---- ...oragePrefixedKeyBlockchainStorageTest.java | 16 ++++ gradle.properties | 2 +- 3 files changed, 94 insertions(+), 18 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java index e5c1f11889..9134ca91c0 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java @@ -140,47 +140,96 @@ public class KeyValueStoragePrefixedKeyBlockchainStorage implements BlockchainSt return blockchainStorage.get(Bytes.concatenate(prefix, key).toArrayUnsafe()).map(Bytes::wrap); } - public void migrateVariables() { + /** + * One time migration of variables from the blockchain storage to the dedicated variable storage. + * To avoid state inconsistency in case of a downgrade done without running the storage + * revert-variables subcommand it fails giving the possibility to retry the downgrade procedure. + */ + private void migrateVariables() { final var blockchainUpdater = updater(); final var variablesUpdater = variablesStorage.updater(); get(VARIABLES_PREFIX, CHAIN_HEAD_HASH.getBytes()) .map(this::bytesToHash) .ifPresent( - ch -> { - variablesUpdater.setChainHead(ch); - LOG.info("Migrated key {} to variables storage", CHAIN_HEAD_HASH); - }); + bch -> + variablesStorage + .getChainHead() + .ifPresentOrElse( + vch -> { + if (!vch.equals(bch)) { + logInconsistencyAndFail(CHAIN_HEAD_HASH, bch, vch); + } + }, + () -> { + variablesUpdater.setChainHead(bch); + LOG.info("Migrated key {} to variables storage", CHAIN_HEAD_HASH); + })); get(VARIABLES_PREFIX, FINALIZED_BLOCK_HASH.getBytes()) .map(this::bytesToHash) .ifPresent( - fh -> { - variablesUpdater.setFinalized(fh); - LOG.info("Migrated key {} to variables storage", FINALIZED_BLOCK_HASH); + bfh -> { + variablesStorage + .getFinalized() + .ifPresentOrElse( + vfh -> { + if (!vfh.equals(bfh)) { + logInconsistencyAndFail(FINALIZED_BLOCK_HASH, bfh, vfh); + } + }, + () -> { + variablesUpdater.setFinalized(bfh); + LOG.info("Migrated key {} to variables storage", FINALIZED_BLOCK_HASH); + }); }); get(VARIABLES_PREFIX, SAFE_BLOCK_HASH.getBytes()) .map(this::bytesToHash) .ifPresent( - sh -> { - variablesUpdater.setSafeBlock(sh); - LOG.info("Migrated key {} to variables storage", SAFE_BLOCK_HASH); + bsh -> { + variablesStorage + .getSafeBlock() + .ifPresentOrElse( + vsh -> { + if (!vsh.equals(bsh)) { + logInconsistencyAndFail(SAFE_BLOCK_HASH, bsh, vsh); + } + }, + () -> { + variablesUpdater.setSafeBlock(bsh); + LOG.info("Migrated key {} to variables storage", SAFE_BLOCK_HASH); + }); }); get(VARIABLES_PREFIX, FORK_HEADS.getBytes()) .map(bytes -> RLP.input(bytes).readList(in -> this.bytesToHash(in.readBytes32()))) .ifPresent( - fh -> { - variablesUpdater.setForkHeads(fh); - LOG.info("Migrated key {} to variables storage", FORK_HEADS); + bfh -> { + final var vfh = variablesStorage.getForkHeads(); + if (vfh.isEmpty()) { + variablesUpdater.setForkHeads(bfh); + LOG.info("Migrated key {} to variables storage", FORK_HEADS); + } else if (!List.copyOf(vfh).equals(bfh)) { + logInconsistencyAndFail(FORK_HEADS, bfh, vfh); + } }); get(Bytes.EMPTY, SEQ_NO_STORE.getBytes()) .ifPresent( - sns -> { - variablesUpdater.setLocalEnrSeqno(sns); - LOG.info("Migrated key {} to variables storage", SEQ_NO_STORE); + bsns -> { + variablesStorage + .getLocalEnrSeqno() + .ifPresentOrElse( + vsns -> { + if (!vsns.equals(bsns)) { + logInconsistencyAndFail(SEQ_NO_STORE, bsns, vsns); + } + }, + () -> { + variablesUpdater.setLocalEnrSeqno(bsns); + LOG.info("Migrated key {} to variables storage", SEQ_NO_STORE); + }); }); blockchainUpdater.removeVariables(); @@ -189,6 +238,17 @@ public class KeyValueStoragePrefixedKeyBlockchainStorage implements BlockchainSt blockchainUpdater.commit(); } + private static void logInconsistencyAndFail( + final VariablesStorage.Keys key, final Object bch, final Object vch) { + LOG.error( + "Inconsistency found when migrating {} to variables storage," + + " probably this is due to a downgrade done without running the `storage revert-variables`" + + " subcommand first, see https://github.com/hyperledger/besu/pull/5471", + key); + throw new IllegalStateException( + key + " mismatch: blockchain storage value=" + bch + ", variables storage value=" + vch); + } + public static class Updater implements BlockchainStorage.Updater { private final KeyValueStorageTransaction blockchainTransaction; diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorageTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorageTest.java index cd072780cf..1e89581a09 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorageTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorageTest.java @@ -14,14 +14,17 @@ */ package org.hyperledger.besu.ethereum.storage.keyvalue; +import static org.hyperledger.besu.ethereum.chain.VariablesStorage.Keys.CHAIN_HEAD_HASH; import static org.hyperledger.besu.ethereum.chain.VariablesStorage.Keys.FINALIZED_BLOCK_HASH; import static org.hyperledger.besu.ethereum.chain.VariablesStorage.Keys.SAFE_BLOCK_HASH; +import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.SAMPLE_CHAIN_HEAD; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.assertNoVariablesInStorage; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.assertVariablesPresentInVariablesStorage; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.assertVariablesReturnedByBlockchainStorage; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.getSampleVariableValues; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.populateBlockchainStorage; import static org.hyperledger.besu.ethereum.core.VariablesStorageHelper.populateVariablesStorage; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import org.hyperledger.besu.ethereum.chain.VariablesStorage; @@ -100,4 +103,17 @@ public class KeyValueStoragePrefixedKeyBlockchainStorageTest { assertVariablesReturnedByBlockchainStorage(blockchainStorage, variableValues); } + + @Test + public void failIfInconsistencyDetectedDuringVariablesMigration() { + populateBlockchainStorage(kvBlockchain, variableValues); + // create and inconsistency putting a different chain head in variables storage + variableValues.put(CHAIN_HEAD_HASH, SAMPLE_CHAIN_HEAD.shiftLeft(1)); + populateVariablesStorage(kvVariables, variableValues); + assertThrows( + IllegalStateException.class, + () -> + new KeyValueStoragePrefixedKeyBlockchainStorage( + kvBlockchain, variablesStorage, blockHeaderFunctions)); + } } diff --git a/gradle.properties b/gradle.properties index 1ae6296416..9a85a6a064 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=23.4.4-SNAPSHOT +version=23.4.4 org.gradle.welcome=never # Set exports/opens flags required by Google Java Format and ErrorProne plugins. (JEP-396)