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

Handle Hex String in EA Telemetry #14827

Merged
merged 24 commits into from
Oct 24, 2024

Conversation

cawthorne
Copy link
Contributor

@cawthorne cawthorne commented Oct 17, 2024

#DF-20580
The EA Telemetry currently fails for EAs where their result is a hex string:
https://smartcontract-it.atlassian.net/browse/DF-20580

https://chainlink-core.slack.com/archives/C0117GGJB6Y/p1729091653229629

Alternative implementation which edits ToDecimal directly, but effects other services is here:
#14841

@cawthorne cawthorne requested a review from a team as a code owner October 17, 2024 13:17
mxiao-cll
mxiao-cll previously approved these changes Oct 17, 2024
Copy link
Contributor

github-actions bot commented Oct 17, 2024

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

Copy link
Contributor

@george-dorin george-dorin left a comment

Choose a reason for hiding this comment

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

It seems that the behavior of not being able to convert hex to decimal is expected by Median and Mercury. As an alternative, perhaps we could add the hex conversion logic specifically for telemetry parsing?

@cawthorne cawthorne changed the title Handle Hex String in Utils.ToDecimal Handle Hex String in EA Telemetry Oct 18, 2024
@george-dorin george-dorin added this pull request to the merge queue Oct 23, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2024
* Handle Hex String in Utils.ToDecimal

* add tests

* add changeset

* add #added to changeset

* handle hex strings only in EA Telemetry

* isolate hex tests

* update changset

* lint

* fix error return usage

* fix comment

* fix tests

* lint

* try different return

* test bump

* test bump

* test bump 2

* lint

* lint

* return decimal.Decimal{} instead of decimal.Zero in error case

* update functon comments

* use assert.InDelta instead of assert.Equal
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 23, 2024
@george-dorin george-dorin added this pull request to the merge queue Oct 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 23, 2024
@george-dorin george-dorin added this pull request to the merge queue Oct 24, 2024
Merged via the queue into develop with commit a9f86b5 Oct 24, 2024
132 checks passed
@george-dorin george-dorin deleted the feature/handle-hex-string-in-utils-ToDecimal branch October 24, 2024 07:47
cedric-cordenier pushed a commit that referenced this pull request Oct 24, 2024
* Handle Hex String in Utils.ToDecimal

* add tests

* add changeset

* add #added to changeset

* handle hex strings only in EA Telemetry

* isolate hex tests

* update changset

* lint

* fix error return usage

* fix comment

* fix tests

* lint

* try different return

* test bump

* test bump

* test bump 2

* lint

* lint

* return decimal.Decimal{} instead of decimal.Zero in error case

* update functon comments

* use assert.InDelta instead of assert.Equal
KMontag42 pushed a commit that referenced this pull request Oct 24, 2024
* Handle Hex String in Utils.ToDecimal

* add tests

* add changeset

* add #added to changeset

* handle hex strings only in EA Telemetry

* isolate hex tests

* update changset

* lint

* fix error return usage

* fix comment

* fix tests

* lint

* try different return

* test bump

* test bump

* test bump 2

* lint

* lint

* return decimal.Decimal{} instead of decimal.Zero in error case

* update functon comments

* use assert.InDelta instead of assert.Equal
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.

5 participants