-
Notifications
You must be signed in to change notification settings - Fork 68
update go-ethereum & tidy up #839
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
Conversation
WalkthroughThis PR updates the go-ethereum dependency version across the project and regenerates L2TokenRegistry bindings to reflect a modified contract that now returns a struct containing token info and a HasBalanceSlot boolean flag. Downstream code in token-price-oracle is adjusted to access the nested Info field. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
bindings/go.sumis excluded by!**/*.sumcontracts/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumnode/go.sumis excluded by!**/*.sumops/l2-genesis/go.sumis excluded by!**/*.sumops/tools/go.sumis excluded by!**/*.sumoracle/go.sumis excluded by!**/*.sumtoken-price-oracle/go.sumis excluded by!**/*.sumtx-submitter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
Makefile(1 hunks)bindings/bin/l2tokenregistry_deployed.hex(1 hunks)bindings/bindings/l2tokenregistry.go(2 hunks)bindings/bindings/l2tokenregistry_more.go(1 hunks)bindings/go.mod(1 hunks)contracts/go.mod(1 hunks)go-ethereum(1 hunks)node/go.mod(1 hunks)ops/l2-genesis/go.mod(1 hunks)ops/tools/go.mod(1 hunks)oracle/go.mod(1 hunks)token-price-oracle/client/bitget_sdk_test.go(0 hunks)token-price-oracle/go.mod(1 hunks)token-price-oracle/updater/token_price.go(3 hunks)tx-submitter/go.mod(1 hunks)
💤 Files with no reviewable changes (1)
- token-price-oracle/client/bitget_sdk_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: check
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (15)
go-ethereum (1)
1-1:⚠️ Review context incomplete: verify consistency of dependency changes across the codebase.The provided code shows only the go-ethereum submodule commit update, but the AI summary indicates significant downstream changes:
- L2TokenRegistry bindings regenerated with modified return types (now returning a struct with
InfoandHasBalanceSlot)- Multiple
go.modfiles updated with the new go-ethereum versiontoken-price-oracle/updater/token_price.gomodified to accesstokenInfo.InfofieldThese related files are not provided for review, making it difficult to assess whether the dependency change is correctly integrated throughout the codebase.
Please verify:
- That the new go-ethereum commit
03910bc750a2301be4c1410b9f3c4d3741df251eis a legitimate upstream commit- That all
go.modfiles are consistently updated to the new version- That bindings regeneration is complete and correct (especially L2TokenRegistry struct changes)
- That
token-price-oracleand other downstream code correctly handle the new struct return type forGetTokenInfoTo confirm this change is sound, please review the complete PR diff and ensure:
- All files mentioned in the AI summary (Makefile, bindings/, token-price-oracle/) are included and consistent
- The struct field access changes (
tokenInfo.Info) are applied everywhere the return type changed- No other code relies on the previous flat return type for
GetTokenInfotoken-price-oracle/go.mod (1)
11-11: LGTM!The go-ethereum dependency update is consistent with other modules in this PR.
bindings/go.mod (1)
7-7: LGTM!The dependency version update is consistent with the broader PR changes. According to the AI summary, the L2TokenRegistry bindings have already been regenerated to match this version.
ops/l2-genesis/go.mod (1)
9-9: LGTM!The go-ethereum dependency update is consistent with other modules in this PR.
contracts/go.mod (1)
9-9: LGTM!The go-ethereum dependency update is consistent with other modules in this PR.
ops/tools/go.mod (1)
8-8: LGTM!The go-ethereum dependency update is consistent with other modules in this PR.
tx-submitter/go.mod (1)
12-12: LGTM!The go-ethereum dependency update is consistent with other modules in this PR.
Makefile (1)
2-3: LGTM! Makefile variables updated consistently.The Ethereum submodule reference has been updated to use the
morph-v2.1.0tag, and the target version aligns with the pseudo-version used across all go.mod files. The submodule checkout logic (line 47) will use this tag to ensure the correct version is checked out.token-price-oracle/updater/token_price.go (2)
271-291: Correct adaptation to the new binding structure.The code properly accesses the nested
Infofield to retrieve token attributes (IsActive,TokenAddress,Decimals,Scale) following the contract binding change whereGetTokenInfonow returns a composite struct.Note that the
HasBalanceSlotfield from the new return type is not used in this function. If this field is relevant for future use cases, consider adding a TODO comment or documenting why it's intentionally omitted here.
433-441: No functional changes in this segment.These are non-functional whitespace changes. The
shouldUpdatePricelogic remains unchanged.bindings/bin/l2tokenregistry_deployed.hex (1)
1-1: Auto-generated deployed bytecode.This file contains the compiled contract bytecode which should be regenerated whenever the contract source changes. Ensure this was generated using the same compilation pipeline and compiler version as other contract artifacts.
bindings/bindings/l2tokenregistry_more.go (1)
16-16: Auto-generated binding updated correctly.The
L2TokenRegistryDeployedBinconstant is regenerated to match the updated contract bytecode. The "Code generated - DO NOT EDIT" header is correctly in place.bindings/bindings/l2tokenregistry.go (2)
439-480: Auto-generated binding correctly reflects the new contract interface.The
GetTokenInfomethod now returns a composite struct containing:
Info: TheIL2TokenRegistryTokenInfostruct with token detailsHasBalanceSlot: A boolean flag indicating whether a balance slot is configuredThis is auto-generated code from
abigen. The changes are consistent across all method variants (Caller, Session, CallerSession).
49-50: ABI and Bin metadata updated.The ABI now includes the updated
getTokenInfosignature returning(TokenInfo info, bool hasBalanceSlot)along with new error definitions. This is auto-generated metadata that aligns with the contract changes.node/go.mod (1)
14-14: Dependency update correctly applied across all modules.The go-ethereum dependency bump to
v1.10.14-0.20251219060125-03910bc750a2has been consistently updated across all 8 go.mod files and verified in corresponding go.sum files with matching hash values. The workspace is properly synchronized.
| github.com/go-kit/kit v0.12.0 | ||
| github.com/morph-l2/externalsign v0.3.1 | ||
| github.com/morph-l2/go-ethereum v1.10.14-0.20251203083507-49fa27bcab24 | ||
| github.com/morph-l2/go-ethereum v1.10.14-0.20251219060125-03910bc750a2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verification script is broken and will not execute properly.
The embedded script concatenates the commit hash with extraneous characters, resulting in an invalid argument to git cat-file. To verify the commit, use: git rev-parse 03910bc750a2 or check the morph-l2/go-ethereum repository directly. The pseudo-version format itself is valid and follows Go's semantic versioning for commits on v1.10.14.
🤖 Prompt for AI Agents
In oracle/go.mod at line 10 the pseudo-version entry contains a commit hash used
by a verification script that currently passes extraneous characters to git
cat-file; update the verification step to extract the clean commit hash (e.g.,
run git rev-parse 03910bc750a2 or strip non-hex characters) before calling git
cat-file, or change the script to derive the commit via git rev-parse on the
pseudo-version suffix so the exact commit id "03910bc750a2" is supplied to git
cat-file for verification.
Summary by CodeRabbit
Chores
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.