search

Consensys / web3signer

Web3Signer is an open-source signing service capable of signing on multiple platforms (Ethereum1 and 2, Filecoin) using private keys stored in an external vault, or encrypted on a disk.
https://docs.web3signer.consensys.net/
Apache License 2.0
200 stars 76 forks source link

Support for Optimism op-signer #999

Open quickchase opened 5 months ago

quickchase commented 5 months ago

This is a feature request to support using web3signer as a remote signer for the Optimism op-stack / op-signer

The primary source code for the op-signer is located here and their services can connect to a remote signer, source code here

In testing, we found that there are at least two primary hurdles in using web3signer as a remote signer for OP-Stack (Sequencer, Bather, and Proposer).

  1. web3signer does not currently support an input field
  2. op-service requires a health check endpoint exposed at /healthz

With regards to number 1 above, we found that a simple addition of input alongside data in the following files allows web3sign to correctly sign transactions for the batcher and proposer:

core/service/jsonrpc/EthSendTransactionJsonParameters.java
core/service/jsonrpc/handlers/sendtransaction/transaction/EthTransaction.java

Related issue: https://github.com/ethereum/go-ethereum/issues/15628 (also linked in code above)

We were able to get web3signer to sign valid batcher and proposer transactions, so I don't THINK this would be a huge lift...

usmansaleem commented 5 months ago

Acknowledged. Let us discuss and prioritize this feature with the team.

usmansaleem commented 5 months ago

@non-fungible-nelson @jframe FYI.

non-fungible-nelson commented 5 months ago

@quickchase what is Optimism's appetite to open a PR? We can provide code-reviews and insights as well. Otherwise, we may have to backlog this issue with an uncertain timeline.

non-fungible-nelson commented 5 months ago

The health check - @usmansaleem we should discuss.

usmansaleem commented 5 months ago

@quickchase @non-fungible-nelson Web3Signer currently exposes /healthcheck which uses Vertx Healthcheck to generate the statuses. https://vertx.io/docs/vertx-health-check/java/. /healthz seems to be kubernetes preferred endpoint. We can expose an alias for /healthcheck as /healthz if https://github.com/ethereum-optimism/optimism/blob/develop/op-node/node/server.go#L97 is able to handle the output of /healthcheck that we have? (My Go is a bit Rusty, no-pun-intended)

usmansaleem commented 5 months ago

vertx dev hint: router.get("/health*") should be able to handle both usecases.

quickchase commented 5 months ago

Actually, I think I misspoke about the /healthz endpoint (though it would be a nice to have for docker deployments) and the actual problem is here:

https://github.com/ethereum-optimism/optimism/blob/7ee8cc86400ea1757f4851ca67e32b0f4362cdb7/op-service/signer/client.go#L89

They're sending a JSON-RPC method of health_status and when you send that to their nodes it returns:

{"jsonrpc":"2.0","id":1,"result":"OK"}