Use zero values for fee if we cannot come to consensus (#174)

* Use zero values for fee if we cannot come to consensus It is better to generate a report that will validate for free, rather than no report at all, if we cannot come to consensus on a valid fee. * Use Sign instead
smartcontractkit · Sep 12, 2023 · fec1da7 · fec1da7
1 parent cb83289
commit fec1da7
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 7 deletions.
diff --git a/pkg/reportingplugins/mercury/aggregate_functions.go b/pkg/reportingplugins/mercury/aggregate_functions.go
@@ -6,6 +6,8 @@ import (
 	"sort"
 )
 
+var Zero = big.NewInt(0)
+
 // NOTE: All aggregate functions assume at least one element in the passed slice
 // The passed slice might be mutated (sorted)
 
@@ -128,7 +130,7 @@ func GetConsensusLinkFee(paos []PAOLinkFee, f int) (*big.Int, error) {
 	var validLinkFees []*big.Int
 	for _, pao := range paos {
 		fee, valid := pao.GetLinkFee()
-		if valid {
+		if valid && fee.Sign() >= 0 {
 			validLinkFees = append(validLinkFees, fee)
 		}
 	}
@@ -151,7 +153,7 @@ func GetConsensusNativeFee(paos []PAONativeFee, f int) (*big.Int, error) {
 	var validNativeFees []*big.Int
 	for _, pao := range paos {
 		fee, valid := pao.GetNativeFee()
-		if valid {
+		if valid && fee.Sign() >= 0 {
 			validNativeFees = append(validNativeFees, fee)
 		}
 	}

diff --git a/pkg/reportingplugins/mercury/aggregate_functions_test.go b/pkg/reportingplugins/mercury/aggregate_functions_test.go
@@ -321,6 +321,23 @@ func Test_AggregateFunctions(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, big.NewInt(3), linkFee)
 		})
+		t.Run("treats zero values as valid", func(t *testing.T) {
+			paos := NewValidParsedAttributedObservations()
+			for i := range paos {
+				paos[i].LinkFee = big.NewInt(0)
+			}
+			linkFee, err := GetConsensusLinkFee(convertLinkFee(paos), f)
+			require.NoError(t, err)
+			assert.Equal(t, big.NewInt(0), linkFee)
+		})
+		t.Run("treats negative values as invalid", func(t *testing.T) {
+			paos := NewValidParsedAttributedObservations()
+			for i := range paos {
+				paos[i].LinkFee = big.NewInt(int64(0 - i))
+			}
+			_, err := GetConsensusLinkFee(convertLinkFee(paos), f)
+			assert.EqualError(t, err, "fewer than f+1 observations have a valid linkFee (got: 1/4)")
+		})
 
 		t.Run("fails when fewer than f+1 linkFees are valid", func(t *testing.T) {
 			invalidMPaos := convertLinkFee(invalidPaos)
@@ -336,7 +353,23 @@ func Test_AggregateFunctions(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, big.NewInt(3), nativeFee)
 		})
-
+		t.Run("treats zero values as valid", func(t *testing.T) {
+			paos := NewValidParsedAttributedObservations()
+			for i := range paos {
+				paos[i].NativeFee = big.NewInt(0)
+			}
+			nativeFee, err := GetConsensusNativeFee(convertNativeFee(paos), f)
+			require.NoError(t, err)
+			assert.Equal(t, big.NewInt(0), nativeFee)
+		})
+		t.Run("treats negative values as invalid", func(t *testing.T) {
+			paos := NewValidParsedAttributedObservations()
+			for i := range paos {
+				paos[i].NativeFee = big.NewInt(int64(0 - i))
+			}
+			_, err := GetConsensusNativeFee(convertNativeFee(paos), f)
+			assert.EqualError(t, err, "fewer than f+1 observations have a valid nativeFee (got: 1/4)")
+		})
 		t.Run("fails when fewer than f+1 nativeFees are valid", func(t *testing.T) {
 			invalidMPaos := convertNativeFee(invalidPaos)
 			_, err := GetConsensusNativeFee(invalidMPaos, f)

diff --git a/pkg/reportingplugins/mercury/v2/mercury.go b/pkg/reportingplugins/mercury/v2/mercury.go
@@ -343,10 +343,22 @@ func (rp *reportingPlugin) buildReportFields(previousReport ocrtypes.Report, pao
 	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusBenchmarkPrice failed"))
 
 	rf.LinkFee, err = mercury.GetConsensusLinkFee(convertLinkFee(paos), rp.f)
-	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusLinkFee failed"))
+	if err != nil {
+		// It is better to generate a report that will validate for free,
+		// rather than no report at all, if we cannot come to consensus on a
+		// valid fee.
+		rp.logger.Errorw("Cannot come to consensus on LINK fee, falling back to 0", "err", err, "paos", paos)
+		rf.LinkFee = big.NewInt(0)
+	}
 
 	rf.NativeFee, err = mercury.GetConsensusNativeFee(convertNativeFee(paos), rp.f)
-	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusNativeFee failed"))
+	if err != nil {
+		// It is better to generate a report that will validate for free,
+		// rather than no report at all, if we cannot come to consensus on a
+		// valid fee.
+		rp.logger.Errorw("Cannot come to consensus on Native fee, falling back to 0", "err", err, "paos", paos)
+		rf.NativeFee = big.NewInt(0)
+	}
 
 	if int64(rf.Timestamp)+int64(rp.offchainConfig.ExpirationWindow) > math.MaxUint32 {
 		merr = errors.Join(merr, fmt.Errorf("timestamp %d + expiration window %d overflows uint32", rf.Timestamp, rp.offchainConfig.ExpirationWindow))

diff --git a/pkg/reportingplugins/mercury/v2/mercury_test.go b/pkg/reportingplugins/mercury/v2/mercury_test.go
@@ -335,6 +335,27 @@ func Test_Plugin_Report(t *testing.T) {
 			assert.False(t, should)
 			assert.NoError(t, err)
 		})
+		t.Run("uses 0 values for link/native if they are invalid", func(t *testing.T) {
+			codec.observationTimestamp = 42
+			codec.err = nil
+
+			protos := newValidProtos()
+			for i := range protos {
+				protos[i].LinkFeeValid = false
+				protos[i].NativeFeeValid = false
+			}
+			aos := newValidAos(t, protos...)
+
+			should, report, err := rp.Report(ocrtypes.ReportTimestamp{}, previousReport, aos)
+			assert.True(t, should)
+			assert.NoError(t, err)
+
+			assert.True(t, should)
+			assert.Equal(t, codec.builtReport, report)
+			require.NotNil(t, codec.builtReportFields)
+			assert.Equal(t, "0", codec.builtReportFields.LinkFee.String())
+			assert.Equal(t, "0", codec.builtReportFields.NativeFee.String())
+		})
 	})
 
 	t.Run("buildReport failures", func(t *testing.T) {

diff --git a/pkg/reportingplugins/mercury/v3/mercury.go b/pkg/reportingplugins/mercury/v3/mercury.go
@@ -384,10 +384,22 @@ func (rp *reportingPlugin) buildReportFields(previousReport ocrtypes.Report, pao
 	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusAsk failed"))
 
 	rf.LinkFee, err = mercury.GetConsensusLinkFee(convertLinkFee(paos), rp.f)
-	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusLinkFee failed"))
+	if err != nil {
+		// It is better to generate a report that will validate for free,
+		// rather than no report at all, if we cannot come to consensus on a
+		// valid fee.
+		rp.logger.Errorw("Cannot come to consensus on LINK fee, falling back to 0", "err", err, "paos", paos)
+		rf.LinkFee = big.NewInt(0)
+	}
 
 	rf.NativeFee, err = mercury.GetConsensusNativeFee(convertNativeFee(paos), rp.f)
-	merr = errors.Join(merr, pkgerrors.Wrap(err, "GetConsensusNativeFee failed"))
+	if err != nil {
+		// It is better to generate a report that will validate for free,
+		// rather than no report at all, if we cannot come to consensus on a
+		// valid fee.
+		rp.logger.Errorw("Cannot come to consensus on Native fee, falling back to 0", "err", err, "paos", paos)
+		rf.NativeFee = big.NewInt(0)
+	}
 
 	if int64(rf.Timestamp)+int64(rp.offchainConfig.ExpirationWindow) > math.MaxUint32 {
 		merr = errors.Join(merr, fmt.Errorf("timestamp %d + expiration window %d overflows uint32", rf.Timestamp, rp.offchainConfig.ExpirationWindow))

diff --git a/pkg/reportingplugins/mercury/v3/mercury_test.go b/pkg/reportingplugins/mercury/v3/mercury_test.go
@@ -354,6 +354,27 @@ func Test_Plugin_Report(t *testing.T) {
 			assert.False(t, should)
 			assert.NoError(t, err)
 		})
+		t.Run("uses 0 values for link/native if they are invalid", func(t *testing.T) {
+			codec.observationTimestamp = 42
+			codec.err = nil
+
+			protos := newValidProtos()
+			for i := range protos {
+				protos[i].LinkFeeValid = false
+				protos[i].NativeFeeValid = false
+			}
+			aos := newValidAos(t, protos...)
+
+			should, report, err := rp.Report(ocrtypes.ReportTimestamp{}, previousReport, aos)
+			assert.True(t, should)
+			assert.NoError(t, err)
+
+			assert.True(t, should)
+			assert.Equal(t, codec.builtReport, report)
+			require.NotNil(t, codec.builtReportFields)
+			assert.Equal(t, "0", codec.builtReportFields.LinkFee.String())
+			assert.Equal(t, "0", codec.builtReportFields.NativeFee.String())
+		})
 	})
 
 	t.Run("buildReport failures", func(t *testing.T) {