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

consortium-v2: only check the current block in GetBestParentBlock #555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minh-bq
Copy link
Contributor

@minh-bq minh-bq commented 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 #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.

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants