From 1a45097cbf51751efe6295718629a0e09c7361ce Mon Sep 17 00:00:00 2001 From: Lei Date: Fri, 8 Dec 2023 10:55:35 -0800 Subject: [PATCH] small improvements based on comments (#11491) * small improvements based on comments * add unit test --- .../evmregistry/v21/mercury/mercury.go | 6 +--- .../v21/mercury/streams/streams.go | 7 +++-- .../v21/mercury/streams/streams_test.go | 28 ++++++++++++++++++- .../evmregistry/v21/mercury/v03/request.go | 2 +- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/mercury.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/mercury.go index cf6ebeafc6e..f9a3c001c66 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/mercury.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/mercury.go @@ -105,10 +105,6 @@ type StreamsLookup struct { Block uint64 } -func (l *StreamsLookup) IsMercuryVersionUnkown() bool { - return l.FeedParamKey != FeedIDs -} - func (l *StreamsLookup) IsMercuryV02() bool { return l.FeedParamKey == FeedIdHex && l.TimeParamKey == BlockNumber } @@ -117,6 +113,6 @@ func (l *StreamsLookup) IsMercuryV03() bool { return l.FeedParamKey == FeedIDs } -func (l *StreamsLookup) IsMercuryUsingBatchPathV03() bool { +func (l *StreamsLookup) IsMercuryV03UsingBlockNumber() bool { return l.TimeParamKey == BlockNumber } diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams.go index aec23431921..70db40c26bb 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams.go @@ -131,6 +131,7 @@ func (s *streams) buildResult(ctx context.Context, i int, checkResult ocr2keeper // tried to call mercury lookupLggr.Infof("at block %d upkeep %s trying to DecodeStreamsLookupRequest performData=%s", block, upkeepId, hexutil.Encode(checkResults[i].PerformData)) streamsLookupErr, err := s.packer.DecodeStreamsLookupRequest(checkResult.PerformData) + if err != nil { lookupLggr.Debugf("at block %d upkeep %s DecodeStreamsLookupRequest failed: %v", block, upkeepId, err) // user contract did not revert with StreamsLookup error @@ -144,7 +145,7 @@ func (s *streams) buildResult(ctx context.Context, i int, checkResult ocr2keeper return } - // mercury permission checking for v0.3 is done by mercury server + // mercury permission checking for v0.3 is done by mercury server, so no need to check here if streamsLookupResponse.IsMercuryV02() { // check permission on the registry for mercury v0.2 opts := s.buildCallOpts(ctx, block) @@ -159,8 +160,8 @@ func (s *streams) buildResult(ctx context.Context, i int, checkResult ocr2keeper checkResults[i].IneligibilityReason = uint8(mercury.MercuryUpkeepFailureReasonMercuryAccessNotAllowed) return } - } else if streamsLookupResponse.IsMercuryVersionUnkown() { - // if mercury version cannot be determined, set failure reason + } else if !streamsLookupResponse.IsMercuryV03() { + // if mercury version is not v02 or v03, set failure reason lookupLggr.Debugf("at block %d upkeep %s NOT allowed to query Mercury server", block, upkeepId) checkResults[i].IneligibilityReason = uint8(mercury.MercuryUpkeepFailureReasonInvalidRevertDataInput) return diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams_test.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams_test.go index abcc37dca18..32041d0f18b 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams_test.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams/streams_test.go @@ -11,10 +11,11 @@ import ( "testing" "time" + "github.com/pkg/errors" + "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -619,6 +620,31 @@ func TestStreams_StreamsLookup(t *testing.T) { }, hasError: true, }, + { + name: "failure - invalid mercury version", + input: []ocr2keepers.CheckResult{ + { + // This Perform data contains invalid FeedParamKey: {feedIdHex:RandomString [ETD-USD BTC-ETH] blockNumber 100 [48 120 48 48]} + PerformData: hexutil.MustDecode("0xf055e4a200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000001c00000000000000000000000000000000000000000000000000000000000000064000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000166665656449644865783a52616e646f6d537472696e670000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000074554442d5553440000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000074254432d45544800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b626c6f636b4e756d62657200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000043078303000000000000000000000000000000000000000000000000000000000"), + UpkeepID: upkeepIdentifier, + Trigger: ocr2keepers.Trigger{ + BlockNumber: blockNum, + }, + IneligibilityReason: uint8(encoding.UpkeepFailureReasonTargetCheckReverted), + }, + }, + expectedResults: []ocr2keepers.CheckResult{ + { + PerformData: hexutil.MustDecode("0xf055e4a200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000001c00000000000000000000000000000000000000000000000000000000000000064000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000166665656449644865783a52616e646f6d537472696e670000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000074554442d5553440000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000074254432d45544800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b626c6f636b4e756d62657200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000043078303000000000000000000000000000000000000000000000000000000000"), + UpkeepID: upkeepIdentifier, + Trigger: ocr2keepers.Trigger{ + BlockNumber: blockNum, + }, + IneligibilityReason: uint8(mercury.MercuryUpkeepFailureReasonInvalidRevertDataInput), + }, + }, + hasError: true, + }, } for _, tt := range tests { diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/v03/request.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/v03/request.go index cb3c9ef3222..ea465ff8582 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/v03/request.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/v03/request.go @@ -100,7 +100,7 @@ func (c *client) multiFeedsRequest(ctx context.Context, ch chan<- mercury.Mercur params := fmt.Sprintf("%s=%s&%s=%s", mercury.FeedIDs, strings.Join(sl.Feeds, ","), mercury.Timestamp, sl.Time.String()) batchPathV03 := mercuryBatchPathV03 - if sl.IsMercuryUsingBatchPathV03() { + if sl.IsMercuryV03UsingBlockNumber() { batchPathV03 = mercuryBatchPathV03BlockNumber } reqUrl := fmt.Sprintf("%s%s%s", c.mercuryConfig.Credentials().URL, batchPathV03, params)