llo: proto > 0 accepts zero default report cadence on v30, require it on v31

llo/dev/v31/factory.go

@@ -107,6 +107,15 @@ func (f *PluginFactory) NewReportingPlugin(ctx context.Context, cfg ocr3types.Re
 			aggregationFaultTolerance, cfg.F, 2*aggregationFaultTolerance+1, 2*cfg.F+1)
 	}
 
+	// The report cadence is a replicated state transition parameter and gates
+	// every channel (see isReportable). protocol.OffchainConfig.Validate used to
+	// require it to be non-zero on protocol versions 1 and 2 and no longer does,
+	// because v30 could be configured with a zero cadence and fallback to the
+	// channel cadence. v31 enforces the the original rule rather than inheriting that leniency.
+	if offchainConfig.ProtocolVersion > 0 && offchainConfig.DefaultMinReportIntervalNanoseconds == 0 {
+		return nil, nil, fmt.Errorf("NewReportingPlugin: offchain config must set defaultMinReportIntervalNanoseconds to a non-zero value if protocol version is %d", offchainConfig.ProtocolVersion)
+	}
+
 	blobInFlightWaitFactor := f.BlobInFlightWaitFactor
 	if blobInFlightWaitFactor == 0 {
 		blobInFlightWaitFactor = defaultBlobInFlightWaitFactor

llo/dev/v31/flow_test.go

@@ -3,6 +3,7 @@ package llo
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math"
 	"sync"
 	"testing"
@@ -236,6 +237,56 @@ func Test_Factory_NewReportingPlugin_aggregationFaultTolerance(t *testing.T) {
 	})
 }
 
+func Test_Factory_NewReportingPlugin_reportCadence(t *testing.T) {
+	ctx := tests.Context(t)
+	f := NewPluginFactory(PluginFactoryParams{
+		OnchainConfigCodec: mockOnchainConfigCodec{},
+		Logger:             logger.Test(t),
+	})
+
+	encode := func(t *testing.T, protocolVersion uint32, cadence uint64) []byte {
+		t.Helper()
+		var aggregationFaultTolerance uint32 = 1
+		b, err := protocol.OffchainConfig{
+			ProtocolVersion:                     protocolVersion,
+			DefaultMinReportIntervalNanoseconds: cadence,
+			AggregationFaultTolerance:           &aggregationFaultTolerance,
+		}.Encode()
+		require.NoError(t, err)
+		return b
+	}
+
+	newPlugin := func(t *testing.T, offchainConfig []byte) error {
+		t.Helper()
+		p, _, err := f.NewReportingPlugin(ctx, ocr3types.ReportingPluginConfig{N: 4, F: 1, ConfigDigest: ocrtypes.ConfigDigest{9}, OffchainConfig: offchainConfig}, nil)
+		if err == nil {
+			require.NoError(t, p.Close())
+		}
+		return err
+	}
+
+	// protocol.OffchainConfig.Validate accepts a zero cadence so that v30 groups
+	// already configured that way keep running. v31 does not.
+	t.Run("refuses to start on a zero cadence", func(t *testing.T) {
+		for _, protocolVersion := range []uint32{1, 2} {
+			err := newPlugin(t, encode(t, protocolVersion, 0))
+			require.EqualError(t, err, fmt.Sprintf("NewReportingPlugin: offchain config must set defaultMinReportIntervalNanoseconds to a non-zero value if protocol version is %d", protocolVersion))
+		}
+	})
+
+	t.Run("accepts a non-zero cadence", func(t *testing.T) {
+		for _, protocolVersion := range []uint32{1, 2} {
+			require.NoError(t, newPlugin(t, encode(t, protocolVersion, 1000)))
+		}
+	})
+
+	// Validate requires a zero cadence on protocol version 0, so the v31 rule
+	// must not contradict it.
+	t.Run("accepts a zero cadence on protocol version 0", func(t *testing.T) {
+		require.NoError(t, newPlugin(t, encode(t, 0, 0)))
+	})
+}
+
 func Test_Factory_MaxRoundPeriod(t *testing.T) {
 	ctx := tests.Context(t)
 	base := ocr3types.ReportingPluginConfig{N: 4, F: 1, ConfigDigest: ocrtypes.ConfigDigest{9}, OffchainConfig: mustEncodeOffchainConfig(t, 1)}

llo/dev/v31/reports.go

@@ -257,8 +257,9 @@ func (o precursor) formatIsEncodable(format llotypes.ReportFormat, f int, channe
 // saturatingAdd returns a+b, clamped to MaxUint64 instead of wrapping.
 //
 // validAfter is a nanosecond wall-clock timestamp, so it already sits around
-// 1.7e18, and the offchain config bounds DefaultMinReportIntervalNanoseconds
-// only away from zero (see protocol.OffchainConfig.Validate). A large enough
+// 1.7e18, and the offchain config puts no upper bound on
+// DefaultMinReportIntervalNanoseconds (see protocol.OffchainConfig.Validate).
+// A large enough
 // interval would wrap the sum to a small number, the cadence comparison would
 // then always pass, and the interval would silently stop gating anything. The
 // config is replicated, so every oracle would do it identically: a silent loss

llo/protocol/offchain_config.go

@@ -80,9 +80,17 @@ func (c OffchainConfig) Validate() error {
 		// Version 2 is version 1 with a channel vote hash that commits to every
 		// field of the channel definition (see protocol.ChannelHashV2).
 		// Nothing else differs, so the cadence rules are the same.
-		if c.DefaultMinReportIntervalNanoseconds == 0 {
-			return fmt.Errorf("default report cadence must be non-zero if protocol version is %d", c.ProtocolVersion)
-		}
+		//
+		// A zero cadence is accepted. Validate on v30 used to run on the
+		// zero-valued struct before the decoded fields were assigned, so
+		// this rule never fired and groups were configured on-chain with
+		// version 1 and a zero cadence. Those groups have been running with
+		// the minimum-interval gate inert, but using the channels report
+		// cadence. Rejecting the config now would stop every node in them.
+		// Zero keeps meaning zero so that an upgraded node reports exactly
+		// what its not-yet-upgraded peers report. Raising the cadence is a
+		// config change, which lands atomically on a new config digest.
+		// v31 Factory checks this strictly.
 	default:
 		return fmt.Errorf("unknown protocol version: %d", c.ProtocolVersion)
 	}

llo/protocol/offchain_config_test.go

@@ -123,15 +123,13 @@ func Test_OffchainConfig(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, cfg, cfgDecoded)
 		})
-		t.Run("DefaultMinReportIntervalNanoseconds=0 is invalid", func(t *testing.T) {
+		t.Run("DefaultMinReportIntervalNanoseconds=0 is accepted and stays 0", func(t *testing.T) {
 			cfg := OffchainConfig{
 				ProtocolVersion:                     2,
 				DefaultMinReportIntervalNanoseconds: 0,
 			}
 
-			err := cfg.Validate()
-			require.Error(t, err)
-			assert.Contains(t, err.Error(), "default report cadence must be non-zero if protocol version is 2")
+			require.NoError(t, cfg.Validate())
 		})
 	})
 	t.Run("decoding rejects an unknown protocol version", func(t *testing.T) {
@@ -161,14 +159,34 @@ func Test_OffchainConfig(t *testing.T) {
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "default report cadence must be 0 if protocol version is 0")
 	})
-	t.Run("DefaultMinReportIntervalNanoseconds=0 is invalid", func(t *testing.T) {
+	t.Run("DefaultMinReportIntervalNanoseconds=0 decodes to 0", func(t *testing.T) {
+		// Groups are configured on-chain with version 1 and a zero cadence,
+		// from when Validate ran before the decoded fields were assigned. The
+		// value must survive decoding unchanged so an upgraded node reports
+		// exactly what its not-yet-upgraded peers report.
 		cfg := OffchainConfig{
 			ProtocolVersion:                     1,
 			DefaultMinReportIntervalNanoseconds: 0,
+			EnableObservationCompression:        true,
 		}
+		require.NoError(t, cfg.Validate())
 
-		err := cfg.Validate()
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "default report cadence must be non-zero if protocol version is 1")
+		b, err := cfg.Encode()
+		require.NoError(t, err)
+
+		decoded, err := DecodeOffchainConfig(b)
+		require.NoError(t, err)
+		assert.Equal(t, cfg, decoded)
+		assert.Zero(t, decoded.DefaultMinReportIntervalNanoseconds)
+	})
+
+	t.Run("the config observed on running v30 groups decodes", func(t *testing.T) {
+		// protocolVersion=1, defaultMinReportIntervalNanoseconds=0,
+		// enableObservationCompression=true, as written on-chain.
+		decoded, err := DecodeOffchainConfig([]byte{0x08, 0x01, 0x10, 0x00, 0x18, 0x01})
+		require.NoError(t, err)
+		assert.Equal(t, uint32(1), decoded.ProtocolVersion)
+		assert.Zero(t, decoded.DefaultMinReportIntervalNanoseconds)
+		assert.True(t, decoded.EnableObservationCompression)
 	})
 }