Skip to content

Commit

Permalink
Merge pull request #3849 from greymistcube/refactor/context-without-g…
Browse files Browse the repository at this point in the history
…ossip

♻️ Remove `IConsensusMessageCommunicator` from `Context`
  • Loading branch information
greymistcube authored Jun 25, 2024
2 parents e857d5e + ba8220a commit e08cf8e
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 91 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ To be released.
- (Libplanet.Net) Changed `Context.Start()` to throw an
`InvalidOperationException` when `Context` is not in a valid state.
[[#3846]]
- (Libplanet.Net) Removed `IConsensusMessageCommunicator` parameter from
`Context()`. [[#3848], [#3849]]

### Backward-incompatible network protocol changes

Expand Down Expand Up @@ -62,6 +64,8 @@ To be released.
[#3833]: https://github.com/planetarium/libplanet/issues/3833
[#3845]: https://github.com/planetarium/libplanet/pull/3845
[#3846]: https://github.com/planetarium/libplanet/pull/3846
[#3848]: https://github.com/planetarium/libplanet/issues/3848
[#3849]: https://github.com/planetarium/libplanet/issues/3849


Version 4.6.1
Expand Down
22 changes: 17 additions & 5 deletions src/Libplanet.Net/Consensus/ConsensusContext.Event.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ namespace Libplanet.Net.Consensus
{
public partial class ConsensusContext
{
/// <summary>
/// An event that is invoked when a <see cref="ConsensusMsg"/> is published.
/// </summary>
internal event EventHandler<(long Height, ConsensusMsg Message)>? MessagePublished;

/// <inheritdoc cref="Context.ExceptionOccurred"/>
internal event EventHandler<(long Height, Exception)>? ExceptionOccurred;

Expand All @@ -14,9 +19,6 @@ public partial class ConsensusContext
/// <inheritdoc cref="Context.StateChanged"/>
internal event EventHandler<Context.ContextState>? StateChanged;

/// <inheritdoc cref="Context.MessagePublished"/>
internal event EventHandler<(long Height, ConsensusMsg Message)>? MessagePublished;

/// <inheritdoc cref="Context.MessageConsumed"/>
internal event EventHandler<(long Height, ConsensusMsg Message)>? MessageConsumed;

Expand All @@ -25,18 +27,28 @@ public partial class ConsensusContext

private void AttachEventHandlers(Context context)
{
// NOTE: Events for testing and debugging.
context.ExceptionOccurred += (sender, exception) =>
ExceptionOccurred?.Invoke(this, (context.Height, exception));
context.TimeoutProcessed += (sender, eventArgs) =>
TimeoutProcessed?.Invoke(this, (context.Height, eventArgs.Round, eventArgs.Step));
context.StateChanged += (sender, eventArgs) =>
StateChanged?.Invoke(this, eventArgs);
context.MessagePublished += (sender, message) =>
MessagePublished?.Invoke(this, (context.Height, message));
context.MessageConsumed += (sender, message) =>
MessageConsumed?.Invoke(this, (context.Height, message));
context.MutationConsumed += (sender, action) =>
MutationConsumed?.Invoke(this, (context.Height, action));

// NOTE: Events for consensus logic.
context.HeightStarted += (sender, height) =>
_consensusMessageCommunicator.OnStartHeight(height);
context.RoundStarted += (sender, round) =>
_consensusMessageCommunicator.OnStartRound(round);
context.MessageToPublish += (sender, message) =>
{
_consensusMessageCommunicator.PublishMessage(message);
MessagePublished?.Invoke(this, (context.Height, message));
};
}
}
}
1 change: 0 additions & 1 deletion src/Libplanet.Net/Consensus/ConsensusContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,6 @@ private Context CreateContext(long height, BlockCommit? lastCommit)
}

Context context = new Context(
_consensusMessageCommunicator,
_blockChain,
height,
lastCommit,
Expand Down
2 changes: 1 addition & 1 deletion src/Libplanet.Net/Consensus/Context.Async.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void Start()
_logger.Information(
"Starting context for height #{Height}",
Height);
_consensusMessageCommunicator.OnStartHeight(Height);
HeightStarted?.Invoke(this, Height);
ProduceMutation(() => StartRound(0));

// FIXME: Exceptions inside tasks should be handled properly.
Expand Down
21 changes: 16 additions & 5 deletions src/Libplanet.Net/Consensus/Context.Event.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,22 @@ namespace Libplanet.Net.Consensus
{
public partial class Context
{
/// <summary>
/// An event that is invoked when <see cref="Context"/> starts via <see cref="Start"/>.
/// </summary>
internal event EventHandler<long>? HeightStarted;

/// <summary>
/// An event that is invoked when <see cref="Context"/> starts a new round
/// via <see cref="StartRound"/>.
/// </summary>
internal event EventHandler<int>? RoundStarted;

/// <summary>
/// An event that is invoked when a <see cref="ConsensusMsg"/> needs to be published.
/// </summary>
internal event EventHandler<ConsensusMsg>? MessageToPublish;

/// <summary>
/// An event that is invoked when an <see cref="Exception"/> is thrown.
/// </summary>
Expand All @@ -25,11 +41,6 @@ public partial class Context
/// </summary>
internal event EventHandler<ContextState>? StateChanged;

/// <summary>
/// An event that is invoked when a <see cref="ConsensusMsg"/> is published.
/// </summary>
internal event EventHandler<ConsensusMsg>? MessagePublished;

/// <summary>
/// An event that is invoked when a queued <see cref="ConsensusMsg"/> is consumed.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Libplanet.Net/Consensus/Context.Mutate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ private void StartRound(int round)
round,
Round,
ToString());
_consensusMessageCommunicator.OnStartRound(round);

Round = round;
RoundStarted?.Invoke(this, Round);
_heightVoteSet.SetRound(round);

Proposal = null;
Expand Down
14 changes: 2 additions & 12 deletions src/Libplanet.Net/Consensus/Context.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ public partial class Context : IDisposable
{
private readonly ContextTimeoutOption _contextTimeoutOption;

private readonly IConsensusMessageCommunicator _consensusMessageCommunicator;
private readonly BlockChain _blockChain;
private readonly Codec _codec;
private readonly ValidatorSet _validatorSet;
Expand Down Expand Up @@ -107,8 +106,6 @@ public partial class Context : IDisposable
/// <summary>
/// Initializes a new instance of the <see cref="Context"/> class.
/// </summary>
/// <param name="consensusMessageCommunicator">A communicator for receiving
/// <see cref="ConsensusMsg"/> from or publishing to other validators.</param>
/// <param name="blockChain">A blockchain that will be committed, which
/// will be voted by consensus, and used for proposing a block.
/// </param>
Expand All @@ -125,15 +122,13 @@ public partial class Context : IDisposable
/// <param name="contextTimeoutOptions">A <see cref="ContextTimeoutOption"/> for
/// configuring a timeout for each <see cref="ConsensusStep"/>.</param>
public Context(
IConsensusMessageCommunicator consensusMessageCommunicator,
BlockChain blockChain,
long height,
BlockCommit? lastCommit,
PrivateKey privateKey,
ValidatorSet validators,
ContextTimeoutOption contextTimeoutOptions)
: this(
consensusMessageCommunicator,
blockChain,
height,
lastCommit,
Expand All @@ -147,7 +142,6 @@ public Context(
}

private Context(
IConsensusMessageCommunicator consensusMessageCommunicator,
BlockChain blockChain,
long height,
BlockCommit? lastCommit,
Expand All @@ -171,7 +165,6 @@ private Context(
.ForContext("Source", nameof(Context));

_privateKey = privateKey;
_consensusMessageCommunicator = consensusMessageCommunicator;
Height = height;
Round = round;
Step = consensusStep;
Expand Down Expand Up @@ -438,11 +431,8 @@ private TimeSpan TimeoutPropose(long round)
/// </summary>
/// <param name="message">A <see cref="ConsensusMsg"/> to publish.</param>
/// <remarks><see cref="ConsensusMsg"/> should be published to itself.</remarks>
private void PublishMessage(ConsensusMsg message)
{
_consensusMessageCommunicator.PublishMessage(message);
MessagePublished?.Invoke(this, message);
}
private void PublishMessage(ConsensusMsg message) =>
MessageToPublish?.Invoke(this, message);

/// <summary>
/// Validates the given block.
Expand Down
12 changes: 6 additions & 6 deletions test/Libplanet.Net.Tests/Consensus/ContextNonProposerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public async Task EnterPreCommitBlockTwoThird()
stepChangedToPreCommit.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreCommitMsg preCommitMsg)
{
Expand Down Expand Up @@ -162,7 +162,7 @@ public async void EnterPreCommitNilTwoThird()
stepChangedToPreCommit.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreCommitMsg preCommitMsg &&
preCommitMsg.BlockHash.Equals(default))
Expand Down Expand Up @@ -213,7 +213,7 @@ public async Task EnterPreVoteNilOnInvalidBlockHeader()
{
timeoutProcessed = true;
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreVoteMsg vote && vote.PreVote.BlockHash.Equals(default))
{
Expand Down Expand Up @@ -286,7 +286,7 @@ public async Task EnterPreVoteNilOnInvalidBlockContent()
{
timeoutProcessed = true;
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreVoteMsg vote && vote.PreVote.BlockHash.Equals(default))
{
Expand Down Expand Up @@ -348,7 +348,7 @@ public async Task EnterPreVoteNilOnInvalidAction()
{
timeoutProcessed = true;
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreVoteMsg vote && vote.PreVote.BlockHash.Equals(default))
{
Expand Down Expand Up @@ -464,7 +464,7 @@ public async void TimeoutPropose()
stepChangedToPreVote.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreVoteMsg)
{
Expand Down
12 changes: 6 additions & 6 deletions test/Libplanet.Net.Tests/Consensus/ContextProposerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public async Task EnterPreCommitNil()
stepChangedToPreCommit.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreCommitMsg preCommitMsg)
{
Expand Down Expand Up @@ -88,7 +88,7 @@ public async void EnterPreCommitBlock()
stepChangedToPreCommit.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down Expand Up @@ -189,7 +189,7 @@ public async Task EndCommitBlock()
stepChangedToEndCommit.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down Expand Up @@ -239,7 +239,7 @@ public async void EnterPreVoteNil()
stepChangedToPreVote.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusPreVoteMsg vote && vote.PreVote.BlockHash.Equals(default))
{
Expand Down Expand Up @@ -271,7 +271,7 @@ public async void EnterPreVoteBlock()
stepChangedToPreVote.Set();
}
};
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down Expand Up @@ -319,7 +319,7 @@ public async void VoteNilOnSelfProposedInvalidBlock()
height: 2,
lastCommit: block2Commit,
validatorSet: TestUtils.ValidatorSet);
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public async Task EnterValidRoundPreVoteBlock()
}
};
context.TimeoutProcessed += (_, __) => timeoutProcessed = true;
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down Expand Up @@ -150,7 +150,7 @@ public async void EnterValidRoundPreVoteNil()
}
};
context.TimeoutProcessed += (_, __) => timeoutProcessed = true;
context.MessagePublished += (_, message) =>
context.MessageToPublish += (_, message) =>
{
if (message is ConsensusProposalMsg proposalMsg)
{
Expand Down
Loading

0 comments on commit e08cf8e

Please sign in to comment.