Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MERC-6004 Add Mercury v4 schema #13862

Merged
merged 11 commits into from
Aug 2, 2024
Merged

MERC-6004 Add Mercury v4 schema #13862

merged 11 commits into from
Aug 2, 2024

Conversation

martin-cll
Copy link
Contributor

Based off Mercury v3 schema with the addition of the market status field. The market status field is an enum which is represented as a uint32 here. This is the most compact width for both Protobuf and Ethereum ABI (uint8 is padded to 32 bytes).

Almost all of the changes are just pattern matching existing places where we fork on the schema version. The things to check for would be the integration test, the datasource.Observe method, and generally making sure that I haven't missed anything while copying.

Note: this depends on corresponding changes in the chainlink-common and repo and smartcontractkit/chainlink-data-streams#73. I'll update this PR with the appropriate versions once those changes are in.

@@ -295,7 +297,7 @@ func (e *EnhancedTelemetryService[T]) collectMercuryEnhancedTelemetry(d Enhanced
var bt uint64
// v1+v2+v3 fields
bp := big.NewInt(0)
//v1+v3 fields
// v1+v3 fields
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to add v4 telemetry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've added a new enum and field for the market status. @mjk90 do you mind also taking a look at the EnhancedEAMercury proto and see if it makes sense?

Comment on lines +106 to +109
obs.BenchmarkPrice = parsed.benchmarkPrice
obs.Bid = parsed.bid
obs.Ask = parsed.ask
obs.MarketStatus = parsed.marketStatus
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where would we set MarketStatus.Err?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is created by setMarketStatus and then returned by the ds.parse method.

require.Error(t, err)
assert.Contains(t, err.Error(), "benchmarkPrice may not be nil")
assert.Contains(t, err.Error(), "linkFee may not be nil")
assert.Contains(t, err.Error(), "nativeFee may not be nil")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about marketstatus?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zero value means CLOSED so it is a valid value. Unlike the price fields which are pointer valued *big.Int, the market status is a scalar uint32 so it can't be nil either.

@martin-cll martin-cll changed the title Add Mercury v4 schema MERC-6004 Add Mercury v4 schema Jul 24, 2024
@martin-cll martin-cll added this pull request to the merge queue Aug 2, 2024
Merged via the queue into develop with commit 05ef7fd Aug 2, 2024
119 checks passed
@martin-cll martin-cll deleted the ml/mercury-feeds-v4 branch August 2, 2024 17:33
Tofel pushed a commit that referenced this pull request Aug 5, 2024
* Add Mercury v4

* Uncomment out test

* Add v4 telemetry

* MERC-6004 Fix build

* Update chainlink-common

* Add changeset

* Update market status proto enum values

* Add changeset hashtag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants