Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Optimism op-signer #999

Open
quickchase opened this issue Jun 4, 2024 · 7 comments
Open

Support for Optimism op-signer #999

quickchase opened this issue Jun 4, 2024 · 7 comments

Comments

@quickchase
Copy link

quickchase commented Jun 4, 2024

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: ethereum/go-ethereum#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
Copy link
Contributor

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

@usmansaleem
Copy link
Contributor

@non-fungible-nelson @jframe FYI.

@non-fungible-nelson
Copy link
Contributor

non-fungible-nelson commented Jun 5, 2024

@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
Copy link
Contributor

The health check - @usmansaleem we should discuss.

@usmansaleem
Copy link
Contributor

@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
Copy link
Contributor

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

@quickchase
Copy link
Author

quickchase commented Jun 6, 2024

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"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants