Separating Block from Proposal

[colibrishin]

We have a marshaled Block in the Proposal. The previous decision was made due to a tricky proposed block-transferring timing situation. This should be separated at some point in my opinion and should be considered with specification changes.

Block-transferring

A node needs to receive a block and the corresponding proposal before the Propose timeout to validate a proposed block and vote on it. The separation causes the change in the behavior of voting at the Propose -> PreVote stage.

libplanet/Libplanet.Net/Consensus/Context.cs

Lines 413 to 424 in 7a7faba

	private (Block<T>, int)? GetProposal(int round)
	{
	ConsensusProposalMsg? proposal = _messageLog.GetProposal(round);
	if (proposal is { } p)
	{
	var block = BlockMarshaler.UnmarshalBlock<T>(
	(Dictionary)_codec.Decode(p.Proposal.MarshaledBlock));
	return (block, p.Proposal.ValidRound);
	}
	return null;
	}

libplanet/Libplanet.Net/Consensus/Context.Mutate.cs

Lines 36 to 44 in 7a7faba

	Proposal proposal = new ProposalMetadata(
	Height,
	Round,
	DateTimeOffset.UtcNow,
	_privateKey.PublicKey,
	_codec.Encode(proposalValue.MarshalBlock()),
	_validRound).Sign(_privateKey);
	BroadcastMessage(new ConsensusProposalMsg(proposal));

libplanet/Libplanet.Net/Consensus/Context.Mutate.cs

Lines 125 to 146 in 7a7faba

	(Block<T> Block, int ValidRound)? propose = GetProposal(Round);
	if (propose is { } p1 &&
	p1.ValidRound == -1 &&
	Step == Step.Propose)
	{
	_logger.Debug(
	"Entering PreVote step due to proposal message with " +
	"valid round -1. (context: {Context})",
	ToString());
	Step = Step.PreVote;
	if (IsValid(p1.Block) && (_lockedRound == -1 || _lockedValue == p1.Block))
	{
	BroadcastMessage(
	new ConsensusPreVoteMsg(MakeVote(Round, p1.Block.Hash, VoteFlag.PreVote)));
	}
	else
	{
	BroadcastMessage(
	new ConsensusPreVoteMsg(MakeVote(Round, null, VoteFlag.PreVote)));
	}
	}

libplanet/Libplanet.Net/Consensus/MessageLog.cs

Lines 216 to 224 in 7a7faba

	internal ConsensusProposalMsg? GetProposal(int round)
	{
	lock (_lock)
	{
	return _proposals.ContainsKey(round)
	? _proposals[round]
	: null;
	}
	}

Evidence

Evidence is data for convicting the validator whose acts possibly lead to the fault of the consensus, and the example of those are the double vote (exceeding total quorum) and double proposal (minority proposal received validator will fall behind from the consensus).

Assuming that

• Any double vote or proposal created from a node is abnormal.
• If the signatures of the vote or proposal are valid.

I think this could be used as evidence and punish the validator later on. (Speculation) We can simply put the vote or proposal into the evidence list and validates the signature and compare it with the local evidence.

However, If we add the evidence into the Block and Proposal will have the marshaled block and that will cause the enlarging of the block size.

[colibrishin]

We could change the behavior of GetProposal() in Context<T> to return only if the block exists in the MessageLog. If a node didn't receive a block and proposal, then the node cannot PreVote-ing if the block is valid or not, but timeout NIL can be done. I think it will be the least fixing time or effort method that I have in mind.

This method has some possible problems we might encounter, for example, Let's assume we would accept the block message when the proposal is in the MessageLog. If the block message has arrived before the proposal, then it will be hidden for a brief moment by gossip caching.

[riemannulus]

I have some questions for before about this suggestion.

1. Why Evidence is added in Block<T>? Could we use another store?
2. Could you suggest the struct of Evidence and how to handle it? (e.g., https://docs.cosmos.network/v0.45/modules/evidence/01_concepts.html)
3. Is this suggestion critical enough to be discussed during a PoS transition? Is it impossible to separate later?

[riemannulus]

Also, I have some questions about this suggestion.

1. So, what do you think about how to separate Block from Proposal? As you said, we have a block sync timing issue if just separate from Proposal. I wonder your opinion on how to solve this problem.
2. Is storing all ConsensusMessage the only way to punish malicious nodes?

[OnedgeLee]

I think this idea is good, though this can be huge chunk of work.
This can be done by separating uncommitted block sync logic from proposal,
but let this issue as future work would be better for now, since its advantage seems to be relatively small to effect of other tasks.
I agree to do this someday.