search

neo-project / neo-modules

MIT License
60 stars 100 forks source link

Fix prepare request in recovery message #765

Closed ZhangTao1596 closed 1 year ago

ZhangTao1596 commented 1 year ago

ValidatorIndex is uninitialized and unassigned in PrepareRequestMessage of RecoveryMessage. This will cause prepare request reverify failure when recovery.

roman-khimov commented 1 year ago

It can be set on the receiver side as well, somewhere in GetPrepareRequestPayload: https://github.com/neo-project/neo-modules/blob/c6d9640548d409e94873215ffb6ed05f8273c65d/src/DBFTPlugin/Messages/RecoveryMessage/RecoveryMessage.cs#L89-L95

That's what NeoGo currently does in https://github.com/nspcc-dev/neo-go/blob/5eb4ba772f4d0e507d8a77699b3856f340c36fe8/pkg/consensus/recovery_message.go#L205

vncoelho commented 1 year ago

I agree with @roman-khimov , this will also be better for mainnet compatibility at this moment.

vncoelho commented 1 year ago

@ZhangTao1596, I am double checking your commit, in fact, it will not affect payload serialization/de-serialization itself. As we see, currently, it is not assigned and keeps null?

superboyiii commented 1 year ago

@superboyiii, if possible try an extended test.

We use to have Unit Tests for consensus, that we created sometime ago. @erikzhang, do you know why they were not ported for neo-modules?

That Akka tests can cover several cases, such as this.

I will.