Boole hardening: partial-sig size cap, checked runner assertions, role-mapper lockstep test (#2978 items 3, 4, 7)

message/validation/const.go

@@ -51,6 +51,17 @@ const (
 	partialSigMsgTypeSize          = 8 // uint64
 	maxPartialSignatureMsgsSize    = partialSigMsgTypeSize + slotSize + maxPartialSignatureMessages*partialSignatureMsgSize
 	maxEncodedPartialSignatureSize = maxPartialSignatureMsgsSize + maxPartialSignatureMsgsSize/encodingOverheadDivisor + 4
+
+	// preForkMaxPartialSignatureMessages is the pre-boole worst case (RoleCommittee,
+	// min(2*V, V+SYNC_COMMITTEE_SIZE) with the spec's V=1000 bound), matching pre-boole
+	// ssv-spec v1.2.2 maxmsgsize.maxSizePartialSignatureMessages (1512 messages, 217748
+	// bytes). Deliberately above the 1000 enforced before the boole convergence, which
+	// sat slightly below the spec's structural bound. The pinned (post-fork) spec no
+	// longer publishes this constant, so const_test.go guards it against the hardcoded
+	// v1.2.2 value instead.
+	preForkMaxPartialSignatureMessages    = 1512
+	preForkMaxPartialSignatureMsgsSize    = partialSigMsgTypeSize + slotSize + preForkMaxPartialSignatureMessages*partialSignatureMsgSize
+	preForkMaxEncodedPartialSignatureSize = preForkMaxPartialSignatureMsgsSize + preForkMaxPartialSignatureMsgsSize/encodingOverheadDivisor + 4
 )
 
 const (

message/validation/const_test.go

@@ -7,11 +7,22 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// specV122MaxSizePartialSignatureMessages mirrors pre-boole ssv-spec v1.2.2's
+// maxmsgsize.maxSizePartialSignatureMessages (1512 messages). It is unexported there and
+// only one spec version can be pinned, so the value is hardcoded here to guard the
+// pre-fork cap.
+const specV122MaxSizePartialSignatureMessages = 217748
+
 // TestSizeCapsCoverSpecWorstCase guards against our hand-computed size caps
 // drifting below the pinned ssv-spec's worst-case message sizes. If this
 // fails after a spec bump, re-derive the corresponding const.go values.
 func TestSizeCapsCoverSpecWorstCase(t *testing.T) {
 	require.GreaterOrEqual(t, maxEncodedPartialSignatureSize, maxmsgsize.MaxSizeSSVMessageFromPartialSignatureMessages)
 	require.GreaterOrEqual(t, maxEncodedConsensusMsgSize, maxmsgsize.MaxSizeSSVMessageFromQBFTMessage)
 	require.GreaterOrEqual(t, MaxEncodedMsgSize, maxmsgsize.MaxSizeSignedSSVMessageFromQBFTWith2Justification)
+
+	// The pre-fork cap must cover the pre-boole spec's structural worst case but stay
+	// below the post-fork cap (otherwise the fork-aware switch would be pointless).
+	require.GreaterOrEqual(t, preForkMaxEncodedPartialSignatureSize, specV122MaxSizePartialSignatureMessages)
+	require.Less(t, preForkMaxEncodedPartialSignatureSize, maxEncodedPartialSignatureSize)
 }

message/validation/partial_validation.go

@@ -30,10 +30,10 @@ func (mv *messageValidator) validatePartialSignatureMessage(
 ) {
 	ssvMessage := signedSSVMessage.SSVMessage
 
-	if len(ssvMessage.Data) > maxEncodedPartialSignatureSize {
+	if maxSize := mv.currentMaxEncodedPartialSignatureSize(); len(ssvMessage.Data) > maxSize {
 		e := ErrSSVDataTooBig
 		e.got = len(ssvMessage.Data)
-		e.want = maxEncodedPartialSignatureSize
+		e.want = maxSize
 		return nil, e
 	}
 
@@ -79,6 +79,20 @@ func (mv *messageValidator) validatePartialSignatureMessage(
 	return partialSignatureMessages, nil
 }
 
+// currentMaxEncodedPartialSignatureSize returns the acceptance cap for encoded
+// partial-signature message data. The post-fork (boole AggregatorCommittee) worst case is
+// ~5x the pre-fork one, so pre-fork the smaller cap is enforced to keep the decode DoS
+// surface at its pre-boole size. The cap is enforced before decoding, when the message's
+// own slot is not yet known, so unlike the other fork gates in this package the switch is
+// wall-clock based — and flips one epoch before boole activation so that messages for
+// post-fork slots arriving early (clock skew) are never rejected against the smaller cap.
+func (mv *messageValidator) currentMaxEncodedPartialSignatureSize() int {
+	if mv.netCfg.BooleForkAtEpoch(mv.netCfg.EstimatedCurrentEpoch() + 1) {
+		return maxEncodedPartialSignatureSize
+	}
+	return preForkMaxEncodedPartialSignatureSize
+}
+
 func (mv *messageValidator) validatePartialSignatureMessageSemantics(
 	signedSSVMessage *spectypes.SignedSSVMessage,
 	partialSignatureMessages *spectypes.PartialSignatureMessages,

observability/utils/format_test.go

@@ -6,6 +6,7 @@ import (
 	spectypes "github.com/ssvlabs/ssv-spec/types"
 	"github.com/stretchr/testify/require"
 
+	"github.com/ssvlabs/ssv/protocol/v2/message"
 	ssvtypes "github.com/ssvlabs/ssv/protocol/v2/types"
 )
 
@@ -78,3 +79,29 @@ func TestFormatRunnerRole(t *testing.T) {
 		require.NotEqual(t, FormatRunnerRole(ssvtypes.RoleAggregator), FormatRunnerRole(ssvtypes.RoleSyncCommitteeContribution))
 	})
 }
+
+// TestRunnerRoleStringMappersLockstep guards the contract documented on
+// ssvtypes.RunnerRoleToString and message.RunnerRoleToString: the two mappers are
+// independent (one reaches the strings via the spec's String() plus a deprecated-role
+// shim, the other via its own switch) and must produce the same string for every runner
+// role that is valid in any fork. A role added or deprecated in one must be reflected in
+// the other — this test is what fails when they drift.
+func TestRunnerRoleStringMappersLockstep(t *testing.T) {
+	t.Parallel()
+
+	// The full role union across forks, mirroring messageValidator.validRoleUnion.
+	roles := []spectypes.RunnerRole{
+		spectypes.RoleCommittee,
+		spectypes.RoleAggregatorCommittee,
+		spectypes.RoleProposer,
+		spectypes.RoleValidatorRegistration,
+		spectypes.RoleVoluntaryExit,
+		ssvtypes.RoleAggregator,
+		ssvtypes.RoleSyncCommitteeContribution,
+	}
+
+	for _, role := range roles {
+		require.Equal(t, message.RunnerRoleToString(role), FormatRunnerRole(role),
+			"role %d: message.RunnerRoleToString and utils.FormatRunnerRole disagree", role)
+	}
+}

protocol/v2/ssv/validator/committee.go

@@ -563,9 +563,17 @@ func (c *Committee) createRunner(
 
 	switch duty := duty.(type) {
 	case *spectypes.CommitteeDuty:
-		c.Runners[duty.DutySlot()] = r.(*runner.CommitteeRunner)
+		cr, ok := r.(*runner.CommitteeRunner)
+		if !ok {
+			return nil, fmt.Errorf("BUG: runner created for committee duty has type %T, expected *runner.CommitteeRunner", r)
+		}
+		c.Runners[duty.DutySlot()] = cr
 	case *spectypes.AggregatorCommitteeDuty:
-		c.AggregatorRunners[duty.DutySlot()] = r.(*runner.AggregatorCommitteeRunner)
+		ar, ok := r.(*runner.AggregatorCommitteeRunner)
+		if !ok {
+			return nil, fmt.Errorf("BUG: runner created for aggregator committee duty has type %T, expected *runner.AggregatorCommitteeRunner", r)
+		}
+		c.AggregatorRunners[duty.DutySlot()] = ar
 	default:
 		c.logger.Panic("BUG: attempt to create committee runner with non-committee duty type",
 			zap.String("type", fmt.Sprintf("%T", duty)))