search

planetarium / libplanet

Blockchain in C#/.NET for on-chain, decentralized gaming
https://docs.libplanet.io/
GNU Lesser General Public License v2.1
505 stars 139 forks source link

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

Open OnedgeLee opened 2 months ago

OnedgeLee commented 2 months ago

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(). https://github.com/planetarium/libplanet/blob/690a98c0664abf024c89dc52a04b14e9f6943eaa/Libplanet.Net/Consensus/ConsensusContext.cs#L250-L253

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. https://github.com/planetarium/libplanet/blob/690a98c0664abf024c89dc52a04b14e9f6943eaa/Libplanet.Net/Consensus/ConsensusContext.cs#L442-L444

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 commented 2 months ago

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.