From 995dca9539b749a44eae7fe5839ab0fd941c6a0b Mon Sep 17 00:00:00 2001 From: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com> Date: Fri, 8 Mar 2024 16:17:54 +0000 Subject: [PATCH 1/4] Add sip --- sips/consensus_justification.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 sips/consensus_justification.md diff --git a/sips/consensus_justification.md b/sips/consensus_justification.md new file mode 100644 index 0000000..59b0a04 --- /dev/null +++ b/sips/consensus_justification.md @@ -0,0 +1,26 @@ +| Author | Title | Category | Status | Date | +| -------------- | ----------------------- | -------- | ------------------- | ---------- | +| Matheus Franco | Consensus Justification | Core | open-for-discussion | 2024-03-08 | + +## Summary + +Drop decided messages and include a consensus justification nested in post-consensus messages. + +## Motivation + +Decided messages represent $16$% of the exchanged messages in the network and also significantly increase the thresholds of allowed messages in message validation. Furthermore, it's not a necessary part of the protocol and the speed-up that it provides can be provided by a simpler justification in the post-consensus message. + +## Rationale + +As it's done with the pre-consensus justification, that is included in the *Proposal* message, we propose including a consensus justification (an aggregated commit message) in the post-consensus message instead of sending decided messages. + +First of all, the decided messages are useful to allow nodes to start the post-consensus phase if they are "behind" in the protocol execution due to not receiving messages. However, a consensus justification inside a post-consensus message allows a node to accomplish the same result while progressing even further due to the post-consensus signature. Moreover, this proposal aligns the design decisions of the (pre-consensus, consensus) and (consensus, post-consensus) inter-phases. + +## Improvements + +This proposal reduces the number of messages exchanged in the network by $16$%. + +## Drawbacks + +- The post-consensus messages would be bigger due to the aggregated commit message. +- Exporters use decided messages to compute statistics of the network. Therefore, the proposal implies not only a protocol change but also changes in the exporter implementation. \ No newline at end of file From 23cebb095a4c36fcd8bea1cd534b22f32cd1a136 Mon Sep 17 00:00:00 2001 From: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com> Date: Fri, 8 Mar 2024 16:18:46 +0000 Subject: [PATCH 2/4] Add to lists --- all.md | 21 +++++++++++---------- core.md | 21 +++++++++++---------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/all.md b/all.md index ba8c694..37c65ab 100644 --- a/all.md +++ b/all.md @@ -1,12 +1,13 @@ ## All SIPS -| SIP # | Title | Status | -|---------------------------------------|-----------------------------|--------| -| [1](./sips/dkg.md) | DKG | open-for-discussion | -| [2](./sips/msg_struct_encoding.md) | Message struct and encoding | open-for-discussion | -| [3](./sips/qbft_sync.md) | QBFT Sync | open-for-discussion | -| [4](./sips/change_operator.md) | Change operators set | open-for-discussion | -| [5](./sips/ecies_share_encryption.md) | ECIES Share Encryption | open-for-discussion | -| [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | -| [7](./sips/fork_support.md) | Fork Support | open-for-discussion | -| [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | \ No newline at end of file +| SIP # | Title | Status | +|-----------------------------------------|-----------------------------|---------------------| +| [1](./sips/dkg.md) | DKG | open-for-discussion | +| [2](./sips/msg_struct_encoding.md) | Message struct and encoding | open-for-discussion | +| [3](./sips/qbft_sync.md) | QBFT Sync | open-for-discussion | +| [4](./sips/change_operator.md) | Change operators set | open-for-discussion | +| [5](./sips/ecies_share_encryption.md) | ECIES Share Encryption | open-for-discussion | +| [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | +| [7](./sips/fork_support.md) | Fork Support | open-for-discussion | +| [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | +| [11](./sips/consensus_justification.md) | Consenuss justification | open-for-discussion | \ No newline at end of file diff --git a/core.md b/core.md index aec11df..e03c2e6 100644 --- a/core.md +++ b/core.md @@ -1,12 +1,13 @@ ## Core -| SIP # | Title | Status | -|------------------------------------|-----------------------------|--------| -| [1](./sips/dkg.md) | DKG | open-for-discussion | -| [2](./sips/msg_struct_encoding.md) | Message struct and encoding | open-for-discussion | -| [3](./sips/qbft_sync.md) | QBFT Sync | open-for-discussion | -| [4](./sips/change_operator.md) | Change operators set | open-for-discussion | -| [5](./sips/ecies_share_encryption.md) | ECIES Share Encryption | open-for-discussion | -| [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | -| [7](./sips/fork_support.md) | Fork Support | open-for-discussion | -| [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | \ No newline at end of file +| SIP # | Title | Status | +|-----------------------------------------|-----------------------------|---------------------| +| [1](./sips/dkg.md) | DKG | open-for-discussion | +| [2](./sips/msg_struct_encoding.md) | Message struct and encoding | open-for-discussion | +| [3](./sips/qbft_sync.md) | QBFT Sync | open-for-discussion | +| [4](./sips/change_operator.md) | Change operators set | open-for-discussion | +| [5](./sips/ecies_share_encryption.md) | ECIES Share Encryption | open-for-discussion | +| [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | +| [7](./sips/fork_support.md) | Fork Support | open-for-discussion | +| [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | +| [11](./sips/consensus_justification.md) | Consenuss justification | open-for-discussion | \ No newline at end of file From 81e7b3021e3d7c86a1b1221a2198567d332b0399 Mon Sep 17 00:00:00 2001 From: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com> Date: Mon, 11 Mar 2024 07:26:24 +0000 Subject: [PATCH 3/4] Add spec change --- sips/consensus_justification.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/sips/consensus_justification.md b/sips/consensus_justification.md index 59b0a04..9f32c17 100644 --- a/sips/consensus_justification.md +++ b/sips/consensus_justification.md @@ -20,6 +20,22 @@ First of all, the decided messages are useful to allow nodes to start the post-c This proposal reduces the number of messages exchanged in the network by $16$%. +## Spec change + +Within a `SignedPartialSignatureMessage.PartialSignatureMessages`, all `PartialSignatureMessage` objects refer to the same `QBFT Instance`. Thus, we can simply add the justification to the `SignedPartialSignatureMessage` as follows: + +```go +type SignedPartialSignatureMessage struct { + Message PartialSignatureMessages + Signature Signature `ssz-size:"96"` + Signer OperatorID + Justification *SignedMessage // Aggregated commit message (or []*SignedMessage if we ever use another asymmetric scheme that doesn't allow aggregation) +} +``` + +It could also be added to the `PartialSignatureMessages` types. The difference is that this would *hold* the `SignedPartialSignatureMessage.Signature` also to the consensus justification. + + ## Drawbacks - The post-consensus messages would be bigger due to the aggregated commit message. From fdd954c5671663200274ab15d7dba5c7416a1229 Mon Sep 17 00:00:00 2001 From: MatheusFranco99 <48058141+MatheusFranco99@users.noreply.github.com> Date: Tue, 12 Mar 2024 07:43:40 +0000 Subject: [PATCH 4/4] Fix typo. Add processing suggestion --- all.md | 2 +- core.md | 2 +- sips/consensus_justification.md | 47 ++++++++++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/all.md b/all.md index 37c65ab..cfb68b9 100644 --- a/all.md +++ b/all.md @@ -10,4 +10,4 @@ | [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | | [7](./sips/fork_support.md) | Fork Support | open-for-discussion | | [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | -| [11](./sips/consensus_justification.md) | Consenuss justification | open-for-discussion | \ No newline at end of file +| [11](./sips/consensus_justification.md) | Consensus justification | open-for-discussion | \ No newline at end of file diff --git a/core.md b/core.md index e03c2e6..28a3aa8 100644 --- a/core.md +++ b/core.md @@ -10,4 +10,4 @@ | [6](./sips/constant_qbft_timeout.md) | Constant QBFT timeout | open-for-discussion | | [7](./sips/fork_support.md) | Fork Support | open-for-discussion | | [8](./sips/pre_consensus_livness.md) | Pre-Consensus livness fix | open-for-discussion | -| [11](./sips/consensus_justification.md) | Consenuss justification | open-for-discussion | \ No newline at end of file +| [11](./sips/consensus_justification.md) | Consensus justification | open-for-discussion | \ No newline at end of file diff --git a/sips/consensus_justification.md b/sips/consensus_justification.md index 9f32c17..415b053 100644 --- a/sips/consensus_justification.md +++ b/sips/consensus_justification.md @@ -22,6 +22,10 @@ This proposal reduces the number of messages exchanged in the network by $16$%. ## Spec change +### Message structure + +**Adding a justification in post-consensus messages** + Within a `SignedPartialSignatureMessage.PartialSignatureMessages`, all `PartialSignatureMessage` objects refer to the same `QBFT Instance`. Thus, we can simply add the justification to the `SignedPartialSignatureMessage` as follows: ```go @@ -33,7 +37,48 @@ type SignedPartialSignatureMessage struct { } ``` -It could also be added to the `PartialSignatureMessages` types. The difference is that this would *hold* the `SignedPartialSignatureMessage.Signature` also to the consensus justification. +It could even be added to the `PartialSignatureMessages` types. The difference is that this would *hold* the `SignedPartialSignatureMessage.Signature` also to the consensus justification. + +### New post-consensus logic + +To implement the justification logic, once a post-consensus message is received, if the consensus has not been decided yet, we ask the consensus module to process a justification in a similar way that a decided message is processed (except y the feature of holding a state and broadcasting new decided messages). If the processing is successful, we update the runner state and continue with the current post-consensus logic. + +```go +func (b *BaseRunner) basePostConsensusMsgProcessing(runner Runner, signedMsg *types.SignedPartialSignatureMessage) (bool, [][32]byte, error) { + + if !b.hasRunningDuty() { + // do nothing + } + + // If has not decided yet, process the justification + if b.State.DecidedValue == nil && b.State != nil && b.State.RunningInstance != nil { + // Process + decidedMsg, err := b.QBFTController.ProcessConsensusJustification(signedMsg.Justification) + if err != nil { + // return error + } + + // Decode + decidedValue = &types.ConsensusData{} + // ... + + // Set state + b.highestDecidedSlot = decidedValue.Duty.Slot + b.State.DecidedValue = decidedValue + } + // Follow-up with the current post-consensus processing +} +``` + +The justification may be large and there's no need for it in pre-consensus messages. Thus, we also recommend adding a check in the pre-consensus processing to assert that there's no consensus justification. This is useful as a part of a protection against buffer DoS attacks. + +```go +func (b *BaseRunner) basePreConsensusMsgProcessing(runner Runner, signedMsg *types.SignedPartialSignatureMessage) (bool, [][32]byte, error) { + if signedMsg.Justification != nil { + // Return error + } +} +``` ## Drawbacks