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

ConsensusContext.HandleMessage() creates context with earlier validator set #3772

Open
OnedgeLee opened this issue Apr 30, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@OnedgeLee
Copy link
Contributor

OnedgeLee commented Apr 30, 2024

If ConsensusContext receives a message with higher height than current height, it tries to create a new Context with height of received message via ConsensusContext.CreateContext().

if (!_contexts.ContainsKey(height))
{
_contexts[height] = CreateContext(height);
}

But ConsensusContext.CreateContext() creates Context with validator set of ConsensusContext.Height - 1, so validator set of created Context is different from actual one, if ConsensusMessage.Height != ConsensusContext.Height.

_blockChain
.GetWorldState(_blockChain[Height - 1].Hash)
.GetValidatorSet(),

It does not make any critical problems if validator set is static, but when it varies, can be a matter.

Since Context cannot be generated without validator set, and validator set cannot be decided without actual block appending, Context should not be created through this path, IMO.

@OnedgeLee OnedgeLee added the bug Something isn't working label Apr 30, 2024
@OnedgeLee OnedgeLee self-assigned this Apr 30, 2024
@OnedgeLee
Copy link
Contributor Author

For Context, ValidatorSet does not needed, since it has BlockChain, also.
Removing ValidatorSet field and replacing them with BlockChain[Height].GetValidatorSet() would resolve this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant