AntelopeIO / leap

C++ implementation of the Antelope protocol
Other
116 stars 68 forks source link

IF: Unification: Deprecate proposal and new_block messages #1910

Closed arhag closed 11 months ago

arhag commented 12 months ago

We do not need the new_block message anymore. Just get rid of it.

Remove proposal network message and instead generate a proposal to incorporate into qc_chain by generating one from the signed_block network message. We need to extract the relevant proposal information from the signed_block. Right now all that is needed is proposal it builds off of. This can be identified using the previous field of the signed_block. We can have the proposal ID be the block ID now to make this identification easy.

EDIT: We don't need to remove the proposal network message as part of this issue since we will make more significant changes soon that makes that work obsolete.

fcecin commented 11 months ago

Part 1/2 of this issue (remove new block message) is addressed by #1919

For part 2/2 (remove proposal message), my current idea is:

This guarantees that the unit tests keep working with minimal modifications (since the unit tests don't have signed_block, so they will keep using hs_proposal_message internally to emulate it, at least for now).

Then:

Does this look like a good idea?

linh2931 commented 11 months ago

It sounds good. Shoud _message be dropped from hs_proposal_message in the structure's name?

fcecin commented 11 months ago

It sounds good. Shoud _message be dropped from hs_proposal_message in the structure's name?

I'm fiddling with this here and I think it is still called a "message", as that's how it's used in the unit tests. I am not sure if this struct is what's actually sent to the qc_chain in some callback. It is possible we don't even have to mention this struct outside of the hotstuff library, and instead you send something else (either some flat parameters in the callback notifying of a new signed_block being produced or received, or we send in some other structure that we could name hs_proposal for example. But I'm not sure at this point. So for now I'm leaving it as-is.

I think that after the prototype, the unit tests are going to change significantly. I am not sure we will be emulating the network in unit tests going forward. My instinct would be to write tests of the "integration" type, that is, start multiple nodeos processes to test networking. For this to work with IF we may need to expose some extra test APIs. But that would allow us to wipe all these messaging concepts that we are keeping in the prototype to ensure the whole thing can be validated very quickly and we can iterate fast (all the unit tests take 3 seconds to run).

arhag commented 11 months ago

https://github.com/AntelopeIO/leap/pull/1919 is sufficient to close this issue. https://github.com/AntelopeIO/leap/pull/1929 is no longer needed.