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

Fix accepting own prepare request. #596

Merged
merged 2 commits into from
Feb 21, 2019

Conversation

jsolman
Copy link
Contributor

@jsolman jsolman commented Feb 20, 2019

Testing showed the primary was still changing view too soon when recovering it's own prepare request after the merge of #594

[18:51:28.234] initialize: height=22 view=4 index=2 role=Primary
[18:51:28.235] Attempt recover of prepare request from index=2
[18:51:28.236] OnPrepareRequestReceived: height=22 view=4 index=2 tx=1
[18:51:28.263] request change view: height=22 view=4 nv=5 state=Primary, RequestSent, ViewChanging

The check in VerifyRequest() needed to either check additionally for whether the request was sent in addition to being received or just have these checks removed. I checked all places this is called, and it is just from AddTransaction, and all places that call AddTransaction already have verified that either the RequestSent or the RequestReceived flags are sent. So the check can just be removed inside of VerifyRequest().

…equest() already have always verified the consensus state.
@jsolman jsolman mentioned this pull request Feb 20, 2019
@jsolman
Copy link
Contributor Author

jsolman commented Feb 20, 2019

After this change, receiving testing Primary receiving its own change view is working as expected.

@erikzhang erikzhang merged commit 2311524 into consensus/improved_dbft Feb 21, 2019
@erikzhang erikzhang deleted the consensus/fix_accept_own_prepreq branch February 21, 2019 04:36
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
* 2.8.0 exchange doc

* Update v2.7.6.md (neo-project#595)

Add xfs setting for sever.
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.

2 participants