diff --git a/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs b/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs index 69b450a360..ab40e27fc6 100644 --- a/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs +++ b/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs @@ -24,8 +24,8 @@ public VotingManagerTests() this.changesApplied = new List(); this.changesReverted = new List(); - this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny())).Callback((VotingData data) => this.changesApplied.Add(data)); - this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny())).Callback((VotingData data) => this.changesReverted.Add(data)); + this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny(), It.IsAny())).Callback((VotingData data, int _) => this.changesApplied.Add(data)); + this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny(), It.IsAny())).Callback((VotingData data, int _) => this.changesReverted.Add(data)); } [Fact] diff --git a/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs b/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs index 08c469ff2f..1afc87c96d 100644 --- a/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs +++ b/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(); diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs index 2e10360f99..5545699e8f 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs @@ -8,11 +8,13 @@ public interface IPollResultExecutor { /// Applies effect of . /// See . - void ApplyChange(VotingData data); + /// The height from which the change should take effect. + void ApplyChange(VotingData data, int executionHeight); /// Reverts effect of . /// See . - void RevertChange(VotingData data); + /// The height from which the change should take effect. + void RevertChange(VotingData data, int executionHeight); /// Converts to a human readable format. /// See . @@ -40,7 +42,7 @@ public PollResultExecutor(IFederationManager federationManager, ILoggerFactory l } /// - 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; } } /// - 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,13 +120,13 @@ 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) { @@ -132,13 +134,13 @@ private void AddHash(byte[] hashBytes) } } - 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) { @@ -146,4 +148,4 @@ private void RemoveHash(byte[] hashBytes) } } } -} +} \ No newline at end of file diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs index a343806e68..52d60cfdbc 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs +++ b/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); diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs index cd8ac70124..435f27410a 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs +++ b/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; - /// Protects access to . private readonly object locker; private readonly ILogger logger; - private List 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 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>(dbKey) ?? new List(); + 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(this.poaConsensusOptions.GenesisFederationMembers); + + IEnumerable 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(); + 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 GetHashes() + public List GetHashes(int blockHeight = int.MaxValue) { lock (this.locker) { - return new List(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 GetHashes(); + List GetHashes(int blockHeight = int.MaxValue); - void Initialize(); + void Initialize(VotingManager votingManager); } -} +} \ No newline at end of file diff --git a/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs b/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs index 3a6f0b1940..ad2b9d509a 100644 --- a/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs +++ b/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs @@ -30,15 +30,15 @@ public void Should_Allow_Code_With_Valid_Hash() byte[] hash = HashHelper.Keccak256(code); this.hashingStrategy.Setup(h => h.Hash(code)).Returns(hash); - this.hashChecker.Setup(h => h.CheckHashWhitelisted(hash)).Returns(true); + this.hashChecker.Setup(h => h.CheckHashWhitelisted(hash, 0)).Returns(true); - var tx = new ContractTxData(1, 1000, (Gas) 10000, code); + var tx = new ContractTxData(1, 1000, (Gas)10000, code); var sut = new AllowedCodeHashLogic(this.hashChecker.Object, this.hashingStrategy.Object); sut.CheckContractTransaction(tx, 0); - this.hashChecker.Verify(h => h.CheckHashWhitelisted(hash), Times.Once); + this.hashChecker.Verify(h => h.CheckHashWhitelisted(hash, 0), Times.Once); } [Fact] @@ -49,7 +49,7 @@ public void Should_Throw_ConsensusErrorException_If_Hash_Not_Allowed() byte[] hash = HashHelper.Keccak256(code); this.hashingStrategy.Setup(h => h.Hash(code)).Returns(hash); - this.hashChecker.Setup(h => h.CheckHashWhitelisted(hash)).Returns(false); + this.hashChecker.Setup(h => h.CheckHashWhitelisted(hash, 0)).Returns(false); var sut = new AllowedCodeHashLogic(this.hashChecker.Object, this.hashingStrategy.Object); @@ -57,7 +57,7 @@ public void Should_Throw_ConsensusErrorException_If_Hash_Not_Allowed() Assert.Throws(() => sut.CheckContractTransaction(tx, 0)); - this.hashChecker.Verify(h => h.CheckHashWhitelisted(hash), Times.Once); + this.hashChecker.Verify(h => h.CheckHashWhitelisted(hash, 0), Times.Once); } [Fact] @@ -70,7 +70,7 @@ public void Should_Not_Validate_ContractCall() sut.CheckContractTransaction(callTx, 0); this.hashingStrategy.Verify(h => h.Hash(It.IsAny()), Times.Never); - this.hashChecker.Verify(h => h.CheckHashWhitelisted(It.IsAny()), Times.Never); + this.hashChecker.Verify(h => h.CheckHashWhitelisted(It.IsAny(), It.IsAny()), Times.Never); } } -} +} \ No newline at end of file diff --git a/src/Stratis.Bitcoin.Features.SmartContracts.Tests/WhitelistHashCheckerTests.cs b/src/Stratis.Bitcoin.Features.SmartContracts.Tests/WhitelistHashCheckerTests.cs index 693f26e1c3..b0cf242fa5 100644 --- a/src/Stratis.Bitcoin.Features.SmartContracts.Tests/WhitelistHashCheckerTests.cs +++ b/src/Stratis.Bitcoin.Features.SmartContracts.Tests/WhitelistHashCheckerTests.cs @@ -16,11 +16,11 @@ public void Should_Return_False_If_Hash_Not_In_Whitelist() var repository = new Mock(); - repository.Setup(r => r.GetHashes()).Returns(new List()); + repository.Setup(r => r.GetHashes(It.IsAny())).Returns(new List()); var strategy = new WhitelistedHashChecker(repository.Object); - Assert.False(strategy.CheckHashWhitelisted(hash)); + Assert.False(strategy.CheckHashWhitelisted(hash, 0)); } [Fact] @@ -30,11 +30,11 @@ public void Should_Return_True_If_Hash_In_Whitelist() var repository = new Mock(); - repository.Setup(r => r.GetHashes()).Returns(new List { new uint256(hash) }); + repository.Setup(r => r.GetHashes(It.IsAny())).Returns(new List { new uint256(hash) }); var strategy = new WhitelistedHashChecker(repository.Object); - Assert.True(strategy.CheckHashWhitelisted(hash)); + Assert.True(strategy.CheckHashWhitelisted(hash, 0)); } [Fact] @@ -42,14 +42,14 @@ public void Should_Return_False_Invalid_uint256() { var repository = new Mock(); - repository.Setup(r => r.GetHashes()).Returns(new List()); + repository.Setup(r => r.GetHashes(It.IsAny())).Returns(new List()); var strategy = new WhitelistedHashChecker(repository.Object); // uint256 must be 256 bytes wide var invalidUint256Bytes = new byte[] { }; - Assert.False(strategy.CheckHashWhitelisted(invalidUint256Bytes)); + Assert.False(strategy.CheckHashWhitelisted(invalidUint256Bytes, 0)); } } } \ No newline at end of file diff --git a/src/Stratis.Bitcoin.Features.SmartContracts/PoA/Rules/AllowedCodeHashLogic.cs b/src/Stratis.Bitcoin.Features.SmartContracts/PoA/Rules/AllowedCodeHashLogic.cs index 2e3dcba4ca..0c3ad0eb1e 100644 --- a/src/Stratis.Bitcoin.Features.SmartContracts/PoA/Rules/AllowedCodeHashLogic.cs +++ b/src/Stratis.Bitcoin.Features.SmartContracts/PoA/Rules/AllowedCodeHashLogic.cs @@ -27,7 +27,7 @@ public void CheckContractTransaction(ContractTxData txData, Money suppliedBudget byte[] hashedCode = this.hashingStrategy.Hash(txData.ContractExecutionCode); - if (!this.whitelistedHashChecker.CheckHashWhitelisted(hashedCode)) + if (!this.whitelistedHashChecker.CheckHashWhitelisted(hashedCode, blockHeight)) { ThrowInvalidCode(); } diff --git a/src/Stratis.Bitcoin.Features.SmartContracts/PoA/WhitelistedHashChecker.cs b/src/Stratis.Bitcoin.Features.SmartContracts/PoA/WhitelistedHashChecker.cs index 5372192786..81437c2f4d 100644 --- a/src/Stratis.Bitcoin.Features.SmartContracts/PoA/WhitelistedHashChecker.cs +++ b/src/Stratis.Bitcoin.Features.SmartContracts/PoA/WhitelistedHashChecker.cs @@ -21,7 +21,7 @@ public WhitelistedHashChecker(IWhitelistedHashesRepository whitelistedHashesRepo /// /// The bytes of the hash to check. /// True if the hash was found in the whitelisted hashes repository. - public bool CheckHashWhitelisted(byte[] hash) + public bool CheckHashWhitelisted(byte[] hash, int blockHeight) { if (hash.Length != 32) { @@ -29,7 +29,7 @@ public bool CheckHashWhitelisted(byte[] hash) return false; } - List allowedHashes = this.whitelistedHashesRepository.GetHashes(); + List allowedHashes = this.whitelistedHashesRepository.GetHashes(blockHeight); // Now that we've checked the width of the byte array, we don't expect the uint256 constructor to throw any exceptions. var hash256 = new uint256(hash); @@ -40,6 +40,6 @@ public bool CheckHashWhitelisted(byte[] hash) public interface IWhitelistedHashChecker { - bool CheckHashWhitelisted(byte[] hash); + bool CheckHashWhitelisted(byte[] hash, int blockHeight); } } \ No newline at end of file diff --git a/src/Stratis.SmartContracts.IntegrationTests/WhitelistNodeExtensions.cs b/src/Stratis.SmartContracts.IntegrationTests/WhitelistNodeExtensions.cs index a3caacf7a4..db0b16d828 100644 --- a/src/Stratis.SmartContracts.IntegrationTests/WhitelistNodeExtensions.cs +++ b/src/Stratis.SmartContracts.IntegrationTests/WhitelistNodeExtensions.cs @@ -15,7 +15,7 @@ public static void WhitelistCode(this IMockChain chain, byte[] code) { var hasher = node.CoreNode.FullNode.NodeService(); var hash = new uint256(hasher.Hash(code)); - node.CoreNode.FullNode.NodeService().AddHash(hash); + node.CoreNode.FullNode.NodeService().AddHash(hash, 0); } }