Merge pull request #500 from earlbread/fix-block-action-get-state

Fix GetState of block action to return proper state
planetarium · Sep 9, 2019 · cb825cf · cb825cf
2 parents 770879a + 487ade3
commit cb825cf
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 9 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -8,6 +8,11 @@ To be released.
 
 ### Bug fixes
 
+ -  Fix bug where `IAccountStateDelta.GetState()` hadn't returned proper state
+    when the block action is evaluated.  [[#500]]
+
+[#500]: https://github.com/planetarium/libplanet/pull/500
+
 
 Version 0.5.2
 -------------

diff --git a/Libplanet.Tests/Blockchain/BlockChainTest.cs b/Libplanet.Tests/Blockchain/BlockChainTest.cs
@@ -368,7 +368,6 @@ public void Append()
                 Assert.Equal(0, blockRenders[0].Context.BlockIndex);
                 Assert.Equal(1, blockRenders[1].Context.BlockIndex);
 
-                Assert.Null(blockRenders[0].Context.PreviousStates.GetState(minerAddress));
                 Assert.Equal(1, blockRenders[0].NextStates.GetState(minerAddress));
                 Assert.Equal(
                     1,
@@ -450,6 +449,7 @@ public void ExecuteActions()
                 { addresses[2], "baz" },
                 { addresses[3], "qux" },
                 { addresses[4], 2 },
+                { MinerReward.RewardRecordAddress, $"{addresses[4]},{addresses[4]}" },
             };
 
             _blockChain.ExecuteActions(blocks[1], true);
@@ -1319,6 +1319,30 @@ public void EvaluateBlockAction()
             Assert.Equal(2, blockActionEvaluation.OutputStates.GetState(miner));
         }
 
+        [Fact]
+        public void BlockActionWithMultipleAddress()
+        {
+            var miner1 = _fx.Address1;
+            var miner2 = _fx.Address2;
+            var rewardRecordAddress = MinerReward.RewardRecordAddress;
+
+            _blockChain.MineBlock(miner1);
+            _blockChain.MineBlock(miner1);
+            _blockChain.MineBlock(miner2);
+
+            AddressStateMap states = _blockChain.GetStates(
+                new[] { miner1, miner2, MinerReward.RewardRecordAddress });
+
+            int reward1 = (int)states[miner1];
+            int reward2 = (int)states[miner2];
+            string rewardRecord = (string)states[rewardRecordAddress];
+
+            Assert.Equal(2, reward1);
+            Assert.Equal(1, reward2);
+
+            Assert.Equal($"{miner1},{miner1},{miner2}", rewardRecord);
+        }
+
         /// <summary>
         /// Builds a fixture that has incomplete states for blocks other
         /// than the tip, to test <c>GetStates()</c> method's

diff --git a/Libplanet.Tests/Common/Action/MinerReward.cs b/Libplanet.Tests/Common/Action/MinerReward.cs
@@ -16,6 +16,9 @@ public MinerReward(int reward)
             Reward = reward;
         }
 
+        public static Address RewardRecordAddress =>
+            new Address("0000000000000000000000000000000000000000");
+
         public static AsyncLocal<ImmutableList<RenderRecord>>
             RenderRecords { get; } = new AsyncLocal<ImmutableList<RenderRecord>>();
 
@@ -36,6 +39,14 @@ public IAccountStateDelta Execute(IActionContext ctx)
         {
             IAccountStateDelta states = ctx.PreviousStates;
 
+            string rewardRecord = (string)states.GetState(RewardRecordAddress);
+
+            rewardRecord = rewardRecord is null
+                ? ctx.Miner.ToString()
+                : $"{rewardRecord},{ctx.Miner}";
+
+            states = states.SetState(RewardRecordAddress, rewardRecord);
+
             int previousReward = (int?)states?.GetState(ctx.Miner) ?? 0;
             int reward = previousReward + Reward;
 

diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs
@@ -708,16 +708,10 @@ internal ActionEvaluation EvaluateBlockAction(
 
             Address miner = block.Miner.GetValueOrDefault();
 
-            var minerState = GetStates(new[] { miner }, block.PreviousHash)
-                .GetValueOrDefault(miner);
-
             if (lastStates is null)
             {
-                lastStates = new AccountStateDeltaImpl(a => minerState);
-            }
-            else if (lastStates.GetState(miner) is null)
-            {
-                lastStates = lastStates.SetState(miner, minerState);
+                lastStates = new AccountStateDeltaImpl(
+                    a => GetStates(new[] { a }, block.PreviousHash).GetValueOrDefault(a));
             }
 
             return ActionEvaluation.EvaluateActionsGradually(