Prevent node from not advancing in case block was rejected during FV (#…

…3793) * ban peers for rejectuntil duration * tests * Added comment * Added missing field dateTimeProvider
stratisproject · Jul 16, 2019 · e11931b · e11931b
1 parent 2ebed26
commit e11931b
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 13 deletions.
diff --git a/src/Stratis.Bitcoin.Features.Consensus.Tests/Rules/CommonRules/PosCoinViewRuleTest.cs b/src/Stratis.Bitcoin.Features.Consensus.Tests/Rules/CommonRules/PosCoinViewRuleTest.cs
@@ -58,7 +58,7 @@ private async Task<ConsensusManager> CreateConsensusManagerAsync(Dictionary<uint
             var consensus = new ConsensusManager(chainedHeaderTree, this.network, this.loggerFactory.Object, this.chainState.Object, integrityValidator,
                 partialValidator, fullValidator, consensusRuleEngine, new Mock<IFinalizedBlockInfoRepository>().Object, signals,
                 new Mock<IPeerBanning>().Object, initialBlockDownloadState, this.ChainIndexer, new Mock<IBlockPuller>().Object, new Mock<IBlockStore>().Object,
-                new Mock<IConnectionManager>().Object, new Mock<INodeStats>().Object, new Mock<INodeLifetime>().Object, this.consensusSettings);
+                new Mock<IConnectionManager>().Object, new Mock<INodeStats>().Object, new Mock<INodeLifetime>().Object, this.consensusSettings, new DateTimeProvider());
 
             // Mock the coinviews "FetchCoinsAsync" method. We will use the "unspentOutputs" dictionary to track spendable outputs.
             this.coinView.Setup(d => d.FetchCoins(It.IsAny<uint256[]>(), It.IsAny<CancellationToken>()))

diff --git a/src/Stratis.Bitcoin.Tests.Common/ConsensusManagerHelper.cs b/src/Stratis.Bitcoin.Tests.Common/ConsensusManagerHelper.cs
@@ -99,7 +99,7 @@ public static class ConsensusManagerHelper
             var consensus = new ConsensusManager(tree, network, loggerFactory, chainState, new IntegrityValidator(consensusRules, loggerFactory),
                 new PartialValidator(asyncProvider, consensusRules, loggerFactory), new FullValidator(consensusRules, loggerFactory), consensusRules,
                 new Mock<IFinalizedBlockInfoRepository>().Object, new Signals.Signals(loggerFactory, null), peerBanning, new Mock<IInitialBlockDownloadState>().Object, chainIndexer,
-                new Mock<IBlockPuller>().Object, new Mock<IBlockStore>().Object, new Mock<IConnectionManager>().Object, new Mock<INodeStats>().Object, new NodeLifetime(), consensusSettings);
+                new Mock<IBlockPuller>().Object, new Mock<IBlockStore>().Object, new Mock<IConnectionManager>().Object, new Mock<INodeStats>().Object, new NodeLifetime(), consensusSettings, dateTimeProvider);
 
             return consensus;
         }

diff --git a/src/Stratis.Bitcoin.Tests/Consensus/ConsensusTestContext.cs b/src/Stratis.Bitcoin.Tests/Consensus/ConsensusTestContext.cs
@@ -164,7 +164,7 @@ public TestContext()
             ConsensusManager consensusManager = new ConsensusManager(tree, this.Network, this.loggerFactory, this.ChainState.Object, this.IntegrityValidator.Object,
                 this.PartialValidator.Object, this.FullValidator.Object, this.consensusRules,
                 this.FinalizedBlockMock.Object, this.signals, this.peerBanning, this.ibd.Object, this.chainIndexer,
-                this.BlockPuller.Object, this.BlockStore.Object, this.connectionManager, this.nodeStats, this.nodeLifetime, this.ConsensusSettings);
+                this.BlockPuller.Object, this.BlockStore.Object, this.connectionManager, this.nodeStats, this.nodeLifetime, this.ConsensusSettings, this.dateTimeProvider);
 
             this.TestConsensusManager = new TestConsensusManager(consensusManager);
         }

diff --git a/src/Stratis.Bitcoin/Base/BaseFeature.cs b/src/Stratis.Bitcoin/Base/BaseFeature.cs
@@ -436,8 +436,8 @@ public static IFullNodeBuilder UseBaseFeature(this IFullNodeBuilder fullNodeBuil
                         connectionManager: provider.GetService<IConnectionManager>(),
                         nodeStats: provider.GetService<INodeStats>(),
                         nodeLifetime: provider.GetService<INodeLifetime>(),
-                        consensusSettings: provider.GetService<ConsensusSettings>()
-                        ));
+                        consensusSettings: provider.GetService<ConsensusSettings>(),
+                        dateTimeProvider: provider.GetService<IDateTimeProvider>()));
                     services.AddSingleton<IChainedHeaderTree, ChainedHeaderTree>();
                     services.AddSingleton<IHeaderValidator, HeaderValidator>();
                     services.AddSingleton<IIntegrityValidator, IntegrityValidator>();

diff --git a/src/Stratis.Bitcoin/Consensus/ConsensusManager.cs b/src/Stratis.Bitcoin/Consensus/ConsensusManager.cs
@@ -122,6 +122,8 @@ internal class DownloadedCallbacks
         private readonly ConsensusManagerPerformanceCounter performanceCounter;
 
         private readonly ConsensusSettings consensusSettings;
+
+        private readonly IDateTimeProvider dateTimeProvider;
 
         private bool isIbd;
 
@@ -144,7 +146,8 @@ internal class DownloadedCallbacks
             IConnectionManager connectionManager,
             INodeStats nodeStats,
             INodeLifetime nodeLifetime,
-            ConsensusSettings consensusSettings)
+            ConsensusSettings consensusSettings,
+            IDateTimeProvider dateTimeProvider)
         {
             Guard.NotNull(chainedHeaderTree, nameof(chainedHeaderTree));
             Guard.NotNull(network, nameof(network));
@@ -196,6 +199,7 @@ internal class DownloadedCallbacks
             this.ibdState = ibdState;
 
             this.blockPuller = blockPuller;
+            this.dateTimeProvider = dateTimeProvider;
 
             this.consensusSettings = consensusSettings;
             this.maxUnconsumedBlocksDataBytes = consensusSettings.MaxBlockMemoryInMB * 1024 * 1024;
@@ -869,13 +873,9 @@ private async Task<ConnectBlocksResult> ConnectBlockAsync(ChainedHeaderBlock blo
             {
                 var badPeers = new List<int>();
 
-                // Ban the peers only in case block is invalid and not temporary rejected.
-                if (validationContext.RejectUntil == null)
+                lock (this.peerLock)
                 {
-                    lock (this.peerLock)
-                    {
-                        badPeers = this.chainedHeaderTree.PartialOrFullValidationFailed(blockToConnect.ChainedHeader);
-                    }
+                    badPeers = this.chainedHeaderTree.PartialOrFullValidationFailed(blockToConnect.ChainedHeader);
                 }
 
                 var failureResult = new ConnectBlocksResult(false)
@@ -887,6 +887,15 @@ private async Task<ConnectBlocksResult> ConnectBlockAsync(ChainedHeaderBlock blo
                     PeersToBan = badPeers
                 };
 
+                // If set, use the block reject until time as the ban duration instead of the default 10 minute ban.
+                if (validationContext.RejectUntil != null)
+                {
+                    failureResult.BanDurationSeconds = (int)(validationContext.RejectUntil.Value.ToUnixTimestamp() - this.dateTimeProvider.GetAdjustedTimeAsUnixTimestamp());
+
+                    if (failureResult.BanDurationSeconds < 1)
+                        failureResult.BanDurationSeconds = 1;
+                }
+
                 this.logger.LogTrace("(-)[FAILED]:'{0}'", failureResult);
                 return failureResult;
             }
@@ -1416,4 +1425,4 @@ public void Dispose()
             this.reorgLock.Dispose();
         }
     }
-}
+}