-
Notifications
You must be signed in to change notification settings - Fork 1.4k
integration: use polygon services in initConsensusEngine (#16946) #16951
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
Fixes: #16026 --------- Co-authored-by: antonis19 <[email protected]>
dbcc723 to
bef31a6
Compare
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.
Pull Request Overview
This PR updates the integration command to use Polygon services when initializing the consensus engine, aligning with the main Ethereum backend architecture. It modifies the block reader setup to accept explicit parameters instead of node configuration objects and integrates proper Polygon bridge and Heimdall services.
- Refactors
setUpBlockReadertoSetUpBlockReaderwith explicit parameters - Integrates Polygon bridge and Heimdall services in the consensus engine initialization
- Removes duplicate service creation and consolidates initialization logic
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eth/backend.go | Updates function signature and call to use explicit dbReadConcurrency parameter |
| cmd/integration/commands/stages.go | Refactors to use SetUpBlockReader and integrates Polygon services in consensus engine |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| cfg.Dirs = dirs | ||
| allSn, borSn, agg, _, _, _, err := allSnapshots(ctx, db, logger) | ||
| dbReadConcurrency := runtime.GOMAXPROCS(-1) * 16 |
Copilot
AI
Sep 2, 2025
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 magic number 16 should be defined as a named constant to explain its purpose and make it easier to maintain.
| if !config.WithoutHeimdall { | ||
| heimdallClient = heimdall.NewHttpClient(config.HeimdallURL, logger, poshttp.WithApiVersioner(ctx)) | ||
| bridgeClient = bridge.NewHttpClient(config.HeimdallURL, logger, poshttp.WithApiVersioner(ctx)) | ||
| } else { | ||
| heimdallClient = heimdall.NewIdleClient(config.Miner) | ||
| bridgeClient = bridge.NewIdleClient() | ||
| } |
Copilot
AI
Sep 2, 2025
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.
This client initialization logic is duplicated from the main backend. Consider extracting this into a shared function to avoid code duplication.
…6951) cherry-pick of #16946 Fixes: #16026 --------- Co-authored-by: antonis19 <[email protected]>
…17095) Cherry-picks of : #16683 #16729 #16803 #16819 #16880 #16951 #17014 --------- Co-authored-by: antonis19 <[email protected]> Co-authored-by: Mark Holt <[email protected]>
…6951) cherry-pick of #16946 Fixes: #16026 --------- Co-authored-by: antonis19 <[email protected]>
cherry-pick of #16946
Fixes: #16026