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

optimism: set chain-ID of pre-bedrock relayed txs #99

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

protolambda
Copy link
Collaborator

Description

Pre-bedrock relayed txs do not have a signature. When served in the RPC the chain ID should not be computed from the signature values, but rather be set from the chain config, to ensure it's stated correctly in the RPC result. The legacy l2geth didn't include the chain ID in the response at all, but we do want it to be compatible with the latest API spec that includes it.

Tests

Fixes an API result of legacy data, may need a new testing strategy for l2geth data, but this change is simple enough that we can test-run it with:

cast rpc --rpc-url=http://127.0.0.1:8745 eth_getTransactionByHash 0xe1fb9e9031d966b707e154e228ad135956df7d3971bc40a6906848a81a8a98dd

Which previously recorded "chainId":"0x7fffffffffffffee" but should now be "chainId":"0xa"

Metadata

CLI-4072

TODOs

@pcw109550
Copy link
Contributor

FYI, here is op-erigon's fix: testinprod-io/op-erigon#14 for this exact same issue.

@ajsutton ajsutton merged commit 09ade3d into optimism Jun 7, 2023
@ajsutton ajsutton deleted the relayed-tx-chain-id-api-fix branch June 7, 2023 22:22
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

Successfully merging this pull request may close these issues.

3 participants