Skip to content

op-signer, op-node: fix block signer utils#14478

Merged
protolambda merged 2 commits intodevelopfrom
fix-block-signer-utils
Feb 21, 2025
Merged

op-signer, op-node: fix block signer utils#14478
protolambda merged 2 commits intodevelopfrom
fix-block-signer-utils

Conversation

@protolambda
Copy link
Copy Markdown
Contributor

@protolambda protolambda commented Feb 21, 2025 •
edited
Loading

Description

Fixes:

  • improve RPC typing in backwards compatible way
    • The full block payload was being sent with every request, even though only the payload-hash was being used. Likely caused by a Go language mistake/footgun with the missing json tag.
  • improve typing and API to be more reusable outside of p2p package
  • cleanup unused op-signer RPC client struct attribute, log the server version instead

New:

  • V2 block-args type, with more Go-native and less error-prone typing. No weird JSON encoding to signer server anymore with V2. This helps others implement their own signer server also.
  • Client binding for the V2 endpoint

Also see infra-repo PR for the server-side improvements.
ethereum-optimism/infra#203

Tests

  • Update block signer tests
  • Add tests for type encoding
  • Add test to ensure the signing-hash is still computed the same

Note: we are not using eth.ChainID type everywhere yet, so some tests still need to convert from big-int to chain-id, but that will change once we improve the rollup-config typing.

TODO: I'd like to e2e test this, but couldn't yet get the op-signer server running. Integration into kurtosis of more of the infra would be nice.

Additional context

Besides more efficiency and better typing, this also helps with re-use the block-signer code. This is important for the test-sequencer work.

Metadata

@protolambda protolambda added A-op-node Area: op-node A-op-service Area: op-service A-op-signer Area: op-signer labels Feb 21, 2025
@protolambda protolambda requested review from a team as code owners February 21, 2025 01:19
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 21, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 67.46988% with 27 lines in your changes missing coverage. Please review.

Project coverage is 46.45%. Comparing base (426176d) to head (93818dc).
Report is 9 commits behind head on develop.

Files with missing lines Patch % Lines
op-service/signer/blockpayload_args.go 63.88% 9 Missing and 4 partials ⚠️
op-service/signer/client.go 0.00% 7 Missing ⚠️
op-service/eth/types.go 70.00% 6 Missing ⚠️
op-node/p2p/gossip.go 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (426176d) and HEAD (93818dc). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (426176d) HEAD (93818dc)
2 1
Additional details and impacted files 
@@             Coverage Diff              @@
##           develop   #14478       +/-   ##
============================================
- Coverage    77.23%   46.45%   -30.79%     
============================================
  Files          178     1017      +839     
  Lines        10667    87908    +77241     
============================================
+ Hits          8239    40840    +32601     
- Misses        2247    44027    +41780     
- Partials       181     3041     +2860
Flag Coverage Δ
cannon-go-tests-32 61.89% <ø> (ø)
cannon-go-tests-64 56.54% <ø> (ø)
contracts-bedrock-tests 94.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
op-node/p2p/signer.go 62.96% <100.00%> (ø)
op-node/p2p/gossip.go 56.45% <0.00%> (ø)
op-service/eth/types.go 43.40% <70.00%> (ø)
op-service/signer/client.go 0.00% <0.00%> (ø)
op-service/signer/blockpayload_args.go 63.79% <63.88%> (ø)

... and 842 files with indirect coverage changes

@protolambda protolambda mentioned this pull request Feb 21, 2025
Merged
sebastianst
sebastianst approved these changes Feb 21, 2025
View reviewed changes
@protolambda protolambda added this pull request to the merge queue Feb 21, 2025
Merged via the queue into develop with commit 7ceadae Feb 21, 2025
46 checks passed
@protolambda protolambda deleted the fix-block-signer-utils branch February 21, 2025 17:20
@bitwiseguy bitwiseguy mentioned this pull request Feb 21, 2025
Merged
Rjected pushed a commit to paradigmxyz/optimism that referenced this pull request Feb 25, 2025
@protolambda @Rjected
op-signer, op-node: fix block signer utils (ethereum-optimism#14478)
acae4cf 
* op-service: fix block signer utils

* op-service: improve block signer
@protolambda protolambda mentioned this pull request Feb 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sebastianst sebastianst sebastianst approved these changes

Assignees

No one assigned

Labels

A-op-node Area: op-node A-op-service Area: op-service A-op-signer Area: op-signer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@protolambda @sebastianst