code: Multiplexing votes with full proposals is broken

[ancazamfir]

When we receive a vote and we try to multiplex with a proposal we do not check that the full value has also been received.
During testing I found we hit L49 and we decide without a full value.

[cason]

How are we abstracting, at code level, the "value dissemination/propagation" protocol?

I am curious to see the current architecture to get some insights. As also discussed in #365, I think we should render this distinction more clear.

[ancazamfir]

Once we merge #417, a proposal message received by the driver means we have a "complete" proposal. This is an internal message.
There are a few cases for value dissemination:

• PROPSAL only: fully included in a PROPOSAL consensus message over the /consensus topic and validated by the application. We used to have a mock app that was doing this, we should bring it back to have the possibility to test something else than starknet.
• blockParts and PROPOSAL: dissemination happens on /blockparts topic, assembled by the builder, once done the blockHash is sent to consensus together with the validity. In addition we also require a PROPOSAL consensus message to be sent over /consensus. This would be similar to Comet. Currently also required for starknet app but...
• blockParts only: this will be the case for starknet so there will be no explicit PROPOSAL consensus message. As I mentioned in this issue, we can do it because for L28 the information in the Init+Fin gives us both proposal_round and valid_round.

The idea behind the "full proposal" keeper is to abstract all of the above (and everything seen on the wire) and just send an internal proposal event/msg to consensus.

[romac]

PROPSAL only: fully included in a PROPOSAL consensus message over the /consensus topic and validated by the application. We used to have a mock app that was doing this, we should bring it back to have the possibility to test something else than starknet.

I have a branch with a "bench/test" app that does just this, will open a PR once it's ready but it's tricky because right now streaming is partly baked right in the gossip-consensus crate.

[ancazamfir]

PROPSAL only: fully included in a PROPOSAL consensus message over the /consensus topic and validated by the application. We used to have a mock app that was doing this, we should bring it back to have the possibility to test something else than starknet.

I have a branch with a "bench/test" app that does just this, will open a PR once it's ready but it's tricky because right now streaming is partly baked right in the gossip-consensus crate.

But if the app/ builder doesn't stream anything we won't use the /blockpart topic. We could just not create if we run with an app that doesn't need it.

[ancazamfir]

Of course we need to add some config to induce the correct behavior in the full proposer module.

[cason]

From what I understand, abstractly, the data exchanged via the blockparts channel/topic, which is a broadcast channel, is used by the node to produce the PROPOSAL message that we consider in the pseudo-code, isn't it?

This is an elegant solution in my view. We don't need to broadcast a PROPOSAL with a id(v) a la Comet, but just produce this message locally from the streamed data (which, of course, must ship all the content of the abstract PROPOSAL message).

[ancazamfir]

From what I understand, abstractly, the data exchanged via the blockparts channel/topic, which is a broadcast channel, is used by the node to produce the PROPOSAL message that we consider in the pseudo-code, isn't it?

Almost, we are not quite there yet, we need to remove the explicit PROPOSAL message but yes, that is the idea. We were waiting for the clarification for protos for L16 which we now have.