From 73c36854588baa9d02138e951271bd96fa85d75d Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 22 Jul 2019 22:07:45 +0900 Subject: [PATCH 1/5] Make Swarm's direct access code to store atomic It's only temporary code in a transition code. [changelog skip] --- Libplanet/Blockchain/BlockChain.cs | 8 +- Libplanet/Net/Swarm.cs | 168 ++++++++++++++++++----------- Menees.Analyzers.Settings.xml | 2 +- 3 files changed, 113 insertions(+), 65 deletions(-) diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs index 2232e6ed26f..b0da02833fa 100644 --- a/Libplanet/Blockchain/BlockChain.cs +++ b/Libplanet/Blockchain/BlockChain.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Runtime.CompilerServices; using System.Security.Cryptography; @@ -19,7 +20,12 @@ namespace Libplanet.Blockchain public class BlockChain : IReadOnlyList> where T : IAction, new() { - private readonly ReaderWriterLockSlim _rwlock; + // FIXME: The _rwlock field should be private. + [SuppressMessage( + "StyleCop.CSharp.OrderingRules", + "SA1401:FieldsMustBePrivate", + Justification = "Temporary visibility.")] + internal readonly ReaderWriterLockSlim _rwlock; private readonly object _txLock; public BlockChain(IBlockPolicy policy, IStore store) diff --git a/Libplanet/Net/Swarm.cs b/Libplanet/Net/Swarm.cs index 8ccd82822fe..d471180b68c 100644 --- a/Libplanet/Net/Swarm.cs +++ b/Libplanet/Net/Swarm.cs @@ -619,33 +619,55 @@ await SyncBehindsBlocksFromPeersAsync( if (!received) { - // FIXME: Swam should not directly access to the IStore instance, - // but BlockChain should have an indirect interface to its underlying store. - IStore store = _blockChain.Store; - - foreach (Block block in _blockChain) + ReaderWriterLockSlim rwlock = _blockChain._rwlock; + rwlock.EnterUpgradeableReadLock(); + try { - if (store.GetBlockStates(block.Hash) is null) + // FIXME: Swam should not directly access to the IStore instance, + // but BlockChain should have an indirect interface to its underlying store. + IStore store = _blockChain.Store; + + foreach (Block block in _blockChain) { - AccountStateGetter stateGetter; - if (block.PreviousHash is null) - { - stateGetter = _ => null; - } - else + if (store.GetBlockStates(block.Hash) is null) { - stateGetter = a => - _blockChain.GetStates( - new[] { a }, - block.PreviousHash - ).GetValueOrDefault(a); - } + AccountStateGetter stateGetter; + if (block.PreviousHash is null) + { + stateGetter = _ => null; + } + else + { + stateGetter = a => + _blockChain.GetStates( + new[] { a }, + block.PreviousHash + ).GetValueOrDefault(a); + } - IReadOnlyList> evaluations = - block.Evaluate(DateTimeOffset.UtcNow, stateGetter).ToImmutableList(); - _blockChain.SetStates(block, evaluations, buildStateReferences: true); + IReadOnlyList> evaluations = + block.Evaluate(DateTimeOffset.UtcNow, stateGetter) + .ToImmutableList(); + rwlock.EnterWriteLock(); + try + { + _blockChain.SetStates( + block, + evaluations, + buildStateReferences: true + ); + } + finally + { + rwlock.ExitWriteLock(); + } + } } } + finally + { + rwlock.ExitUpgradeableReadLock(); + } } } @@ -960,29 +982,40 @@ await socket.SendMultipartMessageAsync( Message parsedMessage = Message.Parse(reply, reply: true); if (parsedMessage is RecentStates recentStates && !recentStates.Missing) { - // FIXME: Swarm should not directly access to the IStore instance, - // but BlockChain should have an indirect interface to its underlying - // store. - IStore store = BlockChain.Store; - string ns = BlockChain.Id.ToString(); - - _logger.Debug("Starts to store state refs received from {0}.", peer); - foreach (var pair in recentStates.StateReferences) + ReaderWriterLockSlim rwlock = BlockChain._rwlock; + rwlock.EnterWriteLock(); + try { - IImmutableSet
address = ImmutableHashSet.Create(pair.Key); - foreach (HashDigest bHash in pair.Value) + // FIXME: Swarm should not directly access to the IStore instance, + // but BlockChain should have an indirect interface to its underlying + // store. + IStore store = BlockChain.Store; + string ns = BlockChain.Id.ToString(); + + _logger.Debug("Starts to store state refs received from {0}.", peer); + foreach (var pair in recentStates.StateReferences) { - store.StoreStateReference(ns, address, store.GetBlock(bHash)); + IImmutableSet
address = ImmutableHashSet.Create(pair.Key); + foreach (HashDigest bHash in pair.Value) + { + Block block = store.GetBlock(bHash); + store.StoreStateReference(ns, address, block); + } } - } - _logger.Debug("Starts to store block states received from {0}.", peer); - foreach (var pair in recentStates.BlockStates) + _logger.Debug("Starts to store block states received from {0}.", peer); + foreach (var pair in recentStates.BlockStates) + { + store.SetBlockStates(pair.Key, new AddressStateMap(pair.Value)); + } + } + finally { - store.SetBlockStates(pair.Key, new AddressStateMap(pair.Value)); + rwlock.ExitWriteLock(); } - _logger.Debug("Finished to store received state refs and block states."); + _logger.Debug( + "Finished to store received state refs and block states."); return true; } @@ -1501,34 +1534,43 @@ private void TransferRecentStates(GetRecentStates getRecentStates) if (_blockChain.Blocks.ContainsKey(blockHash)) { - // FIXME: Swarm should not directly access to the IStore instance, - // but BlockChain should have an indirect interface to its underlying - // store. - IStore store = _blockChain.Store; - string ns = _blockChain.Id.ToString(); + ReaderWriterLockSlim rwlock = _blockChain._rwlock; + rwlock.EnterReadLock(); + try + { + // FIXME: Swarm should not directly access to the IStore instance, + // but BlockChain should have an indirect interface to its underlying + // store. + IStore store = _blockChain.Store; + string ns = _blockChain.Id.ToString(); - stateRefs = store.ListAddresses(ns).Select(address => + stateRefs = store.ListAddresses(ns).Select(address => + { + ImmutableList> refs = + store.IterateStateReferences(ns, address).Select( + p => p.Item1 + ).Reverse().ToImmutableList(); + return + new KeyValuePair>>( + address, refs + ); + }).ToImmutableDictionary(); + + blockStates = stateRefs.Values + .Select(refs => refs.Last()) + .ToImmutableHashSet() + .Select(bh => + new KeyValuePair< + HashDigest, + IImmutableDictionary + >(bh, store.GetBlockStates(bh)) + ) + .ToImmutableDictionary(); + } + finally { - ImmutableList> refs = - store.IterateStateReferences(ns, address).Select( - p => p.Item1 - ).Reverse().ToImmutableList(); - return - new KeyValuePair>>( - address, refs - ); - }).ToImmutableDictionary(); - - blockStates = stateRefs.Values - .Select(refs => refs.Last()) - .ToImmutableHashSet() - .Select(bh => - new KeyValuePair< - HashDigest, - IImmutableDictionary - >(bh, store.GetBlockStates(bh)) - ) - .ToImmutableDictionary(); + rwlock.ExitReadLock(); + } } var reply = new RecentStates(blockHash, blockStates, stateRefs) diff --git a/Menees.Analyzers.Settings.xml b/Menees.Analyzers.Settings.xml index 8d4a6d26c0e..1524c92b211 100644 --- a/Menees.Analyzers.Settings.xml +++ b/Menees.Analyzers.Settings.xml @@ -2,5 +2,5 @@ 100 200 - 2050 + 2100 From ddb80907981f4bb663dee510397e77f7a4a9cb63 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 22 Jul 2019 23:55:33 +0900 Subject: [PATCH 2/5] Make NullPolicy public --- Libplanet.Tests/Blockchain/BlockChainTest.cs | 18 -------------- Libplanet.Tests/Blockchain/NullPolicy.cs | 26 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 18 deletions(-) create mode 100644 Libplanet.Tests/Blockchain/NullPolicy.cs diff --git a/Libplanet.Tests/Blockchain/BlockChainTest.cs b/Libplanet.Tests/Blockchain/BlockChainTest.cs index df19ee170c4..b54261a3510 100644 --- a/Libplanet.Tests/Blockchain/BlockChainTest.cs +++ b/Libplanet.Tests/Blockchain/BlockChainTest.cs @@ -1299,24 +1299,6 @@ void BuildIndex(Guid id, Block block) return (addresses, new[] { b0, b1 }); } - private sealed class NullPolicy : IBlockPolicy - where T : IAction, new() - { - private readonly InvalidBlockException _exceptionToThrow; - - public NullPolicy(InvalidBlockException exceptionToThrow = null) - { - _exceptionToThrow = exceptionToThrow; - } - - public long GetNextBlockDifficulty(IReadOnlyList> blocks) => - blocks.Any() ? 1 : 0; - - public InvalidBlockException ValidateNextBlock( - IReadOnlyList> blocks, Block nextBlock) => - _exceptionToThrow; - } - private sealed class TestEvaluateAction : IAction { public static readonly Address SignerKey = diff --git a/Libplanet.Tests/Blockchain/NullPolicy.cs b/Libplanet.Tests/Blockchain/NullPolicy.cs new file mode 100644 index 00000000000..f75d84c5e8d --- /dev/null +++ b/Libplanet.Tests/Blockchain/NullPolicy.cs @@ -0,0 +1,26 @@ +using System.Collections.Generic; +using System.Linq; +using Libplanet.Action; +using Libplanet.Blockchain.Policies; +using Libplanet.Blocks; + +namespace Libplanet.Tests.Blockchain +{ + public sealed class NullPolicy : IBlockPolicy + where T : IAction, new() + { + private readonly InvalidBlockException _exceptionToThrow; + + public NullPolicy(InvalidBlockException exceptionToThrow = null) + { + _exceptionToThrow = exceptionToThrow; + } + + public long GetNextBlockDifficulty(IReadOnlyList> blocks) => + blocks.Any() ? 1 : 0; + + public InvalidBlockException ValidateNextBlock( + IReadOnlyList> blocks, Block nextBlock) => + _exceptionToThrow; + } +} From 45125278c0aee73f35086dbdf2800b815c0a6eb4 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 22 Jul 2019 23:58:51 +0900 Subject: [PATCH 3/5] StoreExtension.ListAllStateReferences() method --- CHANGES.md | 3 ++ Libplanet.Tests/Store/StoreExtensionTest.cs | 53 +++++++++++++++++++++ Libplanet/Net/Swarm.cs | 12 +---- Libplanet/Store/IStore.cs | 4 +- Libplanet/Store/StoreExtension.cs | 24 ++++++++++ 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index eec74b6175e..fe327e78e24 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,8 @@ To be released. a feasible user interface so that they can decide whom to trust for themselves. [[#272], [#343]] + - Added `StoreExtension.ListAllStateReferences(this IStore, string)` extension + method. [[#363]] ### Behavioral changes @@ -42,6 +44,7 @@ To be released. [#343]: https://github.com/planetarium/libplanet/pull/343 [#350]: https://github.com/planetarium/libplanet/pull/350 +[#363]: https://github.com/planetarium/libplanet/pull/363 [#365]: https://github.com/planetarium/libplanet/pull/365 [#366]: https://github.com/planetarium/libplanet/pull/366 diff --git a/Libplanet.Tests/Store/StoreExtensionTest.cs b/Libplanet.Tests/Store/StoreExtensionTest.cs index 38a2bf87bca..4f1de6e1d64 100644 --- a/Libplanet.Tests/Store/StoreExtensionTest.cs +++ b/Libplanet.Tests/Store/StoreExtensionTest.cs @@ -1,6 +1,11 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Security.Cryptography; +using Libplanet.Blockchain; using Libplanet.Blocks; +using Libplanet.Crypto; +using Libplanet.Tests.Blockchain; using Libplanet.Tests.Common.Action; using Libplanet.Tx; using Xunit; @@ -74,5 +79,53 @@ public void LookupStateReference(StoreFixture fx) fx.Store.LookupStateReference(fx.StoreNamespace, address, block6) ); } + + [Theory] + [MemberData(nameof(StoreFixtures))] + public void ListAllStateReferences(StoreFixture fx) + { + Address address1 = fx.Address1; + Address address2 = fx.Address2; + Address address3 = new PrivateKey().PublicKey.ToAddress(); + + Transaction tx4 = fx.MakeTransaction( + new[] + { + new DumbAction(address1, "foo1"), + new DumbAction(address2, "foo2"), + } + ); + Block block4 = TestUtils.MineNext(fx.Block3, new[] { tx4 }); + + Transaction tx5 = fx.MakeTransaction( + new[] + { + new DumbAction(address1, "bar1"), + new DumbAction(address3, "bar3"), + } + ); + Block block5 = TestUtils.MineNext(block4, new[] { tx5 }); + + Block block6 = TestUtils.MineNext(block5, new Transaction[0]); + + var chain = new BlockChain(new NullPolicy(), fx.Store); + chain.Append(fx.Block1); + chain.Append(fx.Block2); + chain.Append(fx.Block3); + chain.Append(block4); + chain.Append(block5); + chain.Append(block6); + + IImmutableDictionary>> refs = + fx.Store.ListAllStateReferences(chain.Id.ToString()); + + Assert.Equal( + new HashSet
{ address1, address2, address3 }, + refs.Keys.ToHashSet() + ); + Assert.Equal(new[] { block4.Hash, block5.Hash }, refs[address1]); + Assert.Equal(new[] { block4.Hash }, refs[address2]); + Assert.Equal(new[] { block5.Hash }, refs[address3]); + } } } diff --git a/Libplanet/Net/Swarm.cs b/Libplanet/Net/Swarm.cs index d471180b68c..6a21bfdb1fc 100644 --- a/Libplanet/Net/Swarm.cs +++ b/Libplanet/Net/Swarm.cs @@ -1544,17 +1544,7 @@ private void TransferRecentStates(GetRecentStates getRecentStates) IStore store = _blockChain.Store; string ns = _blockChain.Id.ToString(); - stateRefs = store.ListAddresses(ns).Select(address => - { - ImmutableList> refs = - store.IterateStateReferences(ns, address).Select( - p => p.Item1 - ).Reverse().ToImmutableList(); - return - new KeyValuePair>>( - address, refs - ); - }).ToImmutableDictionary(); + stateRefs = store.ListAllStateReferences(ns); blockStates = stateRefs.Values .Select(refs => refs.Last()) diff --git a/Libplanet/Store/IStore.cs b/Libplanet/Store/IStore.cs index 2759ed1304b..b21fdda60bb 100644 --- a/Libplanet/Store/IStore.cs +++ b/Libplanet/Store/IStore.cs @@ -134,8 +134,8 @@ AddressStateMap states /// The chain namespace. /// The to get state references. /// Ordered pairs of a state reference and a corresponding - /// . The highest index (i.e., the closest to the tip) go - /// first and the lowest index (i.e., the closest to the genesis) go last. + /// . The highest index (i.e., the closest to the tip) goes + /// first and the lowest index (i.e., the closest to the genesis) goes last. /// IEnumerable, long>> IterateStateReferences( string @namespace, diff --git a/Libplanet/Store/StoreExtension.cs b/Libplanet/Store/StoreExtension.cs index caee1420009..dc8161351cd 100644 --- a/Libplanet/Store/StoreExtension.cs +++ b/Libplanet/Store/StoreExtension.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using System.Security.Cryptography; using Libplanet.Action; using Libplanet.Blocks; @@ -50,5 +51,28 @@ public static Tuple, long> LookupStateReference( return null; } + + /// + /// Lists all accounts, that have any states, in the given and + /// their state references. + /// + /// A store object. + /// The namespace to look up state references. + /// A dictionary of account addresses to lists of their corresponding state + /// references. Each list of state references is in ascending order, i.e., the block + /// closest to the genesis goes first and the block closest to the tip goes last. + public static IImmutableDictionary>> + ListAllStateReferences(this IStore store, string @namespace) + { + return store.ListAddresses(@namespace).Select(address => + { + ImmutableList> refs = store + .IterateStateReferences(@namespace, address) + .Select(p => p.Item1) + .Reverse() + .ToImmutableList(); + return new KeyValuePair>>(address, refs); + }).ToImmutableDictionary(); + } } } From b5342123d8466cea0fc3d52f58d0dacd97bfd5a0 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 23 Jul 2019 19:59:18 +0900 Subject: [PATCH 4/5] Make Address to implement IComparable --- CHANGES.md | 2 ++ Libplanet.Tests/AddressTest.cs | 35 ++++++++++++++++++++++++++++++++++ Libplanet/Address.cs | 34 ++++++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index fe327e78e24..f71d74ebd74 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,6 +25,8 @@ To be released. [[#272], [#343]] - Added `StoreExtension.ListAllStateReferences(this IStore, string)` extension method. [[#363]] + - `Address` class became to implement `IComparable
` and + `IComparable` interfaces. [[#363]] ### Behavioral changes diff --git a/Libplanet.Tests/AddressTest.cs b/Libplanet.Tests/AddressTest.cs index a4e54fa0edd..8096ae37b53 100644 --- a/Libplanet.Tests/AddressTest.cs +++ b/Libplanet.Tests/AddressTest.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Linq; using System.Runtime.Serialization.Formatters.Binary; using Libplanet.Crypto; using Xunit; @@ -221,5 +222,39 @@ public void CanSerializeAndDeserialize() Assert.Equal(deserializedAddress, expectedAddress); } + + [Fact] + public void Compare() + { + var random = new System.Random(); + var buffer = new byte[20]; + Address[] addresses = Enumerable.Repeat(0, 50).Select(_ => + { + random.NextBytes(buffer); + return new Address(buffer); + }).ToArray(); + for (int i = 1; i < addresses.Length; i++) + { + IComparable
left = addresses[i - 1]; + Address right = addresses[i]; + string leftString = addresses[i - 1].ToHex().ToLower(), + rightString = right.ToHex().ToLower(); + Assert.Equal( + Math.Min(Math.Max(left.CompareTo(right), 1), -1), + Math.Min(Math.Max(leftString.CompareTo(rightString), 1), -1) + ); + Assert.Equal( + left.CompareTo(right), + (left as IComparable).CompareTo(right) + ); + } + + Assert.Throws(() => + (addresses[0] as IComparable).CompareTo(null) + ); + Assert.Throws(() => + (addresses[0] as IComparable).CompareTo("invalid") + ); + } } } diff --git a/Libplanet/Address.cs b/Libplanet/Address.cs index a5f91bfd952..39746f43baf 100644 --- a/Libplanet/Address.cs +++ b/Libplanet/Address.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics.Contracts; using System.Linq; @@ -39,7 +40,7 @@ namespace Libplanet /// [Serializable] [Equals] - public struct Address : ISerializable + public struct Address : ISerializable, IComparable
, IComparable { /// /// The s size that each takes. @@ -210,6 +211,37 @@ public void GetObjectData( info.AddValue("address", _byteArray.ToArray()); } + int IComparable
.CompareTo(Address other) + { + ImmutableArray self = ByteArray, operand = other.ByteArray; + + for (int i = 0; i < Size; i++) + { + int cmp = ((IComparable)self[i]).CompareTo(operand[i]); + if (cmp != 0) + { + return cmp; + } + } + + return 0; + } + + int IComparable.CompareTo(object obj) + { + if (obj is Address other) + { + return ((IComparable
)this).CompareTo(other); + } + + if (obj is null) + { + throw new ArgumentNullException(nameof(obj)); + } + + throw new ArgumentException(nameof(obj)); + } + private static string ToChecksumAddress(string hex) { byte[] bytes = Encoding.ASCII.GetBytes(hex); From 8886884f7103d5c15e4e712506579733d383de28 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 23 Jul 2019 20:01:54 +0900 Subject: [PATCH 5/5] Refactor duplicate code for executing actions --- Libplanet.Tests/Blockchain/BlockChainTest.cs | 47 +++++++++++-- Libplanet/Blockchain/BlockChain.cs | 73 ++++++++++++++++---- Libplanet/Net/Swarm.cs | 49 +------------ 3 files changed, 101 insertions(+), 68 deletions(-) diff --git a/Libplanet.Tests/Blockchain/BlockChainTest.cs b/Libplanet.Tests/Blockchain/BlockChainTest.cs index b54261a3510..eeffa99555b 100644 --- a/Libplanet.Tests/Blockchain/BlockChainTest.cs +++ b/Libplanet.Tests/Blockchain/BlockChainTest.cs @@ -263,7 +263,11 @@ public void Append() DumbAction.RenderRecords.Value = ImmutableList.Empty; - (Address[] addresses, Block[] blocks) = MakeFixturesForAppendTests(); + ( + IImmutableDictionary expectedStates, + Block[] blocks + ) = MakeFixturesForAppendTests(); + Address[] addresses = expectedStates.Keys.ToArray(); Assert.Empty(_blockChain.Blocks); try @@ -271,11 +275,13 @@ public void Append() _blockChain.Append(blocks[0]); Assert.Contains(blocks[0], _blockChain); Assert.Equal(new[] { blocks[0] }, _blockChain.ToArray()); + Assert.NotNull(_blockChain.Store.GetBlockStates(blocks[0].Hash)); Assert.Empty(DumbAction.RenderRecords.Value); _blockChain.Append(blocks[1]); Assert.Contains(blocks[1], _blockChain); Assert.Equal(blocks, _blockChain.ToArray()); + Assert.NotNull(_blockChain.Store.GetBlockStates(blocks[1].Hash)); var renders = DumbAction.RenderRecords.Value; Assert.Equal(4, renders.Count); Assert.True(renders.All(r => r.Render)); @@ -316,7 +322,7 @@ public void Append() addresses.Select(renders[3].Context.PreviousStates.GetState) ); Assert.Equal( - new[] { "foo", "bar", "baz", "qux" }, + expectedStates.Values, addresses.Select(renders[3].NextStates.GetState) ); } @@ -332,7 +338,7 @@ public void AppendWithoutEvaluateActions() { DumbAction.RenderRecords.Value = ImmutableList.Empty; - (Address[] addresses, Block[] blocks) = MakeFixturesForAppendTests(); + (_, Block[] blocks) = MakeFixturesForAppendTests(); try { @@ -374,6 +380,25 @@ public void AppendValidatesBlock() () => blockChain.Append(_fx.Block1)); } + [Fact] + public void ExecuteActions() + { + (var expectedStates, Block[] blocks) = MakeFixturesForAppendTests(); + _blockChain.Append(blocks[0]); + _blockChain.Append( + blocks[1], + DateTimeOffset.UtcNow, + evaluateActions: false, + renderActions: false + ); + + _blockChain.ExecuteActions(blocks[1], true); + Assert.Equal( + expectedStates.ToImmutableDictionary(), + _blockChain.Store.GetBlockStates(blocks[1].Hash) + ); + } + [Fact] public void FindNextHashes() { @@ -627,7 +652,8 @@ public void CanGetBlockLocator() [InlineData(false)] public void Swap(bool render) { - (Address[] addresses, Block[] blocks) = MakeFixturesForAppendTests(); + (var expectedStates, Block[] blocks) = MakeFixturesForAppendTests(); + Address[] addresses = expectedStates.Keys.ToArray(); foreach (Block block in blocks) { _blockChain.Append(block); @@ -1267,10 +1293,12 @@ void BuildIndex(Guid id, Block block) return (signer, addresses, chain); } - private (Address[] addresses, Block[]) MakeFixturesForAppendTests() + private (ImmutableSortedDictionary, Block[]) + MakeFixturesForAppendTests() { Address[] addresses = Enumerable.Repeat(0, 4) .Select(_ => new PrivateKey().PublicKey.ToAddress()) + .OrderBy(a => a) .ToArray(); Block b0 = TestUtils.MineGenesis(); @@ -1296,7 +1324,14 @@ void BuildIndex(Guid id, Block block) ) ); - return (addresses, new[] { b0, b1 }); + var expectedStates = new SortedDictionary + { + { addresses[0], "foo" }, + { addresses[1], "bar" }, + { addresses[2], "baz" }, + { addresses[3], "qux" }, + }; + return (expectedStates.ToImmutableSortedDictionary(), new[] { b0, b1 }); } private sealed class TestEvaluateAction : IAction diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs index b0da02833fa..f2b973f6026 100644 --- a/Libplanet/Blockchain/BlockChain.cs +++ b/Libplanet/Blockchain/BlockChain.cs @@ -533,7 +533,6 @@ bool renderActions } _rwlock.EnterUpgradeableReadLock(); - ActionEvaluation[] evaluations = null; try { InvalidBlockException e = @@ -568,14 +567,6 @@ bool renderActions nonceDeltas[txSigner] = nonceDelta + 1; } - if (evaluateActions) - { - evaluations = block.Evaluate( - currentTime, - a => GetStates(new[] { a }, tip).GetValueOrDefault(a) - ).ToArray(); - } - _rwlock.EnterWriteLock(); try { @@ -585,11 +576,6 @@ bool renderActions Store.IncreaseTxNonce(ns, pair.Key, pair.Value); } - if (!(evaluations is null)) - { - SetStates(block, evaluations, buildStateReferences: true); - } - Store.AppendIndex(ns, block.Hash); ISet txIds = block.Transactions .Select(t => t.Id) @@ -607,8 +593,65 @@ bool renderActions _rwlock.ExitUpgradeableReadLock(); } - if (!(evaluations is null) && renderActions) + if (evaluateActions) { + ExecuteActions(block, renderActions); + } + } + + /// + /// Evaluates actions in the given and fills states with the + /// results, and renders them if is turned on. + /// + /// A block to execute. + /// Whether to render actions. This is not idempotent; even if + /// the given has executed before in the blockchain, + /// its actions are rendered anyway. + /// This method is idempotent (except for rendering). If the given + /// has executed before, it does not execute it nor mutate states. + /// + internal void ExecuteActions(Block block, bool render) + { + IReadOnlyList> EvaluateActions() + { + AccountStateGetter stateGetter; + if (block.PreviousHash is null) + { + stateGetter = _ => null; + } + else + { + stateGetter = a => + GetStates(new[] { a }, block.PreviousHash).GetValueOrDefault(a); + } + + return block + .Evaluate(DateTimeOffset.UtcNow, stateGetter) + .ToImmutableList(); + } + + IReadOnlyList> evaluations = null; + if (Store.GetBlockStates(block.Hash) is null) + { + evaluations = EvaluateActions(); + _rwlock.EnterWriteLock(); + try + { + SetStates(block, evaluations, buildStateReferences: true); + } + finally + { + _rwlock.ExitWriteLock(); + } + } + + if (render) + { + if (evaluations is null) + { + evaluations = EvaluateActions(); + } + foreach (var evaluation in evaluations) { evaluation.Action.Render( diff --git a/Libplanet/Net/Swarm.cs b/Libplanet/Net/Swarm.cs index 6a21bfdb1fc..dee2e41c1f5 100644 --- a/Libplanet/Net/Swarm.cs +++ b/Libplanet/Net/Swarm.cs @@ -619,54 +619,9 @@ await SyncBehindsBlocksFromPeersAsync( if (!received) { - ReaderWriterLockSlim rwlock = _blockChain._rwlock; - rwlock.EnterUpgradeableReadLock(); - try - { - // FIXME: Swam should not directly access to the IStore instance, - // but BlockChain should have an indirect interface to its underlying store. - IStore store = _blockChain.Store; - - foreach (Block block in _blockChain) - { - if (store.GetBlockStates(block.Hash) is null) - { - AccountStateGetter stateGetter; - if (block.PreviousHash is null) - { - stateGetter = _ => null; - } - else - { - stateGetter = a => - _blockChain.GetStates( - new[] { a }, - block.PreviousHash - ).GetValueOrDefault(a); - } - - IReadOnlyList> evaluations = - block.Evaluate(DateTimeOffset.UtcNow, stateGetter) - .ToImmutableList(); - rwlock.EnterWriteLock(); - try - { - _blockChain.SetStates( - block, - evaluations, - buildStateReferences: true - ); - } - finally - { - rwlock.ExitWriteLock(); - } - } - } - } - finally + foreach (Block block in _blockChain) { - rwlock.ExitUpgradeableReadLock(); + _blockChain.ExecuteActions(block, render: false); } } }