From c94b5dd5312cc75fe2983bdb84a8bfa90d327872 Mon Sep 17 00:00:00 2001 From: Bruno Moura Date: Fri, 2 Oct 2026 09:05:11 +0100 Subject: [PATCH] llo: proto > 0 accepts zero default report cadence on v30, require it on v31 --- llo/dev/v31/factory.go | 9 +++++ llo/dev/v31/flow_test.go | 51 ++++++++++++++++++++++++++++ llo/dev/v31/reports.go | 5 +-- llo/protocol/offchain_config.go | 14 ++++++-- llo/protocol/offchain_config_test.go | 34 ++++++++++++++----- 5 files changed, 100 insertions(+), 13 deletions(-) diff --git a/llo/dev/v31/factory.go b/llo/dev/v31/factory.go index e8a7908..1c72f5c 100644 --- a/llo/dev/v31/factory.go +++ b/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 diff --git a/llo/dev/v31/flow_test.go b/llo/dev/v31/flow_test.go index e763c1a..a5ad0cc 100644 --- a/llo/dev/v31/flow_test.go +++ b/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)} diff --git a/llo/dev/v31/reports.go b/llo/dev/v31/reports.go index 163fd16..faf29b6 100644 --- a/llo/dev/v31/reports.go +++ b/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 diff --git a/llo/protocol/offchain_config.go b/llo/protocol/offchain_config.go index 7dad0d8..5f1282a 100644 --- a/llo/protocol/offchain_config.go +++ b/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) } diff --git a/llo/protocol/offchain_config_test.go b/llo/protocol/offchain_config_test.go index e7f11cf..6f92142 100644 --- a/llo/protocol/offchain_config_test.go +++ b/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) }) }