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,26 @@ 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) {
+	// The post-fork cap is compared against the spec's full-SSVMessage-envelope constant,
+	// which is over-conservative: the cap applies to SSVMessage.Data, the inner encoding.
+	// The pre-fork guard below compares against the inner v1.2.2 constant instead — the
+	// only partial-signature size constant that spec version published.
 	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,23 @@ func (mv *messageValidator) validatePartialSignatureMessage(
 	return partialSignatureMessages, nil
 }
 
+// currentMaxEncodedPartialSignatureSize returns the acceptance cap for encoded
+// partial-signature message data (SSVMessage.Data). The post-fork (boole
+// AggregatorCommittee) worst case is ~3.3x the pre-fork one, so pre-fork the smaller cap
+// is enforced, bounding the inner PartialSignatureMessages decode at the pre-fork worst
+// case (~229 KB vs ~763 KB; the outer SignedSSVMessage decode that already happened is
+// bounded separately by MaxEncodedMsgSize). 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 (mirroring
+// SIP-43's one-epoch prior window) 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,

message/validation/partial_validation_test.go

@@ -0,0 +1,62 @@
+package validation
+
+import (
+	"math"
+	"testing"
+
+	"github.com/attestantio/go-eth2-client/spec/phase0"
+	"github.com/stretchr/testify/require"
+
+	"github.com/ssvlabs/ssv/networkconfig"
+)
+
+// TestCurrentMaxEncodedPartialSignatureSize pins the fork gate of the pre-decode
+// partial-signature size cap: the pre-fork cap applies while boole is unscheduled or more
+// than one epoch away, and the post-fork cap applies from one epoch before activation
+// (the early flip that protects boundary messages) onward.
+func TestCurrentMaxEncodedPartialSignatureSize(t *testing.T) {
+	t.Parallel()
+
+	cfgWithBoole := func(booleEpoch phase0.Epoch) *networkconfig.Network {
+		ssv := *networkconfig.TestNetwork.SSV
+		ssv.Forks = networkconfig.SSVForks{Boole: booleEpoch}
+		return &networkconfig.Network{Beacon: networkconfig.TestNetwork.Beacon, SSV: &ssv}
+	}
+	currentEpoch := networkconfig.TestNetwork.EstimatedCurrentEpoch()
+
+	testCases := []struct {
+		name  string
+		boole phase0.Epoch
+		want  int
+	}{
+		{
+			name:  "unscheduled fork keeps the pre-fork cap",
+			boole: math.MaxUint64,
+			want:  preForkMaxEncodedPartialSignatureSize,
+		},
+		{
+			name:  "fork two epochs away keeps the pre-fork cap",
+			boole: currentEpoch + 2,
+			want:  preForkMaxEncodedPartialSignatureSize,
+		},
+		{
+			name:  "cap flips one epoch before activation",
+			boole: currentEpoch + 1,
+			want:  maxEncodedPartialSignatureSize,
+		},
+		{
+			name:  "active fork uses the post-fork cap",
+			boole: 0,
+			want:  maxEncodedPartialSignatureSize,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			mv := &messageValidator{netCfg: cfgWithBoole(tc.boole)}
+			require.Equal(t, tc.want, mv.currentMaxEncodedPartialSignatureSize())
+		})
+	}
+}

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,44 @@ 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)
+	}
+
+	// Sweep beyond the explicit list so a role added to the spec — which FormatRunnerRole
+	// picks up automatically via (RunnerRole).String() but message.RunnerRoleToString's
+	// hand-written switch would miss — fails here instead of drifting silently. Roles the
+	// spec does not know return "UNDEFINED" and are skipped: divergence on genuinely
+	// unknown values is intentional (the deprecated Alan roles also stringify to
+	// "UNDEFINED" in the spec, but they are covered by the explicit list above).
+	for i := 0; i <= 15; i++ {
+		role := spectypes.RunnerRole(i)
+		if role.String() == "UNDEFINED" {
+			continue
+		}
+		require.Equal(t, message.RunnerRoleToString(role), FormatRunnerRole(role),
+			"role %d is known to the spec but the two mappers disagree", role)
+	}
+}

protocol/v2/ssv/validator/committee.go

@@ -550,6 +550,9 @@ func (c *Committee) createRunner(
 	if err != nil {
 		return nil, fmt.Errorf("create committee runner: %w", err)
 	}
+	if r == nil {
+		return nil, fmt.Errorf("BUG: CreateRunnerFn returned nil runner without error")
+	}
 
 	// Wire the QBFT round-timer factory, bound to a msg ID carrying this duty's role so timeout
 	// events are routed to the matching (committee vs aggregator-committee) slot queue.
@@ -563,15 +566,23 @@ 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)))
 	}
 
-	return r, err
+	return r, nil
 }
 
 func (c *Committee) extractValidatorDuties(duty spectypes.Duty) []*spectypes.ValidatorDuty {