Skip to content
This repository was archived by the owner on Oct 16, 2025. It is now read-only.

fix: PollingBlockTracker.getLatestBlock to handle network failures#313

Merged
cryptodev-2s merged 4 commits intomainfrom
cryptodev2s/fix/block-tracker-get-latest-block-timeout
May 2, 2025
Merged

fix: PollingBlockTracker.getLatestBlock to handle network failures#313
cryptodev-2s merged 4 commits intomainfrom
cryptodev2s/fix/block-tracker-get-latest-block-timeout

Conversation

@cryptodev-2s
Copy link
Copy Markdown
Contributor

@cryptodev-2s cryptodev-2s commented Mar 31, 2025

Fix Block Tracker's getLatestBlock Method to Handle Network Failures

Fixes: #311

@cryptodev-2s cryptodev-2s self-assigned this Mar 31, 2025
@cryptodev-2s cryptodev-2s requested a review from a team as a code owner March 31, 2025 14:51
@cryptodev-2s cryptodev-2s marked this pull request as draft March 31, 2025 14:52
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/fix/block-tracker-get-latest-block-timeout branch from bca3298 to a03f89d Compare April 7, 2025 14:44
@cryptodev-2s cryptodev-2s marked this pull request as ready for review April 7, 2025 14:47
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/fix/block-tracker-get-latest-block-timeout branch from a03f89d to a47ec4c Compare April 8, 2025 15:00
@mcmire
Copy link
Copy Markdown
Contributor

mcmire commented Apr 8, 2025

This looks pretty good so far! There are some duplicate tests now but I think we can clean that up later.

Perhaps to fix the failing tests you could call #rejectPendingLatestBlock in _fetchLatestBlock if there is an error? Just an idea.

@cryptodev-2s
Copy link
Copy Markdown
Contributor Author

This looks pretty good so far! There are some duplicate tests now but I think we can clean that up later.

Perhaps to fix the failing tests you could call #rejectPendingLatestBlock in _fetchLatestBlock if there is an error? Just an idea.

That's exactly what I am doing

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/fix/block-tracker-get-latest-block-timeout branch from 90b37df to 6972ebb Compare April 9, 2025 12:40
Copy link
Copy Markdown
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

I think there are some tests we can clean up, but we can do that in another PR. Implementation looks good and we have some better tests now.

@cryptodev-2s cryptodev-2s merged commit b0541b6 into main May 2, 2025
9 checks passed
@cryptodev-2s cryptodev-2s deleted the cryptodev2s/fix/block-tracker-get-latest-block-timeout branch May 2, 2025 09:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getLatestBlock never resolves (and never stops the polling loop) if it encounters an error fetching the latest block number

2 participants