From 6f49bae20133a9be99781bfd2c8870f5d2604f4c Mon Sep 17 00:00:00 2001 From: quantumagi Date: Mon, 14 Mar 2022 03:56:16 +1100 Subject: [PATCH 1/4] [IBD/Rewind] Optimize VotingManager for batched operations --- .../PoATestsBase.cs | 12 +-- .../VotingManagerTests.cs | 14 +-- .../PoAFeature.cs | 4 +- .../Voting/PollResultExecutor.cs | 26 ++--- .../Voting/VotingManager.cs | 97 ++++++++++++------ .../Voting/WhitelistedHashesRepository.cs | 98 ++++++++++++------- .../Rules/AllowedCodeHashLogicTests.cs | 10 +- .../WhitelistHashCheckerTests.cs | 12 +-- .../PoA/Rules/AllowedCodeHashLogic.cs | 2 +- .../PoA/WhitelistedHashChecker.cs | 6 +- .../WhitelistNodeExtensions.cs | 2 +- 11 files changed, 178 insertions(+), 105 deletions(-) diff --git a/src/Stratis.Bitcoin.Features.PoA.Tests/PoATestsBase.cs b/src/Stratis.Bitcoin.Features.PoA.Tests/PoATestsBase.cs index 6eae3eb7f5..673a8f4b5f 100644 --- a/src/Stratis.Bitcoin.Features.PoA.Tests/PoATestsBase.cs +++ b/src/Stratis.Bitcoin.Features.PoA.Tests/PoATestsBase.cs @@ -44,7 +44,7 @@ public class PoATestsBase protected readonly ChainState chainState; protected readonly IAsyncProvider asyncProvider; protected readonly Mock fullNode; - protected readonly Mock blockRepository; + protected readonly Mock blockStoreQueue; public PoATestsBase(TestPoANetwork network = null) { @@ -53,13 +53,13 @@ public PoATestsBase(TestPoANetwork network = null) this.network = network ?? new TestPoANetwork(); this.consensusOptions = this.network.ConsensusOptions; this.dBreezeSerializer = new DBreezeSerializer(this.network.Consensus.ConsensusFactory); - this.blockRepository = new Mock(); + this.blockStoreQueue = new Mock(); this.ChainIndexer = new ChainIndexer(this.network); IDateTimeProvider dateTimeProvider = new DateTimeProvider(); this.consensusSettings = new ConsensusSettings(NodeSettings.Default(this.network)); - (this.federationManager, this.federationHistory) = CreateFederationManager(this, this.network, this.loggerFactory, this.signals, this.blockRepository.Object); + (this.federationManager, this.federationHistory) = CreateFederationManager(this, this.network, this.loggerFactory, this.signals, this.blockStoreQueue.Object); this.slotsManager = new SlotsManager(this.network, this.federationManager, this.federationHistory, this.ChainIndexer); @@ -71,7 +71,7 @@ public PoATestsBase(TestPoANetwork network = null) this.resultExecutorMock = new Mock(); this.votingManager = new VotingManager(this.federationManager, this.resultExecutorMock.Object, new NodeStats(dateTimeProvider, NodeSettings.Default(this.network), new Mock().Object), dataFolder, - this.dBreezeSerializer, this.signals, this.network, this.ChainIndexer, this.blockRepository.Object); + this.dBreezeSerializer, this.signals, this.network, this.ChainIndexer, this.blockStoreQueue.Object); this.votingManager.Initialize(this.federationHistory); @@ -87,7 +87,7 @@ public PoATestsBase(TestPoANetwork network = null) this.currentHeader = headers.Last(); } - public static (IFederationManager federationManager, IFederationHistory federationHistory) CreateFederationManager(object caller, Network network, LoggerFactory loggerFactory, ISignals signals, BlockStore.IBlockRepository blockRepository) + public static (IFederationManager federationManager, IFederationHistory federationHistory) CreateFederationManager(object caller, Network network, LoggerFactory loggerFactory, ISignals signals, IBlockStoreQueue blockStoreQueue) { string dir = TestBase.CreateTestDir(caller); @@ -114,7 +114,7 @@ public static (IFederationManager federationManager, IFederationHistory federati var header = new BlockHeader(); chainIndexerMock.Setup(x => x.Tip).Returns(new ChainedHeader(header, header.GetHash(), 0)); var votingManager = new VotingManager(federationManager, new Mock().Object, - new Mock().Object, nodeSettings.DataFolder, dbreezeSerializer, signals, network, chainIndexerMock.Object, blockRepository, null); + new Mock().Object, nodeSettings.DataFolder, dbreezeSerializer, signals, network, chainIndexerMock.Object, blockStoreQueue, null); var federationHistory = new Mock(); federationHistory.Setup(x => x.GetFederationMemberForBlock(It.IsAny())).Returns((chainedHeader) => diff --git a/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs b/src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs index 90dae49dc0..2024384891 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] @@ -74,7 +74,7 @@ public void CanVote() ChainedHeaderBlock[] blocks = GetBlocksWithVotingData(votesRequired, votingData, votingRequest.ChainedHeader); // Mock the blocks via the block repository. - this.blockRepository.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => + this.blockStoreQueue.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => { return (hash == votingRequest.ChainedHeader.HashBlock) ? votingRequest.Block : blocks.FirstOrDefault(b => b.ChainedHeader.HashBlock == hash)?.Block; }); @@ -112,7 +112,7 @@ public void AddVoteAfterPollComplete() ChainedHeaderBlock[] blocks = GetBlocksWithVotingData(votesRequired + 1, votingData, votingRequest.ChainedHeader); // Mock the blocks via the block repository. - this.blockRepository.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => + this.blockStoreQueue.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => { return (hash == votingRequest.ChainedHeader.HashBlock) ? votingRequest.Block : blocks.FirstOrDefault(b => b.ChainedHeader.HashBlock == hash)?.Block; }); @@ -157,7 +157,7 @@ public void CanCreateVotingRequest() ChainedHeaderBlock votingRequest = GetBlockWithVotingRequest(memberPubKey); // Mock the blocks via the block repository. - this.blockRepository.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => + this.blockStoreQueue.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => { return votingRequest.Block; }); @@ -183,7 +183,7 @@ public void CanExpireAndUnExpirePollViaBlockDisconnected() ChainedHeaderBlock[] blocks = GetBlocksWithVotingData(1, votingData, votingRequest.ChainedHeader); // Mock the blocks via the block repository. - this.blockRepository.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => + this.blockStoreQueue.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => { return (hash == votingRequest.ChainedHeader.HashBlock) ? votingRequest.Block : blocks.FirstOrDefault(b => b.ChainedHeader.HashBlock == hash)?.Block; }); @@ -232,7 +232,7 @@ public void CanExpireAndUnExpirePollViaNodeRewind() ChainedHeaderBlock[] blocks = GetBlocksWithVotingData(1, votingData, votingRequest.ChainedHeader); // Mock the blocks via the block repository. - this.blockRepository.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => + this.blockStoreQueue.Setup(r => r.GetBlock(It.IsAny())).Returns((uint256 hash) => { return (hash == votingRequest.ChainedHeader.HashBlock) ? votingRequest.Block : blocks.FirstOrDefault(b => b.ChainedHeader.HashBlock == hash)?.Block; }); diff --git a/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs b/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs index 08c469ff2f..ed293aa3d2 100644 --- a/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs +++ b/src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs @@ -137,9 +137,9 @@ public override Task InitializeAsync() } this.federationManager.Initialize(); - this.whitelistedHashesRepository.Initialize(); + this.whitelistedHashesRepository.Initialize(this.votingManager); - if (!this.votingManager.Synchronize(this.chainIndexer.Tip)) + if (!this.votingManager.Synchronize(this.chainIndexer.Tip, false)) throw new System.OperationCanceledException(); this.federationHistory.Initialize(); diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs index 2e10360f99..e400460f80 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) { diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs index e72e4afe85..64f53e14ce 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs @@ -9,7 +9,7 @@ using Stratis.Bitcoin.Configuration.Logging; using Stratis.Bitcoin.EventBus; using Stratis.Bitcoin.EventBus.CoreEvents; -using Stratis.Bitcoin.Features.BlockStore; +using Stratis.Bitcoin.Interfaces; using Stratis.Bitcoin.Primitives; using Stratis.Bitcoin.Signals; using Stratis.Bitcoin.Utilities; @@ -20,7 +20,7 @@ namespace Stratis.Bitcoin.Features.PoA.Voting public sealed class VotingManager : IDisposable { private readonly PoAConsensusOptions poaConsensusOptions; - private readonly IBlockRepository blockRepository; + private readonly IBlockStoreQueue blockStoreQueue; private readonly ChainIndexer chainIndexer; private readonly IFederationManager federationManager; @@ -46,6 +46,7 @@ public sealed class VotingManager : IDisposable private IIdleFederationMembersKicker idleFederationMembersKicker; private readonly INodeLifetime nodeLifetime; + private readonly IInitialBlockDownloadState initialBlockDownloadState; /// In-memory collection of pending polls. /// All access should be protected by . @@ -73,8 +74,9 @@ public VotingManager( ISignals signals, Network network, ChainIndexer chainIndexer, - IBlockRepository blockRepository = null, - INodeLifetime nodeLifetime = null) + IBlockStoreQueue blockStoreQueue = null, + INodeLifetime nodeLifetime = null, + IInitialBlockDownloadState initialBlockDownloadState = null) { this.federationManager = Guard.NotNull(federationManager, nameof(federationManager)); this.pollResultExecutor = Guard.NotNull(pollResultExecutor, nameof(pollResultExecutor)); @@ -92,9 +94,10 @@ public VotingManager( Guard.Assert(this.poaConsensusOptions.PollExpiryBlocks != 0); - this.blockRepository = blockRepository; + this.blockStoreQueue = blockStoreQueue; this.chainIndexer = chainIndexer; this.nodeLifetime = nodeLifetime; + this.initialBlockDownloadState = initialBlockDownloadState; this.isInitialized = false; } @@ -321,7 +324,8 @@ public Poll CreatePendingPoll(DBreeze.Transactions.Transaction transaction, Voti this.PollsRepository.AddPolls(transaction, poll); - this.logger.LogInformation("New poll was created: '{0}'.", poll); + this.logger.LogInformation("Created poll {0} [{1}] at height {2}.", + poll.Id, this.pollResultExecutor.ConvertToString(poll.VotingData), poll.PollStartBlockData.Height); }); return poll; @@ -363,6 +367,29 @@ public List GetFederationFromExecutedPolls() } } + public void GetWhitelistedHashesFromExecutedPolls(IWhitelistedHashesRepository whitelistedHashesRepository) + { + lock (this.locker) + { + var federation = new List(this.poaConsensusOptions.GenesisFederationMembers); + + IEnumerable executedPolls = this.GetExecutedPolls().WhitelistPolls(); + foreach (Poll poll in executedPolls.OrderBy(a => a.PollExecutedBlockData.Height)) + { + var hash = new uint256(poll.VotingData.Data); + + if (poll.VotingData.Key == VoteKey.WhitelistHash) + { + whitelistedHashesRepository.AddHash(hash, poll.PollExecutedBlockData.Height); + } + else if (poll.VotingData.Key == VoteKey.KickFederationMember) + { + whitelistedHashesRepository.RemoveHash(hash, poll.PollExecutedBlockData.Height); + } + } + } + } + public int LastKnownFederationHeight() { return (this.PollsRepository.CurrentTip?.Height ?? 0) + (int)this.network.Consensus.MaxReorgLength - 1; @@ -520,7 +547,9 @@ private void ProcessBlock(DBreeze.Transactions.Transaction transaction, ChainedH else { this.logger.LogDebug("Applying poll '{0}'.", poll); - this.pollResultExecutor.ApplyChange(poll.VotingData); + // Hash-related polls have already been applied at the "voted in favor" height. + if (poll.VotingData.Key != VoteKey.WhitelistHash && poll.VotingData.Key != VoteKey.RemoveHash) + this.pollResultExecutor.ApplyChange(poll.VotingData, (int)(poll.PollVotedInFavorBlockData.Height + this.network.Consensus.MaxReorgLength)); this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = new HashHeightPair(chBlock.ChainedHeader)); this.PollsRepository.UpdatePoll(transaction, poll); @@ -642,6 +671,10 @@ private void ProcessBlock(DBreeze.Transactions.Transaction transaction, ChainedH this.polls.AdjustPoll(poll, poll => poll.PollVotedInFavorBlockData = new HashHeightPair(chBlock.ChainedHeader)); this.PollsRepository.UpdatePoll(transaction, poll); pollsRepositoryModified = true; + + // Update the whitelist hash repository early to allow for scenarios where the voting manager is lagging up to MaxReorgLength blocks. + if (poll.VotingData.Key == VoteKey.RemoveHash || poll.VotingData.Key == VoteKey.WhitelistHash) + this.pollResultExecutor.ApplyChange(poll.VotingData, (int)(poll.PollVotedInFavorBlockData.Height + this.network.Consensus.MaxReorgLength)); } } @@ -671,7 +704,7 @@ private void UnProcessBlock(DBreeze.Transactions.Transaction transaction, Chaine foreach (Poll poll in this.polls.Where(x => !x.IsPending && x.PollExecutedBlockData?.Hash == chBlock.ChainedHeader.HashBlock).ToList()) { this.logger.LogDebug("Reverting poll execution '{0}'.", poll); - this.pollResultExecutor.RevertChange(poll.VotingData); + this.pollResultExecutor.RevertChange(poll.VotingData, (int)(chBlock.ChainedHeader.Height + this.network.Consensus.MaxReorgLength)); this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = null); this.PollsRepository.UpdatePoll(transaction, poll); @@ -732,6 +765,10 @@ private void UnProcessBlock(DBreeze.Transactions.Transaction transaction, Chaine this.polls.AdjustPoll(targetPoll, poll => poll.PollVotedInFavorBlockData = null); this.PollsRepository.UpdatePoll(transaction, targetPoll); pollsRepositoryModified = true; + + // Update the whitelist hash repository early to allow for scenarios where the voting manager is lagging up to MaxReorgLength blocks. + if (targetPoll.VotingData.Key == VoteKey.RemoveHash || targetPoll.VotingData.Key == VoteKey.WhitelistHash) + this.pollResultExecutor.RevertChange(targetPoll.VotingData, (int)(chBlock.ChainedHeader.Height + this.network.Consensus.MaxReorgLength)); } // Pub key of a fed member that created voting data. @@ -787,8 +824,12 @@ public List GetLastKnownFederation() return this.GetModifiedFederation(this.chainIndexer.Tip); } - internal bool Synchronize(ChainedHeader newTip) + internal bool Synchronize(ChainedHeader newTip, bool allowLag = true) { + // Allowing the voting manager to lag makes it more efficient due to multi-block processing. + if (allowLag && (LastKnownFederationHeight() - 1) > newTip?.Height && (this.initialBlockDownloadState?.IsInitialBlockDownload() ?? false)) + return false; + if (newTip?.HashBlock == this.PollsRepository.CurrentTip?.Hash) return true; @@ -811,7 +852,7 @@ internal bool Synchronize(ChainedHeader newTip) for (ChainedHeader header = repoTip; header.Height > fork.Height; header = header.Previous) { - Block block = this.blockRepository.GetBlock(header.HashBlock); + Block block = this.blockStoreQueue.GetBlock(header.HashBlock); this.UnProcessBlock(transaction, new ChainedHeaderBlock(block, header)); } @@ -836,30 +877,30 @@ internal bool Synchronize(ChainedHeader newTip) DBreeze.Transactions.Transaction currentTransaction = this.PollsRepository.GetTransaction(); int i = 0; - foreach (Block block in this.blockRepository.EnumerateBatch(headers)) - { - if (this.nodeLifetime.ApplicationStopping.IsCancellationRequested) + foreach (Block block in this.blockStoreQueue.GetBlocks(headers.Select(h => h.HashBlock).ToList())) { - this.logger.LogTrace("(-)[NODE_DISPOSED]"); - this.PollsRepository.SaveCurrentTip(currentTransaction); - currentTransaction.Commit(); - currentTransaction.Dispose(); + if (this.nodeLifetime.ApplicationStopping.IsCancellationRequested) + { + this.logger.LogTrace("(-)[NODE_DISPOSED]"); + this.PollsRepository.SaveCurrentTip(currentTransaction); + currentTransaction.Commit(); + currentTransaction.Dispose(); - bSuccess = false; - return; - } + bSuccess = false; + return; + } - ChainedHeader header = headers[i++]; + ChainedHeader header = headers[i++]; - this.ProcessBlock(currentTransaction, new ChainedHeaderBlock(block, header)); + this.ProcessBlock(currentTransaction, new ChainedHeaderBlock(block, header)); - if (header.Height % 10000 == 0) - { - var progress = (int)((decimal)header.Height / this.chainIndexer.Tip.Height * 100); - var progressString = $"Synchronizing voting data at height {header.Height} / {this.chainIndexer.Tip.Height} ({progress} %)."; + if (header.Height % 10000 == 0) + { + var progress = (int)((decimal)header.Height / this.chainIndexer.Tip.Height * 100); + var progressString = $"Synchronizing voting data at height {header.Height} / {this.chainIndexer.Tip.Height} ({progress} %)."; - this.logger.LogInformation(progressString); - this.signals.Publish(new FullNodeEvent() { Message = progressString, State = FullNodeState.Initializing.ToString() }); + this.logger.LogInformation(progressString); + this.signals.Publish(new FullNodeEvent() { Message = progressString, State = FullNodeState.Initializing.ToString() }); this.PollsRepository.SaveCurrentTip(currentTransaction); diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs index cd8ac70124..85d7de0c83 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs @@ -1,93 +1,123 @@ -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; + // Dictionary of hash histories. Even list entries are additions and odd entries are removals. + private Dictionary whitelistedHashes; - public WhitelistedHashesRepository(ILoggerFactory loggerFactory, IKeyValueRepository kvRepository) + public WhitelistedHashesRepository(ILoggerFactory loggerFactory) { - this.kvRepository = kvRepository; this.locker = new object(); this.logger = loggerFactory.CreateLogger(this.GetType().FullName); + } - // Load this before initialize to ensure its available to when the Mempool feature initializes. + public void Initialize(VotingManager votingManager) + { + // TODO: Must call Initialize before the Mempool rules try to use this class. lock (this.locker) { - this.whitelistedHashes = this.kvRepository.LoadValueJson>(dbKey) ?? new List(); + this.whitelistedHashes = new Dictionary(); + votingManager.GetWhitelistedHashesFromExecutedPolls(this); } } - public void Initialize() - { - } - - private void SaveHashes() + public void AddHash(uint256 hash, int executionHeight) { lock (this.locker) { - this.kvRepository.SaveValueJson(dbKey, this.whitelistedHashes); + // Retrieve the whitelist history for this hash. + if (!this.whitelistedHashes.TryGetValue(hash, out int[] history)) + { + this.whitelistedHashes[hash] = new int[] { executionHeight }; + return; + } + + // 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 AddHash(uint256 hash) + public void RemoveHash(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]"); - return; + // 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.whitelistedHashes.Add(hash); - - 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); } } 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..ede166a927 100644 --- a/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs +++ b/src/Stratis.Bitcoin.Features.SmartContracts.Tests/Consensus/Rules/AllowedCodeHashLogicTests.cs @@ -30,7 +30,7 @@ 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); @@ -38,7 +38,7 @@ public void Should_Allow_Code_With_Valid_Hash() 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); } } } 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); } } From d36f3e8485b04aa2f8170a1876eac167f763c1f9 Mon Sep 17 00:00:00 2001 From: quantumagi Date: Tue, 8 Nov 2022 19:07:21 +1100 Subject: [PATCH 2/4] Fix --- src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs index 4953f60983..baa299409c 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs @@ -407,7 +407,7 @@ public void GetWhitelistedHashesFromExecutedPolls(IWhitelistedHashesRepository w { whitelistedHashesRepository.AddHash(hash, poll.PollExecutedBlockData.Height); } - else if (poll.VotingData.Key == VoteKey.KickFederationMember) + else if (poll.VotingData.Key == VoteKey.RemoveHash) { whitelistedHashesRepository.RemoveHash(hash, poll.PollExecutedBlockData.Height); } @@ -573,7 +573,7 @@ private void ProcessBlock(PollsRepository.Transaction transaction, ChainedHeader this.logger.LogDebug("Applying poll '{0}'.", poll); // Hash-related polls have already been applied at the "voted in favor" height. if (poll.VotingData.Key != VoteKey.WhitelistHash && poll.VotingData.Key != VoteKey.RemoveHash) - this.pollResultExecutor.ApplyChange(poll.VotingData, (int)(poll.PollVotedInFavorBlockData.Height + this.network.Consensus.MaxReorgLength)); + this.pollResultExecutor.ApplyChange(poll.VotingData, chBlock.ChainedHeader.Height); this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = new HashHeightPair(chBlock.ChainedHeader)); transaction.UpdatePoll(poll); From 477b507f99f16e1279d2cf0f1bcf2612082bbe06 Mon Sep 17 00:00:00 2001 From: quantumagi Date: Tue, 8 Nov 2022 19:09:48 +1100 Subject: [PATCH 3/4] Remove unused method --- .../Voting/VotingManager.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs index baa299409c..c2831af8c7 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs @@ -392,29 +392,6 @@ public List GetFederationFromExecutedPolls() } } - public void GetWhitelistedHashesFromExecutedPolls(IWhitelistedHashesRepository whitelistedHashesRepository) - { - lock (this.locker) - { - var federation = new List(this.poaConsensusOptions.GenesisFederationMembers); - - IEnumerable executedPolls = this.GetExecutedPolls().WhitelistPolls(); - foreach (Poll poll in executedPolls.OrderBy(a => a.PollExecutedBlockData.Height)) - { - var hash = new uint256(poll.VotingData.Data); - - if (poll.VotingData.Key == VoteKey.WhitelistHash) - { - whitelistedHashesRepository.AddHash(hash, poll.PollExecutedBlockData.Height); - } - else if (poll.VotingData.Key == VoteKey.RemoveHash) - { - whitelistedHashesRepository.RemoveHash(hash, poll.PollExecutedBlockData.Height); - } - } - } - } - public int LastKnownFederationHeight() { return (this.PollsRepository.CurrentTip?.Height ?? 0) + (int)this.network.Consensus.MaxReorgLength - 1; From 5fa8ff7e64a604385f354d7f9b07ea764811c0cc Mon Sep 17 00:00:00 2001 From: quantumagi Date: Tue, 8 Nov 2022 19:36:34 +1100 Subject: [PATCH 4/4] Update GetWhitelistedHashesFromExecutedPolls --- .../Voting/WhitelistedHashesRepository.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs b/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs index 435f27410a..241973d973 100644 --- a/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs +++ b/src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs @@ -14,6 +14,7 @@ public class WhitelistedHashesRepository : IWhitelistedHashesRepository private readonly ILogger logger; + private readonly Network network; private readonly PoAConsensusOptions poaConsensusOptions; // Dictionary of hash histories. Even list entries are additions and odd entries are removals. @@ -24,6 +25,7 @@ public WhitelistedHashesRepository(ILoggerFactory loggerFactory, Network network this.locker = new object(); this.logger = loggerFactory.CreateLogger(this.GetType().FullName); + this.network = network; this.poaConsensusOptions = network.Consensus.Options as PoAConsensusOptions; } @@ -47,18 +49,19 @@ private void GetWhitelistedHashesFromExecutedPolls(VotingManager votingManager) { 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)) + IEnumerable approvedPolls = votingManager.GetApprovedPolls().WhitelistPolls(); + + foreach (Poll poll in approvedPolls.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); + this.AddHash(hash, (int)(poll.PollVotedInFavorBlockData.Height + this.network.Consensus.MaxReorgLength)); } else if (poll.VotingData.Key == VoteKey.RemoveHash) { - this.RemoveHash(hash, poll.PollExecutedBlockData.Height); + this.RemoveHash(hash, (int)(poll.PollVotedInFavorBlockData.Height + this.network.Consensus.MaxReorgLength)); } } }