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: query latest block info #31

Merged
merged 5 commits into from
Oct 3, 2024
Merged

Conversation

parketh
Copy link
Collaborator

@parketh parketh commented Oct 1, 2024

Summary

This stacked PR (following #30) adds a new HTTP api route to return the latest block info, including:

  • Latest L2 block
  • Latest BTC finalized L2 block
  • Earliest consecutively BTC finalized L2 Block
  • Latest ETH finalized L2 block

to be displayed by the finality explorer

Test plan

make build
make test

@parketh parketh changed the base branch from main to chore/remove-healthcheck-params October 1, 2024 21:53
Copy link
Collaborator

@bap2pecs bap2pecs left a comment

Choose a reason for hiding this comment

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

lgtm. can you add babylon team as reviewers?

db/bbolt.go Outdated
return types.ErrBlockNotFound
}

// Iterate backwards to find the earliest consecutive block
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is good enough for now. a few possible improvements:

  1. record the earliest consecutive block in the DB. so this becomes O(1)
  2. use the exponential strategy to search backwards so it will be O(logN) - since we only insert consecutively, there won't be a case where we skipped blocks in between

I think 1 is better because if we choose 2, every time people visit the site, we will do the same calculation again. btw can you create a follow-up task in Linear? it will be a good ramp-up task for new team members

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

got it, yes i think 1 make sense (similar to what we have for latest consecutively finalized block)

however this is just a rough and ready code which may be superseded if we refactor to psql. as discussed offline, let's wait for @SebastianElvis to confirm before making this change

Copy link
Member

Choose a reason for hiding this comment

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

Imo option 1 makes more sense as well, and this algorithm and corresponding interfaces will apply even if we switch to psql later on. Wdyt?

Btw, have we made any decision on embedded DB vs psql? I remember the conclusion was to follow embedded DB for now

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes we discussed in TG to use embedded DB. but for production deployment, we use docker containers anyway so it's better to do external DB

but we will support both embedded psql and external psql db mode

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay, so we can leave this one for now since it will be updated by the psql refactor

db/bbolt_test.go Outdated
Comment on lines 193 to 202
first := &types.Block{
BlockHeight: 1,
BlockHash: "0x123",
BlockTimestamp: 1000,
}
third := &types.Block{
BlockHeight: 3,
BlockHash: "0x456",
BlockTimestamp: 1050,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

per my comment above, there won't be a case that we have block 1, 3 but not 2

please correct me if my memory is incorrect

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

got it, i'll update the logic to simply fetch the earliest block in the db.

types/block.go Outdated Show resolved Hide resolved
testutil/mocks/expected_clients_mock.go Outdated Show resolved Hide resolved
Copy link
Member

@SebastianElvis SebastianElvis left a comment

Choose a reason for hiding this comment

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

Lgtm then 👍

@parketh
Copy link
Collaborator Author

parketh commented Oct 3, 2024

thanks @SebastianElvis - i added one more commit to address @bap2pecs comment above on consecutive quorum. feel free to take a quick look! otherwise we can merge the two stacked prs

Base automatically changed from chore/remove-healthcheck-params to main October 3, 2024 11:33
@parketh parketh merged commit 8d38bf7 into main Oct 3, 2024
@parketh parketh deleted the feat/finality-explorer-block-info branch October 3, 2024 11:34
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.

3 participants