[#537] Use empty data Incremental Exporter when gvr does not exist (#574

) * Fixes the issue of delete API where validator was imported without slashing protection data. * Code refactoring to build/use common JsonBuilder for Incremental import/export API package
Consensys · May 26, 2022 · f34a725 · f34a725
1 parent b1023e3
commit f34a725
Show file tree

Hide file tree

Showing 23 changed files with 328 additions and 149 deletions.
diff --git a/...src/test/java/tech/pegasys/web3signer/tests/keymanager/DeleteKeystoresAcceptanceTest.java b/...src/test/java/tech/pegasys/web3signer/tests/keymanager/DeleteKeystoresAcceptanceTest.java
@@ -14,6 +14,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.CoreMatchers.is;
+import static tech.pegasys.web3signer.tests.keymanager.SlashingProtectionDataChoice.WITHOUT_SLASHING_PROTECTION_DATA;
+import static tech.pegasys.web3signer.tests.keymanager.SlashingProtectionDataChoice.WITH_SLASHING_PROTECTION_DATA;
 
 import tech.pegasys.web3signer.core.service.http.ArtifactType;
 import tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresRequestBody;
@@ -35,7 +37,6 @@
 import org.junit.jupiter.api.Test;
 
 public class DeleteKeystoresAcceptanceTest extends KeyManagerTestBase {
-
   private static final String BLS_PRIVATE_KEY_1 =
       "3ee2224386c82ffea477e2adf28a2929f5c349165a4196158c7f3a2ecca40f35";
 
@@ -66,9 +67,12 @@ public class DeleteKeystoresAcceptanceTest extends KeyManagerTestBase {
           + "\"data\" : [ ]\n"
           + "}";
 
+  private static final String EMPTY_SLASHING_DATA_WITHOUT_GVR =
+      "{\"metadata\" : {\n  \"interchange_format_version\" : \"5\"\n},\n\"data\" : [ ]\n}";
+
   @Test
   public void invalidRequestBodyReturnsError() throws URISyntaxException {
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
     final Response response = callDeleteKeystores("{\"invalid\": \"json body\"}");
     response.then().assertThat().statusCode(400);
   }
@@ -79,7 +83,7 @@ public void deletingExistingKeyWithNoSlashingProtectionDataTwiceReturnsNotFound(
     final String pubKey =
         "0xa46bf94016af71e55ca0518fe6a8bd3852e01b3f959780a4faf3bbe461ac553c0a83f232cc5f2a4b827d8d3455b706e4";
     createBlsKey("eth2/bls_keystore_2.json", "otherpassword");
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
     callDeleteKeystores(composeRequestBody(pubKey))
         .then()
         .contentType(ContentType.JSON)
@@ -103,7 +107,7 @@ public void deletingExistingKeyWithNoSlashingProtectionDataTwiceReturnsNotFound(
   @Test
   public void deletingExistingKeyReturnDeleted() throws URISyntaxException {
     createBlsKey("eth2/bls_keystore.json", "somepassword");
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
     callDeleteKeystores(composeRequestBody())
         .then()
         .contentType(ContentType.JSON)
@@ -117,7 +121,7 @@ public void deletingExistingKeyReturnDeleted() throws URISyntaxException {
   @Test
   public void deletingExistingTwiceReturnsNotActive() throws URISyntaxException {
     createBlsKey("eth2/bls_keystore.json", "somepassword");
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
     callDeleteKeystores(composeRequestBody())
         .then()
         .contentType(ContentType.JSON)
@@ -143,7 +147,7 @@ public void deletingExistingTwiceReturnsNotActive() throws URISyntaxException {
   public void deletingRemovesSignerFromActiveSigners() throws URISyntaxException {
     final String firstPubkey = createBlsKey("eth2/bls_keystore.json", "somepassword");
     final String secondPubKey = createBlsKey("eth2/bls_keystore_2.json", "otherpassword");
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
 
     callListKeys()
         .then()
@@ -176,7 +180,7 @@ public void deletingRemovesSignerFromActiveSigners() throws URISyntaxException {
   @Test
   public void deletingReadOnlyKeyReturnError() throws URISyntaxException {
     final String readOnlyPubkey = createRawPrivateKeyFile(BLS_PRIVATE_KEY_1);
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
     callDeleteKeystores(composeRequestBody(readOnlyPubkey))
         .then()
         .contentType(ContentType.JSON)
@@ -192,7 +196,7 @@ public void deletingDisablesSigningForAllWeb3Signers()
       throws URISyntaxException, JsonProcessingException {
     final String firstPubkey = createBlsKey("eth2/bls_keystore.json", "somepassword");
     final String secondPubKey = createBlsKey("eth2/bls_keystore_2.json", "otherpassword");
-    setupSignerWithKeyManagerApi(true);
+    setupSignerWithKeyManagerApi(WITH_SLASHING_PROTECTION_DATA);
 
     final SignerConfiguration signer2Configuration =
         new SignerConfigurationBuilder()
@@ -257,6 +261,23 @@ public void testRequestBodyParsing() throws IOException {
             "0x98d083489b3b06b8740da2dfec5cc3c01b2086363fe023a9d7dc1f907633b1ff11f7b99b19e0533e969862270061d884");
   }
 
+  @Test
+  public void deletingExistingKeyWithNoSlashingProtectionReturnDeleted() throws URISyntaxException {
+
+    createBlsKey("eth2/bls_keystore.json", "somepassword");
+
+    setupSignerWithKeyManagerApi(WITHOUT_SLASHING_PROTECTION_DATA);
+
+    callDeleteKeystores(composeRequestBody())
+        .then()
+        .contentType(ContentType.JSON)
+        .assertThat()
+        .statusCode(200)
+        .body("data[0].status", is("deleted"))
+        .and()
+        .body("slashing_protection", is(EMPTY_SLASHING_DATA_WITHOUT_GVR));
+  }
+
   private String composeRequestBody() {
     return composeRequestBody(
         "0x98d083489b3b06b8740da2dfec5cc3c01b2086363fe023a9d7dc1f907633b1ff11f7b99b19e0533e969862270061d884");

diff --git a/...ance-tests/src/test/java/tech/pegasys/web3signer/tests/keymanager/KeyManagerTestBase.java b/...ance-tests/src/test/java/tech/pegasys/web3signer/tests/keymanager/KeyManagerTestBase.java
@@ -16,6 +16,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static tech.pegasys.web3signer.dsl.utils.WaitUtils.waitFor;
 import static tech.pegasys.web3signer.signing.KeyType.BLS;
+import static tech.pegasys.web3signer.tests.keymanager.SlashingProtectionDataChoice.WITHOUT_SLASHING_PROTECTION_DATA;
 
 import tech.pegasys.signers.bls.keystore.KeyStore;
 import tech.pegasys.signers.bls.keystore.KeyStoreLoader;
@@ -53,11 +54,11 @@ public class KeyManagerTestBase extends AcceptanceTestBase {
   @TempDir protected Path testDirectory;
 
   protected void setupSignerWithKeyManagerApi() throws URISyntaxException {
-    setupSignerWithKeyManagerApi(false);
+    setupSignerWithKeyManagerApi(WITHOUT_SLASHING_PROTECTION_DATA);
   }
 
-  protected void setupSignerWithKeyManagerApi(final boolean insertSlashingProtectionData)
-      throws URISyntaxException {
+  protected void setupSignerWithKeyManagerApi(
+      final SlashingProtectionDataChoice slashingProtectionDataChoice) throws URISyntaxException {
     final SignerConfigurationBuilder builder = new SignerConfigurationBuilder();
     builder
         .withKeyStoreDirectory(testDirectory)
@@ -70,22 +71,24 @@ protected void setupSignerWithKeyManagerApi(final boolean insertSlashingProtecti
         .withKeyManagerApiEnabled(true);
     startSigner(builder.build());
 
-    if (insertSlashingProtectionData) {
-      final SignerConfigurationBuilder importBuilder = new SignerConfigurationBuilder();
-      importBuilder
-          .withMode("eth2")
-          .withSlashingEnabled(true)
-          .withSlashingProtectionDbUrl(signer.getSlashingDbUrl())
-          .withSlashingProtectionDbUsername(DB_USERNAME)
-          .withSlashingProtectionDbPassword(DB_PASSWORD)
-          .withKeyStoreDirectory(testDirectory)
-          .withSlashingImportPath(getResourcePath("slashing/slashingImport_two_entries.json"))
-          .withHttpPort(12345); // prevent wait for Ports file in AT
-
-      final Signer importSigner = new Signer(importBuilder.build(), null);
-      importSigner.start();
-      waitFor(() -> assertThat(importSigner.isRunning()).isFalse());
+    if (slashingProtectionDataChoice == WITHOUT_SLASHING_PROTECTION_DATA) {
+      return;
     }
+
+    final SignerConfigurationBuilder importBuilder = new SignerConfigurationBuilder();
+    importBuilder
+        .withMode("eth2")
+        .withSlashingEnabled(true)
+        .withSlashingProtectionDbUrl(signer.getSlashingDbUrl())
+        .withSlashingProtectionDbUsername(DB_USERNAME)
+        .withSlashingProtectionDbPassword(DB_PASSWORD)
+        .withKeyStoreDirectory(testDirectory)
+        .withSlashingImportPath(getResourcePath("slashing/slashingImport_two_entries.json"))
+        .withHttpPort(12345); // prevent wait for Ports file in AT
+
+    final Signer importSigner = new Signer(importBuilder.build(), null);
+    importSigner.start();
+    waitFor(() -> assertThat(importSigner.isRunning()).isFalse());
   }
 
   public Response callListKeys() {

diff --git a/.../src/test/java/tech/pegasys/web3signer/tests/keymanager/SlashingProtectionDataChoice.java b/.../src/test/java/tech/pegasys/web3signer/tests/keymanager/SlashingProtectionDataChoice.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2022 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.
+ */
+package tech.pegasys.web3signer.tests.keymanager;
+
+public enum SlashingProtectionDataChoice {
+  WITH_SLASHING_PROTECTION_DATA,
+  WITHOUT_SLASHING_PROTECTION_DATA
+}
diff --git a/...ts/src/test/java/tech/pegasys/web3signer/tests/slashing/SlashingExportAcceptanceTest.java b/...ts/src/test/java/tech/pegasys/web3signer/tests/slashing/SlashingExportAcceptanceTest.java
@@ -25,7 +25,7 @@
 import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder;
 import tech.pegasys.web3signer.dsl.utils.MetadataFileHelpers;
 import tech.pegasys.web3signer.signing.KeyType;
-import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeModule;
+import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeJsonProvider;
 import tech.pegasys.web3signer.slashingprotection.interchange.model.SignedAttestation;
 import tech.pegasys.web3signer.tests.AcceptanceTestBase;
 
@@ -34,7 +34,6 @@
 import java.util.List;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.json.JsonMapper;
 import dsl.InterchangeV5Format;
 import dsl.SignedArtifacts;
 import io.restassured.response.Response;
@@ -97,7 +96,7 @@ void slashingDataIsExported(@TempDir Path testDirectory) throws IOException {
     exportSigner.start();
     waitFor(() -> assertThat(exportSigner.isRunning()).isFalse());
 
-    final ObjectMapper mapper = JsonMapper.builder().addModule(new InterchangeModule()).build();
+    final ObjectMapper mapper = new InterchangeJsonProvider().getJsonMapper();
 
     final InterchangeV5Format mappedData =
         mapper.readValue(exportFile.toFile(), InterchangeV5Format.class);

diff --git a/...ts/src/test/java/tech/pegasys/web3signer/tests/slashing/SlashingImportAcceptanceTest.java b/...ts/src/test/java/tech/pegasys/web3signer/tests/slashing/SlashingImportAcceptanceTest.java
@@ -21,7 +21,7 @@
 import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder;
 import tech.pegasys.web3signer.dsl.utils.MetadataFileHelpers;
 import tech.pegasys.web3signer.signing.KeyType;
-import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeModule;
+import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeJsonProvider;
 import tech.pegasys.web3signer.slashingprotection.interchange.model.SignedAttestation;
 import tech.pegasys.web3signer.slashingprotection.interchange.model.SignedBlock;
 import tech.pegasys.web3signer.tests.AcceptanceTestBase;
@@ -34,8 +34,6 @@
 import java.util.List;
 import java.util.Map;
 
-import com.fasterxml.jackson.databind.MapperFeature;
-import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.google.common.io.Resources;
 import dsl.InterchangeV5Format;
 import dsl.SignedArtifacts;
@@ -51,10 +49,7 @@ public class SlashingImportAcceptanceTest extends AcceptanceTestBase {
   public static final String DB_PASSWORD = "postgres";
 
   private static final com.fasterxml.jackson.databind.ObjectMapper objectMapper =
-      JsonMapper.builder()
-          .addModule(new InterchangeModule())
-          .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS)
-          .build();
+      new InterchangeJsonProvider().getJsonMapper();
 
   protected final BLSKeyPair keyPair = BLSTestUtil.randomKeyPair(0);
 

diff --git a/...sys/web3signer/core/service/http/handlers/keymanager/delete/DeleteKeystoresProcessor.java b/...sys/web3signer/core/service/http/handlers/keymanager/delete/DeleteKeystoresProcessor.java
@@ -18,10 +18,12 @@
 import tech.pegasys.web3signer.signing.ValidatorManager;
 import tech.pegasys.web3signer.signing.util.IdentifierUtils;
 import tech.pegasys.web3signer.slashingprotection.SlashingProtection;
+import tech.pegasys.web3signer.slashingprotection.interchange.EmptyDataIncrementalInterchangeV5Exporter;
 import tech.pegasys.web3signer.slashingprotection.interchange.IncrementalExporter;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
@@ -93,11 +95,12 @@ private void setAllResultsToError(
     results.addAll(errorResults);
   }
 
-  private IncrementalExporter createIncrementalExporter(final ByteArrayOutputStream outputStream) {
-    return slashingProtection.isPresent()
-        ? slashingProtection.get().createIncrementalExporter(outputStream)
-        // Using no-op exporter instead of returning an optional so can use try with for closing
-        : new NoOpIncrementalExporter();
+  private IncrementalExporter createIncrementalExporter(final OutputStream outputStream) {
+    return slashingProtection
+        .map(sp -> sp.createIncrementalExporter(outputStream))
+        // nothing to export if slashing protection is not available, hence use no-op exporter so
+        // that outputStream can be closed nicely.
+        .orElseGet(() -> new EmptyDataIncrementalInterchangeV5Exporter(outputStream));
   }
 
   private DeleteKeystoreResult processKeyToDelete(
@@ -151,15 +154,4 @@ private DeleteKeystoreResult attemptToExportWithSlashingData(
           DeleteKeystoreStatus.ERROR, "Error exporting slashing data: " + e.getMessage());
     }
   }
-
-  private static class NoOpIncrementalExporter implements IncrementalExporter {
-    @Override
-    public void export(final String publicKey) {}
-
-    @Override
-    public void finalise() {}
-
-    @Override
-    public void close() throws Exception {}
-  }
 }
diff --git a/...cetests/src/test/java/tech/pegasys/web3signer/slashingprotection/ReferenceTestRunner.java b/...cetests/src/test/java/tech/pegasys/web3signer/slashingprotection/ReferenceTestRunner.java
@@ -18,7 +18,7 @@
 
 import tech.pegasys.web3signer.slashingprotection.dao.MetadataDao;
 import tech.pegasys.web3signer.slashingprotection.dao.ValidatorsDao;
-import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeModule;
+import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeJsonProvider;
 import tech.pegasys.web3signer.slashingprotection.model.AttestationTestModel;
 import tech.pegasys.web3signer.slashingprotection.model.BlockTestModel;
 import tech.pegasys.web3signer.slashingprotection.model.Step;
@@ -34,9 +34,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.google.common.io.Resources;
 import db.DatabaseUtil;
 import db.DatabaseUtil.TestDatabaseInfo;
@@ -58,11 +56,7 @@ public class ReferenceTestRunner {
   private static final String USERNAME = "postgres";
   private static final String PASSWORD = "postgres";
 
-  private static final ObjectMapper objectMapper =
-      JsonMapper.builder()
-          .addModule(new InterchangeModule())
-          .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS)
-          .build();
+  private static final ObjectMapper objectMapper = new InterchangeJsonProvider().getJsonMapper();
   private final ValidatorsDao validators = new ValidatorsDao();
 
   private EmbeddedPostgres slashingDatabase;

diff --git a/...integration-test/java/tech/pegasys/web3signer/slashingprotection/IntegrationTestBase.java b/...integration-test/java/tech/pegasys/web3signer/slashingprotection/IntegrationTestBase.java
@@ -23,15 +23,14 @@
 import tech.pegasys.web3signer.slashingprotection.dao.SignedBlocksDao;
 import tech.pegasys.web3signer.slashingprotection.dao.SigningWatermark;
 import tech.pegasys.web3signer.slashingprotection.dao.ValidatorsDao;
-import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeModule;
+import tech.pegasys.web3signer.slashingprotection.interchange.InterchangeJsonProvider;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.util.List;
 import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.json.JsonMapper;
 import db.DatabaseUtil;
 import db.DatabaseUtil.TestDatabaseInfo;
 import dsl.TestSlashingProtectionParameters;
@@ -45,8 +44,7 @@
 
 public class IntegrationTestBase {
 
-  protected final ObjectMapper mapper =
-      JsonMapper.builder().addModule(new InterchangeModule()).build();
+  protected final ObjectMapper mapper = new InterchangeJsonProvider().getJsonMapper();
 
   protected final ValidatorsDao validators = new ValidatorsDao();
   protected final LowWatermarkDao lowWatermarkDao = new LowWatermarkDao();