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.

Sign up for GitHub

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

Jump to bottom

refactor: QueryLatestFinalizedBlocks(count uint64) -> QueryLatestFinalizedBlock() #347

Merged
merged 13 commits into from
May 23, 2024
Merged

refactor: QueryLatestFinalizedBlocks(count uint64) -> QueryLatestFinalizedBlock() #347

merged 13 commits into from
May 23, 2024

Conversation

bap2pecs
Copy link
Contributor

@bap2pecs bap2pecs commented May 22, 2024 •
edited
Loading

Summary

this changes QueryLatestFinalizedBlocks(count uint64) to QueryLatestFinalizedBlock()

after analyzing, it's only used in latestFinalizedBlocksWithRetry() and it's just checking 1 block. i.e. QueryLatestFinalizedBlocks(1)

I also don't see we need to get N latest finalized block in the near future.

so to make our life easier when we implement the interface function for evm_consumer, i am first making this change

also changed WaitForNFinalizedBlocks to WaitForNFinalizedBlocksAndReturnTipHeight to make the e2e tests happy

Test Plan

make mock-gen
make test
make test-e2e

to run individual test. example:

go test -mod=readonly -timeout=25m -v github.com/babylonchain/finality-provider/itest -count=1 --tags=e2e -run TestMultipleFinalityProviders

@bap2pecs bap2pecs mentioned this pull request May 22, 2024
Closed
@bap2pecs
Copy link
Contributor Author

bap2pecs commented May 22, 2024 •
edited
Loading

this addresses all comments in #341 and fixed tests

@bap2pecs bap2pecs requested a review from gusin13 May 22, 2024 18:55
@bap2pecs bap2pecs changed the base branch from base/eth-support to feat/eth-support May 22, 2024 19:02
@XiangEnze XiangEnze mentioned this pull request May 22, 2024
Open
gitferry
gitferry approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

LGTM! Left a tip about how to fix e2e test

itest/test_manager.go Outdated Show resolved Hide resolved
SebastianElvis
SebastianElvis approved these changes May 23, 2024
View reviewed changes
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!

Base automatically changed from feat/eth-support to base/consumer-chain-support May 23, 2024 01:49
@bap2pecs bap2pecs merged commit 30df90e into base/consumer-chain-support May 23, 2024
1 of 2 checks passed
@bap2pecs bap2pecs deleted the refactor/eth-support branch May 23, 2024 03:26
@babylonchain babylonchain deleted a comment May 26, 2024
@babylonchain babylonchain deleted a comment May 26, 2024
@babylonchain babylonchain deleted a comment May 26, 2024
SebastianElvis pushed a commit that referenced this pull request Jun 5, 2024
@bap2pecs @SebastianElvis
refactor: QueryLatestFinalizedBlocks(count uint64) -> `QueryLatestF…
2cc4a79 
…inalizedBlock()` (#347)
maurolacy pushed a commit that referenced this pull request Jul 5, 2024
@bap2pecs @maurolacy
refactor: QueryLatestFinalizedBlocks(count uint64) -> `QueryLatestF…
fd7f130 
…inalizedBlock()` (#347)
gusin13 pushed a commit that referenced this pull request Jul 9, 2024
@bap2pecs @gusin13
refactor: QueryLatestFinalizedBlocks(count uint64) -> `QueryLatestF…
afe82dd 
…inalizedBlock()` (#347)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SebastianElvis SebastianElvis SebastianElvis approved these changes

@gitferry gitferry gitferry approved these changes

@gusin13 gusin13 Awaiting requested review from gusin13

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bap2pecs @gitferry @SebastianElvis @lesterli