fix(bridge-history+rollup-relayer): update da-codec to prevent zstd deadlock #1724
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughBumps da-codec and go-ethereum pseudo-versions across multiple module files and updates the release tag in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
bridge-history-api/go.sumis excluded by!**/*.sumrollup/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
bridge-history-api/go.mod(1 hunks)common/version/version.go(1 hunks)rollup/go.mod(1 hunks)
🔇 Additional comments (1)
common/version/version.go (1)
8-8: ✓ Version tag bump to v4.5.42 verified – no stale v4.5.41 references found
- Ran a global search across all source files (excluding
vendor/) and confirmed there are no remaining occurrences ofv4.5.41.- CI/release pipeline definitions and docs/Helm charts now consistently reference
v4.5.42.- No logic changes; safe to publish images and binaries under
v4.5.42.
…scroll into fix-update-da-codec
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1724 +/- ##
===========================================
+ Coverage 37.48% 37.53% +0.04%
===========================================
Files 243 243
Lines 20500 20500
===========================================
+ Hits 7685 7694 +9
+ Misses 12006 11990 -16
- Partials 809 816 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
common/go.mod (1)
187-187: da-codec upgraded (indirect): consider a workspace-level replace to hard-pinThis ensures every module resolves the same commit and prevents accidental downgrades via transitive deps. Keep it until an official, tagged release with the fix is cut.
If you maintain a top-level go.work or root go.mod that all modules use, add a workspace-level replace like:
replace github.com/scroll-tech/da-codec => github.com/scroll-tech/da-codec v0.1.3-0.20250822100821-0f1ad362e5b9Then run go mod tidy in each module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
common/go.sumis excluded by!**/*.sumcoordinator/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumrollup/go.sumis excluded by!**/*.sumtests/integration-test/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
common/go.mod(2 hunks)coordinator/go.mod(1 hunks)tests/integration-test/go.mod(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- coordinator/go.mod
⏰ 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). (11)
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: test
- GitHub Check: check
🔇 Additional comments (2)
tests/integration-test/go.mod (1)
8-8: Uniform da-codec pseudo-version across all modules confirmedAll
go.modfiles referencegit.colasdn.top/scroll-tech/da-codec v0.1.3-0.20250822100821-0f1ad362e5b9. No stragglers detected—ready to merge.common/go.mod (1)
18-18: go-ethereum bump looks good; please smoke-test key modulesAll tests are passing and we’ve enumerated every Go file importing our fork of go-ethereum. To guard against subtle API or behavior changes, please perform a quick smoke test on the following areas:
Bridge-history-api
• L1/L2 RPC fetchers (internal/controller/fetcher/l1_fetcher.go, l2_fetcher.go)
• Event parsers (internal/logic/l1_event_parser.go, l2_event_parser.go)
• ABI bindings (abi/backend_abi.go)Common utilities
• RPC helpers (common/utils/rpc.go and rpc_test.go)
• Trie operations (common/utils/withdraw_trie.go and withdraw_trie_test.go)
• Crypto/keystore (common/utils/keystore.go)
• Gas tracing (common/utils/trace_gas.go and trace_gas_test.go)Rollup components
• Transaction construction & signing (internal/controller/sender/estimategas.go, sender.go, transaction_signer.go)
• Chain watchers (internal/controller/watcher/l1_watcher.go, l2_watcher.go)
• ABI interactions (rollup/abi/bridge_abi.go, validium_abi.go)Coordinator & other RPC clients
• Any modules under coordinator/ and database/cmd that call into go-ethereumWith these quick manual checks completed, we can consider the upgrade safe to merge.
There was a problem hiding this comment.
We just need to be careful to not override the bridge history hash function somehow with the l2geth change
5 participants
Purpose or design rationale of this PR
Describe your change. Make sure to answer these three questions: What does this PR do? Why does it do it? How does it do it?
This PR aims to fix the zstd deadlock issue of bridge-history and rollup-relayer by updating the da-codec version.
da-codec pr: scroll-tech/da-codec#61
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit