Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial Signature Verification Aggregation #369

Merged
merged 36 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3ea4b69
Add SignedSSVMessage
MatheusFranco99 Jan 10, 2024
3aebee8
Add SSZ encoding
MatheusFranco99 Jan 10, 2024
0fc89ba
Test structures for SignedSSVMessage
MatheusFranco99 Jan 10, 2024
f919fda
Test utils and encoding test
MatheusFranco99 Jan 10, 2024
5d151fb
Tests
MatheusFranco99 Jan 10, 2024
7965ba8
Generate tests
MatheusFranco99 Jan 10, 2024
1c3ace2
Adapt message validation
MatheusFranco99 Jan 10, 2024
e67c4b5
Fix expected error; Bring back commented tests
MatheusFranco99 Jan 10, 2024
e2e279b
Add comment on signature
MatheusFranco99 Jan 15, 2024
43da24c
Merge branch 'main' into signed-ssv-msg
MatheusFranco99 Feb 19, 2024
9b1750d
Adjust ssz-max in SignedSSVMessage
MatheusFranco99 Feb 19, 2024
c74a5c1
Include RSA check in test
MatheusFranco99 Feb 20, 2024
b9469eb
Generate JSONs
MatheusFranco99 Feb 20, 2024
399dd77
Optimistic approach in pre-consensus messages
MatheusFranco99 Mar 18, 2024
86ac7df
Extend optimistic approach to pre-consensus
MatheusFranco99 Mar 18, 2024
21f4d47
Extend to post-consensus
MatheusFranco99 Mar 18, 2024
97765fd
Add approach to validator reg. and exit duties
MatheusFranco99 Mar 18, 2024
f229766
Add errors
MatheusFranco99 Mar 20, 2024
6da8d6d
Rename fall back function
MatheusFranco99 Mar 20, 2024
1f8b1b0
Check duplicated signature case
MatheusFranco99 Mar 21, 2024
15f5e31
Add in-committee check
MatheusFranco99 Mar 21, 2024
a9d1580
Fix unknown signer error messages
MatheusFranco99 Mar 21, 2024
a7d3d15
Drop expected error in invalid signature since it's deprecated
MatheusFranco99 Mar 21, 2024
cfa6c21
Change invalid beacon sig test to trigger error once quorum is reached
MatheusFranco99 Mar 21, 2024
b45fd73
Drop errors in invalid then quorum test
MatheusFranco99 Mar 21, 2024
836a06b
GenerateTests
MatheusFranco99 Mar 21, 2024
ad95766
Fall back and verify for all roots
MatheusFranco99 Mar 21, 2024
464b763
Test: Invalid quorum then valid quorum
MatheusFranco99 Mar 21, 2024
aaa0b28
Test: invalid quorum then valid quorum for pre-consensus
MatheusFranco99 Mar 21, 2024
603a7ab
Fix name
MatheusFranco99 Mar 21, 2024
fdcf38c
Generate tests
MatheusFranco99 Mar 21, 2024
db72966
Fix lint
MatheusFranco99 Mar 21, 2024
03fa670
remove commits from signed ssv message branch
MatheusFranco99 Mar 25, 2024
6f407fb
Fix revert issues. Generate tests
MatheusFranco99 Mar 25, 2024
5e592ba
Simplify if clause
MatheusFranco99 Mar 27, 2024
96807d4
Merge branch 'main' into reduce_signatures_partial_sig
MatheusFranco99 Mar 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ssv/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion ssv/attester.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 36 additions & 0 deletions ssv/partial_sig_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ssv

import (
"encoding/hex"

"github.com/bloxapp/ssv-spec/types"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -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)])
Expand Down
21 changes: 16 additions & 5 deletions ssv/pre_consensus_justification.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down
10 changes: 8 additions & 2 deletions ssv/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 10 additions & 7 deletions ssv/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
46 changes: 34 additions & 12 deletions ssv/runner_signatures.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Comment on lines +60 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm

This is currently checked by MsgValidation ERR_PSIG_INVALID_SIG_ID?
Since MsgValidation isn't part of spec it is fine that you added this..

But now I made up my mind that MsgValidation should be an inseperable part of spec.
Once we add tests for this, we won't have to do those double checks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that message validation should belong to the spec


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 {
Expand All @@ -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)
}
}
15 changes: 14 additions & 1 deletion ssv/runner_validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}
Comment on lines +33 to +39
Copy link
Contributor

@y0sher y0sher Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. shouldn't we do another check here? or after calling FallBackAndVerifyEachSignature, what if we had a quorum of 4, then we realized one is invalid, don't we want to reconstruct again with the 3 left?
    or this is not possible since its a must we'll try first when have 3?
  2. I guess nothing else to do here regarding notifying the runner we no longer have quorum? since the quorum is calculated on the fly using the container?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. we must have 3
  2. yes

}

func (b *BaseRunner) ValidatePostConsensusMsg(runner Runner, signedMsg *types.SignedPartialSignatureMessage) error {
if !b.hasRunningDuty() {
return errors.New("no running duty")
Expand Down
6 changes: 4 additions & 2 deletions ssv/spectest/all_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var AllTests = []tests.TestF{
postconsensus.PostFinish,
postconsensus.NoRunningDuty,
postconsensus.InvalidMessageSignature,
postconsensus.InvalidBeaconSignature,
postconsensus.InvalidBeaconSignatureInQuorum,
postconsensus.DuplicateMsgDifferentRoots,
postconsensus.DuplicateMsgDifferentRootsThenQuorum,
postconsensus.DuplicateMsg,
Expand All @@ -46,6 +46,7 @@ var AllTests = []tests.TestF{
postconsensus.Quorum13Operators,
postconsensus.InvalidDecidedValue,
postconsensus.InvalidThenQuorum,
postconsensus.InvalidQuorumThenValidQuorum,

newduty.ConsensusNotStarted,
newduty.NotDecided,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
"Quorum": 3
},
"PostConsensusContainer": {
"Signatures": {},
"Signatures": {
"c4551c9873f0588b4a13819ed39bde8c5cb6838b52e18d7b1807f02208c5b515": {
"2": "tVOROqJaP6C4g7uajgC44qC1QJ505Ol2tNefLK9jHg5AH0TCzsEnVIMbWRbYkZchALink2Clu63xVq79esPDOJ3u0sT5DkKAOCC2GebeKpQBZWxXVaMf3/QtNxs52ZBR",
"3": "p4mJsTHfJ9m/maFmyKe6pmptN6IMZMTD6pek2OhyB7k/9gc9iFNrnh/g7ZlxPMgJEs4RyEPe6RPBw5R2ldz9GECRUuE+so0uURYzfZtbDeC31xHu8QAwAdXAmFyA0x9t"
}
},
"Quorum": 3
},
"RunningInstance": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
"Quorum": 3
},
"PostConsensusContainer": {
"Signatures": {},
"Signatures": {
"4ae32f48079745930bc24da670a6fe72052e13e3832728c6aa2580adc0307b83": {
"2": "gMXzIoe/QJBrH+ntuAoJ+kvr4qhj4VUmWphfsoLyOQ2VYjwvIOqDtLlXstFKbKXSCqNmxYecfX8xts3jWxcfHILOEgavz6rA8kiThsqlCSHWx3yDbjuBrISi9kU2a0a4",
"3": "ixo9S76Aot/jH4xVSQ+mWM9hXw0sU7YjMC0Q2dvgW38fhLN3fnXQs0mlzz6PNZwBD8isC5LcqhkTs5UKdCJNs0lEVEi5h4cG3I5+rd1SIzj/ZT9wrrutZjvnRkUF6Nxs"
}
},
"Quorum": 3
},
"RunningInstance": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Loading
Loading