chainlink icon indicating copy to clipboard operation
chainlink copied to clipboard

Handle Hex String in EA Telemetry

Open cawthorne opened this issue 1 year ago • 5 comments

#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: https://github.com/smartcontractkit/chainlink/pull/14841

cawthorne avatar Oct 17 '24 13:10 cawthorne

AER Report: CI Core ran successfully :white_check_mark:

aer_workflow , commit

AER Report: Operator UI CI ran successfully :white_check_mark:

aer_workflow , commit

github-actions[bot] avatar Oct 17 '24 13:10 github-actions[bot]

@george-dorin I managed to get all tests passing with the original approach with changing ToDecimal: https://github.com/smartcontractkit/chainlink/pull/14841

My original patch had a bug in.

I was unsure if your comment It seems that the behavior of not being able to convert hex to decimal is expected by Median and Mercury, was based on the tests failing, or if that is actually the case.

We can choose which approach we want, and close the PR we don't merge in.

cawthorne avatar Oct 18 '24 10:10 cawthorne

@cawthorne I was basing this on the tests, and it seems to me that ToDecimal should know how to interpret hex. However, I'm not entirely sure if other products are expecting the conversion to fail

george-dorin avatar Oct 18 '24 10:10 george-dorin

@george-dorin I am fine with either, perhaps it would make sense to just do the telemetry at first, and then the more general solution could come later if there's a need.

cawthorne avatar Oct 18 '24 14:10 cawthorne