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

Feature/localbridgesync #12

Merged
merged 27 commits into from
Jul 17, 2024
Merged

Feature/localbridgesync #12

merged 27 commits into from
Jul 17, 2024

Conversation

arnaubennassar
Copy link
Contributor

@arnaubennassar arnaubennassar commented Jul 1, 2024

Implementation of the localbridgesyncer. Please do not review the reorgdetector as it's left as a PoC

localbridgesync/driver.go Outdated Show resolved Hide resolved
localbridgesync/driver.go Outdated Show resolved Hide resolved
case <-ctx.Done():
close(downloadedCh)
return
default:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to introduce the polling frequency parameter and use it here (namely declare some ticker here and periodically fetch and notify about the new blocks)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the idea is to don't have frequency, but rather go until the end (tip of the network) and if reached wait for new blocks

reorgdetector/reorgdetector.go Show resolved Hide resolved
reorgdetector/reorgdetector.go Show resolved Hide resolved
reorgdetector/reorgdetector.go Outdated Show resolved Hide resolved
}

func New(ctx context.Context) (*ReorgDetector, error) {
r := &ReorgDetector{}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reminder: don't forget to provide the ethclient.Client reference through a constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Null pointer?

@arnaubennassar arnaubennassar changed the base branch from main to develop July 15, 2024 08:37
@arnaubennassar arnaubennassar self-assigned this Jul 15, 2024
Copy link
Contributor

@joanestebanr joanestebanr left a comment

Choose a reason for hiding this comment

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

The code is very simple and elegant!

localbridgesync/downloader.go Outdated Show resolved Hide resolved
localbridgesync/downloader.go Outdated Show resolved Hide resolved
localbridgesync/driver.go Show resolved Hide resolved
localbridgesync/types.go Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Jul 16, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
3.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

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 🎩 aside from the reorgs comments

)

var (
bridgeEventSignature = crypto.Keccak256Hash([]byte("BridgeEvent(uint8,uint32,address,uint32,address,uint256,bytes,uint32)"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't this declared on cdk-contracts-tooling repo?

}

func New(ctx context.Context) (*ReorgDetector, error) {
r := &ReorgDetector{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Null pointer?

for currentBlock > lastBlockFromClient {
lastBlockFromClient, err = r.ethClient.BlockNumber(ctx)
if err != nil {
// TODO: handle error
Copy link
Contributor

Choose a reason for hiding this comment

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

At least log?

}
block, err := r.ethClient.HeaderByNumber(ctx, big.NewInt(int64(currentBlock)))
if err != nil {
// TODO: handle error
Copy link
Contributor

Choose a reason for hiding this comment

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

Log the err maybe?

@arnaubennassar arnaubennassar merged commit 18a624c into develop Jul 17, 2024
1 of 4 checks passed
@arnaubennassar arnaubennassar deleted the feature/localbridgesync branch July 17, 2024 11:11
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