Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Emit event when a block is finalized from the bft module - Closes #3908 #4061

Merged

Conversation

pablitovicente
Copy link
Contributor

@pablitovicente pablitovicente commented Aug 9, 2019

What was the problem?

An event was not being emitted when finalized height changed in finality manager

How did I solve it?

  • by extending event emitter in finality manager
  • by emitting an event on change of finalized height
  • by listening to the event in the BFT class

How to manually test it?

Review checklist

@pablitovicente pablitovicente self-assigned this Aug 9, 2019
@pablitovicente pablitovicente changed the base branch from development to feature/introduce_bft_consensus August 9, 2019 06:48
@lsilvs
Copy link
Contributor

lsilvs commented Aug 9, 2019

Would be possible to use channel.publish and channel.subscribe instead?

@pablitovicente
Copy link
Contributor Author

Would be possible to use channel.publish and channel.subscribe instead?

There was a talk about this and it seems for now we'll use emit() I do agree that maybe before completing bft we can agree on changing to channel.subscribe/publish on all the code? @nazarhussain @shuse2 ?

@shuse2
Copy link
Collaborator

shuse2 commented Aug 9, 2019

Since BFT will be separate library in the future, so I think it would be better to keep it as emit

@pablitovicente pablitovicente marked this pull request as ready for review August 12, 2019 08:17
@shuse2 shuse2 requested a review from SargeKhan August 12, 2019 12:32
@shuse2 shuse2 merged commit 130169b into feature/introduce_bft_consensus Aug 13, 2019
@shuse2 shuse2 deleted the 3908-emit-block-finalized-event branch August 13, 2019 13:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants