From aa264b3c0c7a05c93ba25612447aac085b78be61 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta <45264458+abdelhamidbakhta@users.noreply.github.com> Date: Tue, 24 Sep 2019 18:38:36 +0200 Subject: [PATCH 01/10] Store db metadata file in the root data directory. (#46) * Update version to 1.2.5-SNAPSHOT (#42) Signed-off-by: Edward Evans Signed-off-by: Abdelhamid Bakhta * Store db metadata file in the root data directory The database metadata file should be stored in the root data directory rather than the database subdirectory. The database subdirectory is owned by the database itself and should not be directly manipulated by the node. - first look in the data directory for the metadata file - if the metadata file is found there, process it as normal - if no metadata file is found in the root directory, look in the database subdirectory - if the file is found here, copy it to the root directory, and run based on the root directory version Signed-off-by: Abdelhamid Bakhta * add logs Signed-off-by: Abdelhamid Bakhta * create database directory if database not detected Signed-off-by: Abdelhamid Bakhta * change plugin API know hash Signed-off-by: Abdelhamid Bakhta --- .../dsl/node/ThreadBesuNodeRunner.java | 4 +- .../org/hyperledger/besu/cli/BesuCommand.java | 4 +- .../besu/services/BesuConfigurationImpl.java | 11 +++ build.gradle | 4 +- plugin-api/build.gradle | 2 +- .../plugin/services/BesuConfiguration.java | 7 ++ .../RocksDBKeyValueStorageFactory.java | 11 +-- .../configuration/DatabaseMetadata.java | 35 ++++++-- .../plugin/services/helper/Conditions.java | 41 +++++++++ .../RocksDBKeyValueStorageFactoryTest.java | 40 +++++++-- .../configuration/DatabaseMetadataTest.java | 87 +++++++++++++++++++ 11 files changed, 221 insertions(+), 25 deletions(-) create mode 100644 plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/helper/Conditions.java create mode 100644 plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index 3655c1f0b24..18ba82c02dd 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -106,8 +106,8 @@ public void startNode(final BesuNode node) { } final StorageServiceImpl storageService = new StorageServiceImpl(); - final BesuConfiguration commonPluginConfiguration = - new BesuConfigurationImpl(Files.createTempDir().toPath()); + final Path path = Files.createTempDir().toPath(); + final BesuConfiguration commonPluginConfiguration = new BesuConfigurationImpl(path, path); final BesuPluginContextImpl besuPluginContext = besuPluginContextMap.computeIfAbsent( node, n -> buildPluginContext(node, storageService, commonPluginConfiguration)); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index f0a88d7c731..cf228fa9253 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -771,7 +771,9 @@ public void run() { private void addConfigurationService() { if (pluginCommonConfiguration == null) { - pluginCommonConfiguration = new BesuConfigurationImpl(dataDir().resolve(DATABASE_PATH)); + final Path dataDir = dataDir(); + pluginCommonConfiguration = + new BesuConfigurationImpl(dataDir.resolve(DATABASE_PATH), dataDir); besuPluginContext.addService(BesuConfiguration.class, pluginCommonConfiguration); } } diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java index aa59167034f..d64ff96d951 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java @@ -21,13 +21,24 @@ public class BesuConfigurationImpl implements BesuConfiguration { private final Path storagePath; + private final Path dataPath; public BesuConfigurationImpl(final Path storagePath) { + this(storagePath, storagePath); + } + + public BesuConfigurationImpl(final Path storagePath, final Path dataPath) { this.storagePath = storagePath; + this.dataPath = dataPath; } @Override public Path getStoragePath() { return storagePath; } + + @Override + public Path getDataPath() { + return dataPath; + } } diff --git a/build.gradle b/build.gradle index c9893b6ab4a..56478c9d47d 100644 --- a/build.gradle +++ b/build.gradle @@ -164,9 +164,7 @@ allprojects { } // Below this line are currently only license header tasks - format 'groovy', { - target '**/src/*/grovy/**/*.groovy' - } + format 'groovy', { target '**/src/*/grovy/**/*.groovy' } // Currently disabled due to referencetest issues // format 'bash', { diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index bc2c4602273..ea380a93f9e 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -56,7 +56,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = '7GpOlkPAtanchkveLlCKOTmafqC8cAa19/s/eLLGFyE=' + knownHash = 'f0SPUj4/aLZKyjAGcDYai021dO8pg5xLaNvALEWxoIg=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BesuConfiguration.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BesuConfiguration.java index 29599dd83dd..646841c514c 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BesuConfiguration.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BesuConfiguration.java @@ -25,4 +25,11 @@ public interface BesuConfiguration { * @return location of the storage in the file system of the client. */ Path getStoragePath(); + + /** + * Location of the data directory in the file system running the client. + * + * @return location of the data directory in the file system of the client. + */ + Path getDataPath(); } diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java index cb465b19f94..4453a0d125a 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java @@ -147,17 +147,18 @@ private boolean requiresInit() { private int readDatabaseVersion(final BesuConfiguration commonConfiguration) throws IOException { final Path databaseDir = commonConfiguration.getStoragePath(); + final Path dataDir = commonConfiguration.getDataPath(); final boolean databaseExists = databaseDir.resolve("IDENTITY").toFile().exists(); final int databaseVersion; if (databaseExists) { - databaseVersion = DatabaseMetadata.fromDirectory(databaseDir).getVersion(); - LOG.debug("Existing database detected at {}. Version {}", databaseDir, databaseVersion); + databaseVersion = DatabaseMetadata.lookUpFrom(databaseDir, dataDir).getVersion(); + LOG.info("Existing database detected at {}. Version {}", dataDir, databaseVersion); } else { databaseVersion = DEFAULT_VERSION; - LOG.info( - "No existing database detected at {}. Using version {}", databaseDir, databaseVersion); + LOG.info("No existing database detected at {}. Using version {}", dataDir, databaseVersion); Files.createDirectories(databaseDir); - new DatabaseMetadata(databaseVersion).writeToDirectory(databaseDir); + Files.createDirectories(dataDir); + new DatabaseMetadata(databaseVersion).writeToDirectory(dataDir); } if (!SUPPORTED_VERSIONS.contains(databaseVersion)) { diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java index 43777475757..4eab6e42be3 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java @@ -23,8 +23,12 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class DatabaseMetadata { + private static final Logger LOG = LogManager.getLogger(); + private static final String METADATA_FILENAME = "DATABASE_METADATA.json"; private static ObjectMapper MAPPER = new ObjectMapper(); private final int version; @@ -38,20 +42,41 @@ public int getVersion() { return version; } - public static DatabaseMetadata fromDirectory(final Path databaseDir) throws IOException { - final File metadataFile = getDefaultMetadataFile(databaseDir); + public static DatabaseMetadata lookUpFrom(final Path databaseDir, final Path dataDir) + throws IOException { + LOG.info("Lookup database metadata file in data directory: {}", dataDir.toString()); + File metadataFile = getDefaultMetadataFile(dataDir); + final boolean shouldLookupInDatabaseDir = !metadataFile.exists(); + if (shouldLookupInDatabaseDir) { + LOG.info( + "Database metadata file not found in data directory. Lookup in database directory: {}", + databaseDir.toString()); + metadataFile = getDefaultMetadataFile(databaseDir); + } + DatabaseMetadata databaseMetadata; try { - return MAPPER.readValue(metadataFile, DatabaseMetadata.class); + databaseMetadata = MAPPER.readValue(metadataFile, DatabaseMetadata.class); } catch (FileNotFoundException fnfe) { - return new DatabaseMetadata(0); + databaseMetadata = new DatabaseMetadata(0); } catch (JsonProcessingException jpe) { throw new IllegalStateException( String.format("Invalid metadata file %s", metadataFile.getAbsolutePath()), jpe); } + if (shouldLookupInDatabaseDir) { + LOG.warn( + "Database metadata file has been copied from old location (database directory). Be aware that the old file might be removed in future release."); + writeToDirectory(databaseMetadata, dataDir); + } + return databaseMetadata; } public void writeToDirectory(final Path databaseDir) throws IOException { - MAPPER.writeValue(getDefaultMetadataFile(databaseDir), this); + writeToDirectory(this, databaseDir); + } + + private static void writeToDirectory( + final DatabaseMetadata databaseMetadata, final Path databaseDir) throws IOException { + MAPPER.writeValue(getDefaultMetadataFile(databaseDir), databaseMetadata); } private static File getDefaultMetadataFile(final Path databaseDir) { diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/helper/Conditions.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/helper/Conditions.java new file mode 100644 index 00000000000..085e9313536 --- /dev/null +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/helper/Conditions.java @@ -0,0 +1,41 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.hyperledger.besu.plugin.services.helper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.assertj.core.api.Condition; + +public class Conditions { + public static final Condition FILE_EXISTS = + new Condition<>(path -> path != null && path.toFile().exists(), "File must exist."); + public static final Condition FILE_DOES_NOT_EXIST = + new Condition<>(path -> path == null || !path.toFile().exists(), "File must not exist."); + + public static Condition shouldContain(final String content) { + return new Condition<>( + path -> { + try { + return content.equals(Files.readString(path)); + } catch (IOException e) { + return false; + } + }, + "File should contain specified content."); + } +} diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java index 8e0581e019c..20780ce79ae 100644 --- a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java @@ -54,8 +54,10 @@ public class RocksDBKeyValueStorageFactoryTest { @Test public void shouldCreateCorrectMetadataFileForLatestVersion() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); final RocksDBKeyValueStorageFactory storageFactory = new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments); @@ -63,47 +65,59 @@ public void shouldCreateCorrectMetadataFileForLatestVersion() throws Exception { // Side effect is creation of the Metadata version file storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat(DatabaseMetadata.fromDirectory(commonConfiguration.getStoragePath()).getVersion()) + assertThat( + DatabaseMetadata.lookUpFrom( + commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) + .getVersion()) .isEqualTo(DEFAULT_VERSION); } @Test public void shouldDetectVersion0DatabaseIfNoMetadataFileFound() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); + Files.createDirectories(tempDataDir); tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); final RocksDBKeyValueStorageFactory storageFactory = new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments); storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat(DatabaseMetadata.fromDirectory(tempDatabaseDir).getVersion()).isZero(); + assertThat(DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir).getVersion()).isZero(); } @Test public void shouldDetectCorrectVersionIfMetadataFileExists() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); - tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); - new DatabaseMetadata(DEFAULT_VERSION).writeToDirectory(tempDatabaseDir); + Files.createDirectories(tempDataDir); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); + final RocksDBKeyValueStorageFactory storageFactory = new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments); storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat(DatabaseMetadata.fromDirectory(tempDatabaseDir).getVersion()) + assertThat(DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir).getVersion()) .isEqualTo(DEFAULT_VERSION); assertThat(storageFactory.isSegmentIsolationSupported()).isTrue(); } @Test public void shouldDetectCorrectVersionInCaseOfRollback() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); + Files.createDirectories(tempDataDir); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); + final RocksDBKeyValueStorageFactory storageFactory = new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments, 1); @@ -117,12 +131,14 @@ public void shouldDetectCorrectVersionInCaseOfRollback() throws Exception { @Test public void shouldThrowExceptionWhenVersionNumberIsInvalid() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); + Files.createDirectories(tempDataDir); tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); - new DatabaseMetadata(-1).writeToDirectory(tempDatabaseDir); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); - + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); + new DatabaseMetadata(-1).writeToDirectory(tempDatabaseDir); assertThatThrownBy( () -> new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments) @@ -132,9 +148,13 @@ public void shouldThrowExceptionWhenVersionNumberIsInvalid() throws Exception { @Test public void shouldSetSegmentationFieldDuringCreation() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); + Files.createDirectories(tempDataDir); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); + final RocksDBKeyValueStorageFactory storageFactory = new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments); storageFactory.create(segment, commonConfiguration, metricsSystem); @@ -143,10 +163,14 @@ public void shouldSetSegmentationFieldDuringCreation() throws Exception { @Test public void shouldThrowExceptionWhenMetaDataFileIsCorrupted() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); - when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + Files.createDirectories(tempDataDir); tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); + when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); + when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); + final String badVersion = "{\"🦄\":1}"; Files.write( tempDatabaseDir.resolve(METADATA_FILENAME), badVersion.getBytes(Charset.defaultCharset())); diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java new file mode 100644 index 00000000000..1e90c890312 --- /dev/null +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java @@ -0,0 +1,87 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.hyperledger.besu.plugin.services.storage.rocksdb.configuration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.plugin.services.helper.Conditions.FILE_DOES_NOT_EXIST; +import static org.hyperledger.besu.plugin.services.helper.Conditions.FILE_EXISTS; +import static org.hyperledger.besu.plugin.services.helper.Conditions.shouldContain; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class DatabaseMetadataTest { + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void getVersion() { + final DatabaseMetadata databaseMetadata = new DatabaseMetadata(42); + assertThat(databaseMetadata).isNotNull(); + assertThat(databaseMetadata.getVersion()).isEqualTo(42); + } + + @Test + public void metaFileShouldBeSoughtIntoDataDirFirst() throws Exception { + final Path tempDataDir = createAndWrite("data", "DATABASE_METADATA.json", "{\"version\":42}"); + final Path tempDatabaseDir = createAndWrite("db", "DATABASE_METADATA.json", "{\"version\":99}"); + final DatabaseMetadata databaseMetadata = + DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir); + assertThat(databaseMetadata).isNotNull(); + assertThat(databaseMetadata.getVersion()).isEqualTo(42); + } + + @Test + public void metaFileNotFoundInDataDirShouldLookupIntoDbDir() throws Exception { + final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); + Files.createDirectories(tempDataDir); + final Path tempDatabaseDir = createAndWrite("db", "DATABASE_METADATA.json", "{\"version\":42}"); + final Path metadataPathInDataDir = tempDataDir.resolve("DATABASE_METADATA.json"); + assertThat(metadataPathInDataDir).is(FILE_DOES_NOT_EXIST); + final DatabaseMetadata databaseMetadata = + DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir); + assertThat(databaseMetadata).isNotNull(); + assertThat(databaseMetadata.getVersion()).isEqualTo(42); + assertThat(metadataPathInDataDir).is(FILE_EXISTS); + assertThat(metadataPathInDataDir).is(shouldContain("{\"version\":42}")); + } + + private Path createAndWrite(final String dir, final String file, final String content) + throws IOException { + return createAndWrite(temporaryFolder, dir, file, content); + } + + private Path createAndWrite( + final TemporaryFolder temporaryFolder, + final String dir, + final String file, + final String content) + throws IOException { + final Path tmpDir = temporaryFolder.newFolder().toPath().resolve(dir); + Files.createDirectories(tmpDir); + createAndWrite(tmpDir.resolve(file), content); + return tmpDir; + } + + private void createAndWrite(final Path path, final String content) throws IOException { + path.toFile().createNewFile(); + Files.writeString(path, content); + } +} From 97a5c61f255c489890a0145472ea2b344bd8c11e Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Wed, 25 Sep 2019 15:08:37 +1200 Subject: [PATCH 02/10] PAN-3175: Private tx nonce errors return same msg as any tx (#48) Signed-off-by: Lucas Saldanha --- .../api/graphql/internal/response/GraphQLError.java | 6 ++---- .../besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java | 6 ++---- .../api/jsonrpc/internal/response/JsonRpcError.java | 2 -- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/response/GraphQLError.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/response/GraphQLError.java index fb8b90b05b4..81f2be76864 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/response/GraphQLError.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/response/GraphQLError.java @@ -73,8 +73,10 @@ public static GraphQLError of(final TransactionInvalidReason transactionInvalidR case UPFRONT_COST_EXCEEDS_BALANCE: return TRANSACTION_UPFRONT_COST_EXCEEDS_BALANCE; case NONCE_TOO_LOW: + case PRIVATE_NONCE_TOO_LOW: return NONCE_TOO_LOW; case INCORRECT_NONCE: + case INCORRECT_PRIVATE_NONCE: return INCORRECT_NONCE; case INTRINSIC_GAS_EXCEEDS_GAS_LIMIT: return INTRINSIC_GAS_EXCEEDS_LIMIT; @@ -87,10 +89,6 @@ public static GraphQLError of(final TransactionInvalidReason transactionInvalidR // Private Transaction Invalid Reasons case PRIVATE_TRANSACTION_FAILED: return PRIVATE_TRANSACTION_FAILED; - case PRIVATE_NONCE_TOO_LOW: - return PRIVATE_NONCE_TOO_LOW; - case INCORRECT_PRIVATE_NONCE: - return INCORRECT_PRIVATE_NONCE; case GAS_PRICE_TOO_LOW: return GAS_PRICE_TOO_LOW; default: diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java index 1194970f9e5..af33eb7c5d5 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java @@ -23,8 +23,10 @@ public static JsonRpcError convertTransactionInvalidReason( final TransactionInvalidReason reason) { switch (reason) { case NONCE_TOO_LOW: + case PRIVATE_NONCE_TOO_LOW: return JsonRpcError.NONCE_TOO_LOW; case INCORRECT_NONCE: + case INCORRECT_PRIVATE_NONCE: return JsonRpcError.INCORRECT_NONCE; case INVALID_SIGNATURE: return JsonRpcError.INVALID_TRANSACTION_SIGNATURE; @@ -39,10 +41,6 @@ public static JsonRpcError convertTransactionInvalidReason( // Private Transaction Invalid Reasons case CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE: return JsonRpcError.CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE; - case PRIVATE_NONCE_TOO_LOW: - return JsonRpcError.PRIVATE_NONCE_TOO_LOW; - case INCORRECT_PRIVATE_NONCE: - return JsonRpcError.INCORRECT_PRIVATE_NONCE; case GAS_PRICE_TOO_LOW: return JsonRpcError.GAS_PRICE_TOO_LOW; diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java index adbfb733cd6..76aa505590c 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java @@ -106,8 +106,6 @@ public enum JsonRpcError { // Private transaction errors ENCLAVE_ERROR(-50100, "Error communicating with enclave"), - PRIVATE_NONCE_TOO_LOW(-50100, "Private transaction nonce too low"), - INCORRECT_PRIVATE_NONCE(-50100, "Private transaction nonce is incorrect"), UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE(-50100, "Unimplemented private transaction type"), PRIVACY_NOT_ENABLED(-50100, "Privacy is not enabled to get the precompiled address"), CREATE_PRIVACY_GROUP_ERROR(-50100, "Error creating privacy group"), From 8acd5d64e1deff7d6379cbf2a530fee9edd997a6 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta <45264458+abdelhamidbakhta@users.noreply.github.com> Date: Wed, 25 Sep 2019 09:04:51 +0200 Subject: [PATCH 03/10] Introduce virtual operation. (#45) * Update version to 1.2.5-SNAPSHOT (#42) Signed-off-by: Edward Evans Signed-off-by: Abdelhamid Bakhta * introduce `VirtualOperation` - add `isVirtualOperation` method to `Operation` interface: determines whether or not the operation has been virtually added to the contract code. - default implementation of `isVirtualOperation` returns `false` - create `VirtualOperation` that wraps a real `Operation` and returns `true` for `isVirtualOperation` method. - change `constructor` of `EVM` to pass a `GasCalculator` used to create the `endOfScriptStop` and `invalidOperation` fields. - change visibility of `EVM.operationAtOffset` to `package` for testing purpose. (add `VisibleForTesting` annotation) - create `EVMTest` to test `operationAtOffset` method . Signed-off-by: Abdelhamid Bakhta --- .../mainnet/MainnetEvmRegistries.java | 10 +-- .../org/hyperledger/besu/ethereum/vm/EVM.java | 17 +++-- .../besu/ethereum/vm/Operation.java | 11 +++ .../vm/operations/VirtualOperation.java | 74 +++++++++++++++++++ .../hyperledger/besu/ethereum/vm/EVMTest.java | 65 ++++++++++++++++ 5 files changed, 166 insertions(+), 11 deletions(-) create mode 100644 ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/operations/VirtualOperation.java create mode 100644 ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/EVMTest.java diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetEvmRegistries.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetEvmRegistries.java index 9214a22aa77..6ff4d657873 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetEvmRegistries.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetEvmRegistries.java @@ -108,7 +108,7 @@ static EVM frontier(final GasCalculator gasCalculator) { registerFrontierOpcodes(registry, gasCalculator, Account.DEFAULT_VERSION); - return new EVM(registry, new InvalidOperation(gasCalculator)); + return new EVM(registry, gasCalculator); } static EVM homestead(final GasCalculator gasCalculator) { @@ -116,7 +116,7 @@ static EVM homestead(final GasCalculator gasCalculator) { registerHomesteadOpcodes(registry, gasCalculator, Account.DEFAULT_VERSION); - return new EVM(registry, new InvalidOperation(gasCalculator)); + return new EVM(registry, gasCalculator); } static EVM byzantium(final GasCalculator gasCalculator) { @@ -124,7 +124,7 @@ static EVM byzantium(final GasCalculator gasCalculator) { registerByzantiumOpcodes(registry, gasCalculator, Account.DEFAULT_VERSION); - return new EVM(registry, new InvalidOperation(gasCalculator)); + return new EVM(registry, gasCalculator); } static EVM constantinople(final GasCalculator gasCalculator) { @@ -132,7 +132,7 @@ static EVM constantinople(final GasCalculator gasCalculator) { registerConstantinopleOpcodes(registry, gasCalculator, Account.DEFAULT_VERSION); - return new EVM(registry, new InvalidOperation(gasCalculator)); + return new EVM(registry, gasCalculator); } static EVM istanbul(final GasCalculator gasCalculator, final BigInteger chainId) { @@ -140,7 +140,7 @@ static EVM istanbul(final GasCalculator gasCalculator, final BigInteger chainId) registerIstanbulOpcodes(registry, gasCalculator, Account.DEFAULT_VERSION, chainId); - return new EVM(registry, new InvalidOperation(gasCalculator)); + return new EVM(registry, gasCalculator); } private static void registerFrontierOpcodes( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/EVM.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/EVM.java index 5416a2bf5f4..95d86a8b78c 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/EVM.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/EVM.java @@ -20,24 +20,29 @@ import org.hyperledger.besu.ethereum.vm.MessageFrame.State; import org.hyperledger.besu.ethereum.vm.ehalt.ExceptionalHaltException; import org.hyperledger.besu.ethereum.vm.ehalt.ExceptionalHaltManager; +import org.hyperledger.besu.ethereum.vm.operations.InvalidOperation; +import org.hyperledger.besu.ethereum.vm.operations.StopOperation; +import org.hyperledger.besu.ethereum.vm.operations.VirtualOperation; import org.hyperledger.besu.util.bytes.BytesValue; import java.util.EnumSet; import java.util.Optional; import java.util.function.BiConsumer; +import com.google.common.annotations.VisibleForTesting; import org.apache.logging.log4j.Logger; public class EVM { private static final Logger LOG = getLogger(); - private static final int STOP_OPCODE = 0x00; private final OperationRegistry operations; private final Operation invalidOperation; + private final Operation endOfScriptStop; - public EVM(final OperationRegistry operations, final Operation invalidOperation) { + public EVM(final OperationRegistry operations, final GasCalculator gasCalculator) { this.operations = operations; - this.invalidOperation = invalidOperation; + this.invalidOperation = new InvalidOperation(gasCalculator); + this.endOfScriptStop = new VirtualOperation(new StopOperation(gasCalculator)); } public void runToHalt(final MessageFrame frame, final OperationTracer operationTracer) @@ -142,12 +147,12 @@ private static void logState(final MessageFrame frame, final Optional curre } } - private Operation operationAtOffset( - final Code code, final int contractAccountVersion, final int offset) { + @VisibleForTesting + Operation operationAtOffset(final Code code, final int contractAccountVersion, final int offset) { final BytesValue bytecode = code.getBytes(); // If the length of the program code is shorter than the required offset, halt execution. if (offset >= bytecode.size()) { - return operations.get(STOP_OPCODE, contractAccountVersion); + return endOfScriptStop; } return operations.getOrDefault(bytecode.get(offset), contractAccountVersion, invalidOperation); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Operation.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Operation.java index 0259fd5ec42..2ddad8662db 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Operation.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Operation.java @@ -68,4 +68,15 @@ default int getStackSizeChange() { boolean getUpdatesProgramCounter(); int getOpSize(); + + /** + * Determines whether or not this operation has been virtually added to the contract code. For + * instance if the contract is not ended by a STOP opcode the {@link EVM} adds an explicit end of + * script stop which can be considered as virtual. + * + * @return a boolean indicating if the operation is virtual. + */ + default boolean isVirtualOperation() { + return false; + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/operations/VirtualOperation.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/operations/VirtualOperation.java new file mode 100644 index 00000000000..728d97752ef --- /dev/null +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/operations/VirtualOperation.java @@ -0,0 +1,74 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.hyperledger.besu.ethereum.vm.operations; + +import org.hyperledger.besu.ethereum.core.Gas; +import org.hyperledger.besu.ethereum.vm.MessageFrame; +import org.hyperledger.besu.ethereum.vm.Operation; + +public class VirtualOperation implements Operation { + + private final Operation delegate; + + public VirtualOperation(final Operation delegate) { + this.delegate = delegate; + } + + @Override + public Gas cost(final MessageFrame frame) { + return delegate.cost(frame); + } + + @Override + public void execute(final MessageFrame frame) { + delegate.execute(frame); + } + + @Override + public int getOpcode() { + return delegate.getOpcode(); + } + + @Override + public String getName() { + return delegate.getName(); + } + + @Override + public int getStackItemsConsumed() { + return delegate.getStackItemsConsumed(); + } + + @Override + public int getStackItemsProduced() { + return delegate.getStackItemsProduced(); + } + + @Override + public boolean getUpdatesProgramCounter() { + return delegate.getUpdatesProgramCounter(); + } + + @Override + public int getOpSize() { + return delegate.getOpSize(); + } + + @Override + public boolean isVirtualOperation() { + return true; + } +} diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/EVMTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/EVMTest.java new file mode 100644 index 00000000000..fa93804fcb2 --- /dev/null +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/EVMTest.java @@ -0,0 +1,65 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.vm; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyByte; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.ethereum.vm.operations.StopOperation; +import org.hyperledger.besu.util.bytes.BytesValue; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class EVMTest { + + private static final int CONTRACT_ACCOUNT_VERSION = 1; + @Mock private OperationRegistry operationRegistry; + @Mock private GasCalculator gasCalculator; + private EVM evm; + + @Before + public void setup() { + evm = new EVM(operationRegistry, gasCalculator); + } + + @Test + public void assertThatEndOfScriptNotExplicitlySetInCodeReturnsAVirtualOperation() { + final Code code = new Code(BytesValue.fromHexString("0x60203560003555606035604035556000")); + final Operation operation = + evm.operationAtOffset(code, CONTRACT_ACCOUNT_VERSION, code.getSize()); + assertThat(operation).isNotNull(); + assertThat(operation.isVirtualOperation()).isTrue(); + } + + @Test + public void assertThatEndOfScriptExplicitlySetInCodeDoesNotReturnAVirtualOperation() { + final Code code = new Code(BytesValue.fromHexString("0x6020356000355560603560403555600000")); + when(operationRegistry.getOrDefault( + anyByte(), eq(CONTRACT_ACCOUNT_VERSION), any(Operation.class))) + .thenReturn(new StopOperation(gasCalculator)); + final Operation operation = + evm.operationAtOffset(code, CONTRACT_ACCOUNT_VERSION, code.getSize() - 1); + assertThat(operation).isNotNull(); + assertThat(operation.isVirtualOperation()).isFalse(); + } +} From 231cd71c7bdd07fec6de00f3aecd9dfda795a478 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta <45264458+abdelhamidbakhta@users.noreply.github.com> Date: Wed, 25 Sep 2019 10:05:20 +0200 Subject: [PATCH 04/10] Disallow comments in Genesis JSON file. (#49) * Disallow comments in Genesis JSON file Signed-off-by: Abdelhamid Bakhta --- .../besu/config/GenesisConfigFile.java | 19 +------------------ .../besu/config/GenesisConfigFileTest.java | 16 ++++++++-------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java index 159734d6acb..04abc8f2dd9 100644 --- a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java +++ b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java @@ -23,7 +23,6 @@ import java.util.Optional; import java.util.stream.Stream; -import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.Streams; import com.google.common.io.Resources; @@ -72,23 +71,7 @@ public static GenesisConfigFile development() { } public static GenesisConfigFile fromConfig(final String jsonString) { - try { - final ObjectNode rootNode = JsonUtil.objectNodeFromString(jsonString, false); - return fromConfig(rootNode); - } catch (final RuntimeException re) { - if (re.getCause() instanceof JsonParseException) { - // we had a runtime exception cause by a jsom parse exception. - // try again with comments enabled - final ObjectNode rootNode = JsonUtil.objectNodeFromString(jsonString, true); - // if we get here comments is what broke things, warn and move on. - LOG.warn( - "The provided genesis file contains comments. " - + "In a future release of Besu this will not be supported."); - return fromConfig(rootNode); - } else { - throw re; - } - } + return fromConfig(JsonUtil.objectNodeFromString(jsonString, false)); } public static GenesisConfigFile fromConfig(final ObjectNode config) { diff --git a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigFileTest.java b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigFileTest.java index aa089c3a51d..28fce6b02e1 100644 --- a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigFileTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigFileTest.java @@ -24,6 +24,7 @@ import java.util.function.Function; import java.util.stream.Collectors; +import com.fasterxml.jackson.core.JsonParseException; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.Test; @@ -208,14 +209,13 @@ public void shouldGetLargeChainId() { } @Test - public void acceptComments() { - // this test will change in the future to reject comments. - final GenesisConfigFile config = - GenesisConfigFile.fromConfig( - "{\"config\": { \"chainId\": 2017 }\n/* C comment }*/\n//C++ comment }\n}"); - - assertThat(config.getConfigOptions().getChainId()).contains(new BigInteger("2017")); - // Unfortunately there is no good (non-flakey) way to assert logs. + public void mustNotAcceptComments() { + assertThatThrownBy( + () -> + GenesisConfigFile.fromConfig( + "{\"config\": { \"chainId\": 2017 }\n/* C comment }*/\n//C++ comment }\n}")) + .hasCauseInstanceOf(JsonParseException.class) + .hasMessageContaining("Unexpected character ('/'"); } @Test From 497e825919415e618567d61b6aa0241033c0ebf4 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 25 Sep 2019 09:07:21 -0600 Subject: [PATCH 05/10] Fix default logging (#47) Default logging should be 'null' - which means that Besu will do nothing to the Log4J2 logging levels and rely on the built in or user supplied log4j2.xml configuration files to set up logging. Signed-off-by: Danno Ferrin --- .../src/main/java/org/hyperledger/besu/cli/BesuCommand.java | 6 +++--- .../besu/cli/subcommands/RetestethSubCommand.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index cf228fa9253..00fcfcc9f94 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -538,8 +538,8 @@ void setBannedNodeIds(final List values) { names = {"--logging", "-l"}, paramLabel = "", description = - "Logging verbosity levels: OFF, FATAL, ERROR, WARN, INFO, DEBUG, TRACE, ALL (default: ${DEFAULT-VALUE})") - private final Level logLevel = Level.INFO; + "Logging verbosity levels: OFF, FATAL, ERROR, WARN, INFO, DEBUG, TRACE, ALL (default: INFO)") + private final Level logLevel = null; @Option( names = {"--miner-enabled"}, @@ -932,7 +932,7 @@ protected void validateP2PInterface(final String p2pInterface) { if (!NetworkUtility.isNetworkInterfaceAvailable(p2pInterface)) { throw new ParameterException(commandLine, failMessage); } - } catch (UnknownHostException | SocketException e) { + } catch (final UnknownHostException | SocketException e) { throw new ParameterException(commandLine, failMessage, e); } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/RetestethSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/RetestethSubCommand.java index 5e1b4809ab3..d04b8d21cbf 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/RetestethSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/RetestethSubCommand.java @@ -62,7 +62,7 @@ public class RetestethSubCommand implements Runnable { paramLabel = "", description = "Logging verbosity levels: OFF, FATAL, WARN, INFO, DEBUG, TRACE, ALL (default: INFO)") - private final Level logLevel = Level.INFO; + private final Level logLevel = null; @SuppressWarnings("FieldMayBeFinal") // Because PicoCLI requires Strings to not be final. @Option( From 78ecf332376de20e8f29f88c8293f1c90ab469a1 Mon Sep 17 00:00:00 2001 From: Ratan Rai Sur Date: Wed, 25 Sep 2019 20:57:37 +0300 Subject: [PATCH 06/10] Fix some mark sweep pruner bugs where nodes that should be kept were being swept (Re-merge of #38) (#50) Adds integration tests to catch bug where the mark storage was being cleared before a mark began. Instead, the mark storage is now cleared upon preparing the mark sweep pruner Fixes bug where the pending marks where not being checked while pruning was occurring. By removing the flush in sweepBefore, the existing tests catch the bug. Signed-off-by: Ratan Rai Sur --- .../worldstate/PrunerIntegrationTest.java | 255 ++++++++++++++++++ .../ethereum/worldstate/MarkSweepPruner.java | 45 ++-- .../besu/ethereum/worldstate/Pruner.java | 27 +- .../worldstate/MarkSweepPrunerTest.java | 100 +------ .../besu/ethereum/worldstate/PrunerTest.java | 20 +- .../kvstore/InMemoryKeyValueStorage.java | 12 +- 6 files changed, 324 insertions(+), 135 deletions(-) create mode 100644 ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/worldstate/PrunerIntegrationTest.java diff --git a/ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/worldstate/PrunerIntegrationTest.java b/ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/worldstate/PrunerIntegrationTest.java new file mode 100644 index 00000000000..4008ef9b667 --- /dev/null +++ b/ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/worldstate/PrunerIntegrationTest.java @@ -0,0 +1,255 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.worldstate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain; + +import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.Block; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator.BlockOptions; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.Hash; +import org.hyperledger.besu.ethereum.core.MutableWorldState; +import org.hyperledger.besu.ethereum.core.TransactionReceipt; +import org.hyperledger.besu.ethereum.core.WorldState; +import org.hyperledger.besu.ethereum.rlp.RLP; +import org.hyperledger.besu.ethereum.storage.keyvalue.WorldStateKeyValueStorage; +import org.hyperledger.besu.ethereum.storage.keyvalue.WorldStatePreimageKeyValueStorage; +import org.hyperledger.besu.ethereum.trie.MerklePatriciaTrie; +import org.hyperledger.besu.ethereum.trie.StoredMerklePatriciaTrie; +import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; +import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage; +import org.hyperledger.besu.testutil.MockExecutorService; +import org.hyperledger.besu.util.bytes.Bytes32; +import org.hyperledger.besu.util.bytes.BytesValue; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +import org.junit.Test; + +public class PrunerIntegrationTest { + + private final BlockDataGenerator gen = new BlockDataGenerator(); + private final NoOpMetricsSystem metricsSystem = new NoOpMetricsSystem(); + private final Map hashValueStore = new HashMap<>(); + private final InMemoryKeyValueStorage stateStorage = new TestInMemoryStorage(hashValueStore); + private final WorldStateStorage worldStateStorage = new WorldStateKeyValueStorage(stateStorage); + private final WorldStateArchive worldStateArchive = + new WorldStateArchive( + worldStateStorage, new WorldStatePreimageKeyValueStorage(new InMemoryKeyValueStorage())); + private final InMemoryKeyValueStorage markStorage = new InMemoryKeyValueStorage(); + private final Block genesisBlock = gen.genesisBlock(); + private final MutableBlockchain blockchain = createInMemoryBlockchain(genesisBlock); + + @Test + public void pruner_smallState_manyOpsPerTx() throws InterruptedException { + testPruner(3, 1, 1, 4, 1000); + } + + @Test + public void pruner_largeState_fewOpsPerTx() throws InterruptedException { + testPruner(2, 5, 5, 6, 5); + } + + @Test + public void pruner_emptyBlocks() throws InterruptedException { + testPruner(5, 0, 2, 5, 10); + } + + @Test + public void pruner_markChainhead() throws InterruptedException { + testPruner(4, 2, 1, 10, 20); + } + + @Test + public void pruner_lowRelativeBlockConfirmations() throws InterruptedException { + testPruner(3, 2, 1, 4, 20); + } + + @Test + public void pruner_highRelativeBlockConfirmations() throws InterruptedException { + testPruner(3, 2, 9, 10, 20); + } + + private void testPruner( + final int numCycles, + final int accountsPerBlock, + final long blockConfirmations, + final int numBlocksToKeep, + final int opsPerTransaction) + throws InterruptedException { + + final var markSweepPruner = + new MarkSweepPruner( + worldStateStorage, blockchain, markStorage, metricsSystem, opsPerTransaction); + final var pruner = + new Pruner( + markSweepPruner, + blockchain, + new MockExecutorService(), + new PruningConfiguration(blockConfirmations, numBlocksToKeep)); + + pruner.start(); + + for (int cycle = 0; cycle < numCycles; ++cycle) { + int numBlockInCycle = + numBlocksToKeep + + 1; // +1 to get it to switch from MARKING_COMPLETE TO SWEEPING on each cycle + var fullyMarkedBlockNum = cycle * numBlockInCycle + 1; + + // This should cause a full mark and sweep cycle + assertThat(pruner.getState()).isEqualByComparingTo(Pruner.State.IDLE); + generateBlockchainData(numBlockInCycle, accountsPerBlock); + assertThat(pruner.getState()).isEqualByComparingTo(Pruner.State.IDLE); + + // Collect the nodes we expect to keep + final Set expectedNodes = new HashSet<>(); + for (int i = fullyMarkedBlockNum; i <= blockchain.getChainHeadBlockNumber(); i++) { + final Hash stateRoot = blockchain.getBlockHeader(i).get().getStateRoot(); + collectWorldStateNodes(stateRoot, expectedNodes); + } + + if (accountsPerBlock != 0) { + assertThat(hashValueStore.size()) + .isGreaterThanOrEqualTo(expectedNodes.size()); // Sanity check + } + + // Assert that blocks from mark point onward are still accessible + for (int i = fullyMarkedBlockNum; i <= blockchain.getChainHeadBlockNumber(); i++) { + final Hash stateRoot = blockchain.getBlockHeader(i).get().getStateRoot(); + assertThat(worldStateArchive.get(stateRoot)).isPresent(); + final WorldState markedState = worldStateArchive.get(stateRoot).get(); + // Traverse accounts and make sure all are accessible + final int expectedAccounts = accountsPerBlock * i; + final long accounts = + markedState.streamAccounts(Bytes32.ZERO, expectedAccounts * 2).count(); + assertThat(accounts).isEqualTo(expectedAccounts); + // Traverse storage to ensure that all storage is accessible + markedState + .streamAccounts(Bytes32.ZERO, expectedAccounts * 2) + .forEach(a -> a.storageEntriesFrom(Bytes32.ZERO, 1000)); + } + + // All other state roots should have been removed + for (int i = 0; i < fullyMarkedBlockNum; i++) { + final BlockHeader curHeader = blockchain.getBlockHeader(i).get(); + if (!curHeader.getStateRoot().equals(Hash.EMPTY_TRIE_HASH)) { + assertThat(worldStateArchive.get(curHeader.getStateRoot())).isEmpty(); + } + } + + // Check that storage contains only the values we expect + assertThat(hashValueStore.size()).isEqualTo(expectedNodes.size()); + assertThat(hashValueStore.values()) + .containsExactlyInAnyOrderElementsOf( + expectedNodes.stream().map(BytesValue::getArrayUnsafe).collect(Collectors.toSet())); + } + + pruner.stop(); + } + + private void generateBlockchainData(final int numBlocks, final int numAccounts) { + Block parentBlock = blockchain.getChainHeadBlock(); + for (int i = 0; i < numBlocks; i++) { + final MutableWorldState worldState = + worldStateArchive.getMutable(parentBlock.getHeader().getStateRoot()).get(); + gen.createRandomContractAccountsWithNonEmptyStorage(worldState, numAccounts); + final Hash stateRoot = worldState.rootHash(); + + final Block block = + gen.block( + BlockOptions.create() + .setStateRoot(stateRoot) + .setBlockNumber(parentBlock.getHeader().getNumber() + 1L) + .setParentHash(parentBlock.getHash())); + final List receipts = gen.receipts(block); + blockchain.appendBlock(block, receipts); + parentBlock = block; + } + } + + private Set collectWorldStateNodes( + final Hash stateRootHash, final Set collector) { + final List storageRoots = new ArrayList<>(); + final MerklePatriciaTrie stateTrie = createStateTrie(stateRootHash); + + // Collect storage roots and code + stateTrie + .entriesFrom(Bytes32.ZERO, 1000) + .forEach( + (key, val) -> { + final StateTrieAccountValue accountValue = + StateTrieAccountValue.readFrom(RLP.input(val)); + stateStorage + .get(accountValue.getCodeHash().getArrayUnsafe()) + .ifPresent(v -> collector.add(BytesValue.wrap(v))); + storageRoots.add(accountValue.getStorageRoot()); + }); + + // Collect state nodes + collectTrieNodes(stateTrie, collector); + // Collect storage nodes + for (Hash storageRoot : storageRoots) { + final MerklePatriciaTrie storageTrie = createStorageTrie(storageRoot); + collectTrieNodes(storageTrie, collector); + } + + return collector; + } + + private void collectTrieNodes( + final MerklePatriciaTrie trie, final Set collector) { + final Bytes32 rootHash = trie.getRootHash(); + trie.visitAll( + (node) -> { + if (node.isReferencedByHash() || node.getHash().equals(rootHash)) { + collector.add(node.getRlp()); + } + }); + } + + private MerklePatriciaTrie createStateTrie(final Bytes32 rootHash) { + return new StoredMerklePatriciaTrie<>( + worldStateStorage::getAccountStateTrieNode, + rootHash, + Function.identity(), + Function.identity()); + } + + private MerklePatriciaTrie createStorageTrie(final Bytes32 rootHash) { + return new StoredMerklePatriciaTrie<>( + worldStateStorage::getAccountStorageTrieNode, + rootHash, + Function.identity(), + Function.identity()); + } + + // Proxy class so that we have access to the constructor that takes our own map + private static class TestInMemoryStorage extends InMemoryKeyValueStorage { + + public TestInMemoryStorage(final Map hashValueStore) { + super(hashValueStore); + } + } +} diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPruner.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPruner.java index efa3b38a0eb..951e265de7d 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPruner.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPruner.java @@ -54,7 +54,7 @@ public class MarkSweepPruner { private final Counter sweptNodesCounter; private volatile long nodeAddedListenerId; private final ReentrantLock markLock = new ReentrantLock(true); - private final Set pendingMarks = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final Set pendingMarks = Collections.newSetFromMap(new ConcurrentHashMap<>()); public MarkSweepPruner( final WorldStateStorage worldStateStorage, @@ -98,7 +98,9 @@ public MarkSweepPruner( public void prepare() { worldStateStorage.removeNodeAddedListener(nodeAddedListenerId); // Just in case. - nodeAddedListenerId = worldStateStorage.addNodeAddedListener(this::markNewNodes); + markStorage.clear(); + pendingMarks.clear(); + nodeAddedListenerId = worldStateStorage.addNodeAddedListener(this::markNodes); } public void cleanup() { @@ -107,7 +109,6 @@ public void cleanup() { public void mark(final Hash rootHash) { markOperationCounter.inc(); - markStorage.clear(); createStateTrie(rootHash) .visitAll( node -> { @@ -119,13 +120,12 @@ public void mark(final Hash rootHash) { markNode(node.getHash()); node.getValue().ifPresent(this::processAccountState); }); - LOG.info("Completed marking used nodes for pruning"); + LOG.debug("Completed marking used nodes for pruning"); } public void sweepBefore(final long markedBlockNumber) { - flushPendingMarks(); sweepOperationCounter.inc(); - LOG.info("Sweeping unused nodes"); + LOG.debug("Sweeping unused nodes"); // Sweep state roots first, walking backwards until we get to a state root that isn't in the // storage long prunedNodeCount = 0; @@ -138,7 +138,7 @@ public void sweepBefore(final long markedBlockNumber) { break; } - if (!markStorage.containsKey(candidateStateRootHash.getArrayUnsafe())) { + if (!isMarked(candidateStateRootHash)) { updater.removeAccountStateTrieNode(candidateStateRootHash); prunedNodeCount++; if (prunedNodeCount % operationsPerTransaction == 0) { @@ -149,11 +149,19 @@ public void sweepBefore(final long markedBlockNumber) { } updater.commit(); // Sweep non-state-root nodes - prunedNodeCount += worldStateStorage.prune(markStorage::containsKey); + prunedNodeCount += worldStateStorage.prune(this::isMarked); sweptNodesCounter.inc(prunedNodeCount); worldStateStorage.removeNodeAddedListener(nodeAddedListenerId); markStorage.clear(); - LOG.info("Completed sweeping unused nodes"); + LOG.debug("Completed sweeping unused nodes"); + } + + private boolean isMarked(final Bytes32 key) { + return pendingMarks.contains(key) || markStorage.containsKey(key.getArrayUnsafe()); + } + + private boolean isMarked(final byte[] key) { + return pendingMarks.contains(Bytes32.wrap(key)) || markStorage.containsKey(key); } private MerklePatriciaTrie createStateTrie(final Bytes32 rootHash) { @@ -182,10 +190,14 @@ private void processAccountState(final BytesValue value) { @VisibleForTesting void markNode(final Bytes32 hash) { - markedNodesCounter.inc(); + markNodes(Collections.singleton(hash)); + } + + private void markNodes(final Collection nodeHashes) { + markedNodesCounter.inc(nodeHashes.size()); markLock.lock(); try { - pendingMarks.add(hash); + pendingMarks.addAll(nodeHashes); maybeFlushPendingMarks(); } finally { markLock.unlock(); @@ -209,15 +221,4 @@ void flushPendingMarks() { markLock.unlock(); } } - - private void markNewNodes(final Collection nodeHashes) { - markedNodesCounter.inc(nodeHashes.size()); - markLock.lock(); - try { - pendingMarks.addAll(nodeHashes); - maybeFlushPendingMarks(); - } finally { - markLock.unlock(); - } - } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/Pruner.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/Pruner.java index b76a7978ebf..fb8c7e5116f 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/Pruner.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/Pruner.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.ethereum.worldstate; +import static com.google.common.base.Preconditions.checkArgument; + import org.hyperledger.besu.ethereum.chain.BlockAddedEvent; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; @@ -23,6 +25,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import com.google.common.annotations.VisibleForTesting; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -48,15 +51,13 @@ public Pruner( this.blockchain = blockchain; this.blocksRetained = pruningConfiguration.getBlocksRetained(); this.blockConfirmations = pruningConfiguration.getBlockConfirmations(); - if (blockConfirmations < 0 || blocksRetained < 0) { - throw new IllegalArgumentException( - String.format( - "blockConfirmations and blocksRetained must be non-negative. blockConfirmations=%d, blocksRetained=%d", - blockConfirmations, blocksRetained)); - } + checkArgument( + blockConfirmations >= 0 && blockConfirmations < blocksRetained, + "blockConfirmations and blocksRetained must be non-negative. blockConfirmations must be less than blockRetained."); } public void start() { + LOG.info("Starting Pruner."); blockchain.observeBlockAdded((event, blockchain) -> handleNewBlock(event)); } @@ -88,7 +89,7 @@ private void handleNewBlock(final BlockAddedEvent event) { private void mark(final BlockHeader header) { markBlockNumber = header.getNumber(); final Hash stateRoot = header.getStateRoot(); - LOG.info( + LOG.debug( "Begin marking used nodes for pruning. Block number: {} State root: {}", markBlockNumber, stateRoot); @@ -100,7 +101,10 @@ private void mark(final BlockHeader header) { } private void sweep() { - LOG.info("Begin sweeping unused nodes for pruning. Retention period: {}", blocksRetained); + LOG.debug( + "Begin sweeping unused nodes for pruning. Keeping full state for blocks {} to {}", + markBlockNumber, + markBlockNumber + blocksRetained); execute( () -> { pruningStrategy.sweepBefore(markBlockNumber); @@ -117,7 +121,12 @@ private void execute(final Runnable action) { } } - private enum State { + @VisibleForTesting + State getState() { + return state.get(); + } + + enum State { IDLE, MARK_BLOCK_CONFIRMATIONS_AWAITING, MARKING, diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPrunerTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPrunerTest.java index efa0c089b98..e2fcddab72a 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPrunerTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/MarkSweepPrunerTest.java @@ -65,105 +65,6 @@ public class MarkSweepPrunerTest { private final Block genesisBlock = gen.genesisBlock(); private final MutableBlockchain blockchain = createInMemoryBlockchain(genesisBlock); - @Test - public void prepareMarkAndSweep_smallState_manyOpsPerTx() { - testPrepareMarkAndSweep(3, 1, 2, 1000); - } - - @Test - public void prepareMarkAndSweep_largeState_fewOpsPerTx() { - testPrepareMarkAndSweep(20, 5, 5, 5); - } - - @Test - public void prepareMarkAndSweep_emptyBlocks() { - testPrepareMarkAndSweep(10, 0, 5, 10); - } - - @Test - public void prepareMarkAndSweep_markChainhead() { - testPrepareMarkAndSweep(10, 2, 10, 20); - } - - @Test - public void prepareMarkAndSweep_markGenesis() { - testPrepareMarkAndSweep(10, 2, 0, 20); - } - - @Test - public void prepareMarkAndSweep_multipleRounds() { - testPrepareMarkAndSweep(10, 2, 10, 20); - testPrepareMarkAndSweep(10, 2, 15, 20); - } - - private void testPrepareMarkAndSweep( - final int numBlocks, - final int accountsPerBlock, - final int markBlockNumber, - final int opsPerTransaction) { - final MarkSweepPruner pruner = - new MarkSweepPruner( - worldStateStorage, blockchain, markStorage, metricsSystem, opsPerTransaction); - final int chainHeight = (int) blockchain.getChainHead().getHeight(); - // Generate blocks up to markBlockNumber - final int blockCountBeforeMarkedBlock = markBlockNumber - chainHeight; - generateBlockchainData(blockCountBeforeMarkedBlock, accountsPerBlock); - - // Prepare - pruner.prepare(); - // Mark - final BlockHeader markBlock = blockchain.getBlockHeader(markBlockNumber).get(); - pruner.mark(markBlock.getStateRoot()); - - // Generate more blocks that should be kept - generateBlockchainData(numBlocks - blockCountBeforeMarkedBlock, accountsPerBlock); - - // Collect the nodes we expect to keep - final Set expectedNodes = collectWorldStateNodes(markBlock.getStateRoot()); - for (int i = markBlockNumber; i <= blockchain.getChainHeadBlockNumber(); i++) { - final Hash stateRoot = blockchain.getBlockHeader(i).get().getStateRoot(); - collectWorldStateNodes(stateRoot, expectedNodes); - } - if (accountsPerBlock != 0 && markBlockNumber > 0) { - assertThat(hashValueStore.size()).isGreaterThan(expectedNodes.size()); // Sanity check - } - - // Sweep - pruner.sweepBefore(markBlock.getNumber()); - - // Assert that blocks from mark point onward are still accessible - for (int i = markBlockNumber; i <= blockchain.getChainHeadBlockNumber(); i++) { - final Hash stateRoot = blockchain.getBlockHeader(i).get().getStateRoot(); - assertThat(worldStateArchive.get(stateRoot)).isPresent(); - final WorldState markedState = worldStateArchive.get(stateRoot).get(); - // Traverse accounts and make sure all are accessible - final int expectedAccounts = accountsPerBlock * i; - final long accounts = markedState.streamAccounts(Bytes32.ZERO, expectedAccounts * 2).count(); - assertThat(accounts).isEqualTo(expectedAccounts); - // Traverse storage to ensure that all storage is accessible - markedState - .streamAccounts(Bytes32.ZERO, expectedAccounts * 2) - .forEach(a -> a.storageEntriesFrom(Bytes32.ZERO, 1000)); - } - - // All other state roots should have been removed - for (int i = 0; i < markBlockNumber; i++) { - final BlockHeader curHeader = blockchain.getBlockHeader(i + 1L).get(); - if (curHeader.getNumber() == markBlock.getNumber()) { - continue; - } - if (!curHeader.getStateRoot().equals(Hash.EMPTY_TRIE_HASH)) { - assertThat(worldStateArchive.get(curHeader.getStateRoot())).isEmpty(); - } - } - - // Check that storage contains only the values we expect - assertThat(hashValueStore.size()).isEqualTo(expectedNodes.size()); - assertThat(hashValueStore.values()) - .containsExactlyInAnyOrderElementsOf( - expectedNodes.stream().map(BytesValue::getArrayUnsafe).collect(Collectors.toSet())); - } - @Test public void mark_marksAllExpectedNodes() { final MarkSweepPruner pruner = @@ -362,6 +263,7 @@ private MerklePatriciaTrie createStorageTrie(final Bytes32 Function.identity()); } + // Proxy class so that we have access to the constructor that takes our own map private static class TestInMemoryStorage extends InMemoryKeyValueStorage { public TestInMemoryStorage(final Map hashValueStore) { diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/PrunerTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/PrunerTest.java index 1b13cdbcdb2..b60fd543481 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/PrunerTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/worldstate/PrunerTest.java @@ -65,7 +65,7 @@ public void shouldMarkCorrectBlockAndSweep() throws InterruptedException { final Pruner pruner = new Pruner( - markSweepPruner, blockchain, mockExecutorService, new PruningConfiguration(0, 0)); + markSweepPruner, blockchain, mockExecutorService, new PruningConfiguration(0, 1)); pruner.start(); final Block block1 = appendBlockWithParent(blockchain, genesisBlock); @@ -159,6 +159,22 @@ public void shouldRejectInvalidArguments() { mockExecutorService, new PruningConfiguration(-1, -2))) .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> + new Pruner( + markSweepPruner, + mockchain, + mockExecutorService, + new PruningConfiguration(10, 8))) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> + new Pruner( + markSweepPruner, + mockchain, + mockExecutorService, + new PruningConfiguration(10, 10))) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -171,7 +187,7 @@ public void shouldCleanUpPruningStrategyOnShutdown() throws InterruptedException final Pruner pruner = new Pruner( - markSweepPruner, blockchain, mockExecutorService, new PruningConfiguration(0, 0)); + markSweepPruner, blockchain, mockExecutorService, new PruningConfiguration(0, 1)); pruner.start(); pruner.stop(); verify(markSweepPruner).cleanup(); diff --git a/services/kvstore/src/main/java/org/hyperledger/besu/services/kvstore/InMemoryKeyValueStorage.java b/services/kvstore/src/main/java/org/hyperledger/besu/services/kvstore/InMemoryKeyValueStorage.java index 7b8efdc7aba..59ae98c626c 100644 --- a/services/kvstore/src/main/java/org/hyperledger/besu/services/kvstore/InMemoryKeyValueStorage.java +++ b/services/kvstore/src/main/java/org/hyperledger/besu/services/kvstore/InMemoryKeyValueStorage.java @@ -77,9 +77,15 @@ public Optional get(final byte[] key) throws StorageException { @Override public long removeAllKeysUnless(final Predicate retainCondition) throws StorageException { - long initialSize = hashValueStore.keySet().size(); - hashValueStore.keySet().removeIf(key -> !retainCondition.test(key.getArrayUnsafe())); - return initialSize - hashValueStore.keySet().size(); + final Lock lock = rwLock.writeLock(); + lock.lock(); + try { + long initialSize = hashValueStore.keySet().size(); + hashValueStore.keySet().removeIf(key -> !retainCondition.test(key.getArrayUnsafe())); + return initialSize - hashValueStore.keySet().size(); + } finally { + lock.unlock(); + } } @Override From 1937f1251d87c5e11ed0cf62025b51dcb5364145 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Wed, 25 Sep 2019 15:29:18 -0400 Subject: [PATCH 07/10] Clean up BesuConfiguration construction (#51) Signed-off-by: Meredith Baxter --- .../besu/tests/acceptance/dsl/privacy/PrivacyNode.java | 10 +++++++--- .../java/org/hyperledger/besu/cli/BesuCommand.java | 2 +- .../besu/services/BesuConfigurationImpl.java | 6 +----- .../test/java/org/hyperledger/besu/PrivacyTest.java | 7 ++++--- .../src/test/java/org/hyperledger/besu/RunnerTest.java | 8 ++++---- .../sync/worldstate/WorldStateDownloaderBenchmark.java | 6 +++--- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java index d3b2af22c97..f01865ffe7f 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.tests.acceptance.dsl.privacy; +import static org.hyperledger.besu.controller.BesuController.DATABASE_PATH; + import org.hyperledger.besu.controller.KeyPairUtil; import org.hyperledger.besu.enclave.Enclave; import org.hyperledger.besu.enclave.EnclaveException; @@ -146,13 +148,14 @@ public void start(final BesuNodeRunner runner) { try { final Path dataDir = Files.createTempDirectory("acctest-privacy"); + final Path dbDir = dataDir.resolve(DATABASE_PATH); privacyParameters = new PrivacyParameters.Builder() .setEnabled(true) .setEnclaveUrl(orion.clientUrl()) .setEnclavePublicKeyUsingFile(orion.getConfig().publicKeys().get(0).toFile()) - .setStorageProvider(createKeyValueStorageProvider(dataDir)) + .setStorageProvider(createKeyValueStorageProvider(dataDir, dbDir)) .setPrivateKeyPath(KeyPairUtil.getDefaultKeyFile(besu.homeDirectory()).toPath()) .build(); } catch (IOException e) { @@ -206,7 +209,8 @@ public NodeConfiguration getConfiguration() { return besu.getConfiguration(); } - private StorageProvider createKeyValueStorageProvider(final Path dbLocation) { + private StorageProvider createKeyValueStorageProvider( + final Path dataLocation, final Path dbLocation) { return new KeyValueStorageProviderBuilder() .withStorageFactory( new RocksDBKeyValuePrivacyStorageFactory( @@ -217,7 +221,7 @@ private StorageProvider createKeyValueStorageProvider(final Path dbLocation) { BACKGROUND_THREAD_COUNT, CACHE_CAPACITY), Arrays.asList(KeyValueSegmentIdentifier.values()))) - .withCommonConfiguration(new BesuConfigurationImpl(dbLocation)) + .withCommonConfiguration(new BesuConfigurationImpl(dataLocation, dbLocation)) .withMetricsSystem(new NoOpMetricsSystem()) .build(); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 00fcfcc9f94..ae661f6fa53 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -773,7 +773,7 @@ private void addConfigurationService() { if (pluginCommonConfiguration == null) { final Path dataDir = dataDir(); pluginCommonConfiguration = - new BesuConfigurationImpl(dataDir.resolve(DATABASE_PATH), dataDir); + new BesuConfigurationImpl(dataDir, dataDir.resolve(DATABASE_PATH)); besuPluginContext.addService(BesuConfiguration.class, pluginCommonConfiguration); } } diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java index d64ff96d951..5c3d5cd5d13 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuConfigurationImpl.java @@ -23,11 +23,7 @@ public class BesuConfigurationImpl implements BesuConfiguration { private final Path storagePath; private final Path dataPath; - public BesuConfigurationImpl(final Path storagePath) { - this(storagePath, storagePath); - } - - public BesuConfigurationImpl(final Path storagePath, final Path dataPath) { + public BesuConfigurationImpl(final Path dataPath, final Path storagePath) { this.storagePath = storagePath; this.dataPath = dataPath; } diff --git a/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java b/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java index 2eea7c57f79..c3eb5b42fd2 100644 --- a/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java +++ b/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java @@ -59,11 +59,12 @@ public class PrivacyTest { @Test public void privacyPrecompiled() throws IOException { final Path dataDir = folder.newFolder().toPath(); + final Path dbDir = dataDir.resolve("database"); final PrivacyParameters privacyParameters = new PrivacyParameters.Builder() .setPrivacyAddress(ADDRESS) .setEnabled(true) - .setStorageProvider(createKeyValueStorageProvider(dataDir)) + .setStorageProvider(createKeyValueStorageProvider(dataDir, dbDir)) .build(); final BesuController besuController = new BesuController.Builder() @@ -92,7 +93,7 @@ public void privacyPrecompiled() throws IOException { assertThat(precompiledContract.getName()).isEqualTo("Privacy"); } - private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { + private StorageProvider createKeyValueStorageProvider(final Path dataDir, final Path dbDir) { return new KeyValueStorageProviderBuilder() .withStorageFactory( new RocksDBKeyValuePrivacyStorageFactory( @@ -103,7 +104,7 @@ private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { BACKGROUND_THREAD_COUNT, CACHE_CAPACITY), Arrays.asList(KeyValueSegmentIdentifier.values()))) - .withCommonConfiguration(new BesuConfigurationImpl(dbAhead)) + .withCommonConfiguration(new BesuConfigurationImpl(dataDir, dbDir)) .withMetricsSystem(new NoOpMetricsSystem()) .build(); } diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java index 01f0dc4a662..6348ac07bf8 100644 --- a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java +++ b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java @@ -148,7 +148,7 @@ private void syncFromGenesis(final SyncMode mode, final GenesisConfigFile genesi .privacyParameters(PrivacyParameters.DEFAULT) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) - .storageProvider(createKeyValueStorageProvider(dbAhead)) + .storageProvider(createKeyValueStorageProvider(dataDirAhead, dbAhead)) .build()) { setupState(blockCount, controller.getProtocolSchedule(), controller.getProtocolContext()); } @@ -167,7 +167,7 @@ private void syncFromGenesis(final SyncMode mode, final GenesisConfigFile genesi .privacyParameters(PrivacyParameters.DEFAULT) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) - .storageProvider(createKeyValueStorageProvider(dbAhead)) + .storageProvider(createKeyValueStorageProvider(dataDirAhead, dbAhead)) .build(); final String listenHost = InetAddress.getLoopbackAddress().getHostAddress(); final JsonRpcConfiguration aheadJsonRpcConfiguration = jsonRpcConfiguration(); @@ -355,7 +355,7 @@ private GenesisConfigFile getFastSyncGenesis() { return GenesisConfigFile.fromConfig(jsonNode); } - private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { + private StorageProvider createKeyValueStorageProvider(final Path dataDir, final Path dbDir) { return new KeyValueStorageProviderBuilder() .withStorageFactory( new RocksDBKeyValueStorageFactory( @@ -366,7 +366,7 @@ private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { BACKGROUND_THREAD_COUNT, CACHE_CAPACITY), Arrays.asList(KeyValueSegmentIdentifier.values()))) - .withCommonConfiguration(new BesuConfigurationImpl(dbAhead)) + .withCommonConfiguration(new BesuConfigurationImpl(dataDir, dbDir)) .withMetricsSystem(new NoOpMetricsSystem()) .build(); } diff --git a/ethereum/eth/src/jmh/java/org/hyperledger/besu/ethereum/eth/sync/worldstate/WorldStateDownloaderBenchmark.java b/ethereum/eth/src/jmh/java/org/hyperledger/besu/ethereum/eth/sync/worldstate/WorldStateDownloaderBenchmark.java index 896dfa45c5f..c720ed53116 100644 --- a/ethereum/eth/src/jmh/java/org/hyperledger/besu/ethereum/eth/sync/worldstate/WorldStateDownloaderBenchmark.java +++ b/ethereum/eth/src/jmh/java/org/hyperledger/besu/ethereum/eth/sync/worldstate/WorldStateDownloaderBenchmark.java @@ -100,7 +100,7 @@ public void setUpUnchangedState() { final EthContext ethContext = ethProtocolManager.ethContext(); final StorageProvider storageProvider = - createKeyValueStorageProvider(tempDir.resolve("database")); + createKeyValueStorageProvider(tempDir, tempDir.resolve("database")); worldStateStorage = storageProvider.createWorldStateStorage(); pendingRequests = @@ -158,7 +158,7 @@ public Optional downloadWorldState() { return rootData; } - private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { + private StorageProvider createKeyValueStorageProvider(final Path dataDir, final Path dbDir) { return new KeyValueStorageProviderBuilder() .withStorageFactory( new RocksDBKeyValueStorageFactory( @@ -169,7 +169,7 @@ private StorageProvider createKeyValueStorageProvider(final Path dbAhead) { DEFAULT_BACKGROUND_THREAD_COUNT, DEFAULT_CACHE_CAPACITY), Arrays.asList(KeyValueSegmentIdentifier.values()))) - .withCommonConfiguration(new BesuConfigurationImpl(dbAhead)) + .withCommonConfiguration(new BesuConfigurationImpl(dataDir, dbDir)) .withMetricsSystem(new NoOpMetricsSystem()) .build(); } From 83ad8e6942164700b0b4bac213d3070e4d2271f8 Mon Sep 17 00:00:00 2001 From: Ivaylo Kirilov Date: Thu, 26 Sep 2019 00:05:44 +0100 Subject: [PATCH 08/10] return enclave key instead of private transaction hash (#53) Signed-off-by: Ivaylo Kirilov Signed-off-by: Lucas Saldanha --- .../privacy/methods/priv/PrivDistributeRawTransaction.java | 5 ++++- .../methods/priv/PrivDistributeRawTransactionTest.java | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransaction.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransaction.java index 63990a45017..93109ef9e43 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransaction.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransaction.java @@ -26,6 +26,7 @@ import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.privacy.PrivateTransaction; import org.hyperledger.besu.ethereum.privacy.PrivateTransactionHandler; +import org.hyperledger.besu.util.bytes.BytesValues; public class PrivDistributeRawTransaction extends AbstractSendTransaction implements JsonRpcMethod { @@ -72,6 +73,8 @@ public JsonRpcResponse response(final JsonRpcRequest request) { request, privateTransaction, privacyGroupId, - () -> new JsonRpcSuccessResponse(request.getId(), privateTransaction.hash().toString())); + () -> + new JsonRpcSuccessResponse( + request.getId(), BytesValues.fromBase64(enclaveKey).toString())); } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransactionTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransactionTest.java index bd11f81655e..9912a0c9ccc 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransactionTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransactionTest.java @@ -28,6 +28,7 @@ import org.hyperledger.besu.ethereum.mainnet.ValidationResult; import org.hyperledger.besu.ethereum.privacy.PrivateTransaction; import org.hyperledger.besu.ethereum.privacy.PrivateTransactionHandler; +import org.hyperledger.besu.util.bytes.BytesValues; import org.junit.Before; import org.junit.Test; @@ -47,7 +48,7 @@ public class PrivDistributeRawTransactionTest { + "200e885ff29e973e2576b6600181d1b0a2b5294e30d9be4a1981" + "ffb33a0b8c8a72657374726963746564"; - final String MOCK_ORION_KEY = ""; + final String MOCK_ORION_KEY = "93Ky7lXwFkMc7+ckoFgUMku5bpr9tz4zhmWmk9RlNng="; private final String MOCK_PRIVACY_GROUP = ""; @Mock private TransactionPool transactionPool; @@ -82,7 +83,7 @@ public void validTransactionHashReturnedAfterDistribute() throws Exception { final JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse( - request.getId(), "0x34d3db0593e0dad50104cc575db154b6e1a57ff3fc0354b2df2f25c8f21aca66"); + request.getId(), BytesValues.fromBase64(MOCK_ORION_KEY).toString()); final JsonRpcResponse actualResponse = method.response(request); From b79afb88652a794f8b45b4415743443655b2ebeb Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 26 Sep 2019 01:15:34 +0200 Subject: [PATCH 09/10] Update download link (#54) Signed-off-by: Nicolas MASSART --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4feb5b4877a..2f10e040e8f 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![Build Status](https://jenkins.pegasys.tech/job/Besu/job/master/badge/icon)](https://jenkins.pegasys.tech/job/Besu/job/master/) [![Documentation Status](https://readthedocs.org/projects/hyperledger-besu/badge/?version=latest)](https://besu.hyperledger.org/en/latest/?badge=latest) [![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://github.com/PegasysEng/besu/blob/master/LICENSE) - [ ![Download](https://api.bintray.com/packages/hyperledger/besu-repo/besu/images/download.svg) ](https://bintray.com/hyperledger/besu-repo/besu/_latestVersion) + [ ![Download](https://api.bintray.com/packages/hyperledger-org/besu-repo/besu/images/download.svg) ](https://bintray.com/hyperledger-org/besu-repo/besu/_latestVersion) [![RocketChat chat](https://open.rocket.chat/images/join-chat.svg)](https://chat.hyperledger.org/channel/besu) From 50c076498643c2d45b1a769a41c5f1e59ea9a0ac Mon Sep 17 00:00:00 2001 From: CJ Hare Date: Thu, 26 Sep 2019 14:39:39 +1000 Subject: [PATCH 10/10] Fixing Typo in validation error message (#55) Signed-off-by: Christopher Hare --- .../org/hyperledger/besu/controller/BesuControllerBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index e1d512eb8e6..6a82b6197b8 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -190,7 +190,7 @@ public BesuController build() { checkNotNull(metricsSystem, "Missing metrics system"); checkNotNull(privacyParameters, "Missing privacy parameters"); checkNotNull(dataDirectory, "Missing data directory"); // Why do we need this? - checkNotNull(clock, "Mising clock"); + checkNotNull(clock, "Missing clock"); checkNotNull(transactionPoolConfiguration, "Missing transaction pool configuration"); checkNotNull(nodeKeys, "Missing node keys"); checkNotNull(storageProvider, "Must supply a storage provider");