Merge pull request #3846 from greymistcube/refactor/context-state

♻️ Changed `Context.Start()` to throw an `Exception` in an invalid state
planetarium · Jun 25, 2024 · 407eb80 · 407eb80
2 parents 901ecb2 + a0966f4
commit 407eb80
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 4 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -31,6 +31,9 @@ To be released.
  -  (Libplanet.Net) Changed `Context()` to accept additional `BlockCommit?`
     typed argument.  Removed `lastCommit` parameter from `Context.Start()`.
     [[#3833], [#3845]]
+ -  (Libplanet.Net) Changed `Context.Start()` to throw an
+    `InvalidOperationException` when `Context` is not in a valid state.
+    [[#3846]]
 
 ### Backward-incompatible network protocol changes
 
@@ -58,6 +61,7 @@ To be released.
 [#3811]: https://github.com/planetarium/libplanet/pull/3811
 [#3833]: https://github.com/planetarium/libplanet/issues/3833
 [#3845]: https://github.com/planetarium/libplanet/pull/3845
+[#3846]: https://github.com/planetarium/libplanet/pull/3846
 
 
 Version 4.6.1

diff --git a/src/Libplanet.Net/Consensus/Context.Async.cs b/src/Libplanet.Net/Consensus/Context.Async.cs
@@ -11,8 +11,17 @@ public partial class Context
         /// <summary>
         /// Starts round #0 of consensus for <see cref="Height"/>.
         /// </summary>
+        /// <exception cref="InvalidOperationException">Thrown when <see cref="Step"/>
+        /// is not <see cref="ConsensusStep.Default"/>.</exception>
         public void Start()
         {
+            if (Step != ConsensusStep.Default)
+            {
+                throw new InvalidOperationException(
+                    $"Context cannot be started unless its state is {ConsensusStep.Default} " +
+                    $"but its current step is {Step}");
+            }
+
             _logger.Information(
                 "Starting context for height #{Height}",
                 Height);

diff --git a/test/Libplanet.Net.Tests/Consensus/ContextTest.cs b/test/Libplanet.Net.Tests/Consensus/ContextTest.cs
@@ -48,7 +48,7 @@ public ContextTest(ITestOutputHelper output)
         }
 
         [Fact(Timeout = Timeout)]
-        public async void StartAsProposer()
+        public async Task StartAsProposer()
         {
             var proposalSent = new AsyncAutoResetEvent();
             var stepChangedToPreVote = new AsyncAutoResetEvent();
@@ -77,7 +77,7 @@ public async void StartAsProposer()
         }
 
         [Fact(Timeout = Timeout)]
-        public async void StartAsProposerWithLastCommit()
+        public async Task StartAsProposerWithLastCommit()
         {
             var stepChangedToPreVote = new AsyncAutoResetEvent();
             ConsensusProposalMsg? proposal = null;
@@ -125,6 +125,24 @@ public async void StartAsProposerWithLastCommit()
             Assert.Equal(lastCommit, proposed.LastCommit);
         }
 
+        [Fact]
+        public async Task CannotStartTwice()
+        {
+            var stepChanged = new AsyncAutoResetEvent();
+            var (_, context) = TestUtils.CreateDummyContext();
+            context.StateChanged += (_, eventArgs) =>
+            {
+                if (eventArgs.Step == ConsensusStep.Propose)
+                {
+                    stepChanged.Set();
+                }
+            };
+            context.Start();
+
+            await stepChanged.WaitAsync();
+            Assert.Throws<InvalidOperationException>(() => context.Start());
+        }
+
         [Fact(Timeout = Timeout)]
         public async Task CanAcceptMessagesAfterCommitFailure()
         {
@@ -211,7 +229,7 @@ public async Task CanAcceptMessagesAfterCommitFailure()
                     VoteFlag.PreCommit).Sign(TestUtils.PrivateKeys[i])));
             }
 
-            await Task.WhenAll(stepChangedToEndCommit.WaitAsync());
+            await stepChangedToEndCommit.WaitAsync();
 
             // Check context has only three votes.
             BlockCommit? commit = context.GetBlockCommit();
@@ -459,7 +477,7 @@ void BroadcastMessage(ConsensusMsg message) =>
         /// </para>
         /// </summary>
         [Fact]
-        public async void CanReplaceProposal()
+        public async Task CanReplaceProposal()
         {
             var codec = new Codec();
             var privateKeys = Enumerable.Range(0, 4).Select(_ => new PrivateKey()).ToArray();