From 5abeb8ddd7de97a56cbf1a31fb58bc36cb546918 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Thu, 15 Oct 2026 11:02:55 -0400 Subject: [PATCH 1/4] Validate Max Aggregatable Exponent Consensus --- consensus/oracle/consensus_execution.go | 11 +++--- consensus/oracle/plugin/plugin_outcome.go | 36 +++++++++++++++++++ .../oracle/plugin/plugin_outcome_test.go | 28 +++++++++++++++ 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 05fcc6d4a..509b05137 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -27,10 +27,11 @@ var ( ErrDecimalExponentOutOfRange = errors.New("decimal exponent out of range") ) -// Comparing decimals rescales them to a common exponent, which costs time and memory -// proportional to the exponent gap, so a single observation with an extreme exponent -// could stall every node computing the median. -const maxDecimalExponent = 1000 +// MaxAggregatableExponent bounds the base-10 exponent of an aggregatable value. +// Comparing decimals with different exponents rescales them to a common exponent, +// and that cost grows with the exponent gap. Every in-tree producer reports +// exponent 0; the symmetric window leaves room for scaled values. +const MaxAggregatableExponent = 128 // Constants for type names used in aggregation logic. var ( @@ -258,7 +259,7 @@ func handleMedianAggregation( filtered, func(val *valuespb.Value) (decimal.Decimal, error) { var d decimal.Decimal - if exp := val.GetDecimalValue().GetExponent(); exp > maxDecimalExponent || exp < -maxDecimalExponent { + if exp := val.GetDecimalValue().GetExponent(); exp > MaxAggregatableExponent || exp < -MaxAggregatableExponent { return d, fmt.Errorf("%w: %d", ErrDecimalExponentOutOfRange, exp) } v, err := values.FromProto(val) diff --git a/consensus/oracle/plugin/plugin_outcome.go b/consensus/oracle/plugin/plugin_outcome.go index 028256326..1577f0dd5 100644 --- a/consensus/oracle/plugin/plugin_outcome.go +++ b/consensus/oracle/plugin/plugin_outcome.go @@ -67,6 +67,42 @@ func (r *reportingPlugin) ValidateObservation(ctx context.Context, outctx ocr3ty lggr.Warnw("observation missing input", "requestID", requestID) return fmt.Errorf("observation from observer %d is missing input for request %s", ao.Observer, requestID) } + + if valueInput, ok := reqObs.Input.GetObservation().(*sdk.SimpleConsensusInputs_Value); ok { + if err := validateAggregatableValue(valueInput.Value); err != nil { + lggr.Warnw("observation contains an aggregatable value outside the supported exponent range", "requestID", requestID, "error", err) + return fmt.Errorf("observation from observer %d has invalid value for request %s: %w", ao.Observer, requestID, err) + } + } + } + + return nil +} + +// validateAggregatableValue rejects decimal exponents that could make decimal +// comparisons allocate unbounded amounts of memory during aggregation. +func validateAggregatableValue(value *valuespb.Value) error { + if value == nil { + return nil + } + + switch v := value.Value.(type) { + case *valuespb.Value_DecimalValue: + if exponent := v.DecimalValue.GetExponent(); exponent > oracle.MaxAggregatableExponent || exponent < -oracle.MaxAggregatableExponent { + return fmt.Errorf("decimal exponent out of range: got %d, allowed [%d, %d]", exponent, -oracle.MaxAggregatableExponent, oracle.MaxAggregatableExponent) + } + case *valuespb.Value_MapValue: + for _, field := range v.MapValue.GetFields() { + if err := validateAggregatableValue(field); err != nil { + return err + } + } + case *valuespb.Value_ListValue: + for _, field := range v.ListValue.GetFields() { + if err := validateAggregatableValue(field); err != nil { + return err + } + } } return nil diff --git a/consensus/oracle/plugin/plugin_outcome_test.go b/consensus/oracle/plugin/plugin_outcome_test.go index 750d46fec..786a1c336 100644 --- a/consensus/oracle/plugin/plugin_outcome_test.go +++ b/consensus/oracle/plugin/plugin_outcome_test.go @@ -4,9 +4,11 @@ import ( "context" "errors" "fmt" + "math" "testing" "time" + "github.com/shopspring/decimal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -24,6 +26,7 @@ import ( "github.com/smartcontractkit/chainlink-protos/cre/go/sdk" "github.com/smartcontractkit/chainlink-protos/cre/go/values" + valuespb "github.com/smartcontractkit/chainlink-protos/cre/go/values/pb" "github.com/smartcontractkit/libocr/commontypes" "github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types" @@ -227,6 +230,31 @@ func Test_ValidateObservation_RequestIDConsistency(t *testing.T) { require.ErrorContains(t, err, "not in the query") } +func Test_ValidateObservation_RejectsExtremeDecimalExponent(t *testing.T) { + t.Parallel() + + lggr := logger.Test(t) + reportingPlugin, _ := createReportingPlugin(t, lggr, 2, 7, 5, defaultMaxLengthBytes) + + md := testMetaData() + reqID := md.RequestID() + qBytes, err := proto.Marshal(&oracletypes.Query{RequestIDs: []string{reqID}}) + require.NoError(t, err) + + ao := makeOutcomeTestObs(t, reqID, md, sdk.AggregationType_AGGREGATION_TYPE_MEDIAN, 0, false) + obs := &oracletypes.Observation{} + require.NoError(t, proto.Unmarshal(ao.Observation, obs)) + obs.Observations[reqID].Input.Observation = &sdk.SimpleConsensusInputs_Value{ + Value: valuespb.NewDecimalValue(decimal.New(1, math.MaxInt32)), + } + ao.Observation, err = proto.Marshal(obs) + require.NoError(t, err) + + err = reportingPlugin.ValidateObservation(t.Context(), ocr3types.OutcomeContext{SeqNr: 1}, qBytes, ao) + require.Error(t, err) + require.ErrorContains(t, err, "decimal exponent out of range") +} + // Test_Outcome_PlusOneErrors checks that when f+1 errors are received, Outcome() embeds the // per-field metadata string ("Consensus metadata: requestId=...") and the descriptor type // string ("Descriptor type: AGGREGATION_TYPE_MEDIAN"). From de23ecd59c9bfe7b017acefb9c046e8bbce1d5d2 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Mon, 19 Oct 2026 09:54:11 -0400 Subject: [PATCH 2/4] move check to capability --- consensus/action/capability.go | 4 +++ consensus/oracle/consensus_execution.go | 29 +++++++++++++++++++++ consensus/oracle/plugin/plugin_outcome.go | 31 +---------------------- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/consensus/action/capability.go b/consensus/action/capability.go index 6c0afa0b2..6f1cbea9c 100644 --- a/consensus/action/capability.go +++ b/consensus/action/capability.go @@ -635,6 +635,10 @@ func decodeObservationType(lggr logger.Logger, input *sdk.SimpleConsensusInputs) lggr.Debugw("failed to decode observation value", "err", err) return fmt.Errorf("failed to decode observation value: %w", err) } + if err := oracle.ValidateAggregatableValue(obs.Value); err != nil { + lggr.Debugw("observation value has an unsupported decimal exponent", "err", err) + return fmt.Errorf("invalid observation value: %w", err) + } lggr.Debugw("received observation value") case *sdk.SimpleConsensusInputs_Error: lggr.Debugw("observation is an error") diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 509b05137..038da4d05 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -33,6 +33,35 @@ var ( // exponent 0; the symmetric window leaves room for scaled values. const MaxAggregatableExponent = 128 +// ValidateAggregatableValue rejects decimal exponents that could make decimal +// comparisons allocate unbounded amounts of memory during aggregation. +func ValidateAggregatableValue(value *valuespb.Value) error { + if value == nil { + return nil + } + + switch v := value.Value.(type) { + case *valuespb.Value_DecimalValue: + if exponent := v.DecimalValue.GetExponent(); exponent > MaxAggregatableExponent || exponent < -MaxAggregatableExponent { + return fmt.Errorf("decimal exponent out of range: got %d, allowed [%d, %d]", exponent, -MaxAggregatableExponent, MaxAggregatableExponent) + } + case *valuespb.Value_MapValue: + for _, field := range v.MapValue.GetFields() { + if err := ValidateAggregatableValue(field); err != nil { + return err + } + } + case *valuespb.Value_ListValue: + for _, field := range v.ListValue.GetFields() { + if err := ValidateAggregatableValue(field); err != nil { + return err + } + } + } + + return nil +} + // Constants for type names used in aggregation logic. var ( typeInt64 = reflect.TypeFor[*valuespb.Value_Int64Value]() diff --git a/consensus/oracle/plugin/plugin_outcome.go b/consensus/oracle/plugin/plugin_outcome.go index 1577f0dd5..963640d50 100644 --- a/consensus/oracle/plugin/plugin_outcome.go +++ b/consensus/oracle/plugin/plugin_outcome.go @@ -69,7 +69,7 @@ func (r *reportingPlugin) ValidateObservation(ctx context.Context, outctx ocr3ty } if valueInput, ok := reqObs.Input.GetObservation().(*sdk.SimpleConsensusInputs_Value); ok { - if err := validateAggregatableValue(valueInput.Value); err != nil { + if err := oracle.ValidateAggregatableValue(valueInput.Value); err != nil { lggr.Warnw("observation contains an aggregatable value outside the supported exponent range", "requestID", requestID, "error", err) return fmt.Errorf("observation from observer %d has invalid value for request %s: %w", ao.Observer, requestID, err) } @@ -79,35 +79,6 @@ func (r *reportingPlugin) ValidateObservation(ctx context.Context, outctx ocr3ty return nil } -// validateAggregatableValue rejects decimal exponents that could make decimal -// comparisons allocate unbounded amounts of memory during aggregation. -func validateAggregatableValue(value *valuespb.Value) error { - if value == nil { - return nil - } - - switch v := value.Value.(type) { - case *valuespb.Value_DecimalValue: - if exponent := v.DecimalValue.GetExponent(); exponent > oracle.MaxAggregatableExponent || exponent < -oracle.MaxAggregatableExponent { - return fmt.Errorf("decimal exponent out of range: got %d, allowed [%d, %d]", exponent, -oracle.MaxAggregatableExponent, oracle.MaxAggregatableExponent) - } - case *valuespb.Value_MapValue: - for _, field := range v.MapValue.GetFields() { - if err := validateAggregatableValue(field); err != nil { - return err - } - } - case *valuespb.Value_ListValue: - for _, field := range v.ListValue.GetFields() { - if err := validateAggregatableValue(field); err != nil { - return err - } - } - } - - return nil -} - func (r *reportingPlugin) Outcome(ctx context.Context, outctx ocr3types.OutcomeContext, query types.Query, attributedObservations []types.AttributedObservation) (ocr3types.Outcome, error) { lggr := logger.With(r.lggr, "seqNr", outctx.SeqNr) From e9623df43244e2342c870cb21290deeadddafd69 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Mon, 19 Oct 2026 10:02:10 -0400 Subject: [PATCH 3/4] remove redundant check --- consensus/oracle/consensus_execution.go | 4 ---- consensus/oracle/consensus_execution_test.go | 11 ----------- 2 files changed, 15 deletions(-) diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 038da4d05..956ad7ac4 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -24,7 +24,6 @@ var ( ErrMoreThanOneValidOutcomeForIdenticalConsensus = errors.New("not identical, multiple values with f+1 occurrences") ErrInsufficientObservations = errors.New("insufficient observations to reach consensus") ErrNoSingleValueTypeMeetsThreshold = errors.New("no single value type meets the minimum observation threshold") - ErrDecimalExponentOutOfRange = errors.New("decimal exponent out of range") ) // MaxAggregatableExponent bounds the base-10 exponent of an aggregatable value. @@ -288,9 +287,6 @@ func handleMedianAggregation( filtered, func(val *valuespb.Value) (decimal.Decimal, error) { var d decimal.Decimal - if exp := val.GetDecimalValue().GetExponent(); exp > MaxAggregatableExponent || exp < -MaxAggregatableExponent { - return d, fmt.Errorf("%w: %d", ErrDecimalExponentOutOfRange, exp) - } v, err := values.FromProto(val) if err != nil { return d, err diff --git a/consensus/oracle/consensus_execution_test.go b/consensus/oracle/consensus_execution_test.go index 111be2e08..12cdd6d03 100644 --- a/consensus/oracle/consensus_execution_test.go +++ b/consensus/oracle/consensus_execution_test.go @@ -440,17 +440,6 @@ func Test_handleMedianAggregation(t *testing.T) { expectedError: errors.New("failed to calculate decimal median: insufficient observations to reach consensus"), f: 1, }, - { - name: "decimal median: extreme exponent is excluded", - observations: []*valuespb.Value{ - valuespb.NewDecimalValue(decimal.New(1, 0)), - valuespb.NewDecimalValue(decimal.New(2, 0)), - valuespb.NewDecimalValue(decimal.New(3, 0)), - {Value: &valuespb.Value_DecimalValue{DecimalValue: &valuespb.Decimal{Coefficient: valuespb.NewBigIntFromInt(big.NewInt(1)), Exponent: math.MaxInt32}}}, - }, - expectedOutcome: valuespb.NewDecimalValue(decimal.New(2, 0)), - f: 1, - }, { name: "bigint median: basic five values", observations: []*valuespb.Value{ From 1d118a69c1e33f2922052f8c3f5d4a8e37cf8a15 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Mon, 19 Oct 2026 10:03:15 -0400 Subject: [PATCH 4/4] add validation helper --- consensus/action/capability.go | 18 ++++++++++++++---- consensus/oracle/consensus_execution.go | 4 ++++ consensus/oracle/consensus_execution_test.go | 11 +++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/consensus/action/capability.go b/consensus/action/capability.go index 6f1cbea9c..28f391e27 100644 --- a/consensus/action/capability.go +++ b/consensus/action/capability.go @@ -254,6 +254,10 @@ func (c *consensusCapability) Simple(ctx context.Context, metadata capabilities. responseAndMetadata := capabilities.ResponseAndMetadata[*valuespb.Value]{} return &responseAndMetadata, caperrors.NewPublicSystemError(fmt.Errorf("failed to decode observation: %s", err), caperrors.InvalidArgument) } + if err := validateSimpleConsensusInput(input); err != nil { + responseAndMetadata := capabilities.ResponseAndMetadata[*valuespb.Value]{} + return &responseAndMetadata, caperrors.NewPublicSystemError(fmt.Errorf("invalid consensus input: %s", err), caperrors.InvalidArgument) + } lggr.Debugw("received simple consensus request", "metadata", metadata) @@ -635,10 +639,6 @@ func decodeObservationType(lggr logger.Logger, input *sdk.SimpleConsensusInputs) lggr.Debugw("failed to decode observation value", "err", err) return fmt.Errorf("failed to decode observation value: %w", err) } - if err := oracle.ValidateAggregatableValue(obs.Value); err != nil { - lggr.Debugw("observation value has an unsupported decimal exponent", "err", err) - return fmt.Errorf("invalid observation value: %w", err) - } lggr.Debugw("received observation value") case *sdk.SimpleConsensusInputs_Error: lggr.Debugw("observation is an error") @@ -656,3 +656,13 @@ func decodeObservationType(lggr logger.Logger, input *sdk.SimpleConsensusInputs) } return nil } + +func validateSimpleConsensusInput(input *sdk.SimpleConsensusInputs) error { + if valueInput, ok := input.GetObservation().(*sdk.SimpleConsensusInputs_Value); ok { + if err := oracle.ValidateAggregatableValue(valueInput.Value); err != nil { + return fmt.Errorf("invalid observation value: %w", err) + } + } + + return nil +} diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 956ad7ac4..038da4d05 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -24,6 +24,7 @@ var ( ErrMoreThanOneValidOutcomeForIdenticalConsensus = errors.New("not identical, multiple values with f+1 occurrences") ErrInsufficientObservations = errors.New("insufficient observations to reach consensus") ErrNoSingleValueTypeMeetsThreshold = errors.New("no single value type meets the minimum observation threshold") + ErrDecimalExponentOutOfRange = errors.New("decimal exponent out of range") ) // MaxAggregatableExponent bounds the base-10 exponent of an aggregatable value. @@ -287,6 +288,9 @@ func handleMedianAggregation( filtered, func(val *valuespb.Value) (decimal.Decimal, error) { var d decimal.Decimal + if exp := val.GetDecimalValue().GetExponent(); exp > MaxAggregatableExponent || exp < -MaxAggregatableExponent { + return d, fmt.Errorf("%w: %d", ErrDecimalExponentOutOfRange, exp) + } v, err := values.FromProto(val) if err != nil { return d, err diff --git a/consensus/oracle/consensus_execution_test.go b/consensus/oracle/consensus_execution_test.go index 12cdd6d03..111be2e08 100644 --- a/consensus/oracle/consensus_execution_test.go +++ b/consensus/oracle/consensus_execution_test.go @@ -440,6 +440,17 @@ func Test_handleMedianAggregation(t *testing.T) { expectedError: errors.New("failed to calculate decimal median: insufficient observations to reach consensus"), f: 1, }, + { + name: "decimal median: extreme exponent is excluded", + observations: []*valuespb.Value{ + valuespb.NewDecimalValue(decimal.New(1, 0)), + valuespb.NewDecimalValue(decimal.New(2, 0)), + valuespb.NewDecimalValue(decimal.New(3, 0)), + {Value: &valuespb.Value_DecimalValue{DecimalValue: &valuespb.Decimal{Coefficient: valuespb.NewBigIntFromInt(big.NewInt(1)), Exponent: math.MaxInt32}}}, + }, + expectedOutcome: valuespb.NewDecimalValue(decimal.New(2, 0)), + f: 1, + }, { name: "bigint median: basic five values", observations: []*valuespb.Value{