Skip to content

Conversation

@amirylm
Copy link
Contributor

@amirylm amirylm commented Nov 16, 2021

Resolve #430

IBFT messages were purged once the instance decided or stopped, the behavior changed in this PR:

  • when instance finished w/ decided - ctrl starts to listen for late commit messages with timeout and purge messages once done
  • when instance finished w/o decided - ctrl purges messages

@amirylm amirylm force-pushed the decided-fix branch 3 times, most recently from d671b51 to 327e01c Compare November 17, 2021 13:37
zap.String("identifier", string(netMsg.SignedMessage.Message.Lambda)))
if err := runningInstance.CommitMsgValidationPipeline().Run(netMsg.SignedMessage); err != nil {
zap.Uint64s("signers", netMsg.SignedMessage.SignerIds))
if err := instance.CommitMsgValidationPipelineV0(identifier, seq, i.ValidatorShare).Run(netMsg.SignedMessage); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

@amir-blox better use the fork and not the implementation directly

}

// CommitMsgValidationPipelineV0 is version 0 of commit message validation
func CommitMsgValidationPipelineV0(identifier []byte, seq uint64, share *storage.Share) pipeline.Pipeline {
Copy link
Contributor

Choose a reason for hiding this comment

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

that seems redundant ..

Copy link
Contributor Author

@amirylm amirylm Nov 17, 2021

Choose a reason for hiding this comment

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

its a function so the late commit (in controller level) and exporter could use it as well

Copy link
Contributor

Choose a reason for hiding this comment

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

we have the instance which has that function why create another?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

late commit is done on controller level when the instance finished and on exporter, in both cases we don't have an instance..

@amirylm
Copy link
Contributor Author

amirylm commented Nov 17, 2021

The ibft instance is not available in late commit scenarios:

  1. ibft controller after the instance was cleared
  2. exporter

Therefore in this PR, a static commit message validation function was added to overcome the missing instance object.
It should be refactored in the future to be accessed via the fork

@amirylm amirylm merged commit a4b0db0 into stage Nov 17, 2021
@amirylm amirylm deleted the decided-fix branch November 17, 2021 16:42
@amirylm amirylm self-assigned this Feb 23, 2022
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.

3 participants