Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix consensus problem for proposed blocks with zero transactions #1053

Merged
merged 5 commits into from
Aug 25, 2019
Merged

Fix consensus problem for proposed blocks with zero transactions #1053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e153188
Fix consensus (CRLF)
shargon Aug 24, 2019
5171efc
Removing unnecessary lines changes - Cleaning PR
vncoelho Aug 24, 2019
28167aa
Reverting all changes on CC
vncoelho Aug 24, 2019
27177d7
Merge branch 'master' into fix-consensus
vncoelho Aug 24, 2019
8c89ca8
Modifying ConsensusContext with proposed changes
vncoelho Aug 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions neo/Consensus/ConsensusContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,11 @@ internal void EnsureMaxBlockSize(IEnumerable<Transaction> txs)
txs = txs.Take((int)maxTransactionsPerBlock);
List<UInt256> hashes = new List<UInt256>();
Transactions = new Dictionary<UInt256, Transaction>();
Block.Transactions = new Transaction[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this.. unused ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug, because in the previous computation of the size was used this object.


// We need to know the expected block size

// Expected block size
var blockSize = GetExpectedBlockSizeWithoutTransactions(txs.Count());

// Iterate transaction until reach the size

foreach (Transaction tx in txs)
{
// Check if maximum block size has been already exceeded with the current selected set
Expand Down
13 changes: 13 additions & 0 deletions neo/Consensus/ConsensusService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ private bool AddTransaction(Transaction tx, bool verify)
return false;
}
context.Transactions[tx.Hash] = tx;
return CheckPrepareResponse();
}

private bool CheckPrepareResponse()
{
if (context.TransactionHashes.Length == context.Transactions.Count)
{
// if we are the primary for this view, but acting as a backup because we recovered our own
Expand Down Expand Up @@ -428,6 +433,14 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
if (context.CommitPayloads[i]?.ConsensusMessage.ViewNumber == context.ViewNumber)
if (!Crypto.Default.VerifySignature(hashData, context.CommitPayloads[i].GetDeserializedMessage<Commit>().Signature, context.Validators[i].EncodePoint(false)))
context.CommitPayloads[i] = null;

if (context.TransactionHashes.Length == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shargon, is this also because of the removal of minner transaction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It's possible, but without transactions, there are no blocks :S

{
// There are no tx so we should act like if all the transactions were filled
CheckPrepareResponse();
return;
}

Dictionary<UInt256, Transaction> mempoolVerified = Blockchain.Singleton.MemPool.GetVerifiedTransactions().ToDictionary(p => p.Hash);
List<Transaction> unverified = new List<Transaction>();
foreach (UInt256 hash in context.TransactionHashes)
Expand Down