From 8aab8c358092c578a9c2022daa82305874a47ccc Mon Sep 17 00:00:00 2001 From: Matthew Pendrey Date: Wed, 29 Oct 2025 17:39:47 +0000 Subject: [PATCH] make type filtering consistent with f+1 threshold used in other consensus checks --- consensus/oracle/consensus_execution.go | 32 ++++++++------------ consensus/oracle/consensus_execution_test.go | 27 ++++++++++++----- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/consensus/oracle/consensus_execution.go b/consensus/oracle/consensus_execution.go index 294584010..7f7b57ef4 100644 --- a/consensus/oracle/consensus_execution.go +++ b/consensus/oracle/consensus_execution.go @@ -42,10 +42,9 @@ func CalculateOutcomeForObservations( observations []*valuespb.Value, consensusDescriptor *sdk.ConsensusDescriptor, defaultValue *valuespb.Value, - minObservations int, f int, ) (*valuespb.Value, error) { - filtered, _, err := filterObservations(observations, minObservations) + filtered, _, err := filterObservations(observations, f+1) if err != nil { return nil, err } @@ -456,12 +455,7 @@ func filterObservations(observationProtos []*valuespb.Value, minObservations int return nil, nil, fmt.Errorf("insufficient observations (%d) to meet minimum (%d)", len(observationProtos), minObservations) } - var dominantType reflect.Type - var highestCount int - var highestCountEqual bool - observationsByType := map[reflect.Type][]*valuespb.Value{} - for _, observation := range observationProtos { if observation.Value == nil { continue @@ -469,22 +463,22 @@ func filterObservations(observationProtos []*valuespb.Value, minObservations int tpe := reflect.TypeOf(observation.Value) observationsByType[tpe] = append(observationsByType[tpe], observation) - count := len(observationsByType[tpe]) - if count > highestCount { - highestCount = count - dominantType = tpe - highestCountEqual = false - } else if count == highestCount { - highestCountEqual = true - } } - if highestCount < minObservations { - return nil, nil, fmt.Errorf("no single type met the minimum observation threshold of %d", minObservations) + var dominantType reflect.Type + for tpe, obsOfType := range observationsByType { + if len(obsOfType) >= minObservations { + if dominantType == nil { + dominantType = tpe + } else { + // More than one type meets the threshold + return nil, nil, ErrMultipleValuesMetThreshold + } + } } - if highestCountEqual { - return nil, nil, ErrMultipleValuesMetThreshold + if dominantType == nil { + return nil, nil, ErrNoValuesMetThreshold } return observationsByType[dominantType], dominantType, nil diff --git a/consensus/oracle/consensus_execution_test.go b/consensus/oracle/consensus_execution_test.go index 7191b415f..91505509f 100644 --- a/consensus/oracle/consensus_execution_test.go +++ b/consensus/oracle/consensus_execution_test.go @@ -25,7 +25,6 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { observations []*valuespb.Value descriptor *sdk.ConsensusDescriptor defaultValue *valuespb.Value - minObs int f int expectedOutcome *valuespb.Value expectedError error @@ -43,7 +42,7 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { Aggregation: sdk.AggregationType_AGGREGATION_TYPE_MEDIAN, }, }, - minObs: 3, + f: 2, expectedOutcome: nil, expectedError: errors.New("insufficient observations"), }, @@ -61,7 +60,7 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { Aggregation: sdk.AggregationType_AGGREGATION_TYPE_MEDIAN, }, }, - minObs: 5, + f: 4, expectedOutcome: values.Proto(values.NewInt64(30)), expectedError: nil, }, @@ -80,13 +79,28 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { Aggregation: sdk.AggregationType_AGGREGATION_TYPE_IDENTICAL, }, }, - minObs: 5, f: 3, expectedOutcome: values.Proto(values.NewInt64(42)), expectedError: nil, }, { - name: "median: mixed types, one dominant (int64) - handled by filtering", + name: "median: mixed types, two eligible types (int64, float64) - error returned", + f: 1, + observations: []*valuespb.Value{ + values.Proto(values.NewInt64(10)), values.Proto(values.NewFloat64(1.0)), + values.Proto(values.NewInt64(20)), values.Proto(values.NewFloat64(2.0)), + values.Proto(values.NewInt64(30)), values.Proto(values.NewInt64(40)), + }, + descriptor: &sdk.ConsensusDescriptor{ + Descriptor_: &sdk.ConsensusDescriptor_Aggregation{ + Aggregation: sdk.AggregationType_AGGREGATION_TYPE_MEDIAN, + }, + }, + expectedError: ErrMultipleValuesMetThreshold, + }, + { + name: "median: mixed types, one eligible type (int64) - handled by filtering", + f: 2, observations: []*valuespb.Value{ values.Proto(values.NewInt64(10)), values.Proto(values.NewFloat64(1.0)), values.Proto(values.NewInt64(20)), values.Proto(values.NewFloat64(2.0)), @@ -155,7 +169,6 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { mustWrap(s{Val: 50, OtherField: "common", PrefixSlice: []int64{1, 2, 6}, Nest: s1{Val: 102}, SuffixSlice: []int64{42, 2, 3}}), }, descriptor: cre.ConsensusAggregationFromTags[s]().Descriptor(), - minObs: 5, f: 2, expectedOutcome: mustWrap(s{ Val: 30, @@ -201,7 +214,6 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { Aggregation: sdk.AggregationType_AGGREGATION_TYPE_UNSPECIFIED, }, }, - minObs: 1, expectedOutcome: nil, expectedError: errors.New("unknown aggregation type"), }, @@ -214,7 +226,6 @@ func Test_CalculateOutcomeForObservations(t *testing.T) { tc.observations, tc.descriptor, tc.defaultValue, - tc.minObs, tc.f, )