From 761f63e7b52700827196b6011401950035317e70 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 2 Jul 2024 10:48:07 -0400 Subject: [PATCH] Validate invariants in report (#67) * Validate invariants in report * Revert "Observations that violate bid<=mid<=ask are invalid (#61)" This reverts commit a4b7359e15809b077ae5ec6dcba764c63b388dce. --- mercury/v3/mercury.go | 23 ++------ mercury/v3/mercury_test.go | 107 ++++++------------------------------- 2 files changed, 19 insertions(+), 111 deletions(-) diff --git a/mercury/v3/mercury.go b/mercury/v3/mercury.go index e0ebd35..9f4f2c8 100644 --- a/mercury/v3/mercury.go +++ b/mercury/v3/mercury.go @@ -166,12 +166,7 @@ func (rp *reportingPlugin) Observation(ctx context.Context, repts types.ReportTi } if bpErr == nil && bidErr == nil && askErr == nil { - if err := validatePrices(obs.Bid.Val, obs.BenchmarkPrice.Val, obs.Ask.Val); err != nil { - rp.logger.Errorw("Cannot generate price observation: invalid bid/mid/ask", "err", err) - p.PricesValid = false - } else { - p.PricesValid = true - } + p.PricesValid = true } var maxFinalizedTimestampErr error @@ -230,13 +225,6 @@ func (rp *reportingPlugin) Observation(ctx context.Context, repts types.ReportTi return proto.Marshal(&p) } -func validatePrices(bid, benchmarkPrice, ask *big.Int) error { - if bid.Cmp(benchmarkPrice) > 0 || benchmarkPrice.Cmp(ask) > 0 { - return fmt.Errorf("invariant violated: expected bid<=mid<=ask, got bid: %s, mid: %s, ask: %s", bid, benchmarkPrice, ask) - } - return nil -} - func parseAttributedObservation(ao types.AttributedObservation) (PAO, error) { var pao parsedAttributedObservation var obs MercuryObservationProto @@ -261,13 +249,6 @@ func parseAttributedObservation(ao types.AttributedObservation) (PAO, error) { if err != nil { return parsedAttributedObservation{}, fmt.Errorf("ask cannot be converted to big.Int: %s", err) } - if err := validatePrices(pao.Bid, pao.BenchmarkPrice, pao.Ask); err != nil { - // NOTE: since nodes themselves are not supposed to set - // PricesValid=true if this invariant is violated, this indicates a - // faulty/misbehaving node and the entire observation should be - // ignored - return parsedAttributedObservation{}, fmt.Errorf("observation claimed to be valid, but contains invalid prices: %w", err) - } pao.PricesValid = true } @@ -428,6 +409,8 @@ func (rp *reportingPlugin) buildReportFields(previousReport types.Report, paos [ func (rp *reportingPlugin) validateReport(rf v3.ReportFields) error { return errors.Join( mercury.ValidateBetween("median benchmark price", rf.BenchmarkPrice, rp.onchainConfig.Min, rp.onchainConfig.Max), + mercury.ValidateBetween("median bid invariant", rf.Bid, rp.onchainConfig.Min, rf.BenchmarkPrice), + mercury.ValidateBetween("median ask invariant", rf.Ask, rf.BenchmarkPrice, rp.onchainConfig.Max), mercury.ValidateBetween("median bid", rf.Bid, rp.onchainConfig.Min, rp.onchainConfig.Max), mercury.ValidateBetween("median ask", rf.Ask, rp.onchainConfig.Min, rp.onchainConfig.Max), mercury.ValidateFee("median link fee", rf.LinkFee), diff --git a/mercury/v3/mercury_test.go b/mercury/v3/mercury_test.go index b2666e3..d6c756d 100644 --- a/mercury/v3/mercury_test.go +++ b/mercury/v3/mercury_test.go @@ -164,34 +164,6 @@ func newValidAos(t *testing.T, protos ...*MercuryObservationProto) (aos []types. return } -func Test_parseAttributedObservation(t *testing.T) { - t.Run("returns error if bid<=mid<=ask is violated, even if observation claims itself to be valid", func(t *testing.T) { - obs := &MercuryObservationProto{ - Timestamp: 42, - - BenchmarkPrice: mercury.MustEncodeValueInt192(big.NewInt(123)), - Bid: mercury.MustEncodeValueInt192(big.NewInt(130)), - Ask: mercury.MustEncodeValueInt192(big.NewInt(120)), - PricesValid: true, - - MaxFinalizedTimestamp: 40, - MaxFinalizedTimestampValid: true, - - LinkFee: mercury.MustEncodeValueInt192(big.NewInt(1.1e18)), - LinkFeeValid: true, - NativeFee: mercury.MustEncodeValueInt192(big.NewInt(2.1e18)), - NativeFeeValid: true, - } - - serialized, err := proto.Marshal(obs) - require.NoError(t, err) - - _, err = parseAttributedObservation(types.AttributedObservation{Observation: serialized, Observer: commontypes.OracleID(42)}) - require.Error(t, err) - assert.Equal(t, "observation claimed to be valid, but contains invalid prices: invariant violated: expected bid<=mid<=ask, got bid: 130, mid: 123, ask: 120", err.Error()) - }) -} - func Test_Plugin_Report(t *testing.T) { dataSource := &testDataSource{} codec := &testReportCodec{ @@ -465,11 +437,23 @@ func Test_Plugin_validateReport(t *testing.T) { assert.Contains(t, err.Error(), "median benchmark price (Value: 150000) is outside of allowable range (Min: 1, Max: 1000)") assert.Contains(t, err.Error(), "median bid (Value: 150000) is outside of allowable range (Min: 1, Max: 1000)") assert.Contains(t, err.Error(), "median ask (Value: 150000) is outside of allowable range (Min: 1, Max: 1000)") - assert.Contains(t, err.Error(), "median link fee (Value: -1) is outside of allowable range (Min: 0, Max: 3138550867693340381917894711603833208051177722232017256447)") assert.Contains(t, err.Error(), "median native fee (Value: -1) is outside of allowable range (Min: 0, Max: 3138550867693340381917894711603833208051177722232017256447)") assert.Contains(t, err.Error(), "observationTimestamp (Value: 43) must be >= validFromTimestamp (Value: 44)") + assert.Contains(t, err.Error(), "median link fee (Value: -1) is outside of allowable range (Min: 0, Max: 3138550867693340381917894711603833208051177722232017256447)") assert.Contains(t, err.Error(), "expiresAt (Value: 42) must be ahead of observation timestamp (Value: 43)") }) + t.Run("bid/ask invariant violation", func(t *testing.T) { + rf := v3.ReportFields{ + BenchmarkPrice: big.NewInt(500), + Bid: big.NewInt(501), + Ask: big.NewInt(499), + } + err := rp.validateReport(rf) + require.Error(t, err) + + assert.Contains(t, err.Error(), "median bid invariant (Value: 501) is outside of allowable range (Min: 1, Max: 500)") + assert.Contains(t, err.Error(), "median ask invariant (Value: 499) is outside of allowable range (Min: 500, Max: 1000)") + }) t.Run("zero values", func(t *testing.T) { rf := v3.ReportFields{} @@ -523,20 +507,16 @@ func Test_Plugin_Observation(t *testing.T) { assert.LessOrEqual(t, len(b), maxObservationLength) }) - validBid := big.NewInt(rand.Int63() - 2) - validBenchmarkPrice := new(big.Int).Add(validBid, big.NewInt(1)) - validAsk := new(big.Int).Add(validBid, big.NewInt(2)) - t.Run("all observations succeeded", func(t *testing.T) { obs := v3.Observation{ BenchmarkPrice: mercurytypes.ObsResult[*big.Int]{ - Val: validBenchmarkPrice, + Val: big.NewInt(rand.Int63()), }, Bid: mercurytypes.ObsResult[*big.Int]{ - Val: validBid, + Val: big.NewInt(rand.Int63()), }, Ask: mercurytypes.ObsResult[*big.Int]{ - Val: validAsk, + Val: big.NewInt(rand.Int63()), }, MaxFinalizedTimestamp: mercurytypes.ObsResult[int64]{ Val: rand.Int63(), @@ -741,61 +721,6 @@ func Test_Plugin_Observation(t *testing.T) { assert.Zero(t, p.Ask) assert.False(t, p.PricesValid) }) - - t.Run("bid<=mid<=ask violation", func(t *testing.T) { - obs := v3.Observation{ - BenchmarkPrice: mercurytypes.ObsResult[*big.Int]{ - Val: big.NewInt(10), - }, - Bid: mercurytypes.ObsResult[*big.Int]{ - Val: big.NewInt(11), - }, - Ask: mercurytypes.ObsResult[*big.Int]{ - Val: big.NewInt(12), - }, - MaxFinalizedTimestamp: mercurytypes.ObsResult[int64]{ - Val: rand.Int63(), - }, - LinkPrice: mercurytypes.ObsResult[*big.Int]{ - Val: big.NewInt(rand.Int63()), - }, - NativePrice: mercurytypes.ObsResult[*big.Int]{ - Val: big.NewInt(rand.Int63()), - }, - } - dataSource.Obs = obs - - parsedObs, err := rp.Observation(context.Background(), types.ReportTimestamp{}, nil) - require.NoError(t, err) - - var p MercuryObservationProto - require.NoError(t, proto.Unmarshal(parsedObs, &p)) - - assert.LessOrEqual(t, p.Timestamp, uint32(time.Now().Unix())) - assert.Equal(t, obs.BenchmarkPrice.Val, mustDecodeBigInt(p.BenchmarkPrice)) - assert.False(t, p.PricesValid) // not valid! - - // other values passed through ok - assert.Equal(t, obs.MaxFinalizedTimestamp.Val, p.MaxFinalizedTimestamp) - assert.True(t, p.MaxFinalizedTimestampValid) - - fee := mercury.CalculateFee(obs.LinkPrice.Val, decimal.NewFromInt32(1)) - assert.Equal(t, fee, mustDecodeBigInt(p.LinkFee)) - assert.True(t, p.LinkFeeValid) - - fee = mercury.CalculateFee(obs.NativePrice.Val, decimal.NewFromInt32(1)) - assert.Equal(t, fee, mustDecodeBigInt(p.NativeFee)) - assert.True(t, p.NativeFeeValid) - - // test benchmark price higher than ask - obs.BenchmarkPrice.Val = big.NewInt(13) - dataSource.Obs = obs - - parsedObs, err = rp.Observation(context.Background(), types.ReportTimestamp{}, nil) - require.NoError(t, err) - require.NoError(t, proto.Unmarshal(parsedObs, &p)) - assert.False(t, p.PricesValid) // not valid! - }) } func newUnparseableAttributedObservation() types.AttributedObservation {