Refactor WhitelistedHashesRepository

src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs

@@ -24,8 +24,8 @@ public VotingManagerTests()
             this.changesApplied = new List<VotingData>();
             this.changesReverted = new List<VotingData>();
 
-            this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny<VotingData>())).Callback((VotingData data) => this.changesApplied.Add(data));
-            this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny<VotingData>())).Callback((VotingData data) => this.changesReverted.Add(data));
+            this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny<VotingData>(), It.IsAny<int>())).Callback((VotingData data, int _) => this.changesApplied.Add(data));
+            this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny<VotingData>(), It.IsAny<int>())).Callback((VotingData data, int _) => this.changesReverted.Add(data));
         }
 
         [Fact]

src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs

@@ -137,7 +137,7 @@ public override Task InitializeAsync()
             }
 
             this.federationManager.Initialize();
-            this.whitelistedHashesRepository.Initialize();
+            this.whitelistedHashesRepository.Initialize(this.votingManager);
 
             if (!this.votingManager.Synchronize(this.chainIndexer.Tip))
                 throw new System.OperationCanceledException();

src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs

@@ -8,11 +8,13 @@ public interface IPollResultExecutor
     {
         /// <summary>Applies effect of <see cref="VotingData"/>.</summary>
         /// <param name="data">See <see cref="VotingData"/>.</param>
-        void ApplyChange(VotingData data);
+        /// <param name="executionHeight">The height from which the change should take effect.</param>
+        void ApplyChange(VotingData data, int executionHeight);
 
         /// <summary>Reverts effect of <see cref="VotingData"/>.</summary>
         /// <param name="data">See <see cref="VotingData"/>.</param>
-        void RevertChange(VotingData data);
+        /// <param name="executionHeight">The height from which the change should take effect.</param>
+        void RevertChange(VotingData data, int executionHeight);
 
         /// <summary>Converts <see cref="VotingData"/> to a human readable format.</summary>
         /// <param name="data">See <see cref="VotingData"/>.</param>
@@ -40,7 +42,7 @@ public PollResultExecutor(IFederationManager federationManager, ILoggerFactory l
         }
 
         /// <inheritdoc />
-        public void ApplyChange(VotingData data)
+        public void ApplyChange(VotingData data, int executionHeight)
         {
             switch (data.Key)
             {
@@ -53,17 +55,17 @@ public void ApplyChange(VotingData data)
                     break;
 
                 case VoteKey.WhitelistHash:
-                    this.AddHash(data.Data);
+                    this.AddHash(data.Data, executionHeight);
                     break;
 
                 case VoteKey.RemoveHash:
-                    this.RemoveHash(data.Data);
+                    this.RemoveHash(data.Data, executionHeight);
                     break;
             }
         }
 
         /// <inheritdoc />
-        public void RevertChange(VotingData data)
+        public void RevertChange(VotingData data, int executionHeight)
         {
             switch (data.Key)
             {
@@ -76,11 +78,11 @@ public void RevertChange(VotingData data)
                     break;
 
                 case VoteKey.WhitelistHash:
-                    this.RemoveHash(data.Data);
+                    this.RemoveHash(data.Data, executionHeight);
                     break;
 
                 case VoteKey.RemoveHash:
-                    this.AddHash(data.Data);
+                    this.AddHash(data.Data, executionHeight);
                     break;
             }
         }
@@ -118,32 +120,32 @@ public void RemoveFederationMember(byte[] federationMemberBytes)
             this.federationManager.RemoveFederationMember(federationMember);
         }
 
-        private void AddHash(byte[] hashBytes)
+        private void AddHash(byte[] hashBytes, int executionHeight)
         {
             try
             {
                 var hash = new uint256(hashBytes);
 
-                this.whitelistedHashesRepository.AddHash(hash);
+                this.whitelistedHashesRepository.AddHash(hash, executionHeight);
             }
             catch (FormatException e)
             {
                 this.logger.LogWarning("Hash had incorrect format: '{0}'.", e.ToString());
             }
         }
 
-        private void RemoveHash(byte[] hashBytes)
+        private void RemoveHash(byte[] hashBytes, int executionHeight)
         {
             try
             {
                 var hash = new uint256(hashBytes);
 
-                this.whitelistedHashesRepository.RemoveHash(hash);
+                this.whitelistedHashesRepository.RemoveHash(hash, executionHeight);
             }
             catch (FormatException e)
             {
                 this.logger.LogWarning("Hash had incorrect format: '{0}'.", e.ToString());
             }
         }
     }
-}
+}

src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs

@@ -531,9 +531,7 @@ private void ProcessBlock(PollsRepository.Transaction transaction, ChainedHeader
                 {
                     this.signals.Publish(new VotingManagerProcessBlock(chBlock, transaction));
 
-                    bool pollsRepositoryModified = false;
-
-                    foreach (Poll poll in this.polls.GetPollsToExecuteOrExpire(chBlock.ChainedHeader.Height))
+                    foreach (Poll poll in this.polls.GetPollsToExecuteOrExpire(chBlock.ChainedHeader.Height).OrderBy(p => p.Id))
                     {
                         if (!poll.IsApproved)
                         {
@@ -547,7 +545,7 @@ private void ProcessBlock(PollsRepository.Transaction transaction, ChainedHeader
                         else
                         {
                             this.logger.LogDebug("Applying poll '{0}'.", poll);
-                            this.pollResultExecutor.ApplyChange(poll.VotingData);
+                            this.pollResultExecutor.ApplyChange(poll.VotingData, chBlock.ChainedHeader.Height);
 
                             this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = new HashHeightPair(chBlock.ChainedHeader));
                             transaction.UpdatePoll(poll);
@@ -691,16 +689,22 @@ private void UnProcessBlock(PollsRepository.Transaction transaction, ChainedHead
         {
             lock (this.locker)
             {
-                foreach (Poll poll in this.polls.Where(x => !x.IsPending && x.PollExecutedBlockData?.Hash == chBlock.ChainedHeader.HashBlock).ToList())
+                foreach (Poll poll in this.polls
+                    .Where(x => !x.IsPending && x.PollExecutedBlockData?.Hash == chBlock.ChainedHeader.HashBlock)
+                    .OrderByDescending(p => p.Id)
+                    .ToList())
                 {
                     this.logger.LogDebug("Reverting poll execution '{0}'.", poll);
-                    this.pollResultExecutor.RevertChange(poll.VotingData);
+                    this.pollResultExecutor.RevertChange(poll.VotingData, chBlock.ChainedHeader.Height);
 
                     this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = null);
                     transaction.UpdatePoll(poll);
                 }
 
-                foreach (Poll poll in this.polls.Where(x => x.IsExpired && !PollsRepository.IsPollExpiredAt(x, chBlock.ChainedHeader.Height - 1, this.network as PoANetwork)).ToList())
+                foreach (Poll poll in this.polls
+                    .Where(x => x.IsExpired && !PollsRepository.IsPollExpiredAt(x, chBlock.ChainedHeader.Height - 1, this.network as PoANetwork))
+                    .OrderByDescending(p => p.Id)
+                    .ToList())
                 {
                     this.logger.LogDebug("Reverting poll expiry '{0}'.", poll);
 

src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs

@@ -1,93 +1,163 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Extensions.Logging;
 using NBitcoin;
-using Stratis.Bitcoin.Persistence;
+using Stratis.Bitcoin.Utilities;
 
 namespace Stratis.Bitcoin.Features.PoA.Voting
 {
     public class WhitelistedHashesRepository : IWhitelistedHashesRepository
     {
-        private const string dbKey = "hashesList";
-
-        private readonly IKeyValueRepository kvRepository;
-
         /// <summary>Protects access to <see cref="whitelistedHashes"/>.</summary>
         private readonly object locker;
 
         private readonly ILogger logger;
 
-        private List<uint256> whitelistedHashes;
+        private readonly PoAConsensusOptions poaConsensusOptions;
 
-        public WhitelistedHashesRepository(ILoggerFactory loggerFactory, IKeyValueRepository kvRepository)
+        // Dictionary of hash histories. Even list entries are additions and odd entries are removals.
+        private Dictionary<uint256, int[]> whitelistedHashes;
+
+        public WhitelistedHashesRepository(ILoggerFactory loggerFactory, Network network)
         {
-            this.kvRepository = kvRepository;
             this.locker = new object();
 
             this.logger = loggerFactory.CreateLogger(this.GetType().FullName);
+            this.poaConsensusOptions = network.Consensus.Options as PoAConsensusOptions;
+        }
 
-            // Load this before initialize to ensure its available to when the Mempool feature initializes.
-            lock (this.locker)
+        public class PollComparer : IComparer<(int height, int id)>
+        {
+            public int Compare((int height, int id) poll1, (int height, int id) poll2)
             {
-                this.whitelistedHashes = this.kvRepository.LoadValueJson<List<uint256>>(dbKey) ?? new List<uint256>();
+                int cmp = poll1.height.CompareTo(poll2.height);
+                if (cmp != 0)
+                    return cmp;
+
+                return poll1.id.CompareTo(poll2.id);
             }
         }
 
-        public void Initialize()
+        static PollComparer pollComparer = new PollComparer();
+
+        private void GetWhitelistedHashesFromExecutedPolls(VotingManager votingManager)
         {
+            lock (this.locker)
+            {
+                var federation = new List<IFederationMember>(this.poaConsensusOptions.GenesisFederationMembers);
+
+                IEnumerable<Poll> executedPolls = votingManager.GetExecutedPolls().WhitelistPolls();
+                foreach (Poll poll in executedPolls.OrderBy(a => (a.PollExecutedBlockData.Height, a.Id), pollComparer))
+                {
+                    var hash = new uint256(poll.VotingData.Data);
+
+                    if (poll.VotingData.Key == VoteKey.WhitelistHash)
+                    {
+                        this.AddHash(hash, poll.PollExecutedBlockData.Height);
+                    }
+                    else if (poll.VotingData.Key == VoteKey.RemoveHash)
+                    {
+                        this.RemoveHash(hash, poll.PollExecutedBlockData.Height);
+                    }
+                }
+            }
         }
 
-        private void SaveHashes()
+        public void Initialize(VotingManager votingManager)
         {
+            // TODO: Must call Initialize before the Mempool rules try to use this class.
             lock (this.locker)
             {
-                this.kvRepository.SaveValueJson(dbKey, this.whitelistedHashes);
+                this.whitelistedHashes = new Dictionary<uint256, int[]>();
+                this.GetWhitelistedHashesFromExecutedPolls(votingManager);
             }
         }
 
-        public void AddHash(uint256 hash)
+        public void AddHash(uint256 hash, int executionHeight)
         {
             lock (this.locker)
             {
-                if (this.whitelistedHashes.Contains(hash))
+                // Retrieve the whitelist history for this hash.
+                if (!this.whitelistedHashes.TryGetValue(hash, out int[] history))
                 {
-                    this.logger.LogTrace("(-)[ALREADY_EXISTS]");
+                    this.whitelistedHashes[hash] = new int[] { executionHeight };
                     return;
                 }
 
-                this.whitelistedHashes.Add(hash);
+                // Keep all history up to and including the executionHeight.
+                int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] > executionHeight, 0, history.Length + 1);
+                Array.Resize(ref history, keep | 1);
+                this.whitelistedHashes[hash] = history;
+
+                // If the history is an even length then add the addition height to signify addition.
+                if ((keep % 2) == 0)
+                {
+                    // Add an even indexed entry to signify an addition.
+                    history[keep] = executionHeight;
+                    return;
+                }
+
+                this.logger.LogTrace("(-)[HASH_ALREADY_EXISTS]");
+                return;
+            }
+        }
+
+        public void RemoveHash(uint256 hash, int executionHeight)
+        {
+            lock (this.locker)
+            {
+                // Retrieve the whitelist history for this hash.
+                if (this.whitelistedHashes.TryGetValue(hash, out int[] history))
+                {
+                    // Keep all history up to and including the executionHeight.
+                    int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] >= executionHeight, 0, history.Length + 1);
+                    Array.Resize(ref history, (keep + 1) & ~1);
+                    this.whitelistedHashes[hash] = history;
+
+                    // If the history is an odd length then add the removal height to signify removal.
+                    if ((keep % 2) != 0)
+                    {
+                        history[keep] = executionHeight;
+                        return;
+                    }
+                }
 
-                this.SaveHashes();
+                this.logger.LogTrace("(-)[HASH_DOESNT_EXIST]");
+                return;
             }
         }
 
-        public void RemoveHash(uint256 hash)
+        private bool ExistsHash(uint256 hash, int blockHeight)
         {
             lock (this.locker)
             {
-                bool removed = this.whitelistedHashes.Remove(hash);
+                // Retrieve the whitelist history for this hash.
+                if (!this.whitelistedHashes.TryGetValue(hash, out int[] history))
+                    return false;
 
-                if (removed)
-                    this.SaveHashes();
+                int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] > blockHeight, 0, history.Length + 1);
+                return (keep % 2) != 0;
             }
         }
 
-        public List<uint256> GetHashes()
+        public List<uint256> GetHashes(int blockHeight = int.MaxValue)
         {
             lock (this.locker)
             {
-                return new List<uint256>(this.whitelistedHashes);
+                return this.whitelistedHashes.Where(k => ExistsHash(k.Key, blockHeight)).Select(k => k.Key).ToList();
             }
         }
     }
 
     public interface IWhitelistedHashesRepository
     {
-        void AddHash(uint256 hash);
+        void AddHash(uint256 hash, int executionHeight);
 
-        void RemoveHash(uint256 hash);
+        void RemoveHash(uint256 hash, int executionHeight);
 
-        List<uint256> GetHashes();
+        List<uint256> GetHashes(int blockHeight = int.MaxValue);
 
-        void Initialize();
+        void Initialize(VotingManager votingManager);
     }
-}
+}