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.

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

Add PreparationHash field to PrepareResponse to prevent replay attacks from malicious primary #576

Merged
merged 1 commit into from
Jan 27, 2019

Conversation

erikzhang
Copy link
Member

No description provided.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Erik, I like it, it moved one step toward the Regeneration.

However, I still fell the need of storing the signature instead of the Hash.
Both (signature and hash) will cost almost the same to store and the signature of the Payload promotes us several other benefits.

@@ -289,10 +293,12 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m

private void OnPrepareResponseReceived(ConsensusPayload payload, PrepareResponse message)
{
if (context.Preparations[payload.ValidatorIndex]) return;
if (context.Preparations[payload.ValidatorIndex] != null) return;
Copy link
Member

Choose a reason for hiding this comment

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

Erik, in #426 we had an speed up here.
If the OnPrepareResponseReceived arrives first we were able to use it as the OnPrepareRequest beucase the PrepareResponse carried the partial signature of the Speaker.

@vncoelho vncoelho merged commit 0e87248 into consensus/improved_dbft Jan 27, 2019
@vncoelho vncoelho deleted the fixes/PrepareResponse branch January 27, 2019 19:49
erikzhang pushed a commit that referenced this pull request Mar 3, 2019
* Add commit phase to consensus algorithm (#534)

* Add commit phase to consensus algorithm

* fix tests

* Prevent repeated sending of `Commit` messages

* RPC call gettransactionheight (#541)

* getrawtransactionheight

Nowadays two calls are need to get a transaction height, `getrawtransaction` with `verbose` and then use the `blockhash`.
Other option is to use `confirmations`, but it can be misleading.

* Minnor fix

* Shargon's tip

* modified

* Allow to use the wallet inside a RPC plugin (#536)

* Clean code

* Clean code

* Minor fix on mempoolVerified

* Add MemoryPool Unit tests. Fix bug on initital start of Persisting the Genesis block.

* Prevent `ConsensusService` from receiving messages before starting (#573)

* Prevent `ConsensusService` from receiving messages before starting

* fixed tests - calling OnStart now

* Consensus recovery log (#572)

* Pass store to `ConsensusService`

* Implement `ISerializable` in `ConsensusContext`

* Start from recovery log

* Fix unit tests due to constructor taking the store.

* Add unit tests for serializing and deserializing the consensus context.

* Combine `ConsensusContext.ChangeView()` and `ConsensusContext.Reset()`

* Add `PreparationHash` field to `PrepareResponse` to prevent replay attacks from malicious primary (#576)

* Fixed a problem where `PrepareResponse.PreparationHash` was not assigned.

* Load context from store only when height matches

* Recover nodes requesting ChangeView when possible (#579)

* Fixes bug in `OnPrepareRequestReceived()`

* Send `RecoveryMessage` only when `message.NewViewNumber <= context.ViewNumber`

* Fix and optimize view changing (#590)

* Allow to ignore the recovery logs

* Add `isRecovering` (#594)

* Fix accepting own prepare request (#596)

* Pick some changes from #575.

* Fixes `Prefixes`

* Restore transactions from saved consensus context (#598)

* Refactoring

* AggressiveInlining (#606)

* Reset Block reference when consensus context is initialized after block persist. (#608)

* Change `ConsensusPayload` for compatibility (#609)
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants