-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stage 3 of dBFT (Commit) #320
Changes from 4 commits
2de2101
17b5ad0
66709f6
e6f52a5
6eb2ae0
1788d01
97322b0
fb1f9a1
2415611
e08e0b0
700ff6f
3d56256
a1272fa
ef134fe
77a5d37
4e12b24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
using System.IO; | ||
using Neo.IO; | ||
|
||
namespace Neo.Consensus | ||
{ | ||
internal class CommitAgreement : ConsensusMessage | ||
{ | ||
/// <summary> | ||
/// Block hash of the signature | ||
/// </summary> | ||
public UInt256 BlockHash; | ||
|
||
/// <summary> | ||
/// Signature | ||
/// </summary> | ||
public byte[] Signature; | ||
|
||
/// <summary> | ||
/// Constructors | ||
/// </summary> | ||
public CommitAgreement() : base(ConsensusMessageType.CommitAgreement) { } | ||
|
||
public override void Deserialize(BinaryReader reader) | ||
{ | ||
base.Deserialize(reader); | ||
|
||
BlockHash = reader.ReadSerializable<UInt256>(); | ||
Signature = reader.ReadBytes(64); | ||
} | ||
|
||
public override void Serialize(BinaryWriter writer) | ||
{ | ||
base.Serialize(writer); | ||
|
||
writer.Write(BlockHash); | ||
writer.Write(Signature); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
using Neo.Core; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using Neo.Core; | ||
using Neo.Cryptography; | ||
using Neo.Cryptography.ECC; | ||
using Neo.IO; | ||
using Neo.Network.Payloads; | ||
using Neo.Wallets; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
|
||
namespace Neo.Consensus | ||
{ | ||
|
@@ -28,21 +28,63 @@ internal class ConsensusContext | |
public byte[] ExpectedView; | ||
public KeyPair KeyPair; | ||
|
||
private UInt256[] Commits; | ||
private Block _header = null; | ||
public bool CommitAgreementSent = false; | ||
|
||
public int M => Validators.Length - (Validators.Length - 1) / 3; | ||
|
||
public bool TryToCommit(ConsensusPayload payload, CommitAgreement message) | ||
{ | ||
// Already received | ||
|
||
if (Commits[payload.ValidatorIndex] != null) | ||
{ | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's be consistent with your single line ifs. I see some with curly bracket and some without. |
||
} | ||
|
||
// Check signature of the validator | ||
|
||
if (!Crypto.Default.VerifySignature | ||
( | ||
message.BlockHash.ToArray(), message.Signature, | ||
Validators[payload.ValidatorIndex].EncodePoint(false) | ||
)) | ||
{ | ||
return false; | ||
} | ||
|
||
// Store received block hash | ||
|
||
Commits[payload.ValidatorIndex] = message.BlockHash; | ||
|
||
// Check count | ||
|
||
if (_header == null) return false; | ||
|
||
return Commits.Where(u => u != null && u == _header.Hash).Count() >= M; | ||
} | ||
|
||
public void ChangeView(byte view_number) | ||
{ | ||
int p = ((int)BlockIndex - view_number) % Validators.Length; | ||
State &= ConsensusState.SignatureSent; | ||
ViewNumber = view_number; | ||
PrimaryIndex = p >= 0 ? (uint)p : (uint)(p + Validators.Length); | ||
|
||
if (State == ConsensusState.Initial) | ||
{ | ||
TransactionHashes = null; | ||
Signatures = new byte[Validators.Length][]; | ||
Commits = new UInt256[Validators.Length]; | ||
CommitAgreementSent = false; | ||
} | ||
|
||
if (MyIndex >= 0) | ||
{ | ||
ExpectedView[MyIndex] = view_number; | ||
} | ||
|
||
_header = null; | ||
} | ||
|
||
|
@@ -54,10 +96,10 @@ public ConsensusPayload MakeChangeView() | |
}); | ||
} | ||
|
||
private Block _header = null; | ||
public Block MakeHeader() | ||
{ | ||
if (TransactionHashes == null) return null; | ||
|
||
if (_header == null) | ||
{ | ||
_header = new Block | ||
|
@@ -71,10 +113,24 @@ public Block MakeHeader() | |
NextConsensus = NextConsensus, | ||
Transactions = new Transaction[0] | ||
}; | ||
|
||
Commits[MyIndex] = _header.Hash; | ||
} | ||
|
||
return _header; | ||
} | ||
|
||
public ConsensusPayload MakeCommitAgreement() | ||
{ | ||
if (_header == null) return null; | ||
|
||
return MakePayload(new CommitAgreement() | ||
{ | ||
BlockHash = _header.Hash, | ||
Signature = _header.Hash.ToArray().Sign(KeyPair) | ||
}); | ||
} | ||
|
||
private ConsensusPayload MakePayload(ConsensusMessage message) | ||
{ | ||
message.ViewNumber = ViewNumber; | ||
|
@@ -122,6 +178,9 @@ public void Reset(Wallet wallet) | |
Signatures = new byte[Validators.Length][]; | ||
ExpectedView = new byte[Validators.Length]; | ||
KeyPair = null; | ||
Commits = new UInt256[Validators.Length]; | ||
CommitAgreementSent = false; | ||
|
||
for (int i = 0; i < Validators.Length; i++) | ||
{ | ||
WalletAccount account = wallet.GetAccount(Validators[i]); | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -85,8 +85,11 @@ private bool CheckPolicy(Transaction tx) | |||
return true; | ||||
} | ||||
|
||||
private void CheckSignatures() | ||||
private void OnCommitAgreement(ConsensusPayload payload, CommitAgreement message) | ||||
{ | ||||
if (context.State.HasFlag(ConsensusState.BlockSent)) return; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the "jump of the cat", Shargon? aheuahueahuea |
||||
if (!context.TryToCommit(payload, message)) return; | ||||
|
||||
if (context.Signatures.Count(p => p != null) >= context.M && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p))) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this double check necessary? Because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean line 95: neo/neo/Consensus/ConsensusService.cs Line 95 in 77a5d37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this method is called when receive a commit, no when you sent your commit, so if you receive 6 commits, and you don't send your commit, you never relay the block. |
||||
{ | ||||
Contract contract = Contract.CreateMultiSigContract(context.M, context.Validators); | ||||
|
@@ -101,12 +104,28 @@ private void CheckSignatures() | |||
sc.Verifiable.Scripts = sc.GetScripts(); | ||||
block.Transactions = context.TransactionHashes.Select(p => context.Transactions[p]).ToArray(); | ||||
Log($"relay block: {block.Hash}"); | ||||
|
||||
if (!localNode.Relay(block)) | ||||
Log($"reject block: {block.Hash}"); | ||||
|
||||
context.State |= ConsensusState.BlockSent; | ||||
} | ||||
} | ||||
|
||||
private void CheckSignatures() | ||||
{ | ||||
if (!context.CommitAgreementSent) | ||||
{ | ||||
if (context.Signatures.Count(p => p != null) >= context.M && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p))) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. merge if loops |
||||
{ | ||||
// Sent i'm ready! | ||||
|
||||
context.CommitAgreementSent = true; | ||||
SignAndRelay(context.MakeCommitAgreement()); | ||||
} | ||||
} | ||||
} | ||||
|
||||
public void Dispose() | ||||
{ | ||||
Log("OnStop"); | ||||
|
@@ -206,61 +225,62 @@ private void InitializeConsensus(byte view_number) | |||
|
||||
private void LocalNode_InventoryReceived(object sender, IInventory inventory) | ||||
{ | ||||
ConsensusPayload payload = inventory as ConsensusPayload; | ||||
if (payload != null) | ||||
if (inventory == null || !(inventory is ConsensusPayload payload)) return; | ||||
|
||||
lock (context) | ||||
{ | ||||
lock (context) | ||||
if (payload.ValidatorIndex == context.MyIndex || | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not see the Is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #233 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I think that we can remove |
||||
payload.ValidatorIndex >= context.Validators.Length || | ||||
payload.Version != ConsensusContext.Version) return; | ||||
|
||||
if (payload.PrevHash != context.PrevHash || payload.BlockIndex != context.BlockIndex) | ||||
{ | ||||
if (payload.ValidatorIndex == context.MyIndex) return; | ||||
// Request blocks | ||||
|
||||
if (payload.Version != ConsensusContext.Version) | ||||
return; | ||||
if (payload.PrevHash != context.PrevHash || payload.BlockIndex != context.BlockIndex) | ||||
if (Blockchain.Default?.Height + 1 < payload.BlockIndex) | ||||
{ | ||||
// Request blocks | ||||
Log($"chain sync: expected={payload.BlockIndex} current: {Blockchain.Default?.Height} nodes={localNode.RemoteNodeCount}"); | ||||
|
||||
if (Blockchain.Default?.Height + 1 < payload.BlockIndex) | ||||
{ | ||||
Log($"chain sync: expected={payload.BlockIndex} current: {Blockchain.Default?.Height} nodes={localNode.RemoteNodeCount}"); | ||||
localNode.RequestGetBlocks(); | ||||
} | ||||
|
||||
localNode.RequestGetBlocks(); | ||||
} | ||||
return; | ||||
} | ||||
|
||||
return; | ||||
} | ||||
ConsensusMessage message; | ||||
try | ||||
{ | ||||
message = ConsensusMessage.DeserializeFrom(payload.Data); | ||||
} | ||||
catch | ||||
{ | ||||
return; | ||||
} | ||||
|
||||
if (payload.ValidatorIndex >= context.Validators.Length) return; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check if payload validator Index could had changed on RequestGetBlocks() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I move this validation to the begining of the method |
||||
ConsensusMessage message; | ||||
try | ||||
{ | ||||
message = ConsensusMessage.DeserializeFrom(payload.Data); | ||||
} | ||||
catch | ||||
{ | ||||
return; | ||||
} | ||||
if (message.ViewNumber != context.ViewNumber && message.Type != ConsensusMessageType.ChangeView) | ||||
return; | ||||
switch (message.Type) | ||||
{ | ||||
case ConsensusMessageType.ChangeView: | ||||
OnChangeViewReceived(payload, (ChangeView)message); | ||||
break; | ||||
case ConsensusMessageType.PrepareRequest: | ||||
OnPrepareRequestReceived(payload, (PrepareRequest)message); | ||||
break; | ||||
case ConsensusMessageType.PrepareResponse: | ||||
OnPrepareResponseReceived(payload, (PrepareResponse)message); | ||||
break; | ||||
} | ||||
if (message.ViewNumber != context.ViewNumber && message.Type != ConsensusMessageType.ChangeView) | ||||
return; | ||||
|
||||
switch (message.Type) | ||||
{ | ||||
case ConsensusMessageType.ChangeView: | ||||
OnChangeViewReceived(payload, (ChangeView)message); | ||||
break; | ||||
case ConsensusMessageType.PrepareRequest: | ||||
OnPrepareRequestReceived(payload, (PrepareRequest)message); | ||||
break; | ||||
case ConsensusMessageType.PrepareResponse: | ||||
OnPrepareResponseReceived(payload, (PrepareResponse)message); | ||||
break; | ||||
case ConsensusMessageType.CommitAgreement: | ||||
OnCommitAgreement(payload, (CommitAgreement)message); | ||||
break; | ||||
} | ||||
} | ||||
} | ||||
|
||||
private void LocalNode_InventoryReceiving(object sender, InventoryReceivingEventArgs e) | ||||
{ | ||||
Transaction tx = e.Inventory as Transaction; | ||||
if (tx != null) | ||||
if (e.Inventory is Transaction tx) | ||||
{ | ||||
lock (context) | ||||
{ | ||||
|
@@ -373,6 +393,8 @@ private void RequestChangeView() | |||
|
||||
private void SignAndRelay(ConsensusPayload payload) | ||||
{ | ||||
if (payload == null) return; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this just for the commit case or it is a general case that has been happening? |
||||
|
||||
ContractParametersContext sc; | ||||
try | ||||
{ | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bool looks like it should belong in ConsensusState
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true, Snowy. It looks like.