Make GetStates() only down until all addresses are found

#189
planetarium · Apr 12, 2019 · 06f459b · 06f459b
1 parent 7787a2b
commit 06f459b
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 4 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -6,6 +6,12 @@ Version 0.2.2
 
 To be released.
 
+ -  Fixed a bug that `BlockChain<T>.GetStates()` had returned slower than
+    necessary for many addresses.  [[#189], [#192]]
+
+[#189]: https://github.com/planetarium/libplanet/issues/189
+[#192]: https://github.com/planetarium/libplanet/pull/192
+
 
 Version 0.2.1
 -------------

diff --git a/Libplanet.Tests/Blockchain/BlockChainTest.cs b/Libplanet.Tests/Blockchain/BlockChainTest.cs
@@ -1,7 +1,9 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using System.Xml.Schema;
 using Libplanet.Action;
 using Libplanet.Blockchain;
 using Libplanet.Blockchain.Policies;
@@ -226,6 +228,56 @@ public void CanGetBlockLocator()
             Assert.Equal(expected, actual);
         }
 
+        // This is a regression test for:
+        // https://github.com/planetarium/libplanet/issues/189#issuecomment-482443607.
+        [Fact]
+        public void GetStatesOnlyDrillsDownUntilRequestedAddressesAreFound()
+        {
+            var tracker = new StoreTracker(_fx.Store);
+            var chain = new BlockChain<DumbAction>(
+                new NullPolicy<DumbAction>(),
+                tracker
+            );
+
+            Block<DumbAction> b = TestUtils.MineGenesis<DumbAction>();
+            chain.Append(b);
+            Address[] addresses = new Address[30];
+            for (int i = 0; i < addresses.Length; ++i)
+            {
+                var privateKey = new PrivateKey();
+                Address address = privateKey.PublicKey.ToAddress();
+                addresses[i] = address;
+                DumbAction[] actions =
+                {
+                    new DumbAction(address, "foo"),
+                    new DumbAction(i < 1 ? address : addresses[i - 1], "bar"),
+                };
+                Transaction<DumbAction>[] txs =
+                {
+                    Transaction<DumbAction>.Create(privateKey, actions),
+                };
+                b = TestUtils.MineNext(b, txs);
+                chain.Append(b);
+            }
+
+            tracker.ClearLogs();
+            int testingDepth = addresses.Length / 2;
+            Address[] targetAddresses = Enumerable.Range(
+                testingDepth,
+                Math.Min(10, addresses.Length - testingDepth - 1)
+            ).Select(i => addresses[i]).ToArray();
+            AddressStateMap result = chain.GetStates(targetAddresses);
+            foreach (Address targetAddress in targetAddresses)
+            {
+                Assert.True(result.ContainsKey(targetAddress));
+            }
+
+            var callCount = tracker.Logs.Where(
+                triple => triple.Item1.Equals("GetBlockStates")
+            ).Select(triple => triple.Item2).Count();
+            Assert.True(testingDepth >= callCount);
+        }
+
         [Fact]
         public void EvaluateActions()
         {
@@ -264,6 +316,18 @@ public void EvaluateActions()
             Assert.Equal(states[TestEvaluateAction.BlockIndexKey], blockIndex);
         }
 
+        private sealed class NullPolicy<T> : IBlockPolicy<T>
+            where T : IAction, new()
+        {
+            public int GetNextBlockDifficulty(IEnumerable<Block<T>> blocks) =>
+                blocks.Any() ? 1 : 0;
+
+            public InvalidBlockException ValidateBlocks(
+                IEnumerable<Block<T>> blocks,
+                DateTimeOffset currentTime
+            ) => null;
+        }
+
         private sealed class TestEvaluateAction : IAction
         {
             public static readonly Address SignerKey =

diff --git a/Libplanet.Tests/Common/Action/DumbAction.cs b/Libplanet.Tests/Common/Action/DumbAction.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Immutable;
 using System.Threading;
 using Libplanet.Action;
@@ -34,7 +35,7 @@ public DumbAction(
         public IImmutableDictionary<string, object> PlainValue =>
             ImmutableDictionary<string, object>.Empty
                 .Add("item", Item)
-                .Add("target_address", TargetAddress)
+                .Add("target_address", TargetAddress.ToByteArray())
                 .Add("record_rehearsal", RecordRehearsal);
 
         public IAccountStateDelta Execute(IActionContext context)
@@ -69,7 +70,7 @@ IImmutableDictionary<string, object> plainValue
         )
         {
             Item = (string)plainValue["item"];
-            TargetAddress = (Address)plainValue["target_address"];
+            TargetAddress = new Address((byte[])plainValue["target_address"]);
             RecordRehearsal = (bool)plainValue["record_rehearsal"];
         }
     }

diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs
@@ -165,17 +165,19 @@ public AddressStateMap GetStates(
                 _rwlock.ExitReadLock();
             }
 
+            ImmutableHashSet<Address> requestedAddresses =
+                addresses.ToImmutableHashSet();
             var states = new AddressStateMap();
             while (offset != null)
             {
                 states = (AddressStateMap)states.SetItems(
                     Store.GetBlockStates(offset.Value)
                     .Where(
-                        kv => addresses.Contains(kv.Key) &&
+                        kv => requestedAddresses.Contains(kv.Key) &&
                         !states.ContainsKey(kv.Key))
                     );
 
-                if (states.Keys.SequenceEqual(addresses))
+                if (requestedAddresses.SetEquals(states.Keys))
                 {
                     break;
                 }