From a43046f0b05067d4e0f1cb7249a472d89d9e55c6 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Thu, 9 Jun 2022 18:16:37 +0600 Subject: [PATCH 01/27] preventing non validator message --- src/Lachain.Core/Consensus/ConsensusManager.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Lachain.Core/Consensus/ConsensusManager.cs b/src/Lachain.Core/Consensus/ConsensusManager.cs index 9bdb6f698..8ffd8138f 100644 --- a/src/Lachain.Core/Consensus/ConsensusManager.cs +++ b/src/Lachain.Core/Consensus/ConsensusManager.cs @@ -122,7 +122,7 @@ public void Dispatch(ConsensusMessage message, ECDSAPublicKey from) if (fromIndex == -1) { Logger.LogWarning( - $"Skipped message for era {era} since we it came from {from.ToHex()} who is not validator for this era"); + $"Skipped message for era {era} since it came from {from.ToHex()} who is not validator for this era"); return; } @@ -290,6 +290,13 @@ private void Run(ulong startingEra) foreach (var (message, from) in savedMessages) { var fromIndex = validators.GetValidatorIndex(from); + if (fromIndex == -1) + { + Logger.LogWarning( + $"Skipped message for era {CurrentEra} since it came from " + + $"{from.ToHex()} who is not validator for this era"); + continue; + } Logger.LogTrace( $"Handling postponed message: {message.PrettyTypeString()}"); broadcaster.Dispatch(message, fromIndex); From 573a5d66ffdca31561cc438a511261098b9b5060 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Thu, 9 Jun 2022 18:57:54 +0600 Subject: [PATCH 02/27] Preventing spam creation of ReliableBroadcast --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 71a16976e..b0f5dd49b 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -165,15 +165,23 @@ public void Dispatch(ConsensusMessage message, int from) var hbbftId = new HoneyBadgerId((int) message.Validator.Era); EnsureProtocol(hbbftId)?.ReceiveMessage(new MessageEnvelope(message, from)); break; + // There are separate instance of ReliableBroadcast for each validator. + // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId case ConsensusMessage.PayloadOneofCase.ValMessage: + if (!ValidateSenderId(message.ValMessage.SenderId)) + break; var reliableBroadcastId = new ReliableBroadcastId(message.ValMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(reliableBroadcastId)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.EchoMessage: + if (!ValidateSenderId(message.ValMessage.SenderId)) + break; var rbIdEchoMsg = new ReliableBroadcastId(message.EchoMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdEchoMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.ReadyMessage: + if (!ValidateSenderId(message.ValMessage.SenderId)) + break; var rbIdReadyMsg = new ReliableBroadcastId(message.ReadyMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdReadyMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); break; @@ -337,6 +345,21 @@ private void ValidateId(IProtocolIdentifier id) if (id.Era != _era) throw new InvalidOperationException($"Era mismatched, expected {_era} got message with {id.Era}"); } + + private bool ValidateSenderId(int senderId) + { + if (_validators is null) + { + Logger.LogWarning("We don't have validators"); + return false; + } + if (senderId < 0 || senderId >= _validators.N) + { + Logger.LogWarning($"Invalid sender id in \"ValMessage\": {senderId}. N: {_validators.N}"); + return false; + } + return true; + } public bool WaitFinish(TimeSpan timeout) { From 2448854e4f98fda24a0e0f6c04edb55c8701d4b6 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Fri, 10 Jun 2022 22:00:40 +0600 Subject: [PATCH 03/27] fixed some issues --- src/Lachain.Consensus/AbstractProtocol.cs | 9 ++++++++- src/Lachain.Consensus/RootProtocol/RootProtocol.cs | 2 +- src/Lachain.Core/Consensus/EraBroadcaster.cs | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index f7eb5fbec..9676d3585 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -144,7 +144,14 @@ public void ReceiveMessage(MessageEnvelope message) { lock (_queueLock) { - if (Terminated){} + if (Terminated) + { + // we should return here instead of enqueueing messages + // because once terminated, the messages are not being processed anymore + // so the queue will just get large unnecessarily + // investigating if the above is true, if true will return from here + return; + } // Logger.LogTrace($"{Id}: got message after termination"); _queue.Enqueue(message); Monitor.Pulse(_queueLock); diff --git a/src/Lachain.Consensus/RootProtocol/RootProtocol.cs b/src/Lachain.Consensus/RootProtocol/RootProtocol.cs index c809165d2..92348e7ee 100644 --- a/src/Lachain.Consensus/RootProtocol/RootProtocol.cs +++ b/src/Lachain.Consensus/RootProtocol/RootProtocol.cs @@ -192,7 +192,7 @@ private void TrySignHeader() Logger.LogTrace("TrySignHeader"); if (_receipts is null || _nonce is null || _blockProducer is null) { - Logger.LogTrace($"Not ready yet: _hasges {_receipts is null}, _nonce {_nonce is null}, _blockProducer {_blockProducer is null}"); + Logger.LogTrace($"Not ready yet: _hashes {_receipts is null}, _nonce {_nonce is null}, _blockProducer {_blockProducer is null}"); return; } diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index b0f5dd49b..a251f4492 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -174,13 +174,13 @@ public void Dispatch(ConsensusMessage message, int from) EnsureProtocol(reliableBroadcastId)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.EchoMessage: - if (!ValidateSenderId(message.ValMessage.SenderId)) + if (!ValidateSenderId(message.EchoMessage.SenderId)) break; var rbIdEchoMsg = new ReliableBroadcastId(message.EchoMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdEchoMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.ReadyMessage: - if (!ValidateSenderId(message.ValMessage.SenderId)) + if (!ValidateSenderId(message.ReadyMessage.SenderId)) break; var rbIdReadyMsg = new ReliableBroadcastId(message.ReadyMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdReadyMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); @@ -355,7 +355,7 @@ private bool ValidateSenderId(int senderId) } if (senderId < 0 || senderId >= _validators.N) { - Logger.LogWarning($"Invalid sender id in \"ValMessage\": {senderId}. N: {_validators.N}"); + Logger.LogWarning($"Invalid sender id in consensus message: {senderId}. N: {_validators.N}"); return false; } return true; From 5cc5c2185eed0b95e035e98d742dcadc8aae24e2 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Sat, 11 Jun 2022 00:06:07 +0600 Subject: [PATCH 04/27] handling checking in EnsureProtocol --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index a251f4492..16a79b864 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -165,23 +165,15 @@ public void Dispatch(ConsensusMessage message, int from) var hbbftId = new HoneyBadgerId((int) message.Validator.Era); EnsureProtocol(hbbftId)?.ReceiveMessage(new MessageEnvelope(message, from)); break; - // There are separate instance of ReliableBroadcast for each validator. - // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId case ConsensusMessage.PayloadOneofCase.ValMessage: - if (!ValidateSenderId(message.ValMessage.SenderId)) - break; var reliableBroadcastId = new ReliableBroadcastId(message.ValMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(reliableBroadcastId)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.EchoMessage: - if (!ValidateSenderId(message.EchoMessage.SenderId)) - break; var rbIdEchoMsg = new ReliableBroadcastId(message.EchoMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdEchoMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.ReadyMessage: - if (!ValidateSenderId(message.ReadyMessage.SenderId)) - break; var rbIdReadyMsg = new ReliableBroadcastId(message.ReadyMessage.SenderId, (int) message.Validator.Era); EnsureProtocol(rbIdReadyMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); break; @@ -308,7 +300,11 @@ public void Terminate() ); RegisterProtocols(new[] {coin}); return coin; + // There are separate instance of ReliableBroadcast for each validator. + // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId case ReliableBroadcastId rbcId: + if (!ValidateSenderId(rbcId.SenderId)) + return null; var rbc = new ReliableBroadcast(rbcId, _validators, this); RegisterProtocols(new[] {rbc}); return rbc; From 39f6c3144a322bde059afd3210ad21a52fffccda Mon Sep 17 00:00:00 2001 From: tbssajal Date: Sat, 11 Jun 2022 02:16:43 +0600 Subject: [PATCH 05/27] preventing CommonCoin spam creation --- .../BinaryAgreement/BinaryAgreement.cs | 4 +- src/Lachain.Consensus/CommonCoin/CoinToss.cs | 22 ++++++++ src/Lachain.Core/Consensus/EraBroadcaster.cs | 56 ++++++++++++++++++- 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 src/Lachain.Consensus/CommonCoin/CoinToss.cs diff --git a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs index e67fbc7d6..093cd8500 100644 --- a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs +++ b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs @@ -128,13 +128,13 @@ private void TryProgressEpoch() _currentValues = _binaryBroadcastsResults[_currentEpoch - 1]; var coinId = new CoinId(_agreementId.Era, _agreementId.AssociatedValidatorId, _currentEpoch); - if ((_currentEpoch / 2) % 3 == 2) + if (CoinToss.CreateCoinId(_currentEpoch)) { Broadcaster.InternalRequest(new ProtocolRequest(Id, coinId, null)); } else { - _coins[_currentEpoch] = ((_currentEpoch / 2) % 3) != 0; + _coins[_currentEpoch] = CoinToss.TossCoin(_currentEpoch) != 0; } _currentEpoch += 1; diff --git a/src/Lachain.Consensus/CommonCoin/CoinToss.cs b/src/Lachain.Consensus/CommonCoin/CoinToss.cs new file mode 100644 index 000000000..ed2a4e2c3 --- /dev/null +++ b/src/Lachain.Consensus/CommonCoin/CoinToss.cs @@ -0,0 +1,22 @@ +namespace Lachain.Consensus.CommonCoin +{ + public static class CoinToss + { + // Starting epoch for which BinaryAgreement requests for CommonCoin + public static long StartingEpochForCommonCoin = 5; + public static long TossCoin(long epoch) + { + return (epoch / 2) % 3; + } + + public static bool CreateCoinId(long epoch) + { + return TossCoin(epoch) == 2; + } + + public static long PreviousEpoch(long epoch) + { + return epoch - 6; + } + } +} \ No newline at end of file diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 16a79b864..f446242dd 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -273,6 +273,14 @@ public void Terminate() _callback.Clear(); } + // Each ProtocolId is created only once to prevent spamming, Protocols are mapped against ProtocolId, so each + // Protocol will also be created only once, after achieving result, Protocol terminate and no longer process any + // messages. So if the required protocol (lets say 'parent protocol') that will use the result of newly created + // protocols (lets say 'child protocol') is not created before 'child protocol' returns result, then the + // 'parent protocol' will never be able to use their results and will get stuck + // For ReliableBroadcast, this is not a problem, because it waits for at least F + 1 inputs from validators. + // But for other protocols (CommonCoin, BinaryBroadcast) may have problem if spammed from malicious validators. + // BinaryAgreement is created only from InternalRequest, not from ExternalMessage, so it is safe as well. [MethodImpl(MethodImplOptions.Synchronized)] private IConsensusProtocol? EnsureProtocol(IProtocolIdentifier id) { @@ -292,6 +300,8 @@ public void Terminate() RegisterProtocols(new[] {bb}); return bb; case CoinId coinId: + if (!ValidateCoinId(coinId)) + return null; var coin = new CommonCoin( coinId, _validators, _wallet.GetThresholdSignatureKeyForBlock((ulong) _era - 1) ?? @@ -300,8 +310,6 @@ public void Terminate() ); RegisterProtocols(new[] {coin}); return coin; - // There are separate instance of ReliableBroadcast for each validator. - // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId case ReliableBroadcastId rbcId: if (!ValidateSenderId(rbcId.SenderId)) return null; @@ -342,6 +350,9 @@ private void ValidateId(IProtocolIdentifier id) throw new InvalidOperationException($"Era mismatched, expected {_era} got message with {id.Era}"); } + // There are separate instance of ReliableBroadcast for each validator. + // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId + // Sender id is basically validator's id, so it must be between 0 and N-1 inclusive private bool ValidateSenderId(int senderId) { if (_validators is null) @@ -357,6 +368,47 @@ private bool ValidateSenderId(int senderId) return true; } + // CommonCoin returns result immediately after getting a valid input and then terminates + // This input could be via network from another validator or its own generated + // RootProtocol requests for special type of CoinId, for this type check if RootProtocol exists + // BinaryAgreement requests another type of CoinId and it generates them sequentially + private bool ValidateCoinId(CoinId coinId) + { + if (coinId.Agreement == -1 && coinId.Epoch == 0) + { + // This type of coinId is created from RootProtocol or via network from another validator + // If it is via network and RootProtocol is not created yet, then RootProtocol will not get + // the result from this CommonCoin, and will get stuck + return _callback.TryGetValue(coinId, out var _); + } + else if (!(_validators is null) && coinId.Agreement >= 0 && coinId.Agreement < _validators.N) + { + // BinaryAgreement requests such CommonCoin + if (!_callback.TryGetValue(coinId, out var binaryAgreementId) || + !_registry.TryGetValue(binaryAgreementId, out var binaryAgreement) || + binaryAgreement.Terminated) + return false; + // BinaryAgreement uses CommonCoin in this pattern: false, true, result of CommonCoin + // For each odd epoch. So, epoch 1 => false, epoch 3 => true, epoch 5 => result of CommonCoin + // and the cycle continues. So starting from epoch 5, at an interval of epoch 6 (5 , 11 , 17, ..), + // CommonCoin is requested + var createCoinId = coinId.Epoch > 0 && CoinToss.CreateCoinId(coinId.Epoch); + if (!createCoinId) + return false; + // If this CommonCoin is not the first one, check if the previous one is terminated + if (coinId.Epoch > CoinToss.StartingEpochForCommonCoin) + { + var previousCoinId = new CoinId(coinId.Era, coinId.Agreement, CoinToss.PreviousEpoch(coinId.Epoch)); + if (!_registry.TryGetValue(previousCoinId, out var previousCommonCoin) || + !previousCommonCoin.Terminated) + return false; + } + return true; + } + else + return false; + } + public bool WaitFinish(TimeSpan timeout) { return EnsureProtocol(new RootProtocolId(_era))?.WaitFinish(timeout) ?? true; From 6e68f13ce211a643eb9f61aecdb0139f887bfc59 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Sun, 12 Jun 2022 23:36:27 +0600 Subject: [PATCH 06/27] updated checking common coin spamming --- src/Lachain.Consensus/CommonCoin/CoinToss.cs | 17 ++++++++------- src/Lachain.Core/Consensus/EraBroadcaster.cs | 23 ++++---------------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/Lachain.Consensus/CommonCoin/CoinToss.cs b/src/Lachain.Consensus/CommonCoin/CoinToss.cs index ed2a4e2c3..bb0601c2d 100644 --- a/src/Lachain.Consensus/CommonCoin/CoinToss.cs +++ b/src/Lachain.Consensus/CommonCoin/CoinToss.cs @@ -2,21 +2,22 @@ namespace Lachain.Consensus.CommonCoin { public static class CoinToss { - // Starting epoch for which BinaryAgreement requests for CommonCoin - public static long StartingEpochForCommonCoin = 5; + /* + RootProtocol requests for one CommonCoin protocol + BinaryAgreement requests many CommonCoin protocols sequentially + For each odd epoch BinaryAgreements takes false, true, result of common coin + So for epoch 1 => false, epoch 3 => true, epoch 5 => result of common coin + and the cycle continues + */ public static long TossCoin(long epoch) { return (epoch / 2) % 3; } + // epoch will be odd and TossCoin value will be 2 public static bool CreateCoinId(long epoch) { - return TossCoin(epoch) == 2; - } - - public static long PreviousEpoch(long epoch) - { - return epoch - 6; + return ((epoch & 1) != 0) && TossCoin(epoch) == 2; } } } \ No newline at end of file diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index f446242dd..66c924bda 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -281,6 +281,7 @@ public void Terminate() // For ReliableBroadcast, this is not a problem, because it waits for at least F + 1 inputs from validators. // But for other protocols (CommonCoin, BinaryBroadcast) may have problem if spammed from malicious validators. // BinaryAgreement is created only from InternalRequest, not from ExternalMessage, so it is safe as well. + // For each protocol, the corresponding 'parent protocol' is stored in _callback dictionary [MethodImpl(MethodImplOptions.Synchronized)] private IConsensusProtocol? EnsureProtocol(IProtocolIdentifier id) { @@ -370,15 +371,14 @@ private bool ValidateSenderId(int senderId) // CommonCoin returns result immediately after getting a valid input and then terminates // This input could be via network from another validator or its own generated - // RootProtocol requests for special type of CoinId, for this type check if RootProtocol exists - // BinaryAgreement requests another type of CoinId and it generates them sequentially + // If the 'parent protocol' (mentioned above) is not created when this protocol is requested + // then the 'parent protocol' will not get the result and will get stuck' + // So check if the 'parent protocol' exists private bool ValidateCoinId(CoinId coinId) { if (coinId.Agreement == -1 && coinId.Epoch == 0) { // This type of coinId is created from RootProtocol or via network from another validator - // If it is via network and RootProtocol is not created yet, then RootProtocol will not get - // the result from this CommonCoin, and will get stuck return _callback.TryGetValue(coinId, out var _); } else if (!(_validators is null) && coinId.Agreement >= 0 && coinId.Agreement < _validators.N) @@ -388,21 +388,6 @@ private bool ValidateCoinId(CoinId coinId) !_registry.TryGetValue(binaryAgreementId, out var binaryAgreement) || binaryAgreement.Terminated) return false; - // BinaryAgreement uses CommonCoin in this pattern: false, true, result of CommonCoin - // For each odd epoch. So, epoch 1 => false, epoch 3 => true, epoch 5 => result of CommonCoin - // and the cycle continues. So starting from epoch 5, at an interval of epoch 6 (5 , 11 , 17, ..), - // CommonCoin is requested - var createCoinId = coinId.Epoch > 0 && CoinToss.CreateCoinId(coinId.Epoch); - if (!createCoinId) - return false; - // If this CommonCoin is not the first one, check if the previous one is terminated - if (coinId.Epoch > CoinToss.StartingEpochForCommonCoin) - { - var previousCoinId = new CoinId(coinId.Era, coinId.Agreement, CoinToss.PreviousEpoch(coinId.Epoch)); - if (!_registry.TryGetValue(previousCoinId, out var previousCommonCoin) || - !previousCommonCoin.Terminated) - return false; - } return true; } else From 76109fcc29df4955e8495ddf9df74c103727aec2 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 01:24:44 +0600 Subject: [PATCH 07/27] removed unnecessary lines --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 66c924bda..0ee4cd189 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -381,7 +381,7 @@ private bool ValidateCoinId(CoinId coinId) // This type of coinId is created from RootProtocol or via network from another validator return _callback.TryGetValue(coinId, out var _); } - else if (!(_validators is null) && coinId.Agreement >= 0 && coinId.Agreement < _validators.N) + else if (ValidateSenderId((int)coinId.Agreement)) { // BinaryAgreement requests such CommonCoin if (!_callback.TryGetValue(coinId, out var binaryAgreementId) || From 039b13b3c65b1e92faf54dea4079fdd764da50fd Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 01:48:55 +0600 Subject: [PATCH 08/27] prevention of spam creation of BinaryBroadcastId --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 40 ++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 0ee4cd189..11f30ae6b 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -297,6 +297,8 @@ public void Terminate() switch (id) { case BinaryBroadcastId bbId: + if (!ValidateBinaryBroadcastId(bbId)) + return null; var bb = new BinaryBroadcast(bbId, _validators, this); RegisterProtocols(new[] {bb}); return bb; @@ -312,7 +314,7 @@ public void Terminate() RegisterProtocols(new[] {coin}); return coin; case ReliableBroadcastId rbcId: - if (!ValidateSenderId(rbcId.SenderId)) + if (!ValidateSenderId((long) rbcId.SenderId)) return null; var rbc = new ReliableBroadcast(rbcId, _validators, this); RegisterProtocols(new[] {rbc}); @@ -354,7 +356,7 @@ private void ValidateId(IProtocolIdentifier id) // There are separate instance of ReliableBroadcast for each validator. // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId // Sender id is basically validator's id, so it must be between 0 and N-1 inclusive - private bool ValidateSenderId(int senderId) + private bool ValidateSenderId(long senderId) { if (_validators is null) { @@ -381,7 +383,7 @@ private bool ValidateCoinId(CoinId coinId) // This type of coinId is created from RootProtocol or via network from another validator return _callback.TryGetValue(coinId, out var _); } - else if (ValidateSenderId((int)coinId.Agreement)) + else if (ValidateSenderId(coinId.Agreement)) { // BinaryAgreement requests such CommonCoin if (!_callback.TryGetValue(coinId, out var binaryAgreementId) || @@ -394,6 +396,38 @@ private bool ValidateCoinId(CoinId coinId) return false; } + // BinaryBroadcast needs at least F + 1 responses from validators to reach result + // So malicious validators are not a threat in terms of reaching wrong result or + // Creating BinaryBroadcast too early so that its 'parent protocol', BinaryAgreement + // does not get the result. But we need to stop spam creation of this protocol as it + // can be created too many times with too many epochs + // BinaryAgreement creates BinaryBroadcast sequentially, for each even epoch using the + // result of previous CommonCoin as estimation. Different honest validators can start + // the protocol in different time and broadcast due to network latency, but none of them + // will reach a verdict without at least F + 1 response, so we can assume that if a + // BinaryBroadcast protocol is created and broadcasted by an honest validator, then that + // honest validator reached a valid verdict with at least F + 1 response in a previously + // created BinaryBroadcast protocol, and so the current validator (this node) has also + // reached the same verdict in the previous BinaryBroadcast protocol. + // So we can check if the previous BinaryBroadcast protocol has terminated, if so, then + // we can create another BinaryBroadcast protocol + private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) + { + if (!ValidateSenderId(binaryBroadcastId.Agreement) || binaryBroadcastId.Epoch < 0) + return false; + else if (binaryBroadcastId.Epoch > 0 && (binaryBroadcastId.Epoch & 1) == 0) // positive and even + { + var previousBinaryBroadcastId = + new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, binaryBroadcastId.Epoch - 2); + if (!_registry.TryGetValue(previousBinaryBroadcastId, out var previousBinaryBroadcast) || + !previousBinaryBroadcast.Terminated) + return false; + return true; + } + else + return true; + } + public bool WaitFinish(TimeSpan timeout) { return EnsureProtocol(new RootProtocolId(_era))?.WaitFinish(timeout) ?? true; From a11e588b02674711718f809d529bd8a5748ae6ff Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 13:17:44 +0600 Subject: [PATCH 09/27] removed unnecessary comment --- src/Lachain.Consensus/AbstractProtocol.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index 9676d3585..3d5a23727 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -149,7 +149,6 @@ public void ReceiveMessage(MessageEnvelope message) // we should return here instead of enqueueing messages // because once terminated, the messages are not being processed anymore // so the queue will just get large unnecessarily - // investigating if the above is true, if true will return from here return; } // Logger.LogTrace($"{Id}: got message after termination"); From 88010e2429bb287581dead3beb238516388a6d38 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 13:25:42 +0600 Subject: [PATCH 10/27] updated comment --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 11f30ae6b..001d7155f 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -279,7 +279,7 @@ public void Terminate() // protocols (lets say 'child protocol') is not created before 'child protocol' returns result, then the // 'parent protocol' will never be able to use their results and will get stuck // For ReliableBroadcast, this is not a problem, because it waits for at least F + 1 inputs from validators. - // But for other protocols (CommonCoin, BinaryBroadcast) may have problem if spammed from malicious validators. + // But some protocol (CommonCoin) may have problem if spammed from malicious validators. // BinaryAgreement is created only from InternalRequest, not from ExternalMessage, so it is safe as well. // For each protocol, the corresponding 'parent protocol' is stored in _callback dictionary [MethodImpl(MethodImplOptions.Synchronized)] From c85a0ef9ae7c36e21f3351e5ccc8b91dc964b69a Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 13:45:22 +0600 Subject: [PATCH 11/27] cleared queue while terminating --- src/Lachain.Consensus/AbstractProtocol.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index 3d5a23727..f6a9bce30 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -94,6 +94,9 @@ public void Terminate() if (Terminated) return; Logger.LogTrace($"{Id}: protocol is terminated"); Terminated = true; + // We can empty the _queue because the messages will no longer be processed + // This will free some memory in case of spam messages + _queue.Clear(); Monitor.Pulse(_queueLock); } } From 54f90b3ccd855d69eb9ca69a1c09d5e856e57884 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Mon, 13 Jun 2022 14:14:56 +0600 Subject: [PATCH 12/27] handled termination --- src/Lachain.Consensus/AbstractProtocol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index f6a9bce30..ce9bc869a 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -137,7 +137,7 @@ public void Start() catch (Exception e) { Logger.LogError($"{Id}: exception occured while processing message: {e}"); - Terminated = true; + Terminate(); break; } } From cf27e021ce6dff5a6f7dbfcc8a99d00d325a5248 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Tue, 14 Jun 2022 14:29:29 +0600 Subject: [PATCH 13/27] handled exception --- .../ReliableBroadcast/ReliableBroadcast.cs | 5 +++-- src/Lachain.Core/Consensus/EraBroadcaster.cs | 13 ++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs index 5dd71f667..11065331c 100644 --- a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs +++ b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs @@ -261,14 +261,15 @@ private void Abort() private bool CheckEchoMessage(ECHOMessage msg, int from) { var value = msg.Data.Keccak(); - for (int i = from + _merkleTreeSize, j = 0; i > 1; i /= 2, ++j) + int i, j; + for (i = from + _merkleTreeSize, j = 0; i > 1 && j < msg.MerkleProof.Count; i /= 2, ++j) { value = (i & 1) == 0 ? value.ToBytes().Concat(msg.MerkleProof[j].ToBytes()).Keccak() // we are left sibling : msg.MerkleProof[j].ToBytes().Concat(value.ToBytes()).Keccak(); // we are right sibling } - return msg.MerkleTreeRoot.Equals(value); + return msg.MerkleTreeRoot.Equals(value) && i == 1; } private void AugmentInput(List input) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 001d7155f..8d366d683 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -287,13 +287,20 @@ public void Terminate() { ValidateId(id); if (_registry.TryGetValue(id, out var existingProtocol)) return existingProtocol; - Logger.LogTrace($"Creating protocol {id} on demand"); if (_terminated) { Logger.LogTrace($"Protocol {id} not created since broadcaster is terminated"); return null; } + var protocol = CreateProtocol(id); + if (!(protocol is null)) + Logger.LogTrace($"Created protocol {id} on demand"); + return protocol; + } + + private IConsensusProtocol? CreateProtocol(IProtocolIdentifier id) + { switch (id) { case BinaryBroadcastId bbId: @@ -424,8 +431,8 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) return false; return true; } - else - return true; + else + return binaryBroadcastId.Epoch == 0; } public bool WaitFinish(TimeSpan timeout) From d8235cbb6e88d1ab34ee5ad52cf7e4e21cf82788 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Tue, 14 Jun 2022 21:26:42 +0600 Subject: [PATCH 14/27] fixed issue with node stop --- src/Lachain.Consensus/AbstractProtocol.cs | 8 +- .../BinaryAgreement/BinaryAgreement.cs | 6 +- src/Lachain.Consensus/CommonCoin/CoinToss.cs | 2 +- .../IConsensusBroadcaster.cs | 1 + .../ReliableBroadcast/ReliableBroadcast.cs | 6 +- .../Consensus/ConsensusManager.cs | 14 +-- src/Lachain.Core/Consensus/EraBroadcaster.cs | 106 ++++++++++++------ .../BroadcastSimulator.cs | 5 + 8 files changed, 100 insertions(+), 48 deletions(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index ce9bc869a..1de2d7678 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -96,7 +96,8 @@ public void Terminate() Terminated = true; // We can empty the _queue because the messages will no longer be processed // This will free some memory in case of spam messages - _queue.Clear(); + // _queue.Clear(); + Broadcaster.HandleTermination(Id); Monitor.Pulse(_queueLock); } } @@ -138,6 +139,7 @@ public void Start() { Logger.LogError($"{Id}: exception occured while processing message: {e}"); Terminate(); + // Terminated = true; break; } } @@ -152,9 +154,9 @@ public void ReceiveMessage(MessageEnvelope message) // we should return here instead of enqueueing messages // because once terminated, the messages are not being processed anymore // so the queue will just get large unnecessarily - return; + // return; } - // Logger.LogTrace($"{Id}: got message after termination"); + Logger.LogTrace($"{Id}: got message after termination"); _queue.Enqueue(message); Monitor.Pulse(_queueLock); } diff --git a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs index 093cd8500..ea41eee12 100644 --- a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs +++ b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs @@ -128,13 +128,15 @@ private void TryProgressEpoch() _currentValues = _binaryBroadcastsResults[_currentEpoch - 1]; var coinId = new CoinId(_agreementId.Era, _agreementId.AssociatedValidatorId, _currentEpoch); - if (CoinToss.CreateCoinId(_currentEpoch)) + // if (CoinToss.CreateCoinId(_currentEpoch)) + if ((_currentEpoch / 2) % 3 == 2) { Broadcaster.InternalRequest(new ProtocolRequest(Id, coinId, null)); } else { - _coins[_currentEpoch] = CoinToss.TossCoin(_currentEpoch) != 0; + _coins[_currentEpoch] = ((_currentEpoch / 2) % 3) != 0; + // _coins[_currentEpoch] = CoinToss.TossCoin(_currentEpoch) != 0; } _currentEpoch += 1; diff --git a/src/Lachain.Consensus/CommonCoin/CoinToss.cs b/src/Lachain.Consensus/CommonCoin/CoinToss.cs index bb0601c2d..10b0c7c56 100644 --- a/src/Lachain.Consensus/CommonCoin/CoinToss.cs +++ b/src/Lachain.Consensus/CommonCoin/CoinToss.cs @@ -17,7 +17,7 @@ public static long TossCoin(long epoch) // epoch will be odd and TossCoin value will be 2 public static bool CreateCoinId(long epoch) { - return ((epoch & 1) != 0) && TossCoin(epoch) == 2; + return (epoch > 0) && ((epoch & 1) != 0) && TossCoin(epoch) == 2; } } } \ No newline at end of file diff --git a/src/Lachain.Consensus/IConsensusBroadcaster.cs b/src/Lachain.Consensus/IConsensusBroadcaster.cs index 99d946abb..1b34a5951 100644 --- a/src/Lachain.Consensus/IConsensusBroadcaster.cs +++ b/src/Lachain.Consensus/IConsensusBroadcaster.cs @@ -6,6 +6,7 @@ namespace Lachain.Consensus { public interface IConsensusBroadcaster { + void HandleTermination(IProtocolIdentifier id); void RegisterProtocols(IEnumerable protocols); /* diff --git a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs index 11065331c..66453f0ff 100644 --- a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs +++ b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs @@ -261,15 +261,15 @@ private void Abort() private bool CheckEchoMessage(ECHOMessage msg, int from) { var value = msg.Data.Keccak(); - int i, j; - for (i = from + _merkleTreeSize, j = 0; i > 1 && j < msg.MerkleProof.Count; i /= 2, ++j) + // This can throw exception if msg.MerkleProof has less elements than the depth of the MerkleTree + for (int i = from + _merkleTreeSize, j = 0; i > 1; i /= 2, ++j) { value = (i & 1) == 0 ? value.ToBytes().Concat(msg.MerkleProof[j].ToBytes()).Keccak() // we are left sibling : msg.MerkleProof[j].ToBytes().Concat(value.ToBytes()).Keccak(); // we are right sibling } - return msg.MerkleTreeRoot.Equals(value) && i == 1; + return msg.MerkleTreeRoot.Equals(value); } private void AugmentInput(List input) diff --git a/src/Lachain.Core/Consensus/ConsensusManager.cs b/src/Lachain.Core/Consensus/ConsensusManager.cs index 8ffd8138f..4e9911ef5 100644 --- a/src/Lachain.Core/Consensus/ConsensusManager.cs +++ b/src/Lachain.Core/Consensus/ConsensusManager.cs @@ -290,13 +290,13 @@ private void Run(ulong startingEra) foreach (var (message, from) in savedMessages) { var fromIndex = validators.GetValidatorIndex(from); - if (fromIndex == -1) - { - Logger.LogWarning( - $"Skipped message for era {CurrentEra} since it came from " - + $"{from.ToHex()} who is not validator for this era"); - continue; - } + // if (fromIndex == -1) + // { + // Logger.LogWarning( + // $"Skipped message for era {CurrentEra} since it came from " + // + $"{from.ToHex()} who is not validator for this era"); + // continue; + // } Logger.LogTrace( $"Handling postponed message: {message.PrettyTypeString()}"); broadcaster.Dispatch(message, fromIndex); diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 8d366d683..a036cb404 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -37,6 +37,12 @@ public class EraBroadcaster : IConsensusBroadcaster private bool _terminated; private int _myIdx; private IPublicConsensusKeySet? _validators; + private int _createdBbId = 0; + private int _createdCoinId = 0; + private int _terminatedBbId = 0; + private int _terminatedCoinId = 0; + private const int _maxAllowedBbId = 10; + private const int _maxAllowedCoinId = 10; public bool Ready => _validators != null; @@ -273,6 +279,21 @@ public void Terminate() _callback.Clear(); } + public void HandleTermination(IProtocolIdentifier id) + { + switch (id) + { + case BinaryAgreementId bbId: + _terminatedBbId++; + break; + case CoinId coinId: + _terminatedCoinId++; + break; + default: + break; + } + } + // Each ProtocolId is created only once to prevent spamming, Protocols are mapped against ProtocolId, so each // Protocol will also be created only once, after achieving result, Protocol terminate and no longer process any // messages. So if the required protocol (lets say 'parent protocol') that will use the result of newly created @@ -308,6 +329,7 @@ public void Terminate() return null; var bb = new BinaryBroadcast(bbId, _validators, this); RegisterProtocols(new[] {bb}); + _createdBbId++; return bb; case CoinId coinId: if (!ValidateCoinId(coinId)) @@ -319,6 +341,7 @@ public void Terminate() this ); RegisterProtocols(new[] {coin}); + _createdCoinId++; return coin; case ReliableBroadcastId rbcId: if (!ValidateSenderId((long) rbcId.SenderId)) @@ -378,61 +401,80 @@ private bool ValidateSenderId(long senderId) return true; } - // CommonCoin returns result immediately after getting a valid input and then terminates - // This input could be via network from another validator or its own generated - // If the 'parent protocol' (mentioned above) is not created when this protocol is requested - // then the 'parent protocol' will not get the result and will get stuck' - // So check if the 'parent protocol' exists + // Check if parent protocol is terminated + // Check validity + // Check if not terminated protocols are not too many private bool ValidateCoinId(CoinId coinId) { if (coinId.Agreement == -1 && coinId.Epoch == 0) { // This type of coinId is created from RootProtocol or via network from another validator - return _callback.TryGetValue(coinId, out var _); + return true; } else if (ValidateSenderId(coinId.Agreement)) { // BinaryAgreement requests such CommonCoin - if (!_callback.TryGetValue(coinId, out var binaryAgreementId) || - !_registry.TryGetValue(binaryAgreementId, out var binaryAgreement) || - binaryAgreement.Terminated) + // Checking if the epoch argument is valid + var createCoinId = CoinToss.CreateCoinId(coinId.Epoch); + if (!createCoinId) + { + Logger.LogInformation($"Invalid CoinId: {coinId}"); + return false; + } + // Checking if BA is terminated + if (_callback.TryGetValue(coinId, out var binaryAgreementId) && + _registry.TryGetValue(binaryAgreementId, out var binaryAgreement) && + binaryAgreement.Terminated) + { + Logger.LogInformation($"BinaryAgreement {binaryAgreementId} for CoinId {coinId} is already terminated"); return false; + } + // Allow creation of new CC if not too many are present + if (_createdCoinId - _terminatedCoinId > _maxAllowedCoinId) + { + Logger.LogInformation($"Too many CoinId created, created: {_createdCoinId} and terminated " + + $"{_terminatedCoinId}, max allowed {_maxAllowedCoinId}"); + return false; + } return true; } else return false; } - // BinaryBroadcast needs at least F + 1 responses from validators to reach result - // So malicious validators are not a threat in terms of reaching wrong result or - // Creating BinaryBroadcast too early so that its 'parent protocol', BinaryAgreement - // does not get the result. But we need to stop spam creation of this protocol as it - // can be created too many times with too many epochs - // BinaryAgreement creates BinaryBroadcast sequentially, for each even epoch using the - // result of previous CommonCoin as estimation. Different honest validators can start - // the protocol in different time and broadcast due to network latency, but none of them - // will reach a verdict without at least F + 1 response, so we can assume that if a - // BinaryBroadcast protocol is created and broadcasted by an honest validator, then that - // honest validator reached a valid verdict with at least F + 1 response in a previously - // created BinaryBroadcast protocol, and so the current validator (this node) has also - // reached the same verdict in the previous BinaryBroadcast protocol. - // So we can check if the previous BinaryBroadcast protocol has terminated, if so, then - // we can create another BinaryBroadcast protocol + // Check if parent protocol is terminated + // Check validity + // Check if not terminated protocols are not too many private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) { - if (!ValidateSenderId(binaryBroadcastId.Agreement) || binaryBroadcastId.Epoch < 0) + if (!ValidateSenderId(binaryBroadcastId.Agreement) + || binaryBroadcastId.Epoch < 0 || (binaryBroadcastId.Epoch & 1) != 0) + { + Logger.LogInformation($"Invalid BbId: {binaryBroadcastId}"); return false; - else if (binaryBroadcastId.Epoch > 0 && (binaryBroadcastId.Epoch & 1) == 0) // positive and even + } + else if (binaryBroadcastId.Epoch > 0) // positive and even { - var previousBinaryBroadcastId = - new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, binaryBroadcastId.Epoch - 2); - if (!_registry.TryGetValue(previousBinaryBroadcastId, out var previousBinaryBroadcast) || - !previousBinaryBroadcast.Terminated) + // Checking if BA is terminated + if (_callback.TryGetValue(binaryBroadcastId, out var binaryAgreementId) && + _registry.TryGetValue(binaryAgreementId, out var binaryAgreement) && + binaryAgreement.Terminated) + { + Logger.LogInformation($"BinaryAgreement {binaryAgreementId} for BinaryBroadcastId " + + $"{binaryBroadcastId} is already terminated"); return false; + } + // Allow creation of new BB if not too many are present + if (_createdBbId - _terminatedBbId > _maxAllowedBbId) + { + Logger.LogInformation($"Too many BinaryBroadcastId created, created: {_createdBbId} and terminated " + + $"{_terminatedBbId}, max allowed {_maxAllowedBbId}"); + return false; + } return true; } - else - return binaryBroadcastId.Epoch == 0; + else // 0 epoch + return true; } public bool WaitFinish(TimeSpan timeout) diff --git a/test/Lachain.ConsensusTest/BroadcastSimulator.cs b/test/Lachain.ConsensusTest/BroadcastSimulator.cs index edbffd003..9a65d35c8 100644 --- a/test/Lachain.ConsensusTest/BroadcastSimulator.cs +++ b/test/Lachain.ConsensusTest/BroadcastSimulator.cs @@ -49,6 +49,11 @@ public IConsensusProtocol GetProtocolById(IProtocolIdentifier id) return Registry[id]; } + public void HandleTermination(IProtocolIdentifier id) + { + return; + } + [MethodImpl(MethodImplOptions.Synchronized)] public void Terminate() { From 37da8498c9736fc6b95b0c8b6214cf66485256b6 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Tue, 14 Jun 2022 21:38:10 +0600 Subject: [PATCH 15/27] added log --- src/Lachain.Consensus/AbstractProtocol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index 1de2d7678..63a9fdb05 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -151,12 +151,12 @@ public void ReceiveMessage(MessageEnvelope message) { if (Terminated) { + Logger.LogTrace($"{Id}: got message after termination"); // we should return here instead of enqueueing messages // because once terminated, the messages are not being processed anymore // so the queue will just get large unnecessarily // return; } - Logger.LogTrace($"{Id}: got message after termination"); _queue.Enqueue(message); Monitor.Pulse(_queueLock); } From 57f84a1f62a79c11e1c8c54595aec3b3654402fe Mon Sep 17 00:00:00 2001 From: tbssajal Date: Tue, 14 Jun 2022 22:36:54 +0600 Subject: [PATCH 16/27] added extra log --- src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs index 66453f0ff..e524fd122 100644 --- a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs +++ b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs @@ -132,7 +132,7 @@ private void HandleValMessage(ValMessage val, int validator) } Logger.LogTrace( - $"Protocol {Id} got VAL message from {validator} ({validatorPubKey}), sending ECHO" + $"Protocol {Id} got VAL message {val} from {validator} ({validatorPubKey}), sending ECHO" ); _sentValMessage[validator] = true; From 113434c6b19436369e782f0f46de201edf9d7895 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Tue, 14 Jun 2022 23:01:56 +0600 Subject: [PATCH 17/27] added commenst --- .../ReliableBroadcast/ReliableBroadcast.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs index e524fd122..4c99a76fe 100644 --- a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs +++ b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs @@ -136,6 +136,15 @@ private void HandleValMessage(ValMessage val, int validator) ); _sentValMessage[validator] = true; + // Before sending echo, we can check if validator == val.SenderId, means if the val message is from the correct validator + // Because one validator cannot produce val message of another validator, it can only send echo. + // If we don't check this condition, there could be potential issue, for example, a malicious validator (id = x) sends a + // val message that has random shards but correct MerkleProof and uses val.SenderId = y (another validator), and sends to + // validator with id = z. It will be constructed as echo message and sent to everyone by validator, id = z, this echo will + // pass the CheckEchoMessage(). Now every honest validator will think that val message of validator of id = y is confirmed + // by echo message from validator of id = z. When the correct val message of id = y will come to id = z, he will send again + // but others will not accept it, because id = z already sent echo for id = y, (but it was from malicious id = x), because + // the correct for each pair is received only once. Broadcaster.Broadcast(CreateEchoMessage(val)); } From dc6133ecda94efb30da046f268788faa14d608b7 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 15 Jun 2022 16:04:35 +0600 Subject: [PATCH 18/27] fixed some bugs --- src/Lachain.Consensus/AbstractProtocol.cs | 1 - src/Lachain.Consensus/CommonCoin/CoinToss.cs | 11 +++ .../IConsensusBroadcaster.cs | 1 - src/Lachain.Core/Consensus/EraBroadcaster.cs | 99 ++++++++++--------- .../BroadcastSimulator.cs | 5 - 5 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index 63a9fdb05..ef0ebf036 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -97,7 +97,6 @@ public void Terminate() // We can empty the _queue because the messages will no longer be processed // This will free some memory in case of spam messages // _queue.Clear(); - Broadcaster.HandleTermination(Id); Monitor.Pulse(_queueLock); } } diff --git a/src/Lachain.Consensus/CommonCoin/CoinToss.cs b/src/Lachain.Consensus/CommonCoin/CoinToss.cs index 10b0c7c56..6592cc5d7 100644 --- a/src/Lachain.Consensus/CommonCoin/CoinToss.cs +++ b/src/Lachain.Consensus/CommonCoin/CoinToss.cs @@ -19,5 +19,16 @@ public static bool CreateCoinId(long epoch) { return (epoch > 0) && ((epoch & 1) != 0) && TossCoin(epoch) == 2; } + + public static long NextCoinCreationEpoch(long epoch) + { + return epoch + 6; + } + + public static long TotalValidCommonCoin(long lowEpoch, long highEpoch) + { + if (!CreateCoinId(lowEpoch) || !CreateCoinId(highEpoch)) return -1; + return (highEpoch - lowEpoch) / 6 + 1; + } } } \ No newline at end of file diff --git a/src/Lachain.Consensus/IConsensusBroadcaster.cs b/src/Lachain.Consensus/IConsensusBroadcaster.cs index 1b34a5951..99d946abb 100644 --- a/src/Lachain.Consensus/IConsensusBroadcaster.cs +++ b/src/Lachain.Consensus/IConsensusBroadcaster.cs @@ -6,7 +6,6 @@ namespace Lachain.Consensus { public interface IConsensusBroadcaster { - void HandleTermination(IProtocolIdentifier id); void RegisterProtocols(IEnumerable protocols); /* diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index a036cb404..206c911ff 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -37,10 +37,8 @@ public class EraBroadcaster : IConsensusBroadcaster private bool _terminated; private int _myIdx; private IPublicConsensusKeySet? _validators; - private int _createdBbId = 0; - private int _createdCoinId = 0; - private int _terminatedBbId = 0; - private int _terminatedCoinId = 0; + private long[] _minEpochCC; + private long[] _minEpochBb; private const int _maxAllowedBbId = 10; private const int _maxAllowedCoinId = 10; @@ -76,6 +74,17 @@ public void SetValidatorKeySet(IPublicConsensusKeySet keySet) { _validators = keySet; _myIdx = _validators.GetValidatorIndex(_wallet.EcdsaKeyPair.PublicKey); + InitializeCounter(); + } + + private void InitializeCounter() + { + if (_validators is null) + throw new Exception("We don't have validators"); + _minEpochBb = new long[_validators.N]; + _minEpochCC = new long[_validators.N]; + for (int i = 0; i < _validators.N; i++) + _minEpochCC[i] = 5; } public void RegisterProtocols(IEnumerable protocols) @@ -279,30 +288,9 @@ public void Terminate() _callback.Clear(); } - public void HandleTermination(IProtocolIdentifier id) - { - switch (id) - { - case BinaryAgreementId bbId: - _terminatedBbId++; - break; - case CoinId coinId: - _terminatedCoinId++; - break; - default: - break; - } - } - // Each ProtocolId is created only once to prevent spamming, Protocols are mapped against ProtocolId, so each // Protocol will also be created only once, after achieving result, Protocol terminate and no longer process any - // messages. So if the required protocol (lets say 'parent protocol') that will use the result of newly created - // protocols (lets say 'child protocol') is not created before 'child protocol' returns result, then the - // 'parent protocol' will never be able to use their results and will get stuck - // For ReliableBroadcast, this is not a problem, because it waits for at least F + 1 inputs from validators. - // But some protocol (CommonCoin) may have problem if spammed from malicious validators. - // BinaryAgreement is created only from InternalRequest, not from ExternalMessage, so it is safe as well. - // For each protocol, the corresponding 'parent protocol' is stored in _callback dictionary + // messages. [MethodImpl(MethodImplOptions.Synchronized)] private IConsensusProtocol? EnsureProtocol(IProtocolIdentifier id) { @@ -329,7 +317,6 @@ public void HandleTermination(IProtocolIdentifier id) return null; var bb = new BinaryBroadcast(bbId, _validators, this); RegisterProtocols(new[] {bb}); - _createdBbId++; return bb; case CoinId coinId: if (!ValidateCoinId(coinId)) @@ -341,7 +328,6 @@ public void HandleTermination(IProtocolIdentifier id) this ); RegisterProtocols(new[] {coin}); - _createdCoinId++; return coin; case ReliableBroadcastId rbcId: if (!ValidateSenderId((long) rbcId.SenderId)) @@ -403,7 +389,15 @@ private bool ValidateSenderId(long senderId) // Check if parent protocol is terminated // Check validity - // Check if not terminated protocols are not too many + // Check if non-terminated protocols are not too many + // Yet this could be exploited in this way: an honest node process CC sequentially, CC-1, CC-3, CC-5,..., + // Lets say we have CC-11 and we are allowing at most 10 non-terminated CC. A malicious validator can + // propose for CC-671, CC-677, .. (10 CC like this) together. They will be allowed and maybe never terminated + // because they are too far. So a proposal from honest validator will be rejected. + // We can handle the proposal sequenitally, but due to network latency, messages from other validators may not + // reach us sequenitally. So we propose this idea: allow max 'X' (for now X = 10) non-terminated CC and also + // check that: for the lowest non-terminated CC (CC-L) and the highest non-termianted CC (CC-H) are such that + // the total number of valid CC in between CC-L and CC-H (including) is at most 'X'. private bool ValidateCoinId(CoinId coinId) { if (coinId.Agreement == -1 && coinId.Epoch == 0) @@ -411,7 +405,7 @@ private bool ValidateCoinId(CoinId coinId) // This type of coinId is created from RootProtocol or via network from another validator return true; } - else if (ValidateSenderId(coinId.Agreement)) + if (ValidateSenderId(coinId.Agreement)) { // BinaryAgreement requests such CommonCoin // Checking if the epoch argument is valid @@ -430,21 +424,28 @@ private bool ValidateCoinId(CoinId coinId) return false; } // Allow creation of new CC if not too many are present - if (_createdCoinId - _terminatedCoinId > _maxAllowedCoinId) + var minEpoch = _minEpochCC[coinId.Agreement]; + var minCoinId = new CoinId(coinId.Era, coinId.Agreement, minEpoch); + while (_registry.TryGetValue(minCoinId, out var minCommonCoin) && minCommonCoin.Terminated) { - Logger.LogInformation($"Too many CoinId created, created: {_createdCoinId} and terminated " - + $"{_terminatedCoinId}, max allowed {_maxAllowedCoinId}"); + minEpoch = CoinToss.NextCoinCreationEpoch(minEpoch); + minCoinId = new CoinId(coinId.Era, coinId.Agreement, minEpoch); + } + + _minEpochCC[coinId.Agreement] = minEpoch; + var diff = CoinToss.TotalValidCommonCoin(minEpoch, coinId.Epoch); + if (diff < 0 || diff > _maxAllowedCoinId) + { + Logger.LogInformation($"Too many CoinId created, request CoinId: {coinId}, non terminated CoinId of minimum " + + $"epoch: {minEpoch}, max CoinId allowed {_maxAllowedCoinId}"); return false; } return true; } - else - return false; + return false; } - // Check if parent protocol is terminated - // Check validity - // Check if not terminated protocols are not too many + // same logic as ValidateCoinId private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) { if (!ValidateSenderId(binaryBroadcastId.Agreement) @@ -453,7 +454,7 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) Logger.LogInformation($"Invalid BbId: {binaryBroadcastId}"); return false; } - else if (binaryBroadcastId.Epoch > 0) // positive and even + if (binaryBroadcastId.Epoch > 0) // positive and even { // Checking if BA is terminated if (_callback.TryGetValue(binaryBroadcastId, out var binaryAgreementId) && @@ -465,16 +466,26 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) return false; } // Allow creation of new BB if not too many are present - if (_createdBbId - _terminatedBbId > _maxAllowedBbId) + var minEpoch = _minEpochBb[binaryBroadcastId.Agreement]; + var minBbId = new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, minEpoch); + while (_registry.TryGetValue(minBbId, out var minBb) && minBb.Terminated) + { + minEpoch += 2; + minBbId = new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, minEpoch); + } + + _minEpochBb[binaryBroadcastId.Agreement] = minEpoch; + var diff = (binaryBroadcastId.Epoch - minEpoch) / 2 + 1; + if (diff > _maxAllowedBbId) { - Logger.LogInformation($"Too many BinaryBroadcastId created, created: {_createdBbId} and terminated " - + $"{_terminatedBbId}, max allowed {_maxAllowedBbId}"); + Logger.LogInformation($"Too many BbId created, request BbId: {binaryBroadcastId}, non terminated" + + $" BbId of minimum epoch: {minEpoch}, max BbId allowed {_maxAllowedBbId}"); return false; } return true; } - else // 0 epoch - return true; + // 0 epoch + return true; } public bool WaitFinish(TimeSpan timeout) diff --git a/test/Lachain.ConsensusTest/BroadcastSimulator.cs b/test/Lachain.ConsensusTest/BroadcastSimulator.cs index 9a65d35c8..edbffd003 100644 --- a/test/Lachain.ConsensusTest/BroadcastSimulator.cs +++ b/test/Lachain.ConsensusTest/BroadcastSimulator.cs @@ -49,11 +49,6 @@ public IConsensusProtocol GetProtocolById(IProtocolIdentifier id) return Registry[id]; } - public void HandleTermination(IProtocolIdentifier id) - { - return; - } - [MethodImpl(MethodImplOptions.Synchronized)] public void Terminate() { From b6ed44b578fed40a6e1c6baa4ccb2127ed4905c0 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 15 Jun 2022 16:31:13 +0600 Subject: [PATCH 19/27] added some fixes --- src/Lachain.Consensus/AbstractProtocol.cs | 6 ++---- .../BinaryAgreement/BinaryAgreement.cs | 6 ++---- .../Consensus/ConsensusManager.cs | 19 ++++++++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index ef0ebf036..f2ac214fe 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -96,7 +96,7 @@ public void Terminate() Terminated = true; // We can empty the _queue because the messages will no longer be processed // This will free some memory in case of spam messages - // _queue.Clear(); + _queue.Clear(); Monitor.Pulse(_queueLock); } } @@ -138,7 +138,6 @@ public void Start() { Logger.LogError($"{Id}: exception occured while processing message: {e}"); Terminate(); - // Terminated = true; break; } } @@ -150,11 +149,10 @@ public void ReceiveMessage(MessageEnvelope message) { if (Terminated) { - Logger.LogTrace($"{Id}: got message after termination"); // we should return here instead of enqueueing messages // because once terminated, the messages are not being processed anymore // so the queue will just get large unnecessarily - // return; + return; } _queue.Enqueue(message); Monitor.Pulse(_queueLock); diff --git a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs index ea41eee12..093cd8500 100644 --- a/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs +++ b/src/Lachain.Consensus/BinaryAgreement/BinaryAgreement.cs @@ -128,15 +128,13 @@ private void TryProgressEpoch() _currentValues = _binaryBroadcastsResults[_currentEpoch - 1]; var coinId = new CoinId(_agreementId.Era, _agreementId.AssociatedValidatorId, _currentEpoch); - // if (CoinToss.CreateCoinId(_currentEpoch)) - if ((_currentEpoch / 2) % 3 == 2) + if (CoinToss.CreateCoinId(_currentEpoch)) { Broadcaster.InternalRequest(new ProtocolRequest(Id, coinId, null)); } else { - _coins[_currentEpoch] = ((_currentEpoch / 2) % 3) != 0; - // _coins[_currentEpoch] = CoinToss.TossCoin(_currentEpoch) != 0; + _coins[_currentEpoch] = CoinToss.TossCoin(_currentEpoch) != 0; } _currentEpoch += 1; diff --git a/src/Lachain.Core/Consensus/ConsensusManager.cs b/src/Lachain.Core/Consensus/ConsensusManager.cs index 4e9911ef5..79593f284 100644 --- a/src/Lachain.Core/Consensus/ConsensusManager.cs +++ b/src/Lachain.Core/Consensus/ConsensusManager.cs @@ -132,6 +132,7 @@ public void Dispatch(ConsensusMessage message, ECDSAPublicKey from) { lock (_postponedMessages) { + // This queue can get very long and may cause node shut down due to insufficient memory _postponedMessages .PutIfAbsent(era, new List<(ConsensusMessage message, ECDSAPublicKey from)>()) .Add((message, from)); @@ -290,13 +291,17 @@ private void Run(ulong startingEra) foreach (var (message, from) in savedMessages) { var fromIndex = validators.GetValidatorIndex(from); - // if (fromIndex == -1) - // { - // Logger.LogWarning( - // $"Skipped message for era {CurrentEra} since it came from " - // + $"{from.ToHex()} who is not validator for this era"); - // continue; - // } + // If a validator from some previous era sends message for some future era, the + // message will come here, but it could be that the validator is not a validator + // anymore, in that case the fromIndex will be -1 and may halt some process. + // So we need to check this and discard such messages + if (fromIndex == -1) + { + Logger.LogWarning( + $"Skipped message for era {CurrentEra} since it came from " + + $"{from.ToHex()} who is not validator for this era"); + continue; + } Logger.LogTrace( $"Handling postponed message: {message.PrettyTypeString()}"); broadcaster.Dispatch(message, fromIndex); From de2ed83780af7f85c34b799c6d85cb1d605c9009 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 15 Jun 2022 20:25:42 +0600 Subject: [PATCH 20/27] removed unnecessary lines --- src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs index 4c99a76fe..28d68fd31 100644 --- a/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs +++ b/src/Lachain.Consensus/ReliableBroadcast/ReliableBroadcast.cs @@ -132,7 +132,7 @@ private void HandleValMessage(ValMessage val, int validator) } Logger.LogTrace( - $"Protocol {Id} got VAL message {val} from {validator} ({validatorPubKey}), sending ECHO" + $"Protocol {Id} got VAL message from {validator} ({validatorPubKey}), sending ECHO" ); _sentValMessage[validator] = true; From aadca1ea791b44bd1c67d733c5d59b3629bf80fd Mon Sep 17 00:00:00 2001 From: tbssajal Date: Fri, 17 Jun 2022 14:27:39 +0600 Subject: [PATCH 21/27] fixed some issue --- src/Lachain.Consensus/AbstractProtocol.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/AbstractProtocol.cs b/src/Lachain.Consensus/AbstractProtocol.cs index f2ac214fe..5da1da5c6 100644 --- a/src/Lachain.Consensus/AbstractProtocol.cs +++ b/src/Lachain.Consensus/AbstractProtocol.cs @@ -137,7 +137,7 @@ public void Start() catch (Exception e) { Logger.LogError($"{Id}: exception occured while processing message: {e}"); - Terminate(); + Terminated = true; break; } } @@ -152,6 +152,7 @@ public void ReceiveMessage(MessageEnvelope message) // we should return here instead of enqueueing messages // because once terminated, the messages are not being processed anymore // so the queue will just get large unnecessarily + Monitor.Pulse(_queueLock); return; } _queue.Enqueue(message); From 1ab08d8ef261eb386f205ec5d2bc88336afb79c6 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Fri, 24 Jun 2022 22:11:28 +0600 Subject: [PATCH 22/27] modified handling spamming --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index e1bcdcd53..5dcebe899 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -431,7 +431,8 @@ private bool ValidateCoinId(CoinId coinId) return false; } // Checking if BA is terminated - if (_callback.TryGetValue(coinId, out var binaryAgreementId) && + var binaryAgreementId = CreateBaId(coinId.Agreement); + if (!(binaryAgreementId is null) && _registry.TryGetValue(binaryAgreementId, out var binaryAgreement) && binaryAgreement.Terminated) { @@ -471,8 +472,9 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) } if (binaryBroadcastId.Epoch > 0) // positive and even { + var binaryAgreementId = CreateBaId(binaryBroadcastId.Agreement); // Checking if BA is terminated - if (_callback.TryGetValue(binaryBroadcastId, out var binaryAgreementId) && + if (!(binaryAgreementId is null) && _registry.TryGetValue(binaryAgreementId, out var binaryAgreement) && binaryAgreement.Terminated) { @@ -503,6 +505,11 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) return true; } + private BinaryAgreementId? CreateBaId(long senderId) + { + return ValidateSenderId(senderId) ? new BinaryAgreementId(_era, senderId) : null; + } + public bool WaitFinish(TimeSpan timeout) { return EnsureProtocol(new RootProtocolId(_era))?.WaitFinish(timeout) ?? true; From d4960bcdf704af24214f345cc95891fefce94bea Mon Sep 17 00:00:00 2001 From: tbssajal Date: Fri, 24 Jun 2022 22:11:53 +0600 Subject: [PATCH 23/27] handled header spamming --- .../RootProtocol/RootProtocol.cs | 48 +++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/src/Lachain.Consensus/RootProtocol/RootProtocol.cs b/src/Lachain.Consensus/RootProtocol/RootProtocol.cs index 92348e7ee..7b94e1402 100644 --- a/src/Lachain.Consensus/RootProtocol/RootProtocol.cs +++ b/src/Lachain.Consensus/RootProtocol/RootProtocol.cs @@ -30,6 +30,7 @@ public class RootProtocol : AbstractProtocol private MultiSig? _multiSig; private ulong _cycleDuration; private bool _useNewChainId; + private bool[] _receivedHeader; private readonly List> _signatures = new List>(); @@ -43,6 +44,11 @@ public RootProtocol(RootProtocolId id, IPublicConsensusKeySet wallet, ECDSAPriva _validatorAttendanceRepository = validatorAttendanceRepository; _cycleDuration = cycleDuration; _useNewChainId = useNewChainId; + // _receivedHeader is used to check if a validator has sent valid SignedHeaderMessage. + // We should receive at most one valid SignedHeaderMessage from a validator + _receivedHeader = new bool[N]; + for (int i = 0; i < N; i++) + _receivedHeader[i] = false; } public override void ProcessMessage(MessageEnvelope envelope) @@ -78,13 +84,27 @@ public override void ProcessMessage(MessageEnvelope envelope) Logger.LogWarning($"Header we have {_header}"); Logger.LogWarning($"Header we received {signedHeaderMessage.Header}"); } + + // Random message can raise exception like recover id in signature verification can be out of range or + // public key cannot be serialized + var verified = false; + try + { + verified = _crypto.VerifySignatureHashed( + signedHeaderMessage.Header.Keccak().ToBytes(), + signedHeaderMessage.Signature.Encode(), + Wallet.EcdsaPublicKeySet[idx].EncodeCompressed(), + _useNewChainId + ); + } + catch (Exception exception) + { + var pubKey = Broadcaster.GetPublicKeyById(idx)!.ToHex(); + Logger.LogWarning($"Faulty behaviour: exception occured trying to verify SignedHeaderMessage " + + $"from {idx} ({pubKey}): {exception}"); + } - if (!_crypto.VerifySignatureHashed( - signedHeaderMessage.Header.Keccak().ToBytes(), - signedHeaderMessage.Signature.Encode(), - Wallet.EcdsaPublicKeySet[idx].EncodeCompressed(), - _useNewChainId - )) + if (!verified) { _lastMessage = $"Incorrect signature of header {signedHeaderMessage.Header.Keccak().ToHex()} from validator {idx}"; @@ -92,8 +112,10 @@ public override void ProcessMessage(MessageEnvelope envelope) $"Incorrect signature of header {signedHeaderMessage.Header.Keccak().ToHex()} from validator {idx}" ); } - else + else if (!_receivedHeader[idx]) { + // received verified header from validator + _receivedHeader[idx] = true; Logger.LogTrace("Add signatures"); _lastMessage = "Add signatures"; _signatures.Add(new Tuple( @@ -110,6 +132,13 @@ public override void ProcessMessage(MessageEnvelope envelope) message.SignedHeaderMessage.Header.Index / _cycleDuration); _validatorAttendanceRepository.SaveState(validatorAttendance.ToBytes()); } + else + { + // discard this header because already received a valid header from this validator + var pubKey = Broadcaster.GetPublicKeyById(idx)!.ToHex(); + Logger.LogWarning($"Already received verified header from {idx} ({pubKey}). Validator {idx} " + + $"({pubKey}) tried to send SignedHeaderMessage more than once"); + } CheckSignatures(); } @@ -246,7 +275,10 @@ private void CheckSignatures() .Aggregate((x, y) => x.Value > y.Value ? x : y); Logger.LogTrace($"bestHeader.Value {bestHeader.Value} Wallet.N {Wallet.N} Wallet.F {Wallet.F}"); if (bestHeader.Value < Wallet.N - Wallet.F) return; - Logger.LogTrace($"Received {bestHeader.Value} signatures for block header"); + Logger.LogTrace( + $"Received {bestHeader.Value} signatures for block header: height: {bestHeader.Key.Index}, hash: " + + $"{bestHeader.Key.Keccak().ToHex()}. My block header height: {_header.Index}, hash: {_header.Keccak().ToHex()}." + + $" My header hash matches with bestheader hash: {_header.Keccak().Equals(bestHeader.Key.Keccak())}"); _multiSig = new MultiSig {Quorum = (uint) (Wallet.N - Wallet.F)}; _multiSig.Validators.AddRange(Wallet.EcdsaPublicKeySet); foreach (var (header, signature) in _signatures) From 1085036427ab46e2346e0d9c720ae20295d76e3a Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 3 Aug 2022 21:26:34 +0600 Subject: [PATCH 24/27] storing external messages when protocol is not created yet, removed the functionality to restrict creation of protocols --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 193 +++++++++++-------- 1 file changed, 110 insertions(+), 83 deletions(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 5dcebe899..489a6b27d 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -38,10 +38,6 @@ public class EraBroadcaster : IConsensusBroadcaster private bool _terminated; private int _myIdx; private IPublicConsensusKeySet? _validators; - private long[] _minEpochCC; - private long[] _minEpochBb; - private const int _maxAllowedBbId = 10; - private const int _maxAllowedCoinId = 10; public bool Ready => _validators != null; @@ -57,6 +53,9 @@ public class EraBroadcaster : IConsensusBroadcaster private readonly IDictionary _registry = new ConcurrentDictionary(); + private readonly IDictionary> _postponedMessages = + new ConcurrentDictionary>(); + public EraBroadcaster( long era, IConsensusMessageDeliverer consensusMessageDeliverer, IPrivateWallet wallet, IValidatorAttendanceRepository validatorAttendanceRepository @@ -75,17 +74,6 @@ public void SetValidatorKeySet(IPublicConsensusKeySet keySet) { _validators = keySet; _myIdx = _validators.GetValidatorIndex(_wallet.EcdsaKeyPair.PublicKey); - InitializeCounter(); - } - - private void InitializeCounter() - { - if (_validators is null) - throw new Exception("We don't have validators"); - _minEpochBb = new long[_validators.N]; - _minEpochCC = new long[_validators.N]; - for (int i = 0; i < _validators.N; i++) - _minEpochCC[i] = 5; } public void RegisterProtocols(IEnumerable protocols) @@ -106,7 +94,7 @@ public void Broadcast(ConsensusMessage message) } var payload = _messageFactory.ConsensusMessage(message); - foreach (var publicKey in _validators.EcdsaPublicKeySet) + foreach (var publicKey in _validators!.EcdsaPublicKeySet) { if (publicKey.Equals(_wallet.EcdsaKeyPair.PublicKey)) { @@ -140,7 +128,7 @@ public void SendToValidator(ConsensusMessage message, int index) } var payload = _messageFactory.ConsensusMessage(message); - _consensusMessageDeliverer.SendTo(_validators.EcdsaPublicKeySet[index], payload); + _consensusMessageDeliverer.SendTo(_validators!.EcdsaPublicKeySet[index], payload); } public void Dispatch(ConsensusMessage message, int from) @@ -165,49 +153,73 @@ public void Dispatch(ConsensusMessage message, int from) return; } + IProtocolIdentifier protocolId; switch (message.PayloadCase) { case ConsensusMessage.PayloadOneofCase.Bval: - var idBval = new BinaryBroadcastId(message.Validator.Era, message.Bval.Agreement, + protocolId = new BinaryBroadcastId(message.Validator.Era, message.Bval.Agreement, message.Bval.Epoch); - EnsureProtocol(idBval)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.Aux: - var idAux = new BinaryBroadcastId(message.Validator.Era, message.Aux.Agreement, message.Aux.Epoch); - EnsureProtocol(idAux)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new BinaryBroadcastId(message.Validator.Era, message.Aux.Agreement, message.Aux.Epoch); break; case ConsensusMessage.PayloadOneofCase.Conf: - var idConf = new BinaryBroadcastId(message.Validator.Era, message.Conf.Agreement, + protocolId = new BinaryBroadcastId(message.Validator.Era, message.Conf.Agreement, message.Conf.Epoch); - EnsureProtocol(idConf)?.ReceiveMessage(new MessageEnvelope(message, from)); break; case ConsensusMessage.PayloadOneofCase.Coin: - var idCoin = new CoinId(message.Validator.Era, message.Coin.Agreement, message.Coin.Epoch); - EnsureProtocol(idCoin)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new CoinId(message.Validator.Era, message.Coin.Agreement, message.Coin.Epoch); break; case ConsensusMessage.PayloadOneofCase.Decrypted: - var hbbftId = new HoneyBadgerId((int) message.Validator.Era); - EnsureProtocol(hbbftId)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new HoneyBadgerId((int) message.Validator.Era); break; case ConsensusMessage.PayloadOneofCase.ValMessage: - var reliableBroadcastId = new ReliableBroadcastId(message.ValMessage.SenderId, (int) message.Validator.Era); - EnsureProtocol(reliableBroadcastId)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new ReliableBroadcastId(message.ValMessage.SenderId, (int) message.Validator.Era); break; case ConsensusMessage.PayloadOneofCase.EchoMessage: - var rbIdEchoMsg = new ReliableBroadcastId(message.EchoMessage.SenderId, (int) message.Validator.Era); - EnsureProtocol(rbIdEchoMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new ReliableBroadcastId(message.EchoMessage.SenderId, (int) message.Validator.Era); break; case ConsensusMessage.PayloadOneofCase.ReadyMessage: - var rbIdReadyMsg = new ReliableBroadcastId(message.ReadyMessage.SenderId, (int) message.Validator.Era); - EnsureProtocol(rbIdReadyMsg)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new ReliableBroadcastId(message.ReadyMessage.SenderId, (int) message.Validator.Era); break; case ConsensusMessage.PayloadOneofCase.SignedHeaderMessage: - var rootId = new RootProtocolId(message.Validator.Era); - EnsureProtocol(rootId)?.ReceiveMessage(new MessageEnvelope(message, from)); + protocolId = new RootProtocolId(message.Validator.Era); break; default: throw new InvalidOperationException($"Unknown message type {message}"); } + + HandleExternalMessage(protocolId, new MessageEnvelope(message, from)); + } + + private void HandleExternalMessage(IProtocolIdentifier protocolId, MessageEnvelope message) + { + // For external message we don't create new protocols. each protocol is requested for some result + // by another protocol (maybe itself) via internal message. When protocols are created by internal message + // we deliver the external messages to that protocol, otherwise store them to deliver later + if (ValidateProtocolId(protocolId)) + { + if (message.External) + { + var protocol = GetProtocolById(protocolId); + if (protocol is null) + { + lock (_postponedMessages) + { + _postponedMessages + .PutIfAbsent(protocolId, new List()) + .Add(message); + } + } + else protocol.ReceiveMessage(message); + } + else Logger.LogWarning("Internal message should not be here"); + } + else + { + var from = message.ValidatorIndex; + Logger.LogWarning($"Invalid protocol id {protocolId} from validator {GetPublicKeyById(from)!.ToHex()} ({from})"); + } } [MethodImpl(MethodImplOptions.Synchronized)] @@ -238,6 +250,21 @@ public void InternalRequest(ProtocolRequest re if (_registry.TryGetValue(request.To, out var protocol)) protocol?.ReceiveMessage(new MessageEnvelope(request, GetMyId())); + + if (!(protocol is null)) + { + lock (_postponedMessages) + { + if (_postponedMessages.TryGetValue(request.To, out var savedMessages)) + { + foreach (var message in savedMessages) + { + protocol.ReceiveMessage(message); + } + _postponedMessages.Remove(request.To); + } + } + } } public void InternalResponse(ProtocolResult result) @@ -276,7 +303,7 @@ public int GetMyId() public int GetIdByPublicKey(ECDSAPublicKey publicKey) { - return _validators.GetValidatorIndex(publicKey); + return _validators!.GetValidatorIndex(publicKey); } public ECDSAPublicKey? GetPublicKeyById(int id) @@ -301,6 +328,10 @@ public void Terminate() _registry.Clear(); _callback.Clear(); + lock (_postponedMessages) + { + _postponedMessages.Clear(); + } } // Each ProtocolId is created only once to prevent spamming, Protocols are mapped against ProtocolId, so each @@ -325,19 +356,16 @@ public void Terminate() private IConsensusProtocol? CreateProtocol(IProtocolIdentifier id) { + if (!ValidateProtocolId(id)) return null; switch (id) { case BinaryBroadcastId bbId: - if (!ValidateBinaryBroadcastId(bbId)) - return null; - var bb = new BinaryBroadcast(bbId, _validators, this); + var bb = new BinaryBroadcast(bbId, _validators!, this); RegisterProtocols(new[] {bb}); return bb; case CoinId coinId: - if (!ValidateCoinId(coinId)) - return null; var coin = new CommonCoin( - coinId, _validators, + coinId, _validators!, _wallet.GetThresholdSignatureKeyForBlock((ulong) _era - 1) ?? throw new InvalidOperationException($"No TS keys present for era {_era}"), this @@ -345,22 +373,20 @@ public void Terminate() RegisterProtocols(new[] {coin}); return coin; case ReliableBroadcastId rbcId: - if (!ValidateSenderId((long) rbcId.SenderId)) - return null; - var rbc = new ReliableBroadcast(rbcId, _validators, this); + var rbc = new ReliableBroadcast(rbcId, _validators!, this); RegisterProtocols(new[] {rbc}); return rbc; case BinaryAgreementId baId: - var ba = new BinaryAgreement(baId, _validators, this); + var ba = new BinaryAgreement(baId, _validators!, this); RegisterProtocols(new[] {ba}); return ba; case CommonSubsetId acsId: - var acs = new CommonSubset(acsId, _validators, this); + var acs = new CommonSubset(acsId, _validators!, this); RegisterProtocols(new[] {acs}); return acs; case HoneyBadgerId hbId: var hb = new HoneyBadger( - hbId, _validators, + hbId, _validators!, _wallet.GetTpkePrivateKeyForBlock((ulong) _era - 1) ?? throw new InvalidOperationException($"No TPKE keys present for era {_era}"), this @@ -368,7 +394,7 @@ public void Terminate() RegisterProtocols(new[] {hb}); return hb; case RootProtocolId rootId: - var root = new RootProtocol(rootId, _validators, _wallet.EcdsaKeyPair.PrivateKey, + var root = new RootProtocol(rootId, _validators!, _wallet.EcdsaKeyPair.PrivateKey, this, _validatorAttendanceRepository, StakingContract.CycleDuration, HardforkHeights.IsHardfork_9Active((ulong)_era)); RegisterProtocols(new[] {root}); @@ -383,6 +409,38 @@ private void ValidateId(IProtocolIdentifier id) if (id.Era != _era) throw new InvalidOperationException($"Era mismatched, expected {_era} got message with {id.Era}"); } + + private bool ValidateProtocolId(IProtocolIdentifier id) + { + switch (id) + { + case BinaryBroadcastId bbId: + return ValidateBinaryBroadcastId(bbId); + case CoinId coinId: + return ValidateCoinId(coinId); + case ReliableBroadcastId rbcId: + // need to validate the sender id only + return ValidateSenderId((long) rbcId.SenderId); + case BinaryAgreementId baId: + // created only by internal request, external messages never reach BinaryAgreementId + // so no need to validate + return true; + case CommonSubsetId acsId: + // created only by internal request, external messages never reach BinaryAgreementId + // so no need to validate + return true; + case HoneyBadgerId hbId: + // only has era in the fields + ValidateId(hbId); + return true; + case RootProtocolId rootId: + // only has era in the fields + ValidateId(rootId); + return true; + default: + return false; + } + } // There are separate instance of ReliableBroadcast for each validator. // Check if the SenderId is one of the validator's id before creating ReliableBroadcastId @@ -430,6 +488,7 @@ private bool ValidateCoinId(CoinId coinId) Logger.LogInformation($"Invalid CoinId: {coinId}"); return false; } + // Checking if BA is terminated var binaryAgreementId = CreateBaId(coinId.Agreement); if (!(binaryAgreementId is null) && @@ -439,23 +498,7 @@ private bool ValidateCoinId(CoinId coinId) Logger.LogInformation($"BinaryAgreement {binaryAgreementId} for CoinId {coinId} is already terminated"); return false; } - // Allow creation of new CC if not too many are present - var minEpoch = _minEpochCC[coinId.Agreement]; - var minCoinId = new CoinId(coinId.Era, coinId.Agreement, minEpoch); - while (_registry.TryGetValue(minCoinId, out var minCommonCoin) && minCommonCoin.Terminated) - { - minEpoch = CoinToss.NextCoinCreationEpoch(minEpoch); - minCoinId = new CoinId(coinId.Era, coinId.Agreement, minEpoch); - } - - _minEpochCC[coinId.Agreement] = minEpoch; - var diff = CoinToss.TotalValidCommonCoin(minEpoch, coinId.Epoch); - if (diff < 0 || diff > _maxAllowedCoinId) - { - Logger.LogInformation($"Too many CoinId created, request CoinId: {coinId}, non terminated CoinId of minimum " - + $"epoch: {minEpoch}, max CoinId allowed {_maxAllowedCoinId}"); - return false; - } + return true; } return false; @@ -482,23 +525,7 @@ private bool ValidateBinaryBroadcastId(BinaryBroadcastId binaryBroadcastId) $"{binaryBroadcastId} is already terminated"); return false; } - // Allow creation of new BB if not too many are present - var minEpoch = _minEpochBb[binaryBroadcastId.Agreement]; - var minBbId = new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, minEpoch); - while (_registry.TryGetValue(minBbId, out var minBb) && minBb.Terminated) - { - minEpoch += 2; - minBbId = new BinaryBroadcastId(binaryBroadcastId.Era, binaryBroadcastId.Agreement, minEpoch); - } - - _minEpochBb[binaryBroadcastId.Agreement] = minEpoch; - var diff = (binaryBroadcastId.Epoch - minEpoch) / 2 + 1; - if (diff > _maxAllowedBbId) - { - Logger.LogInformation($"Too many BbId created, request BbId: {binaryBroadcastId}, non terminated" - + $" BbId of minimum epoch: {minEpoch}, max BbId allowed {_maxAllowedBbId}"); - return false; - } + return true; } // 0 epoch From 8e74fc5fb828705ba782ecf12ce53598bb977b83 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 3 Aug 2022 21:27:04 +0600 Subject: [PATCH 25/27] added limit on era for postponed messages --- src/Lachain.Core/Consensus/ConsensusManager.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Lachain.Core/Consensus/ConsensusManager.cs b/src/Lachain.Core/Consensus/ConsensusManager.cs index 79593f284..ade61874b 100644 --- a/src/Lachain.Core/Consensus/ConsensusManager.cs +++ b/src/Lachain.Core/Consensus/ConsensusManager.cs @@ -128,7 +128,7 @@ public void Dispatch(ConsensusMessage message, ECDSAPublicKey from) broadcaster.Dispatch(message, fromIndex); } - else + else if (IsEraNearFuture(era, CurrentEra)) { lock (_postponedMessages) { @@ -141,6 +141,14 @@ public void Dispatch(ConsensusMessage message, ECDSAPublicKey from) } } + private bool IsEraNearFuture(long era, long currentEra) + { + long allowedEra = 5; + if (era < currentEra) return false; + if (era - currentEra <= allowedEra) return true; + return false; + } + public void Start(ulong startingEra) { _networkManager.AdvanceEra(startingEra); @@ -151,6 +159,14 @@ private void FinishEra() { lock (_erasLock) { + // Sometimes _postponedMessages messages are not cleared if the current node is not a validator + // but messages maybe inserted in case the nodes does not have validators set yet and someone sent + // consensus message to this node. So it needs to be cleared for every era + lock (_postponedMessages) + { + _postponedMessages.Remove(CurrentEra); + } + var broadcaster = _eras[CurrentEra]; lock (broadcaster) { From 5bd58bdeaf51668a568ebd574a00209620a10e0d Mon Sep 17 00:00:00 2001 From: tbssajal Date: Wed, 3 Aug 2022 22:35:55 +0600 Subject: [PATCH 26/27] removed unnecessary comments --- src/Lachain.Core/Consensus/EraBroadcaster.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 489a6b27d..576cbafd9 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -265,6 +265,7 @@ public void InternalRequest(ProtocolRequest re } } } + else Logger.LogWarning($"protocol {request.To} not created? something went wrong"); } public void InternalResponse(ProtocolResult result) @@ -462,15 +463,6 @@ private bool ValidateSenderId(long senderId) // Check if parent protocol is terminated // Check validity - // Check if non-terminated protocols are not too many - // Yet this could be exploited in this way: an honest node process CC sequentially, CC-1, CC-3, CC-5,..., - // Lets say we have CC-11 and we are allowing at most 10 non-terminated CC. A malicious validator can - // propose for CC-671, CC-677, .. (10 CC like this) together. They will be allowed and maybe never terminated - // because they are too far. So a proposal from honest validator will be rejected. - // We can handle the proposal sequenitally, but due to network latency, messages from other validators may not - // reach us sequenitally. So we propose this idea: allow max 'X' (for now X = 10) non-terminated CC and also - // check that: for the lowest non-terminated CC (CC-L) and the highest non-termianted CC (CC-H) are such that - // the total number of valid CC in between CC-L and CC-H (including) is at most 'X'. private bool ValidateCoinId(CoinId coinId) { if (coinId.Agreement == -1 && coinId.Epoch == 0) From 53085f239b5191be13c396c0400b8b8927a0c3c2 Mon Sep 17 00:00:00 2001 From: tbssajal Date: Thu, 4 Aug 2022 18:45:17 +0600 Subject: [PATCH 27/27] removed unnecessary logs and added comments --- src/Lachain.Consensus/HoneyBadger/HoneyBadger.cs | 4 ++++ src/Lachain.Core/Consensus/EraBroadcaster.cs | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Lachain.Consensus/HoneyBadger/HoneyBadger.cs b/src/Lachain.Consensus/HoneyBadger/HoneyBadger.cs index 201e40c7f..1f8de5e26 100644 --- a/src/Lachain.Consensus/HoneyBadger/HoneyBadger.cs +++ b/src/Lachain.Consensus/HoneyBadger/HoneyBadger.cs @@ -128,6 +128,8 @@ private void CheckResult() new ProtocolResult>(_honeyBadgerId, _result)); } + // TODO: (investigate) the share is comming from ReliableBroadcast and the shareId of the share should match the senderId + // of that ReliableBroadcast, otherwise we might replace one share with another in _receivedShares array private void HandleCommonSubset(ProtocolResult> result) { Logger.LogTrace($"Common subset finished {result.From}"); @@ -160,6 +162,8 @@ private ConsensusMessage CreateDecryptedMessage(PartiallyDecryptedShare share) return message; } + // DecryptorId of the message should match the senderId, otherwise the message should be discarded + // because HoneyBadger does not accept two messages for same DecryptorId and shareId // We need to handle this message carefully like how about decoding a random message with random length // and the value of 'share.ShareId' needs to be checked. If it is out of range, it can throw exception private void HandleDecryptedMessage(TPKEPartiallyDecryptedShareMessage msg, int senderId) diff --git a/src/Lachain.Core/Consensus/EraBroadcaster.cs b/src/Lachain.Core/Consensus/EraBroadcaster.cs index 576cbafd9..0fcdbb84e 100644 --- a/src/Lachain.Core/Consensus/EraBroadcaster.cs +++ b/src/Lachain.Core/Consensus/EraBroadcaster.cs @@ -265,7 +265,6 @@ public void InternalRequest(ProtocolRequest re } } } - else Logger.LogWarning($"protocol {request.To} not created? something went wrong"); } public void InternalResponse(ProtocolResult result)