MystenLabs / mysticeti

Mysticeti: Low-Latency DAG Consensus with Fast Commit Path
Apache License 2.0
55 stars 28 forks source link

Pass epoch to committee & validate on connection. #51

Closed akichidis closed 1 year ago

akichidis commented 1 year ago

I've tried a few different directions but I found my self having to do some bigger refactoring to work (ex like wrapping the network message on a wrapper type with metadata to pass down epoch). There are alternatives here but found that the least intrusive without having to also touch part of the Network component.

andll commented 1 year ago

Let's discuss this little bit

We have to include epoch to the StatementBlock, because otherwise it's a vulnerability from BFT perspective - malicious validator can claim he is on new epoch and feed signed blocks from old epoch to other validators, thus breaking the protocol

Now after we do this, I think for practical point of view things will just work...

Then we could some time later just change code a bit to include epoch to the handshake (so we avoid extra round trip during connection introduced by message in this PR). This I think can be done with much lower priority, perhaps when we decided more firmly on which network stack to use

What do you think?

akichidis commented 1 year ago

We have to include epoch to the StatementBlock, because otherwise it's a vulnerability from BFT perspective - malicious validator can claim he is on new epoch and feed signed blocks from old epoch to other validators, thus breaking the protocol

100% agreed, this would be done as separate PR as we want to do anyways for BFT reasons as you said.

Now after we do this, I think for practical point of view things will just work...

Then we could some time later just change code a bit to include epoch to the handshake (so we avoid extra round trip during connection introduced by message in this PR). This I think can be done with much lower priority, perhaps when we decided more firmly on which network stack to use

What do you think?

My intention was to minimise any possible aftereffects from having any sort of communication with validators of different epoch during reconfig and get us to a close parity with NW. Sure we can do things more efficiently but for now just wanted to have something in place, that's I chose the path of least changes. If you believe that we can live without this PR I am happy to drop it.

akichidis commented 1 year ago

Closing for now. Talked offline about it as there is not immediate need for it for functional level.