OPB-02: Raise error on oversized chain id#3620
Conversation
|
|
This PR changes implementation code, but doesn't include a changeset. Did you forget to add one? |
maurelian
force-pushed
the
op-node/fillbytes-size-checks
branch
from
e69f994 to
caa4690
Compare
maurelian
force-pushed
the
op-node/fillbytes-size-checks
branch
from
caa4690 to
bd3d1b7
Compare
maurelian
force-pushed
the
op-node/fillbytes-size-checks
branch
from
bd3d1b7 to
80b79bb
Compare
protolambda
left a comment
protolambda
left a comment
There was a problem hiding this comment.
LGTM. Technically the FillBytes usages are all safe; chain ID is loaded exclusively from local configuration. And the other usages from OPB-2 are all through other safe code paths (deposit fields are from test only code-paths, and basefee size is checked by the json decoder). Still nice to catch any potential issues when the code is used wrong or called in unexpected ways however. Nice fix. The other OPB-02 fixes can also go in a separate PR.
|
The PR description needs to be updated. I also am re-running the e2e tests b/c they timed out. It seems like a flake given these changes, but we'll have to see. |
|
@trianglesphere seems to all be working now |
|
This PR has been added to the merge queue, and will be merged soon. |
|
This PR is next in line to be merged, and will be merged as soon as checks pass. |
1 similar comment
|
This PR is next in line to be merged, and will be merged as soon as checks pass. |
|
CircleCI errored out and didn't mark completed jobs as done, so they hung. Restarted the jobs to get this through. |
5 participants
Description
Fixes OPB-02.
This is a draft, POC for how I'd do this in signer.go. Two other locations still need fixing which I'll do in a subsequent PR once this is approved.
Tests
No new tests are added as it's unclear how this would actually be reachable, however it was decided to include it as "future proofing".