Fix inconsistent latest_block_hash
#1424
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ahmadkaouk @librelois @koushiro @sorpaas Please help take a review. |
librelois
reviewed
May 14, 2024
librelois
reviewed
May 14, 2024
librelois
reviewed
May 14, 2024
|
@boundless-forest the overall design seems good, but we need a migration and/or a breaking change section in this PR's description to indicate that there is a migration to run or that node operators should resync the sql backend after the upgrade |
librelois
approved these changes
May 14, 2024
I have deleted the broken db operation. Let's focus on fixing only one issue in this PR. |
koushiro
reviewed
May 15, 2024
koushiro
reviewed
May 15, 2024
koushiro
approved these changes
May 15, 2024
boundless-forest
added a commit
to darwinia-network/frontier
that referenced
this pull request
Sep 23, 2024
* Add `best_hash` method * Update earliest tag matched value * Fix CI tests * Code clean * Fix review comments * Update comment * Fix compile after solving conflicts * Remove the broken changes * Update trait bound * Simplify trait bound
boundless-forest
added a commit
to darwinia-network/frontier
that referenced
this pull request
Sep 23, 2024
* Add `best_hash` method * Update earliest tag matched value * Fix CI tests * Code clean * Fix review comments * Update comment * Fix compile after solving conflicts * Remove the broken changes * Update trait bound * Simplify trait bound
AurevoirXavier
pushed a commit
to darwinia-network/frontier
that referenced
this pull request
Sep 23, 2024
* Fix inconsistent `latest_block_hash` (polkadot-evm#1424) * Add `best_hash` method * Update earliest tag matched value * Fix CI tests * Code clean * Fix review comments * Update comment * Fix compile after solving conflicts * Remove the broken changes * Update trait bound * Simplify trait bound * FIX ci
This was referenced Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We(Darwinia) have noticed that there are some unexpected cases when indexing evm logs with the third party tools, like ponder. After some investment, I found out that the root cause is the latest block hash is sometimes inconsistent when using different RPC methods. For instance, the indexing tool generally first queries the latest block number with
eth_getBlockNumber(latest), which returns theclient.info().best_hashin the current implementation, then it tries to fetch the block data witheth_getBlockByHash(hash), but the block data isn't sync to the kv/sql db in time, so the indexing tool misunderstands that the block doesn't have any logs. But in fact it does. Of course, the indexing tool will track the finalize tag and fetch these blocks data again and compared to the previously stored data, it would find the same block hash but with different block data information. We have found that this can be easily reproduced with the frontier sql backend. Some discussion can be found moonbeam-foundation/moonbeam#2788.This PR changes the
latest_block_hashresponse from the underlying substrate clientbest_hashto dblatest_block_hash(). This only applies to the sql db now.