Error deleting validator

[pablomendezroyo]

Context

1. Import a new validator (without slashing protection) onto the web3signer using POST /eth/v1/keystores
2. Delete such validator using DELETE /eth/v1/keystores
3. Error

Note the validator was imported without slashing protection data, it still was without slashing protection data at the time of trying to be deleted (the client was still syncing)

The error

Error when deleting keystores

2022-03-17 07:53:01.155+00:00 | vert.x-eventloop-thread-1 | ERROR | RoutingContext | Unhandled exception in router
java.lang.RuntimeException: Error deleting keystores
	at tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresProcessor.process(DeleteKeystoresProcessor.java:75) ~[web3signer-core-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresHandler.handle(DeleteKeystoresHandler.java:65) ~[web3signer-core-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresHandler.handle(DeleteKeystoresHandler.java:33) ~[web3signer-core-develop.jar:21.10.5+27-g222755c]
	at io.vertx.ext.web.impl.BlockingHandlerDecorator.lambda$handle$0(BlockingHandlerDecorator.java:48) ~[vertx-web-4.2.3.jar:4.2.3]
	at io.vertx.core.impl.ContextImpl.lambda$null$0(ContextImpl.java:159) ~[vertx-core-4.2.3.jar:4.2.3]
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100) ~[vertx-core-4.2.3.jar:4.2.3]
	at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$1(ContextImpl.java:157) ~[vertx-core-4.2.3.jar:4.2.3]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) ~[?:?]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-common-4.1.72.Final.jar:4.1.72.Final]
	at java.lang.Thread.run(Unknown Source) [?:?]
Caused by: java.lang.RuntimeException: No genesis validators root for slashing protection data
	at tech.pegasys.web3signer.slashingprotection.interchange.InterchangeV5Exporter.startInterchangeExport(InterchangeV5Exporter.java:94) ~[web3signer-slashing-protection-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.slashingprotection.interchange.InterchangeV5Exporter$IncrementalInterchangeV5Exporter.<init>(InterchangeV5Exporter.java:267) ~[web3signer-slashing-protection-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.slashingprotection.interchange.InterchangeV5Exporter.createIncrementalExporter(InterchangeV5Exporter.java:75) ~[web3signer-slashing-protection-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.slashingprotection.interchange.InterchangeV5Manager.createIncrementalExporter(InterchangeV5Manager.java:86) ~[web3signer-slashing-protection-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.slashingprotection.DbSlashingProtection.createIncrementalExporter(DbSlashingProtection.java:175) ~[web3signer-slashing-protection-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresProcessor.createIncrementalExporter(DeleteKeystoresProcessor.java:98) ~[web3signer-core-develop.jar:21.10.5+27-g222755c]
	at tech.pegasys.web3signer.core.service.http.handlers.keymanager.delete.DeleteKeystoresProcessor.process(DeleteKeystoresProcessor.java:56) ~[web3signer-core-develop.jar:21.10.5+27-g222755c]
	... 10 more

[siladu]

Hi @pablomendezroyo, it looks like you've hit an edge case whereby the slashing protection database is completely empty and an export of slashing protection data is attempted. The genesis validators root is created for the first time upon signing something or when importing slashing protection data.

We will look at getting this tidied up, thanks for reporting.

Please note, the key manager API is still in early access and so it is expected it may contain bugs until we are satisfied it is production-ready.

[siladu]

Note for whoever implements this fix...

We discussed adding the genesis validators root into the database before the first object is signed, e.g. at startup or when a validator is registered. However the GVR currently comes in via the signing request from the validator client. Although we could determine it, we don't really want to introduce this logic into web3signer if we can help it.

Instead, we think outputting an empty data field in the interchange format is a better solution. We believe other clients have done something similar in other export scenarios.

The implementation in DeleteKeystoresProcessor could check the slashingProtection hasDataToExport and might look something like...

  private IncrementalExporter createIncrementalExporter(final ByteArrayOutputStream outputStream) {
    return slashingProtection.isPresent() && slashingProtection.hasDataToExport()
        ? slashingProtection.get().createIncrementalExporter(outputStream)
        // Using no-op exporter instead of returning an optional so can use try with for closing
        : new NoOpIncrementalExporter();
  }

[siladu]

The problem can be recreated by adding this (failing) test in DeleteKeystoresAcceptanceTest...

@Test
public void deletingExistingKeyWithNoSlashingProtectionReturnDeleted() throws URISyntaxException {
  createBlsKey("eth2/bls_keystore.json", "somepassword");
  setupSignerWithKeyManagerApi(false);
  callDeleteKeystores(composeRequestBody())
      .then()
      .contentType(ContentType.JSON)
      .assertThat()
      .statusCode(200)
      .body("data[0].status", is("deleted"))
      .and()
      .body("slashing_protection", is(emptySlashingData));
}