fix: force start duties in ValidatorRegistrationRunner (#1188)

* fix: force start duties in `ValidatorRegistrationRunner`

* update to spec PR

* update spec JSONs


---------

Co-authored-by: Lior Rutenberg <liorr@blox.io>

go.mod

@@ -223,5 +223,8 @@ replace github.com/google/flatbuffers => github.com/google/flatbuffers v1.11.0
 
 replace github.com/dgraph-io/ristretto => github.com/dgraph-io/ristretto v0.1.1-0.20211108053508-297c39e6640f
 
-//TODO remove this replace when the following PR is merged https://github.com/bloxapp/eth2-key-manager/pull/100
+// TODO: remove this replace when the following PR is merged https://github.com/bloxapp/eth2-key-manager/pull/100
 replace github.com/bloxapp/eth2-key-manager => github.com/bloxapp/eth2-key-manager v1.3.2-0.20231022162227-e2b8264a29a5
+
+// TODO: remove this replace when the following PR is merged https://github.com/bloxapp/ssv-spec/pull
+replace github.com/bloxapp/ssv-spec => github.com/moshe-blox/ssv-spec v0.0.0-20231105135956-a64e63f6e35a

go.sum

@@ -56,8 +56,6 @@ github.com/bits-and-blooms/bitset v1.7.0 h1:YjAGVd3XmtK9ktAbX8Zg2g2PwLIMjGREZJHl
 github.com/bits-and-blooms/bitset v1.7.0/go.mod h1:gIdJ4wp64HaoK2YrL1Q5/N7Y16edYb8uY+O0FJTyyDA=
 github.com/bloxapp/eth2-key-manager v1.3.2-0.20231022162227-e2b8264a29a5 h1:vjrMmMH15Bo0QF+228CuEZvCI+OuPyJRco82Gj/WyTI=
 github.com/bloxapp/eth2-key-manager v1.3.2-0.20231022162227-e2b8264a29a5/go.mod h1:cT+qAJfnAzNz9StFoHQ8xAkyU2eyEukd6xfxvcBWuZA=
-github.com/bloxapp/ssv-spec v0.3.3 h1:iNomqWQjxDDQouHMjl27PmH1hUolJ4u8QQ+HX/TQQcg=
-github.com/bloxapp/ssv-spec v0.3.3/go.mod h1:zPJR7YnG5iZ6I0h6EzfVly8bTBXaZwcx4TyJ8pzYVd8=
 github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g=
 github.com/btcsuite/btcd/btcec/v2 v2.3.2 h1:5n0X6hX0Zk+6omWcihdYvdAlGf2DfasC0GMf7DClJ3U=
 github.com/btcsuite/btcd/btcec/v2 v2.3.2/go.mod h1:zYzJ8etWJQIv1Ogk7OzpWjowwOdXY1W/17j2MW85J04=
@@ -521,6 +519,8 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
 github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw=
+github.com/moshe-blox/ssv-spec v0.0.0-20231105135956-a64e63f6e35a h1:I9d5JgkAFj3twwA3D7KkMxvp1wvMnkScmSGDvlf8J7o=
+github.com/moshe-blox/ssv-spec v0.0.0-20231105135956-a64e63f6e35a/go.mod h1:zPJR7YnG5iZ6I0h6EzfVly8bTBXaZwcx4TyJ8pzYVd8=
 github.com/moul/http2curl v1.0.0/go.mod h1:8UbvGypXm98wA/IqH45anm5Y2Z6ep6O31QGOAZ3H0fQ=
 github.com/mr-tron/base58 v1.1.2/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc=
 github.com/mr-tron/base58 v1.1.3/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc=

protocol/v2/ssv/runner/runner.go

@@ -271,9 +271,10 @@ func (b *BaseRunner) hasRunningDuty() bool {
 }
 
 func (b *BaseRunner) ShouldProcessDuty(duty *spectypes.Duty) error {
-	if b.QBFTController.Height >= specqbft.Height(duty.Slot) {
+	// assume StartingDuty is not nil if state is not nil
+	if b.State != nil && b.State.StartingDuty.Slot >= duty.Slot {
 		return errors.Errorf("duty for slot %d already passed. Current height is %d", duty.Slot,
-			b.QBFTController.Height)
+			b.State.StartingDuty.Slot)
 	}
 	return nil
 }

protocol/v2/ssv/runner/validator_registration.go

@@ -54,13 +54,7 @@ func NewValidatorRegistrationRunner(
 }
 
 func (r *ValidatorRegistrationRunner) StartNewDuty(logger *zap.Logger, duty *spectypes.Duty) error {
-	// Note: Unlike the other runners, this doesn't call BaseRunner.baseStartNewDuty because
-	// that requires a QBFTController which ValidatorRegistrationRunner doesn't have.
-	if r.HasRunningDuty() {
-		return errors.New("already running duty")
-	}
-	r.BaseRunner.baseSetupForNewDuty(duty)
-	return r.executeDuty(logger, duty)
+	return r.BaseRunner.baseStartNewDuty(logger, r, duty)
 }
 
 // HasRunningDuty returns true if a duty is already running (StartNewDuty called and returned nil)

protocol/v2/ssv/runner/voluntary_exit.go

@@ -50,13 +50,7 @@ func NewVoluntaryExitRunner(
 }
 
 func (r *VoluntaryExitRunner) StartNewDuty(logger *zap.Logger, duty *spectypes.Duty) error {
-	// Note: Unlike the other runners, this doesn't call BaseRunner.baseStartNewDuty because
-	// that requires a QBFTController which VoluntaryExitRunner doesn't have.
-	if r.HasRunningDuty() {
-		return errors.New("already running duty")
-	}
-	r.BaseRunner.baseSetupForNewDuty(duty)
-	return r.executeDuty(logger, duty)
+	return r.BaseRunner.baseStartNewDuty(logger, r, duty)
 }
 
 // HasRunningDuty returns true if a duty is already running (StartNewDuty called and returned nil)

protocol/v2/ssv/spectest/ssv_mapping_test.go

@@ -175,11 +175,13 @@ func newRunnerDutySpecTestFromMap(t *testing.T, m map[string]interface{}) *Start
 	require.NoError(t, json.Unmarshal(byts, duty))
 
 	outputMsgs := make([]*spectypes.SignedPartialSignatureMessage, 0)
-	for _, msg := range m["OutputMessages"].([]interface{}) {
-		byts, _ = json.Marshal(msg)
-		typedMsg := &spectypes.SignedPartialSignatureMessage{}
-		require.NoError(t, json.Unmarshal(byts, typedMsg))
-		outputMsgs = append(outputMsgs, typedMsg)
+	if v, ok := m["OutputMessages"].([]interface{}); ok {
+		for _, msg := range v {
+			byts, _ = json.Marshal(msg)
+			typedMsg := &spectypes.SignedPartialSignatureMessage{}
+			require.NoError(t, json.Unmarshal(byts, typedMsg))
+			outputMsgs = append(outputMsgs, typedMsg)
+		}
 	}
 
 	ks := testingutils.KeySetForShare(&spectypes.Share{Quorum: uint64(baseRunnerMap["Share"].(map[string]interface{})["Quorum"].(float64))})