From 3d872c2b4f0c30f6d490dcb90dbd25f6830834c0 Mon Sep 17 00:00:00 2001 From: Paulo Aboim Pinto Date: Tue, 18 Sep 2018 22:48:43 +0200 Subject: [PATCH] Make BlockProcessor with less dependencies (#354) * RocksDbRepository reach 42% cover * Rename abstract class ICrypto to Crypto. I'm sure this was once an interface but it's not anymore, threfore should not carry the prefix "I" that identify the interfaces. * fix some merge problems * Increase Server tests coverage to 94%. Black listed peers try to connect test * fix merge issues * * Remove protected fields from TestBase and replace with overload usage of the method RandomInt. * Small clean up on the UtCrypto * Create UtBlockchain and create first tests. more tests will be added soon! * fix issues from the merge * Adding testing information to the team Contributing page. * Small CleanUp the test class with properties that aren't been used anymore. * UtBlockChain: InitializeBlockchain_IsGenesisBlock_InitializeComplete * Firsts UtBlockProcessor tests * AddBlock tests added * * UtBlockProcessor: Run test * Fix an inverted if logic. The code only enter in the While loop if the cancellation was requested and should be the other way around. * Cleanup * Replace BlockProcessor dependency from Prompt and replace with IBlockPool * Remove unused properties from BlockProcessor * ContainsBlock method in BlockProcessor signature changed to Private because is not used anywhere else but internally * UtBlockProcessor - Loop test: When the received block is not the expected next. * Remove comment * Remove async modifier from methods that aren't implemented * . * Merge branch 'development' of https://github.com/aboimpinto/neo-sharp into development * Add test class, #301 * Write tests for Add method. #301 * Refactor the Block / BlockHeader in Signed and Unsigned objects (mutable and immutable). The intenstion of this refactor is to bring better readability of the code and remove logic code from the DTO. * Implement the UpdateHash methods in the creation of the Signed * Signed and Unsigned strucuture return GenenisBlock * Draft of the BinaryCustomSeralization of TransactionBase * Revert "Remove stack logs (#321)" This reverts commit 34c676aad4bce133f64b8234180fd4014ee76377. * Add BlockOperationsManager and WitnessOperationManager that will be resposable to Sign and Verify the block and the witness * Initial work, removing logic from the model objects and add it into Witness. Transaction, BlockHeader and Block OperationManager classes. In this classes the verify method will be implemented. #306, #307 * Cleanup and remove code that was not merged. * regreassion test for the case where the current block is null. This happen when the node start without genesis block. * Remove dependencies from BlockProcessor and remove BlockHeader references. BlockProcessor use only Block object. #350, #351 * now the Persist consider the genesis block for the case of start a node in a clean environment --- .../Processing/BlockHeaderPersister.cs | 20 +++- .../Blockchain/Processing/BlockPersister.cs | 94 +++++++++++++++++++ .../Blockchain/Processing/BlockProcessor.cs | 52 +++------- .../Blockchain/Processing/IBlockPersister.cs | 19 ++++ .../DI/Modules/BlockchainModule.cs | 1 + .../Handlers/BlockHeadersMessageHandler.cs | 16 ++-- .../Blockchain/Processing/UtBlockProcessor.cs | 48 +--------- 7 files changed, 149 insertions(+), 101 deletions(-) create mode 100644 src/NeoSharp.Core/Blockchain/Processing/BlockPersister.cs create mode 100644 src/NeoSharp.Core/Blockchain/Processing/IBlockPersister.cs diff --git a/src/NeoSharp.Core/Blockchain/Processing/BlockHeaderPersister.cs b/src/NeoSharp.Core/Blockchain/Processing/BlockHeaderPersister.cs index 079d515b..4e8508b4 100644 --- a/src/NeoSharp.Core/Blockchain/Processing/BlockHeaderPersister.cs +++ b/src/NeoSharp.Core/Blockchain/Processing/BlockHeaderPersister.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using NeoSharp.Core.Extensions; @@ -28,11 +29,20 @@ public async Task Persist(params BlockHeader[] blockHeaders) { if (blockHeaders == null) throw new ArgumentNullException(nameof(blockHeaders)); - var blockHeadersToPersist = blockHeaders - .Where(bh => bh != null && bh.Index > LastBlockHeader?.Index) - .Distinct(bh => bh.Index) - .OrderBy(bh => bh.Index) - .ToList(); + var blockHeadersToPersist = new List(); + if (this.LastBlockHeader == null) + { + // Persisting the Genesis block + blockHeadersToPersist = blockHeaders.ToList(); + } + else + { + blockHeadersToPersist = blockHeaders + .Where(bh => bh != null && bh.Index > LastBlockHeader?.Index) + .Distinct(bh => bh.Index) + .OrderBy(bh => bh.Index) + .ToList(); + } foreach (var blockHeader in blockHeadersToPersist) { diff --git a/src/NeoSharp.Core/Blockchain/Processing/BlockPersister.cs b/src/NeoSharp.Core/Blockchain/Processing/BlockPersister.cs new file mode 100644 index 00000000..19ebc056 --- /dev/null +++ b/src/NeoSharp.Core/Blockchain/Processing/BlockPersister.cs @@ -0,0 +1,94 @@ +using System; +using System.Threading.Tasks; +using NeoSharp.Core.Models; +using NeoSharp.Core.Persistence; + +namespace NeoSharp.Core.Blockchain.Processing +{ + public class BlockPersister : IBlockPersister + { + #region Private Fields + private readonly IRepository _repository; + private readonly IBlockHeaderPersister _blockHeaderPersister; + private readonly ITransactionPersister _transactionPersister; + private readonly ITransactionPool _transactionPool; + #endregion + + #region Constructor + public BlockPersister( + IRepository repository, + IBlockHeaderPersister blockHeaderPersister, + ITransactionPersister transactionPersister, + ITransactionPool transactionPool) + { + this._repository = repository; + this._blockHeaderPersister = blockHeaderPersister; + this._transactionPersister = transactionPersister; + this._transactionPool = transactionPool; + } + #endregion + + #region IBlockPersister Implementation + public Block LastPersistedBlock { get; private set; } + + public event EventHandler OnBlockHeadersPersisted; + + public async Task Persist(params Block[] blocks) + { + foreach (var block in blocks) + { + this.LastPersistedBlock = block; + + var blockHeader = await this._repository.GetBlockHeader(block.Hash); + if (blockHeader == null) + { + await this._blockHeaderPersister.Persist(block.GetBlockHeader()); + } + + foreach (var transaction in block.Transactions) + { + await this._transactionPersister.Persist(transaction); + this._transactionPool.Remove(transaction.Hash); + } + } + } + + public async Task Persist(params BlockHeader[] blockHeaders) + { + try + { + this._blockHeaderPersister.OnBlockHeadersPersisted += this.HandleBlockHandlePersisted; + await this._blockHeaderPersister.Persist(blockHeaders); + } + finally + { + this._blockHeaderPersister.OnBlockHeadersPersisted -= this.HandleBlockHandlePersisted; + } + } + + public async Task IsBlockPersisted(Block block) + { + var blockHeader = await this._repository.GetBlockHeader(block.Hash); + + if (blockHeader?.Type == HeaderType.Extended) + { + throw new InvalidOperationException($"The block \"{block.Hash.ToString(true)}\" exists already on the blockchain."); + } + + if (blockHeader != null && blockHeader.Hash != block.Hash) + { + throw new InvalidOperationException($"The block \"{block.Hash.ToString(true)}\" has an invalid hash."); + } + + return false; + } + #endregion + + #region Private Method + private void HandleBlockHandlePersisted(object sender, BlockHeader[] e) + { + this.OnBlockHeadersPersisted?.Invoke(sender, e); + } + #endregion + } +} \ No newline at end of file diff --git a/src/NeoSharp.Core/Blockchain/Processing/BlockProcessor.cs b/src/NeoSharp.Core/Blockchain/Processing/BlockProcessor.cs index 4cc920b4..6ced25ec 100644 --- a/src/NeoSharp.Core/Blockchain/Processing/BlockProcessor.cs +++ b/src/NeoSharp.Core/Blockchain/Processing/BlockProcessor.cs @@ -4,7 +4,6 @@ using NeoSharp.Core.Helpers; using NeoSharp.Core.Models; using NeoSharp.Core.Models.OperationManger; -using NeoSharp.Core.Persistence; using NeoSharp.Core.Types; namespace NeoSharp.Core.Blockchain.Processing @@ -14,12 +13,9 @@ public class BlockProcessor : IBlockProcessor private static readonly TimeSpan DefaultBlockPollingInterval = TimeSpan.FromMilliseconds(100); private readonly IBlockPool _blockPool; - private readonly ITransactionPool _transactionPool; - private readonly IRepository _repository; private readonly IAsyncDelayer _asyncDelayer; - private readonly IBlockHeaderPersister _blockHeaderPersister; - private readonly ITransactionPersister _transactionPersister; private readonly IBlockOperationsManager _blockOperationsManager; + private readonly IBlockPersister _blockPersister; private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); private Block _currentBlock; @@ -27,20 +23,14 @@ public class BlockProcessor : IBlockProcessor public BlockProcessor( IBlockPool blockPool, - ITransactionPool transactionPool, - IRepository repository, IAsyncDelayer asyncDelayer, - IBlockHeaderPersister blockHeaderPersister, - ITransactionPersister transactionPersister, - IBlockOperationsManager blockOperationsManager) + IBlockOperationsManager blockOperationsManager, + IBlockPersister blockPersister) { _blockPool = blockPool ?? throw new ArgumentNullException(nameof(blockPool)); - _transactionPool = transactionPool ?? throw new ArgumentNullException(nameof(transactionPool)); - _repository = repository ?? throw new ArgumentNullException(nameof(repository)); _asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); - _blockHeaderPersister = blockHeaderPersister ?? throw new ArgumentNullException(nameof(blockHeaderPersister)); - _transactionPersister = transactionPersister ?? throw new ArgumentNullException(nameof(transactionPersister)); _blockOperationsManager = blockOperationsManager; + _blockPersister = blockPersister ?? throw new ArgumentNullException(nameof(_blockPersister)); } // TODO: We will read the current block from Blockchain @@ -63,7 +53,7 @@ public void Run(Block currentBlock) continue; } - await Persist(block); + await this._blockPersister.Persist(block); _blockPool.Remove(nextBlockHeight); _currentBlock = block; @@ -78,7 +68,10 @@ public async Task AddBlock(Block block) { if (block == null) throw new ArgumentNullException(nameof(block)); - _blockOperationsManager.Sign(block); + if (block.Hash == null) + { + _blockOperationsManager.Sign(block); + } var blockHash = block.Hash; @@ -90,18 +83,10 @@ public async Task AddBlock(Block block) throw new InvalidOperationException($"The block \"{blockHash.ToString(true)}\" was already queued to be added."); } - var blockHeader = await _repository.GetBlockHeader(blockHash); - if (blockHeader?.Type == HeaderType.Extended) + if (!await this._blockPersister.IsBlockPersisted(block)) { - throw new InvalidOperationException($"The block \"{blockHash.ToString(true)}\" exists already on the blockchain."); + _blockPool.Add(block); } - - if (blockHeader != null && blockHeader.Hash != block.Hash) - { - throw new InvalidOperationException($"The block \"{blockHash.ToString(true)}\" has an invalid hash."); - } - - _blockPool.Add(block); } public void Dispose() @@ -109,20 +94,5 @@ public void Dispose() _cancellationTokenSource.Cancel(); _cancellationTokenSource.Dispose(); } - - protected virtual async Task Persist(Block block) - { - var blockHeader = await _repository.GetBlockHeader(block.Hash); - if (blockHeader == null) - { - await _blockHeaderPersister.Persist(block.GetBlockHeader()); - } - - foreach (var transaction in block.Transactions) - { - await _transactionPersister.Persist(transaction); - _transactionPool.Remove(transaction.Hash); - } - } } } \ No newline at end of file diff --git a/src/NeoSharp.Core/Blockchain/Processing/IBlockPersister.cs b/src/NeoSharp.Core/Blockchain/Processing/IBlockPersister.cs new file mode 100644 index 00000000..cfd08c5a --- /dev/null +++ b/src/NeoSharp.Core/Blockchain/Processing/IBlockPersister.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using NeoSharp.Core.Models; + +namespace NeoSharp.Core.Blockchain.Processing +{ + public interface IBlockPersister + { + Block LastPersistedBlock { get; } + + event EventHandler OnBlockHeadersPersisted; + + Task Persist(params Block[] block); + + Task Persist(params BlockHeader[] blockHeaders); + + Task IsBlockPersisted(Block blocks); + } +} diff --git a/src/NeoSharp.Core/DI/Modules/BlockchainModule.cs b/src/NeoSharp.Core/DI/Modules/BlockchainModule.cs index fd6d9148..efd6507d 100644 --- a/src/NeoSharp.Core/DI/Modules/BlockchainModule.cs +++ b/src/NeoSharp.Core/DI/Modules/BlockchainModule.cs @@ -15,6 +15,7 @@ public void Register(IContainerBuilder containerBuilder) containerBuilder.RegisterSingleton(); #region Processing + containerBuilder.RegisterSingleton(); containerBuilder.RegisterSingleton(); containerBuilder.RegisterSingleton(); containerBuilder.RegisterSingleton(); diff --git a/src/NeoSharp.Core/Messaging/Handlers/BlockHeadersMessageHandler.cs b/src/NeoSharp.Core/Messaging/Handlers/BlockHeadersMessageHandler.cs index e29460ed..add831a6 100644 --- a/src/NeoSharp.Core/Messaging/Handlers/BlockHeadersMessageHandler.cs +++ b/src/NeoSharp.Core/Messaging/Handlers/BlockHeadersMessageHandler.cs @@ -16,31 +16,30 @@ public class BlockHeadersMessageHandler : IMessageHandler { private const int MaxBlocksCountToSync = 500; - private readonly IBlockHeaderPersister _blockHeaderPersister; + private readonly IBlockPersister _blockPersister; private readonly IBlockchain _blockchain; private readonly ILogger _logger; - public BlockHeadersMessageHandler(IBlockHeaderPersister blockHeaderPersister, IBlockchain blockchain, ILogger logger) + public BlockHeadersMessageHandler(IBlockPersister blockPersister, IBlockchain blockchain, ILogger logger) { - _blockHeaderPersister = blockHeaderPersister ?? throw new ArgumentNullException(nameof(blockHeaderPersister)); + _blockPersister = blockPersister ?? throw new ArgumentNullException(nameof(blockPersister)); _blockchain = blockchain ?? throw new ArgumentNullException(nameof(blockchain)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } public async Task Handle(BlockHeadersMessage message, IPeer sender) { - EventHandler blockHeadersPersisted = - async (_, blockHeaders) => await BlockHeadersPersisted(sender, blockHeaders); + async void HeadersPersisted(object _, BlockHeader[] blockHeaders) => await BlockHeadersPersisted(sender, blockHeaders); try { - _blockHeaderPersister.OnBlockHeadersPersisted += blockHeadersPersisted; + _blockPersister.OnBlockHeadersPersisted += HeadersPersisted; - await _blockHeaderPersister.Persist(message.Payload.Headers ?? new BlockHeader[0]); + await _blockPersister.Persist(message.Payload.Headers ?? new BlockHeader[0]); } finally { - _blockHeaderPersister.OnBlockHeadersPersisted -= blockHeadersPersisted; + _blockPersister.OnBlockHeadersPersisted -= HeadersPersisted; } if (_blockchain.LastBlockHeader.Index < sender.Version.CurrentBlockIndex) @@ -76,7 +75,6 @@ private static async Task SynchronizeBlocks(IPeer source, IReadOnlyCollection(); - blockPoolMock - .Setup(x => x.Contains(block.Hash)) - .Returns(false); - - var repositoryMock = this.AutoMockContainer.GetMock(); - repositoryMock - .Setup(x => x.GetBlockHeader(block.Hash)) - .ReturnsAsync(expectedBlockHeader); - - var testee = this.AutoMockContainer.Create(); - - await testee.AddBlock(block); - } - [TestMethod] public async Task AddBlock_ValidBlockNotInBlockPoolInBlockChainWithTheRightBlockHeaderType_BlockAddedToBlockPool() { @@ -234,9 +199,7 @@ public void Run_WhenAddBlockThisIsProcessed_OnBlockProcessedEventRaised() .Setup(x => x.TryGet(1, out newBlock)) .Returns(true); - var blockHeaderPersisterMock = this.AutoMockContainer.GetMock(); - var transactionProcessorMock = this.AutoMockContainer.GetMock>(); - var repositoryMock = this.AutoMockContainer.GetMock(); + var blockPersisterMock = this.AutoMockContainer.GetMock(); var testee = this.AutoMockContainer.Create(); @@ -254,14 +217,7 @@ public void Run_WhenAddBlockThisIsProcessed_OnBlockProcessedEventRaised() waitForBlockProcessedEvent.WaitOne(); - transactionProcessorMock.Verify(x => x.Persist(transactionInNewBlock)); - - blockHeaderPersisterMock.Verify(x => x.Persist(It.Is(blockHeader => - blockHeader.ConsensusData == newBlock.ConsensusData && - blockHeader.Hash == newBlock.Hash && - blockHeader.Index == newBlock.Index && - blockHeader.Timestamp == newBlock.Timestamp && - blockHeader.Version == newBlock.Version))); + blockPersisterMock.Verify(x => x.Persist(newBlock)); } [TestMethod]