consortium/v2: Change rules to choose best parent #422
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
Current rule to choose best parent block does not take into consideration the justified block height of the canonical chain and new chain (if upcoming block has smaller height than that of canonical chain). This might lead to in-turn miner producing block that cannot be inserted into canonical chain because it does not satisfy all the conditions for a reorg.
minh-bq
added a commit
to minh-bq/ronin
that referenced
this pull request
Sep 4, 2024
In commit 94bcf95 ("fix: miner does not mine its inturn block"), we make the miner go backward in the chain to find if it can create an inturn block which makes the chain has high difficulty instead of just creating the new block on the current block. However, after finality is introduced, this scenario can happen Current chain 10 <- 11 (diff: 3) <- 12 (diff: 3) And the miner can create block 11 with difficulty 7. However, block 12 has enough finality vote to justify block 11 already. So if the miner still tries to create block 11 with difficulty 7, that block will be rejected because the old chain has higher justified block number (11 vs maximum 10) and this miner does not participate to make a new block in this scenario. This is undesirable as this miner can be viewed as down even though it is still online. PR axieinfinity#422 tries to address the above issue but it reverts the purpose of commit 94bcf95 when the TODO is not resolved. This commit tries to simplify the logic of GetBestParentBlock to resolve the issue. GetBestParentBlock does not go backward and tries to revert more than 1 block anymore, it only tries to revert 1 block (i.e. the current block) if possible. With the above example, the miner only tries to revert block 12 if it can create a new block 12 with difficult 7 and does not try to revert block 11. It works with the assumption that in case the current block 12 with difficulty 3 justify block 11, it means there are enough finality votes for block 11 and the newly created block 12 with difficulty 7 can justify block 11 too. So justified block number in the newly created chain is the same as the old one but the new one will have higher difficulty.
minh-bq
added a commit
to minh-bq/ronin
that referenced
this pull request
Sep 4, 2024
In commit 94bcf95 ("fix: miner does not mine its inturn block"), we make the miner go backward in the chain to find if it can create an inturn block which makes the chain has high difficulty instead of just creating the new block on the current block. However, after finality is introduced, this scenario can happen Current chain 10 <- 11 (diff: 3) <- 12 (diff: 3) And the miner can create block 11 with difficulty 7. However, block 12 has enough finality vote to justify block 11 already. So if the miner still tries to create block 11 with difficulty 7, that block will be rejected because the old chain has higher justified block number (11 vs maximum 10) and this miner does not participate to make a new block in this scenario. This is undesirable as this miner can be viewed as down even though it is still online. PR axieinfinity#422 tries to address the above issue but it reverts the purpose of commit 94bcf95 when the TODO is not resolved. This commit tries to simplify the logic of GetBestParentBlock to resolve the issue. GetBestParentBlock does not go backward and tries to revert more than 1 block anymore, it only tries to revert 1 block (i.e. the current block) if possible. With the above example, the miner only tries to revert block 12 if it can create a new block 12 with difficult 7 and does not try to revert block 11. It works with the assumption that in case the current block 12 with difficulty 3 justify block 11, it means there are enough finality votes for block 11 and the newly created block 12 with difficulty 7 can justify block 11 too. So justified block number in the newly created chain is the same as the old one but the new one will have higher difficulty.
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.
Current rule to choose best parent block does not take into consideration the justified block height of the canonical chain and new chain (if upcoming block has smaller height than that of canonical chain). This might lead to in-turn miner producing block that cannot be inserted into canonical chain because it does not satisfy all the conditions for a reorg.