RocksDB plugin is hard wired, rather then discovered (#1934)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

acceptance-tests/build.gradle

@@ -33,6 +33,7 @@ dependencies {
   testSupportImplementation project(':metrics:core')
   testSupportImplementation project(':pantheon')
   testSupportImplementation project(':plugin-api')
+  testSupportImplementation project(':plugins:rocksdb')
   testSupportImplementation project(':services:kvstore')
   testSupportImplementation project(':testutil')
   testSupportImplementation project(':util')

acceptance-tests/src/test-support/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java

@@ -34,6 +34,7 @@ import tech.pegasys.pantheon.plugin.services.PantheonConfiguration;
 import tech.pegasys.pantheon.plugin.services.PantheonEvents;
 import tech.pegasys.pantheon.plugin.services.PicoCLIOptions;
 import tech.pegasys.pantheon.plugin.services.StorageService;
+import tech.pegasys.pantheon.plugin.services.storage.rocksdb.RocksDBPlugin;
 import tech.pegasys.pantheon.services.PantheonConfigurationImpl;
 import tech.pegasys.pantheon.services.PantheonEventsImpl;
 import tech.pegasys.pantheon.services.PantheonPluginContextImpl;
@@ -90,6 +91,9 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
 
     pantheonPluginContext.addService(PantheonConfiguration.class, commonPluginConfiguration);
 
+    // register built-in plugins
+    new RocksDBPlugin().register(pantheonPluginContext);
+
     return pantheonPluginContext;
   }
 

ethereum/core/build.gradle

@@ -35,9 +35,6 @@ dependencies {
   implementation project(':plugin-api')
   implementation project(':services:kvstore')
 
-  // Runtime dependency gets the Plugin included in the distribution
-  runtime project(":plugins:rocksdb")
-
   implementation 'com.fasterxml.jackson.core:jackson-databind'
   implementation 'com.google.guava:guava'
   implementation 'io.vertx:vertx-core'

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueSegmentIdentifier.java

@@ -15,14 +15,25 @@ package tech.pegasys.pantheon.ethereum.storage.keyvalue;
 import tech.pegasys.pantheon.plugin.services.storage.SegmentIdentifier;
 
 public enum KeyValueSegmentIdentifier implements SegmentIdentifier {
-  BLOCKCHAIN,
-  WORLD_STATE,
-  PRIVATE_TRANSACTIONS,
-  PRIVATE_STATE,
-  PRUNING_STATE;
+  BLOCKCHAIN((byte) 1),
+  WORLD_STATE((byte) 2),
+  PRIVATE_TRANSACTIONS((byte) 3),
+  PRIVATE_STATE((byte) 4),
+  PRUNING_STATE((byte) 5);
+
+  private final byte[] id;
+
+  KeyValueSegmentIdentifier(final byte... id) {
+    this.id = id;
+  }
 
   @Override
   public String getName() {
     return name();
   }
+
+  @Override
+  public byte[] getId() {
+    return id;
+  }
 }

pantheon/build.gradle

@@ -45,6 +45,7 @@ dependencies {
   implementation project(':metrics:core')
   implementation project(':nat')
   implementation project(':plugin-api')
+  implementation project(':plugins:rocksdb')
   implementation project(':services:kvstore')
 
   implementation 'com.fasterxml.jackson.core:jackson-databind'
@@ -61,7 +62,6 @@ dependencies {
   runtime 'org.apache.logging.log4j:log4j-core'
   runtime 'org.apache.logging.log4j:log4j-slf4j-impl'
 
-  testImplementation project(':plugins:rocksdb')
   testImplementation project(':testutil')
   testImplementation project(path: ':ethereum:core', configuration: 'testSupportArtifacts')
 

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -96,6 +96,7 @@ import tech.pegasys.pantheon.plugin.services.PantheonEvents;
 import tech.pegasys.pantheon.plugin.services.PicoCLIOptions;
 import tech.pegasys.pantheon.plugin.services.StorageService;
 import tech.pegasys.pantheon.plugin.services.metrics.MetricCategory;
+import tech.pegasys.pantheon.plugin.services.storage.rocksdb.RocksDBPlugin;
 import tech.pegasys.pantheon.services.PantheonConfigurationImpl;
 import tech.pegasys.pantheon.services.PantheonEventsImpl;
 import tech.pegasys.pantheon.services.PantheonPluginContextImpl;
@@ -747,7 +748,6 @@ public class PantheonCommand implements DefaultCommandValues, Runnable {
   public void run() {
     try {
       prepareLogging();
-      addConfigurationService();
       logger.info("Starting Pantheon version: {}", PantheonInfo.version());
       checkOptions().configure().controller().startPlugins().startSynchronization();
     } catch (final Exception e) {
@@ -756,8 +756,10 @@ public class PantheonCommand implements DefaultCommandValues, Runnable {
   }
 
   private void addConfigurationService() {
-    pluginCommonConfiguration = new PantheonConfigurationImpl(dataDir().resolve(DATABASE_PATH));
-    pantheonPluginContext.addService(PantheonConfiguration.class, pluginCommonConfiguration);
+    if (pluginCommonConfiguration == null) {
+      pluginCommonConfiguration = new PantheonConfigurationImpl(dataDir().resolve(DATABASE_PATH));
+      pantheonPluginContext.addService(PantheonConfiguration.class, pluginCommonConfiguration);
+    }
   }
 
   @VisibleForTesting
@@ -830,6 +832,10 @@ public class PantheonCommand implements DefaultCommandValues, Runnable {
   private PantheonCommand preparePlugins() {
     pantheonPluginContext.addService(PicoCLIOptions.class, new PicoCLIOptionsImpl(commandLine));
     pantheonPluginContext.addService(StorageService.class, storageService);
+
+    // register built-in plugins
+    new RocksDBPlugin().register(pantheonPluginContext);
+
     pantheonPluginContext.registerPlugins(pluginsDir());
     return this;
   }
@@ -987,8 +993,9 @@ public class PantheonCommand implements DefaultCommandValues, Runnable {
 
   public PantheonControllerBuilder<?> getControllerBuilder() {
     try {
+      addConfigurationService();
       return controllerBuilderFactory
-          .fromEthNetworkConfig(updateNetworkConfig(getNetwork()))
+          .fromEthNetworkConfig(updateNetworkConfig(getNetwork()), genesisConfigOverrides)
           .synchronizerConfiguration(buildSyncConfig())
           .ethProtocolConfiguration(ethProtocolOptions.toDomainObject())
           .dataDirectory(dataDir())

pantheon/src/main/java/tech/pegasys/pantheon/services/StorageServiceImpl.java

@@ -12,6 +12,7 @@
  */
 package tech.pegasys.pantheon.services;
 
+import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueSegmentIdentifier;
 import tech.pegasys.pantheon.plugin.services.StorageService;
 import tech.pegasys.pantheon.plugin.services.exception.StorageException;
 import tech.pegasys.pantheon.plugin.services.storage.KeyValueStorageFactory;
@@ -28,7 +29,7 @@ public class StorageServiceImpl implements StorageService {
   private final Map<String, KeyValueStorageFactory> factories;
 
   public StorageServiceImpl() {
-    this.segments = List.of(Segment.values());
+    this.segments = List.of(KeyValueSegmentIdentifier.values());
     this.factories = new ConcurrentHashMap<>();
   }
 
@@ -42,19 +43,6 @@ public class StorageServiceImpl implements StorageService {
     return segments;
   }
 
-  private enum Segment implements SegmentIdentifier {
-    BLOCKCHAIN,
-    WORLD_STATE,
-    PRIVATE_TRANSACTIONS,
-    PRIVATE_STATE,
-    PRUNING_STATE;
-
-    @Override
-    public String getName() {
-      return name();
-    }
-  }
-
   public KeyValueStorageFactory getByName(final String name) {
     return Optional.ofNullable(factories.get(name))
         .orElseThrow(

pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java

@@ -51,6 +51,7 @@ import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration;
 import tech.pegasys.pantheon.ethereum.storage.StorageProvider;
 import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration;
 import tech.pegasys.pantheon.plugin.services.PantheonConfiguration;
+import tech.pegasys.pantheon.plugin.services.PicoCLIOptions;
 import tech.pegasys.pantheon.plugin.services.storage.KeyValueStorageFactory;
 import tech.pegasys.pantheon.services.PantheonPluginContextImpl;
 import tech.pegasys.pantheon.services.StorageServiceImpl;
@@ -65,6 +66,7 @@ import java.nio.file.Path;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -112,6 +114,7 @@ public abstract class CommandTestAbstract {
   @Mock protected PantheonConfiguration commonPluginConfiguration;
   @Mock protected KeyValueStorageFactory rocksDBStorageFactory;
   @Mock protected KeyValueStorageFactory rocksDBSPrivacyStorageFactory;
+  @Mock protected PicoCLIOptions cliOptions;
 
   @Mock protected Logger mockLogger;
   @Mock protected PantheonPluginContextImpl mockPantheonPluginContext;
@@ -147,8 +150,9 @@ public abstract class CommandTestAbstract {
   public void initMocks() throws Exception {
 
     // doReturn used because of generic PantheonController
-    doReturn(mockControllerBuilder).when(mockControllerBuilderFactory).fromEthNetworkConfig(any());
-
+    doReturn(mockControllerBuilder)
+        .when(mockControllerBuilderFactory)
+        .fromEthNetworkConfig(any(), any());
     when(mockControllerBuilder.synchronizerConfiguration(any())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.ethProtocolConfiguration(any())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.transactionPoolConfiguration(any()))
@@ -198,6 +202,9 @@ public abstract class CommandTestAbstract {
     when(mockRunnerBuilder.build()).thenReturn(mockRunner);
 
     when(storageService.getByName("rocksdb")).thenReturn(rocksDBStorageFactory);
+
+    when(mockPantheonPluginContext.getService(PicoCLIOptions.class))
+        .thenReturn(Optional.of(cliOptions));
   }
 
   // Display outputs for debug purpose

pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java

@@ -17,6 +17,7 @@ import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isNotNull;
 import static org.mockito.Mockito.atLeast;
@@ -165,7 +166,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     verify(mockRunnerBuilder).ethNetworkConfig(ethNetworkArg.capture());
     verify(mockRunnerBuilder).build();
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(ethNetworkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(ethNetworkArg.capture(), any());
     final ArgumentCaptor<MiningParameters> miningArg =
         ArgumentCaptor.forClass(MiningParameters.class);
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
@@ -332,7 +333,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
             .setBootNodes(nodes)
             .build();
     verify(mockControllerBuilder).dataDirectory(eq(Paths.get("/opt/pantheon").toAbsolutePath()));
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(eq(networkConfig));
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(eq(networkConfig), any());
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
     assertThat(syncConfigurationCaptor.getValue().getSyncMode()).isEqualTo(SyncMode.FAST);
@@ -890,7 +891,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
 
     parseCommand("--genesis-file", genesisFile.toString());
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue().getGenesisConfig())
@@ -926,7 +927,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue().getGenesisConfig())
@@ -949,7 +950,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue().getGenesisConfig())
@@ -2343,7 +2344,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue()).isEqualTo(EthNetworkConfig.getNetworkConfig(DEV));
@@ -2359,7 +2360,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue()).isEqualTo(EthNetworkConfig.getNetworkConfig(RINKEBY));
@@ -2375,7 +2376,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue()).isEqualTo(EthNetworkConfig.getNetworkConfig(ROPSTEN));
@@ -2391,7 +2392,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue()).isEqualTo(EthNetworkConfig.getNetworkConfig(GOERLI));
@@ -2432,7 +2433,7 @@ public class PantheonCommandTest extends CommandTestAbstract {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
 
-    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture());
+    verify(mockControllerBuilderFactory).fromEthNetworkConfig(networkArg.capture(), any());
     verify(mockControllerBuilder).build();
 
     assertThat(networkArg.getValue().getBootNodes())

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 = 'xJ1uQYYa5Z4h7qIVsKMLsUFHAjj4FXAeyCtX6PY4z0g='
+  knownHash = 'sRinlNFG0IbW1DXvVnpa/a4puB5WD0g2GReAGkIl6dc='
 }
 check.dependsOn('checkAPIChanges')
 

plugin-api/src/main/java/tech/pegasys/pantheon/plugin/services/storage/SegmentIdentifier.java

@@ -22,9 +22,16 @@ import tech.pegasys.pantheon.plugin.Unstable;
 public interface SegmentIdentifier {
 
   /**
-   * Identifier for the segment consistent throughout the lifetime of the segment.
+   * Name for the segment consistent throughout the lifetime of the segment.
    *
    * @return unique name of the segment.
    */
   String getName();
+
+  /**
+   * Identifier for the segment consistent throughout the lifetime of the segment.
+   *
+   * @return unique id of the segment.
+   */
+  byte[] getId();
 }

plugins/rocksdb/build.gradle

@@ -33,15 +33,12 @@ dependencies {
   implementation project(':services:kvstore')
 
   implementation 'com.fasterxml.jackson.core:jackson-databind'
-  implementation 'com.google.auto.service:auto-service'
   implementation 'com.google.guava:guava'
   implementation 'info.picocli:picocli'
   implementation 'io.prometheus:simpleclient'
   implementation 'org.apache.logging.log4j:log4j-api'
   implementation 'org.rocksdb:rocksdbjni'
 
-  annotationProcessor 'com.google.auto.service:auto-service'
-
   runtime 'org.apache.logging.log4j:log4j-core'
 
   testImplementation project(':testutil')

plugins/rocksdb/src/main/java/tech/pegasys/pantheon/plugin/services/storage/rocksdb/RocksDBPlugin.java

@@ -24,13 +24,11 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
 
-import com.google.auto.service.AutoService;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-@AutoService(PantheonPlugin.class)
 public class RocksDBPlugin implements PantheonPlugin {
 
   private static final Logger LOG = LogManager.getLogger();

plugins/rocksdb/src/main/java/tech/pegasys/pantheon/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java

@@ -78,7 +78,7 @@ public class RocksDBColumnarKeyValueStorage
     try {
       final List<ColumnFamilyDescriptor> columnDescriptors =
           segments.stream()
-              .map(segment -> new ColumnFamilyDescriptor(getId(segment)))
+              .map(segment -> new ColumnFamilyDescriptor(segment.getId()))
               .collect(Collectors.toList());
       columnDescriptors.add(
           new ColumnFamilyDescriptor(
@@ -111,7 +111,7 @@ public class RocksDBColumnarKeyValueStorage
           segments.stream()
               .collect(
                   Collectors.toMap(
-                      segment -> BytesValue.wrap(getId(segment)), SegmentIdentifier::getName));
+                      segment -> BytesValue.wrap(segment.getId()), SegmentIdentifier::getName));
 
       final ImmutableMap.Builder<String, ColumnFamilyHandle> builder = ImmutableMap.builder();
 
@@ -213,10 +213,6 @@ public class RocksDBColumnarKeyValueStorage
     }
   }
 
-  private byte[] getId(final SegmentIdentifier name) {
-    return name.getName().getBytes(StandardCharsets.UTF_8);
-  }
-
   private class RocksDbTransaction implements Transaction<ColumnFamilyHandle> {
 
     private final org.rocksdb.Transaction innerTx;

plugins/rocksdb/src/test/java/tech/pegasys/pantheon/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java

@@ -59,7 +59,7 @@ public class RocksDBKeyValueStorageFactoryTest {
         new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments);
 
     // Side effect is creation of the Metadata version file
-    storageFactory.create(() -> "block-chain", commonConfiguration, metricsSystem);
+    storageFactory.create(segment, commonConfiguration, metricsSystem);
 
     assertEquals(
         DEFAULT_VERSION,
@@ -91,7 +91,7 @@ public class RocksDBKeyValueStorageFactoryTest {
     final RocksDBKeyValueStorageFactory storageFactory =
         new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments);
 
-    storageFactory.create(() -> "block-chain", commonConfiguration, metricsSystem);
+    storageFactory.create(segment, commonConfiguration, metricsSystem);
 
     assertEquals(DEFAULT_VERSION, DatabaseMetadata.fromDirectory(tempDatabaseDir).getVersion());
     assertTrue(storageFactory.isSegmentIsolationSupported());
@@ -108,7 +108,7 @@ public class RocksDBKeyValueStorageFactoryTest {
     assertThatThrownBy(
             () ->
                 new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments)
-                    .create(() -> "segment-does-not-matter", commonConfiguration, metricsSystem))
+                    .create(segment, commonConfiguration, metricsSystem))
         .isInstanceOf(StorageException.class);
   }
 
@@ -125,7 +125,7 @@ public class RocksDBKeyValueStorageFactoryTest {
     assertThatThrownBy(
             () ->
                 new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments)
-                    .create(() -> "bad-version", commonConfiguration, metricsSystem))
+                    .create(segment, commonConfiguration, metricsSystem))
         .isInstanceOf(IllegalStateException.class);
 
     final String badValue = "{\"version\":\"iomedae\"}";
@@ -135,7 +135,7 @@ public class RocksDBKeyValueStorageFactoryTest {
     assertThatThrownBy(
             () ->
                 new RocksDBKeyValueStorageFactory(() -> rocksDbConfiguration, segments)
-                    .create(() -> "bad-value", commonConfiguration, metricsSystem))
+                    .create(segment, commonConfiguration, metricsSystem))
         .isInstanceOf(IllegalStateException.class);
   }
 }

plugins/rocksdb/src/test/java/tech/pegasys/pantheon/plugin/services/storage/rocksdb/unsegmented/RocksDBColumnarKeyValueStorageTest.java

@@ -89,9 +89,11 @@ public class RocksDBColumnarKeyValueStorageTest extends AbstractKeyValueStorageT
     FOO(new byte[] {1}),
     BAR(new byte[] {2});
 
+    private final byte[] id;
     private final String nameAsUtf8;
 
     TestSegment(final byte[] id) {
+      this.id = id;
       this.nameAsUtf8 = new String(id, StandardCharsets.UTF_8);
     }
 
@@ -99,6 +101,11 @@ public class RocksDBColumnarKeyValueStorageTest extends AbstractKeyValueStorageT
     public String getName() {
       return nameAsUtf8;
     }
+
+    @Override
+    public byte[] getId() {
+      return id;
+    }
   }
 
   private SegmentedKeyValueStorage<ColumnFamilyHandle> createSegmentedStore() throws Exception {