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

Spec alignment review #1509

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 9 additions & 12 deletions operator/validator/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1197,11 +1197,10 @@ func SetupCommitteeRunners(
//logger.Debug("leader", zap.Int("operator_id", int(leader)))
return leader
},
Storage: options.Storage.Get(convert.RunnerRole(role)),
Network: options.Network,
Timer: roundtimer.New(ctx, options.NetworkConfig.Beacon, role, nil),
SignatureVerification: true,
CutOffRound: specqbft.Round(specqbft.CutoffRound),
Storage: options.Storage.Get(convert.RunnerRole(role)),
Network: options.Network,
Timer: roundtimer.New(ctx, options.NetworkConfig.Beacon, role, nil),
CutOffRound: specqbft.Round(specqbft.CutoffRound),
}
config.ValueCheckF = valueCheckF

Expand Down Expand Up @@ -1258,11 +1257,10 @@ func SetupRunners(
//logger.Debug("leader", zap.Int("operator_id", int(leader)))
return leader
},
Storage: options.Storage.Get(convert.RunnerRole(role)),
Network: options.Network,
Timer: roundtimer.New(ctx, options.NetworkConfig.Beacon, role, nil),
SignatureVerification: true,
CutOffRound: specqbft.Round(specqbft.CutoffRound),
Storage: options.Storage.Get(convert.RunnerRole(role)),
Network: options.Network,
Timer: roundtimer.New(ctx, options.NetworkConfig.Beacon, role, nil),
CutOffRound: specqbft.Round(specqbft.CutoffRound),
}
config.ValueCheckF = valueCheckF

Expand Down Expand Up @@ -1298,8 +1296,7 @@ func SetupRunners(
qbftCtrl := buildController(spectypes.RoleSyncCommitteeContribution, syncCommitteeContributionValueCheckF)
runners[role] = runner.NewSyncCommitteeAggregatorRunner(options.NetworkConfig, options.NetworkConfig.Beacon.GetBeaconNetwork(), shareMap, qbftCtrl, options.Beacon, options.Network, options.Signer, options.OperatorSigner, syncCommitteeContributionValueCheckF, 0)
case spectypes.RoleValidatorRegistration:
qbftCtrl := buildController(spectypes.RoleValidatorRegistration, nil)
runners[role] = runner.NewValidatorRegistrationRunner(options.NetworkConfig, options.NetworkConfig.Beacon.GetBeaconNetwork(), shareMap, qbftCtrl, options.Beacon, options.Network, options.Signer, options.OperatorSigner)
runners[role] = runner.NewValidatorRegistrationRunner(options.NetworkConfig, options.NetworkConfig.Beacon.GetBeaconNetwork(), shareMap, options.Beacon, options.Network, options.Signer, options.OperatorSigner)
case spectypes.RoleVoluntaryExit:
runners[role] = runner.NewVoluntaryExitRunner(options.NetworkConfig, options.NetworkConfig.Beacon.GetBeaconNetwork(), shareMap, options.Beacon, options.Network, options.Signer, options.OperatorSigner)
}
Expand Down
23 changes: 8 additions & 15 deletions protocol/v2/qbft/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,19 @@ type IConfig interface {
GetStorage() qbftstorage.QBFTStore
// GetTimer returns round timer
GetTimer() roundtimer.Timer
// VerifySignatures returns if signature is checked
VerifySignatures() bool
// GetRoundCutOff returns the round cut off
GetCutOffRound() specqbft.Round
}

type Config struct {
BeaconSigner spectypes.BeaconSigner
Domain spectypes.DomainType
ValueCheckF specqbft.ProposedValueCheckF
ProposerF specqbft.ProposerF
Storage qbftstorage.QBFTStore
Network specqbft.Network
Timer roundtimer.Timer
SignatureVerification bool
CutOffRound specqbft.Round
BeaconSigner spectypes.BeaconSigner
Domain spectypes.DomainType
ValueCheckF specqbft.ProposedValueCheckF
ProposerF specqbft.ProposerF
Storage qbftstorage.QBFTStore
Network specqbft.Network
Timer roundtimer.Timer
CutOffRound specqbft.Round
}

// GetShareSigner returns a BeaconSigner instance
Expand Down Expand Up @@ -83,7 +80,3 @@ func (c *Config) GetTimer() roundtimer.Timer {
func (c *Config) GetCutOffRound() specqbft.Round {
return c.CutOffRound
}

func (c *Config) VerifySignatures() bool {
return c.SignatureVerification
}
2 changes: 1 addition & 1 deletion protocol/v2/qbft/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (c *Controller) addAndStoreNewInstance() *instance.Instance {
func (c *Controller) GetRoot() ([32]byte, error) {
marshaledRoot, err := json.Marshal(c)
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not encode state")
return [32]byte{}, errors.Wrap(err, "could not encode controller")
}
ret := sha256.Sum256(marshaledRoot)
return ret, nil
Expand Down
2 changes: 1 addition & 1 deletion protocol/v2/qbft/controller/decided.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func ValidateDecided(
return errors.Wrap(err, "invalid decided msg")
}

if err := msg.SignedMessage.Validate(); err != nil {
if err := msg.Validate(); err != nil {
return errors.Wrap(err, "invalid decided")
}

Expand Down
8 changes: 3 additions & 5 deletions protocol/v2/qbft/instance/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,9 @@ func BaseCommitValidationVerifySignature(
return err
}

if config.VerifySignatures() {
// verify signature
if err := spectypes.Verify(msg.SignedMessage, operators); err != nil {
return errors.Wrap(err, "msg signature invalid")
}
// verify signature
if err := spectypes.Verify(msg.SignedMessage, operators); err != nil {
return errors.Wrap(err, "msg signature invalid")
}

return nil
Expand Down
4 changes: 1 addition & 3 deletions protocol/v2/qbft/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,8 @@ func (i *Instance) Broadcast(logger *zap.Logger, msg *spectypes.SignedSSVMessage
if !i.CanProcessMessages() {
return errors.New("instance stopped processing messages")
}
msgID := spectypes.MessageID{}
copy(msgID[:], i.State.ID)

return i.config.GetNetwork().Broadcast(msgID, msg)
return i.GetConfig().GetNetwork().Broadcast(msg.SSVMessage.GetID(), msg)
}

func allSigners(all []*specqbft.ProcessingMessage) []spectypes.OperatorID {
Expand Down
10 changes: 4 additions & 6 deletions protocol/v2/qbft/instance/prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func validSignedPrepareForHeightRoundAndRootIgnoreSignature(
return errors.New("wrong msg round")
}

if err := msg.SignedMessage.Validate(); err != nil {
if err := msg.Validate(); err != nil {
return errors.Wrap(err, "prepareData invalid")
}

Expand Down Expand Up @@ -150,11 +150,9 @@ func validSignedPrepareForHeightRoundAndRootVerifySignature(
return err
}

if config.VerifySignatures() {
// Verify signature
if err := spectypes.Verify(msg.SignedMessage, operators); err != nil {
return errors.Wrap(err, "msg signature invalid")
}
// Verify signature
if err := spectypes.Verify(msg.SignedMessage, operators); err != nil {
return errors.Wrap(err, "msg signature invalid")
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion protocol/v2/qbft/instance/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func isValidProposal(
return errors.New("proposal leader invalid")
}

if err := msg.SignedMessage.Validate(); err != nil {
if err := msg.Validate(); err != nil {
return errors.Wrap(err, "proposal invalid")
}

Expand Down
10 changes: 4 additions & 6 deletions protocol/v2/qbft/instance/round_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func validRoundChangeForDataIgnoreSignature(
return errors.New("msg allows 1 signer")
}

if err := msg.SignedMessage.Validate(); err != nil {
if err := msg.Validate(); err != nil {
return errors.Wrap(err, "roundChange invalid")
}

Expand Down Expand Up @@ -353,11 +353,9 @@ func validRoundChangeForDataVerifySignature(
return err
}

if config.VerifySignatures() {
// Verify signature
if err := spectypes.Verify(msg.SignedMessage, state.CommitteeMember.Committee); err != nil {
return errors.Wrap(err, "msg signature invalid")
}
// Verify signature
if err := spectypes.Verify(msg.SignedMessage, state.CommitteeMember.Committee); err != nil {
return errors.Wrap(err, "msg signature invalid")
}

return nil
Expand Down
9 changes: 4 additions & 5 deletions protocol/v2/qbft/testing/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ var TestingConfig = func(logger *zap.Logger, keySet *testingutils.TestKeySet, ro
ProposerF: func(state *specqbft.State, round specqbft.Round) types.OperatorID {
return 1
},
Storage: TestingStores(logger).Get(role),
Network: testingutils.NewTestingNetwork(1, keySet.OperatorKeys[1]),
Timer: roundtimer.NewTestingTimer(),
SignatureVerification: true,
CutOffRound: testingutils.TestingCutOffRound,
Storage: TestingStores(logger).Get(role),
Network: testingutils.NewTestingNetwork(1, keySet.OperatorKeys[1]),
Timer: roundtimer.NewTestingTimer(),
CutOffRound: testingutils.TestingCutOffRound,
}
}

Expand Down
4 changes: 2 additions & 2 deletions protocol/v2/ssv/runner/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (r *AggregatorRunner) expectedPostConsensusRootsAndDomain() ([]ssz.HashRoot
cd := &spectypes.ValidatorConsensusData{}
err := cd.Decode(r.GetState().DecidedValue)
if err != nil {
return nil, spectypes.DomainAggregateAndProof, err
return nil, spectypes.DomainError, errors.Wrap(err, "could not create consensus data")
}
aggregateAndProof, err := cd.GetAggregateAndProof()
if err != nil {
Expand Down Expand Up @@ -370,7 +370,7 @@ func (r *AggregatorRunner) Decode(data []byte) error {
func (r *AggregatorRunner) GetRoot() ([32]byte, error) {
marshaledRoot, err := r.Encode()
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not encode DutyRunnerState")
return [32]byte{}, errors.Wrap(err, "could not encode AggregatorRunner")
}
ret := sha256.Sum256(marshaledRoot)
return ret, nil
Expand Down
12 changes: 6 additions & 6 deletions protocol/v2/ssv/runner/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ func (cr *CommitteeRunner) Decode(data []byte) error {
func (cr *CommitteeRunner) GetRoot() ([32]byte, error) {
marshaledRoot, err := cr.Encode()
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not encode DutyRunnerState")
return [32]byte{}, errors.Wrap(err, "could not encode CommitteeRunner")
}
ret := sha256.Sum256(marshaledRoot)
return ret, nil
}

func (cr *CommitteeRunner) MarshalJSON() ([]byte, error) {
type CommitteeAlias struct {
type CommitteeRunnerAlias struct {
BaseRunner *BaseRunner
beacon beacon.BeaconNode
network specqbft.Network
Expand All @@ -118,7 +118,7 @@ func (cr *CommitteeRunner) MarshalJSON() ([]byte, error) {
}

// Create object and marshal
alias := &CommitteeAlias{
alias := &CommitteeRunnerAlias{
BaseRunner: cr.BaseRunner,
beacon: cr.beacon,
network: cr.network,
Expand All @@ -133,7 +133,7 @@ func (cr *CommitteeRunner) MarshalJSON() ([]byte, error) {
}

func (cr *CommitteeRunner) UnmarshalJSON(data []byte) error {
type CommitteeAlias struct {
type CommitteeRunnerAlias struct {
BaseRunner *BaseRunner
beacon beacon.BeaconNode
network specqbft.Network
Expand All @@ -145,7 +145,7 @@ func (cr *CommitteeRunner) UnmarshalJSON(data []byte) error {
}

// Unmarshal the JSON data into the auxiliary struct
aux := &CommitteeAlias{}
aux := &CommitteeRunnerAlias{}
if err := json.Unmarshal(data, &aux); err != nil {
return err
}
Expand Down Expand Up @@ -369,7 +369,7 @@ func (cr *CommitteeRunner) ProcessPostConsensus(logger *zap.Logger, signedMsg *s
// If the reconstructed signature verification failed, fall back to verifying each partial signature
// TODO should we return an error here? maybe other sigs are fine?
if err != nil {
for _, root := range roots {
for root := range rootSet {
cr.BaseRunner.FallBackAndVerifyEachSignature(cr.BaseRunner.State.PostConsensusContainer, root,
share.Committee, validator)
}
Expand Down
22 changes: 0 additions & 22 deletions protocol/v2/ssv/runner/compact.go

This file was deleted.

2 changes: 1 addition & 1 deletion protocol/v2/ssv/runner/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ func (r *ProposerRunner) Decode(data []byte) error {
func (r *ProposerRunner) GetRoot() ([32]byte, error) {
marshaledRoot, err := r.Encode()
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not encode DutyRunnerState")
return [32]byte{}, errors.Wrap(err, "could not encode ProposerRunner")
}
ret := sha256.Sum256(marshaledRoot)
return ret, nil
Expand Down
1 change: 0 additions & 1 deletion protocol/v2/ssv/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ func (b *BaseRunner) baseConsensusMsgProcessing(logger *zap.Logger, runner Runne
}

decidedMsg, err := b.QBFTController.ProcessMsg(logger, msg)
b.compactInstanceIfNeeded(msg)
if err != nil {
return false, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion protocol/v2/ssv/runner/runner_signatures.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (b *BaseRunner) validatePartialSigMsgForSlot(
slot spec.Slot,
) error {
if err := psigMsgs.Validate(); err != nil {
return errors.Wrap(err, "SignedPartialSignatureMessage invalid")
return errors.Wrap(err, "PartialSignatureMessages invalid")
}

if psigMsgs.Slot != slot {
Expand Down
6 changes: 3 additions & 3 deletions protocol/v2/ssv/runner/runner_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (pcs *State) MarshalJSON() ([]byte, error) {
RunningInstance *instance.Instance
DecidedValue []byte
Finished bool
BeaconDuty *spectypes.ValidatorDuty `json:"ValidatorDuty,omitempty"`
ValidatorDuty *spectypes.ValidatorDuty `json:"ValidatorDuty,omitempty"`
CommitteeDuty *spectypes.CommitteeDuty `json:"CommitteeDuty,omitempty"`
}

Expand All @@ -86,8 +86,8 @@ func (pcs *State) MarshalJSON() ([]byte, error) {
}

if pcs.StartingDuty != nil {
if BeaconDuty, ok := pcs.StartingDuty.(*spectypes.ValidatorDuty); ok {
alias.BeaconDuty = BeaconDuty
if ValidatorDuty, ok := pcs.StartingDuty.(*spectypes.ValidatorDuty); ok {
alias.ValidatorDuty = ValidatorDuty
} else if committeeDuty, ok := pcs.StartingDuty.(*spectypes.CommitteeDuty); ok {
alias.CommitteeDuty = committeeDuty
} else {
Expand Down
2 changes: 1 addition & 1 deletion protocol/v2/ssv/runner/sync_committee_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func (r *SyncCommitteeAggregatorRunner) Decode(data []byte) error {
func (r *SyncCommitteeAggregatorRunner) GetRoot() ([32]byte, error) {
marshaledRoot, err := r.Encode()
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not encode DutyRunnerState")
return [32]byte{}, errors.Wrap(err, "could not encode SyncCommitteeAggregatorRunner")
}
ret := sha256.Sum256(marshaledRoot)
return ret, nil
Expand Down
Loading
Loading