Spec - Partial Signature Verification Aggregation (#369)

* Add SignedSSVMessage * Add SSZ encoding * Test structures for SignedSSVMessage * Test utils and encoding test * Tests * Generate tests * Adapt message validation * Fix expected error; Bring back commented tests * Add comment on signature * Adjust ssz-max in SignedSSVMessage * Include RSA check in test * Generate JSONs * Optimistic approach in pre-consensus messages * Extend optimistic approach to pre-consensus * Extend to post-consensus * Add approach to validator reg. and exit duties * Add errors * Rename fall back function * Check duplicated signature case * Add in-committee check * Fix unknown signer error messages * Drop expected error in invalid signature since it's deprecated * Change invalid beacon sig test to trigger error once quorum is reached * Drop errors in invalid then quorum test * GenerateTests * Fall back and verify for all roots * Test: Invalid quorum then valid quorum * Test: invalid quorum then valid quorum for pre-consensus * Fix name * Generate tests * Fix lint * remove commits from signed ssv message branch * Fix revert issues. Generate tests * Simplify if clause
ssvlabs · Mar 28, 2024 · ebb6014 · ebb6014
1 parent 7a3b1de
commit ebb6014
Show file tree

Hide file tree

Showing 67 changed files with 5,883 additions and 120 deletions.
diff --git a/ssv/aggregator.go b/ssv/aggregator.go
@@ -3,6 +3,7 @@ package ssv
 import (
 	"crypto/sha256"
 	"encoding/json"
+
 	"github.com/attestantio/go-eth2-client/spec/phase0"
 	"github.com/bloxapp/ssv-spec/qbft"
 	"github.com/bloxapp/ssv-spec/types"
@@ -70,7 +71,9 @@ func (r *AggregatorRunner) ProcessPreConsensus(signedMsg *types.SignedPartialSig
 	// reconstruct selection proof sig
 	fullSig, err := r.GetState().ReconstructBeaconSig(r.GetState().PreConsensusContainer, root, r.GetShare().ValidatorPubKey)
 	if err != nil {
-		return errors.Wrap(err, "could not reconstruct selection proof sig")
+		// If the reconstructed signature verification failed, fall back to verifying each partial signature
+		r.BaseRunner.FallBackAndVerifyEachSignature(r.GetState().PreConsensusContainer, root)
+		return errors.Wrap(err, "got pre-consensus quorum but it has invalid signatures")
 	}
 
 	duty := r.GetState().StartingDuty
@@ -162,7 +165,11 @@ func (r *AggregatorRunner) ProcessPostConsensus(signedMsg *types.SignedPartialSi
 	for _, root := range roots {
 		sig, err := r.GetState().ReconstructBeaconSig(r.GetState().PostConsensusContainer, root, r.GetShare().ValidatorPubKey)
 		if err != nil {
-			return errors.Wrap(err, "could not reconstruct post consensus signature")
+			// If the reconstructed signature verification failed, fall back to verifying each partial signature
+			for _, root := range roots {
+				r.BaseRunner.FallBackAndVerifyEachSignature(r.GetState().PostConsensusContainer, root)
+			}
+			return errors.Wrap(err, "got post-consensus quorum but it has invalid signatures")
 		}
 		specSig := phase0.BLSSignature{}
 		copy(specSig[:], sig)

diff --git a/ssv/attester.go b/ssv/attester.go
@@ -3,6 +3,7 @@ package ssv
 import (
 	"crypto/sha256"
 	"encoding/json"
+
 	"github.com/attestantio/go-eth2-client/spec/phase0"
 	"github.com/bloxapp/ssv-spec/qbft"
 	"github.com/bloxapp/ssv-spec/types"
@@ -126,7 +127,11 @@ func (r *AttesterRunner) ProcessPostConsensus(signedMsg *types.SignedPartialSign
 	for _, root := range roots {
 		sig, err := r.GetState().ReconstructBeaconSig(r.GetState().PostConsensusContainer, root, r.GetShare().ValidatorPubKey)
 		if err != nil {
-			return errors.Wrap(err, "could not reconstruct post consensus signature")
+			// If the reconstructed signature verification failed, fall back to verifying each partial signature
+			for _, root := range roots {
+				r.BaseRunner.FallBackAndVerifyEachSignature(r.GetState().PostConsensusContainer, root)
+			}
+			return errors.Wrap(err, "got post-consensus quorum but it has invalid signatures")
 		}
 		specSig := phase0.BLSSignature{}
 		copy(specSig[:], sig)

diff --git a/ssv/partial_sig_container.go b/ssv/partial_sig_container.go
@@ -2,6 +2,7 @@ package ssv
 
 import (
 	"encoding/hex"
+
 	"github.com/bloxapp/ssv-spec/types"
 	"github.com/pkg/errors"
 )
@@ -31,6 +32,41 @@ func (ps *PartialSigContainer) AddSignature(sigMsg *types.PartialSignatureMessag
 	}
 }
 
+// Returns if container has signature for signer and signing root
+func (ps *PartialSigContainer) HasSigner(signer types.OperatorID, signingRoot [32]byte) bool {
+	if ps.Signatures[rootHex(signingRoot)] == nil {
+		return false
+	}
+	return ps.Signatures[rootHex(signingRoot)][signer] != nil
+}
+
+// Return signature for given root and signer
+func (ps *PartialSigContainer) GetSignature(signer types.OperatorID, signingRoot [32]byte) (types.Signature, error) {
+	if ps.Signatures[rootHex(signingRoot)] == nil {
+		return nil, errors.New("Dont have signature for the given signing root")
+	}
+	if ps.Signatures[rootHex(signingRoot)][signer] == nil {
+		return nil, errors.New("Dont have signature on signing root for the given signer")
+	}
+	return ps.Signatures[rootHex(signingRoot)][signer], nil
+}
+
+// Return signature map for given root
+func (ps *PartialSigContainer) GetSignatures(signingRoot [32]byte) map[types.OperatorID][]byte {
+	return ps.Signatures[rootHex(signingRoot)]
+}
+
+// Remove signer from signature map
+func (ps *PartialSigContainer) Remove(signer uint64, signingRoot [32]byte) {
+	if ps.Signatures[rootHex(signingRoot)] == nil {
+		return
+	}
+	if ps.Signatures[rootHex(signingRoot)][signer] == nil {
+		return
+	}
+	delete(ps.Signatures[rootHex(signingRoot)], signer)
+}
+
 func (ps *PartialSigContainer) ReconstructSignature(root [32]byte, validatorPubKey []byte) ([]byte, error) {
 	// Reconstruct signatures
 	signature, err := types.ReconstructSignatures(ps.Signatures[rootHex(root)])

diff --git a/ssv/pre_consensus_justification.go b/ssv/pre_consensus_justification.go
@@ -51,6 +51,7 @@ func (b *BaseRunner) validatePreConsensusJustifications(data *types.ConsensusDat
 	signers := make(map[types.OperatorID]bool)
 	roots := make(map[[32]byte]bool)
 	rootCount := 0
+	partialSigContainer := NewPartialSigContainer(b.Share.Quorum)
 	for i, msg := range data.PreConsensusJustifications {
 		if err := msg.Validate(); err != nil {
 			return err
@@ -73,29 +74,39 @@ func (b *BaseRunner) validatePreConsensusJustifications(data *types.ConsensusDat
 		}
 
 		// validate roots
-		for _, msgRoot := range msg.Message.Messages {
+		for _, partialSigMessage := range msg.Message.Messages {
 			// validate roots
 			if i == 0 {
 				// check signer did not sign duplicate root
-				if roots[msgRoot.SigningRoot] {
+				if roots[partialSigMessage.SigningRoot] {
 					return errors.New("duplicate signed root")
 				}
 
 				// record roots
-				roots[msgRoot.SigningRoot] = true
+				roots[partialSigMessage.SigningRoot] = true
 			} else {
 				// compare roots
-				if !roots[msgRoot.SigningRoot] {
+				if !roots[partialSigMessage.SigningRoot] {
 					return errors.New("inconsistent roots")
 				}
 			}
+			partialSigContainer.AddSignature(partialSigMessage)
 		}
 
-		// verify sigs and duty.slot == msg.slot
+		// verify duty.slot == msg.slot
 		if err := b.validatePartialSigMsgForSlot(msg, data.Duty.Slot); err != nil {
 			return err
 		}
 	}
+
+	// Verify the reconstructed signature for each root
+	for root := range roots {
+		_, err := b.State.ReconstructBeaconSig(partialSigContainer, root, b.Share.ValidatorPubKey)
+		if err != nil {
+			return errors.Wrap(err, "wrong pre-consensus partial signature")
+		}
+	}
+
 	return nil
 }
 

diff --git a/ssv/proposer.go b/ssv/proposer.go
@@ -75,7 +75,9 @@ func (r *ProposerRunner) ProcessPreConsensus(signedMsg *types.SignedPartialSigna
 	// randao is relevant only for block proposals, no need to check type
 	fullSig, err := r.GetState().ReconstructBeaconSig(r.GetState().PreConsensusContainer, root, r.GetShare().ValidatorPubKey)
 	if err != nil {
-		return errors.Wrap(err, "could not reconstruct randao sig")
+		// If the reconstructed signature verification failed, fall back to verifying each partial signature
+		r.BaseRunner.FallBackAndVerifyEachSignature(r.GetState().PreConsensusContainer, root)
+		return errors.Wrap(err, "got pre-consensus quorum but it has invalid signatures")
 	}
 
 	duty := r.GetState().StartingDuty
@@ -189,7 +191,11 @@ func (r *ProposerRunner) ProcessPostConsensus(signedMsg *types.SignedPartialSign
 	for _, root := range roots {
 		sig, err := r.GetState().ReconstructBeaconSig(r.GetState().PostConsensusContainer, root, r.GetShare().ValidatorPubKey)
 		if err != nil {
-			return errors.Wrap(err, "could not reconstruct post consensus signature")
+			// If the reconstructed signature verification failed, fall back to verifying each partial signature
+			for _, root := range roots {
+				r.BaseRunner.FallBackAndVerifyEachSignature(r.GetState().PostConsensusContainer, root)
+			}
+			return errors.Wrap(err, "got post-consensus quorum but it has invalid signatures")
 		}
 		specSig := phase0.BLSSignature{}
 		copy(specSig[:], sig)

diff --git a/ssv/runner.go b/ssv/runner.go
@@ -167,7 +167,7 @@ func (b *BaseRunner) basePostConsensusMsgProcessing(runner Runner, signedMsg *ty
 	return hasQuorum, roots, errors.Wrap(err, "could not process post-consensus partial signature msg")
 }
 
-// basePartialSigMsgProcessing adds an already validated partial msg to the container, checks for quorum and returns true (and roots) if quorum exists
+// basePartialSigMsgProcessing adds a validated (without signature verification) partial msg to the container, checks for quorum and returns true (and roots) if quorum exists
 func (b *BaseRunner) basePartialSigMsgProcessing(
 	signedMsg *types.SignedPartialSignatureMessage,
 	container *PartialSigContainer,
@@ -177,14 +177,17 @@ func (b *BaseRunner) basePartialSigMsgProcessing(
 	for _, msg := range signedMsg.Message.Messages {
 		prevQuorum := container.HasQuorum(msg.SigningRoot)
 
-		container.AddSignature(msg)
-
-		if prevQuorum {
-			continue
+		// Check if it has two signatures for the same signer
+		if container.HasSigner(msg.Signer, msg.SigningRoot) {
+			b.resolveDuplicateSignature(container, msg)
+		} else {
+			container.AddSignature(msg)
 		}
 
-		quorum := container.HasQuorum(msg.SigningRoot)
-		if quorum {
+		hasQuorum := container.HasQuorum(msg.SigningRoot)
+
+		if hasQuorum && !prevQuorum {
+			// Notify about first quorum only
 			roots = append(roots, msg.SigningRoot)
 			anyQuorum = true
 		}

diff --git a/ssv/runner_signatures.go b/ssv/runner_signatures.go
@@ -45,35 +45,34 @@ func (b *BaseRunner) signPostConsensusMsg(runner Runner, msg *types.PartialSigna
 	}, nil
 }
 
+// Validate message content without verifying signatures
 func (b *BaseRunner) validatePartialSigMsgForSlot(
 	signedMsg *types.SignedPartialSignatureMessage,
 	slot spec.Slot,
 ) error {
 	if err := signedMsg.Validate(); err != nil {
 		return errors.Wrap(err, "SignedPartialSignatureMessage invalid")
 	}
-
 	if signedMsg.Message.Slot != slot {
 		return errors.New("invalid partial sig slot")
 	}
 
-	if err := signedMsg.GetSignature().VerifyByOperators(signedMsg, b.Share.DomainType, types.PartialSignatureType, b.Share.Committee); err != nil {
-		return errors.Wrap(err, "failed to verify PartialSignature")
-	}
-
-	for _, msg := range signedMsg.Message.Messages {
-		if err := b.verifyBeaconPartialSignature(msg); err != nil {
-			return errors.Wrap(err, "could not verify Beacon partial Signature")
+	// Check if signer is in committee
+	signerInCommittee := false
+	for _, operator := range b.Share.Committee {
+		if operator.OperatorID == signedMsg.Signer {
+			signerInCommittee = true
+			break
 		}
 	}
+	if !signerInCommittee {
+		return errors.New("unknown signer")
+	}
 
 	return nil
 }
 
-func (b *BaseRunner) verifyBeaconPartialSignature(msg *types.PartialSignatureMessage) error {
-	signer := msg.Signer
-	signature := msg.PartialSignature
-	root := msg.SigningRoot
+func (b *BaseRunner) verifyBeaconPartialSignature(signer uint64, signature types.Signature, root [32]byte) error {
 
 	for _, n := range b.Share.Committee {
 		if n.GetID() == signer {
@@ -95,3 +94,26 @@ func (b *BaseRunner) verifyBeaconPartialSignature(msg *types.PartialSignatureMes
 	}
 	return errors.New("unknown signer")
 }
+
+// Stores the container's existing signature or the new one, depending on their validity. If both are invalid, remove the existing one
+func (b *BaseRunner) resolveDuplicateSignature(container *PartialSigContainer, msg *types.PartialSignatureMessage) {
+
+	// Check previous signature validity
+	previousSignature, err := container.GetSignature(msg.Signer, msg.SigningRoot)
+	if err == nil {
+		err = b.verifyBeaconPartialSignature(msg.Signer, previousSignature, msg.SigningRoot)
+		if err == nil {
+			// Keep the previous sigature since it's correct
+			return
+		}
+	}
+
+	// Previous signature is incorrect or doesn't exist
+	container.Remove(msg.Signer, msg.SigningRoot)
+
+	// Hold the new signature, if correct
+	err = b.verifyBeaconPartialSignature(msg.Signer, msg.PartialSignature, msg.SigningRoot)
+	if err == nil {
+		container.AddSignature(msg)
+	}
+}
diff --git a/ssv/runner_validations.go b/ssv/runner_validations.go
@@ -2,11 +2,12 @@ package ssv
 
 import (
 	"bytes"
+	"sort"
+
 	spec "github.com/attestantio/go-eth2-client/spec/phase0"
 	"github.com/bloxapp/ssv-spec/types"
 	ssz "github.com/ferranbt/fastssz"
 	"github.com/pkg/errors"
-	"sort"
 )
 
 func (b *BaseRunner) ValidatePreConsensusMsg(runner Runner, signedMsg *types.SignedPartialSignatureMessage) error {
@@ -26,6 +27,18 @@ func (b *BaseRunner) ValidatePreConsensusMsg(runner Runner, signedMsg *types.Sig
 	return b.verifyExpectedRoot(runner, signedMsg, roots, domain)
 }
 
+// Verify each signature in container removing the invalid ones
+func (b *BaseRunner) FallBackAndVerifyEachSignature(container *PartialSigContainer, root [32]byte) {
+
+	signatures := container.GetSignatures(root)
+
+	for operatorID, signature := range signatures {
+		if err := b.verifyBeaconPartialSignature(operatorID, signature, root); err != nil {
+			container.Remove(operatorID, root)
+		}
+	}
+}
+
 func (b *BaseRunner) ValidatePostConsensusMsg(runner Runner, signedMsg *types.SignedPartialSignatureMessage) error {
 	if !b.hasRunningDuty() {
 		return errors.New("no running duty")

diff --git a/ssv/spectest/all_tests.go b/ssv/spectest/all_tests.go
@@ -27,7 +27,7 @@ var AllTests = []tests.TestF{
 	postconsensus.PostFinish,
 	postconsensus.NoRunningDuty,
 	postconsensus.InvalidMessageSignature,
-	postconsensus.InvalidBeaconSignature,
+	postconsensus.InvalidBeaconSignatureInQuorum,
 	postconsensus.DuplicateMsgDifferentRoots,
 	postconsensus.DuplicateMsgDifferentRootsThenQuorum,
 	postconsensus.DuplicateMsg,
@@ -46,6 +46,7 @@ var AllTests = []tests.TestF{
 	postconsensus.Quorum13Operators,
 	postconsensus.InvalidDecidedValue,
 	postconsensus.InvalidThenQuorum,
+	postconsensus.InvalidQuorumThenValidQuorum,
 
 	newduty.ConsensusNotStarted,
 	newduty.NotDecided,
@@ -130,9 +131,10 @@ var AllTests = []tests.TestF{
 	preconsensus.ValidMessage13Operators,
 	preconsensus.InconsistentBeaconSigner,
 	preconsensus.UnknownSigner,
-	preconsensus.InvalidBeaconSignature,
+	preconsensus.InvalidBeaconSignatureInQuorum,
 	preconsensus.InvalidMessageSignature,
 	preconsensus.InvalidThenQuorum,
+	preconsensus.InvalidQuorumThenValidQuorum,
 
 	valcheckduty.WrongValidatorIndex,
 	valcheckduty.WrongValidatorPK,

diff --git a/..._invalid_beacon_signature/aggregator.json → ...eacon_signature_in_quorum/aggregator.json b/..._invalid_beacon_signature/aggregator.json → ...eacon_signature_in_quorum/aggregator.json
@@ -6,7 +6,12 @@
 								"Quorum": 3
 						},
 						"PostConsensusContainer": {
-								"Signatures": {},
+								"Signatures": {
+										"c4551c9873f0588b4a13819ed39bde8c5cb6838b52e18d7b1807f02208c5b515": {
+												"2": "tVOROqJaP6C4g7uajgC44qC1QJ505Ol2tNefLK9jHg5AH0TCzsEnVIMbWRbYkZchALink2Clu63xVq79esPDOJ3u0sT5DkKAOCC2GebeKpQBZWxXVaMf3/QtNxs52ZBR",
+												"3": "p4mJsTHfJ9m/maFmyKe6pmptN6IMZMTD6pek2OhyB7k/9gc9iFNrnh/g7ZlxPMgJEs4RyEPe6RPBw5R2ldz9GECRUuE+so0uURYzfZtbDeC31xHu8QAwAdXAmFyA0x9t"
+										}
+								},
 								"Quorum": 3
 						},
 						"RunningInstance": {

diff --git a/...us_invalid_beacon_signature/attester.json → ..._beacon_signature_in_quorum/attester.json b/...us_invalid_beacon_signature/attester.json → ..._beacon_signature_in_quorum/attester.json
@@ -6,7 +6,12 @@
 								"Quorum": 3
 						},
 						"PostConsensusContainer": {
-								"Signatures": {},
+								"Signatures": {
+										"4ae32f48079745930bc24da670a6fe72052e13e3832728c6aa2580adc0307b83": {
+												"2": "gMXzIoe/QJBrH+ntuAoJ+kvr4qhj4VUmWphfsoLyOQ2VYjwvIOqDtLlXstFKbKXSCqNmxYecfX8xts3jWxcfHILOEgavz6rA8kiThsqlCSHWx3yDbjuBrISi9kU2a0a4",
+												"3": "ixo9S76Aot/jH4xVSQ+mWM9hXw0sU7YjMC0Q2dvgW38fhLN3fnXQs0mlzz6PNZwBD8isC5LcqhkTs5UKdCJNs0lEVEi5h4cG3I5+rd1SIzj/ZT9wrrutZjvnRkUF6Nxs"
+										}
+								},
 								"Quorum": 3
 						},
 						"RunningInstance": {

diff --git a/...n_signature/proposer (blinded block).json → ...e_in_quorum/proposer (blinded block).json b/...n_signature/proposer (blinded block).json → ...e_in_quorum/proposer (blinded block).json
@@ -6,7 +6,12 @@
 								"Quorum": 3
 						},
 						"PostConsensusContainer": {
-								"Signatures": {},
+								"Signatures": {
+										"734ce0b3fea9558ef2605043bbb947d7f02e9f6270ab5fc19eb4a2f6dd987976": {
+												"2": "lmjeRzuAjlk2ps3OFi3Z+d3aGKTTdnrVXLe0ZfGFhDW4HkfWYIaooo8qmSbQ/HXlCkzvIwYSwXbMUciwd/KPCeEkffUSNFpUFJQaq+Lwp5f80+Qd60+qbJuiTh8ONE7h",
+												"3": "uD3l8cu35ELGd/P94ZA43ofjo/rhX0JzKw/yYGJ+vSWFfayDsi+7XXAcnlYPr8VvCmShQ5qUKhcaIUJw7xww7Ai9rH5anamvnyVLMS8ziQ0EbPwoxQh3oRbbYT7RT0aa"
+										}
+								},
 								"Quorum": 3
 						},
 						"RunningInstance": {