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

feat: calculate network id #212

Open
wants to merge 6 commits into
base: release/v0.5.0
Choose a base branch
from
Open

Conversation

ToniRamirezM
Copy link
Contributor

Description

Calculate network ID instead of reading it from config

cmd/run.go Outdated
reorgDetectorL2 *reorgdetector.ReorgDetector,
l2Client *ethclient.Client,
) *bridgesync.BridgeSync {
if !isNeeded([]string{cdkcommon.RPC, cdkcommon.AGGSENDER}, components) {
return nil
}

ethermanClient, err := newEtherman(c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing c config.Config to be able to create a ethClient, inject the etherClient. There are 2 functions on run.go that create the common instance for all classes:

  • runL1ClientIfNeeded
  • runL2ClientIfNeeded

cmd/run.go Outdated
@@ -77,8 +77,8 @@ func start(cliCtx *cli.Context) error {

l1InfoTreeSync := runL1InfoTreeSyncerIfNeeded(cliCtx.Context, components, *c, l1Client, reorgDetectorL1)
claimSponsor := runClaimSponsorIfNeeded(cliCtx.Context, components, l2Client, c.ClaimSponsor)
l1BridgeSync := runBridgeSyncL1IfNeeded(cliCtx.Context, components, c.BridgeL1Sync, reorgDetectorL1, l1Client)
l2BridgeSync := runBridgeSyncL2IfNeeded(cliCtx.Context, components, c.BridgeL2Sync, reorgDetectorL2, l2Client)
l1BridgeSync := runBridgeSyncL1IfNeeded(cliCtx.Context, components, *c, reorgDetectorL1, l1Client)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that is not related to your PR, but renaming c for config or even for cfg would be really nice

Copy link

sonarcloud bot commented Dec 3, 2024

Copy link
Contributor

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM, left a minor comments.


return 0, fmt.Errorf("error calling SMC RollupManager.RollupAddressToID(%s). Err: %w", rollupAddr.String(), err)
}
log.Infof("rollupID: %d (obtenied from SMC: %s )", rollupID, contracts.Banana.RollupManager.String())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.Infof("rollupID: %d (obtenied from SMC: %s )", rollupID, contracts.Banana.RollupManager.String())
log.Infof("rollupID: %d (obtained from contract: %s )", rollupID, contracts.Banana.RollupManager.String())

if err != nil {
log.Errorf("error getting rollupID from %s : %+v", contracts.Banana.RollupManager.String(), err)

return 0, fmt.Errorf("error calling SMC RollupManager.RollupAddressToID(%s). Err: %w", rollupAddr.String(), err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return 0, fmt.Errorf("error calling SMC RollupManager.RollupAddressToID(%s). Err: %w", rollupAddr.String(), err)
return 0, fmt.Errorf("error calling contract RollupManager.RollupAddressToID(%s). Err: %w", rollupAddr.String(), err)

}
rollupID, err := contracts.Banana.RollupManager.RollupAddressToID(&bind.CallOpts{Pending: false}, rollupAddr)
if err != nil {
log.Errorf("error getting rollupID from %s : %+v", contracts.Banana.RollupManager.String(), err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.Errorf("error getting rollupID from %s : %+v", contracts.Banana.RollupManager.String(), err)
log.Errorf("error getting rollupID from %s: %v", contracts.Banana.RollupManager.String(), err)

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.

4 participants