Change output format for price feed check #52
Conversation
There was a problem hiding this comment.
The output is not as required by ticket.
For price feed checks we expect:
Found <address> on <price feed provider>
Name: <price feed name>
Decimals: <decimals>
<if this is an implementation address> <in red> this is an implementation contract with a proxy address <proxy address> </in red>
For coin gecko we expect:
Found <address> on <token validator provider>
Name: <token name>
Symbol: <token symbol>
Decimals: <decimals>
The output is literally the same, you can check the output image. |
nivcertora
dismissed
yoav-el-certora’s stale review
Same output, no need to change
| if address.lower() != price_feed.address.lower(): | ||
| color = pp.Colors.FAILURE | ||
| message += f"This is an implementation contract with a proxy address\n" | ||
| message += f"Origin Address: {price_feed.address}\n" |
There was a problem hiding this comment.
This was not requested
There was a problem hiding this comment.
this is an implementation contract with a proxy address </in red>
There was a problem hiding this comment.
This line - message += f"Origin Address: {price_feed.address}\n"
There was a problem hiding this comment.
It was but they made a mistake, they don't need the proxy address in the sentence as it is already printed. they want the original.
| message += f"Proxy address: {price_feed.proxy_address}\n" | ||
| if address.lower() != price_feed.address.lower(): | ||
| color = pp.Colors.FAILURE | ||
| message += f"This is an implementation contract with a proxy address\n" |
There was a problem hiding this comment.
Only this sentence should be printed in red
There was a problem hiding this comment.
It's not possible to do. and better to put the entire message to not miss it.
There was a problem hiding this comment.
But everything in red makes you think this is a failure
There was a problem hiding this comment.
It is a failure according to the ticket.
There was a problem hiding this comment.
If the contract uses the proxy address instead of the original they want it in red
There was a problem hiding this comment.
From ticket description, they want only the last line.
Please check this with Michael.
There was a problem hiding this comment.
ill merge it and if have a issue ill talk to him.
There was a problem hiding this comment.
Check with @MichaelMorami before merge.
nivcertora
requested review from
yoav-el-certora and
MichaelMorami
and removed request for
yoav-el-certora
https://certora.atlassian.net/browse/CERT-7874
https://certora.atlassian.net/browse/CERT-7875
PR Summary
Refactor PriceFeedData and Enhance PriceFeedCheck Messaging
Remove Address Fields:
addressandproxy_addressfrom the__str__method inPriceFeedDatato streamline its string representation.Enhance Messaging in PriceFeedCheck:
PriceFeedCheckto includeproxy_addressandorigin_addressin success messages when they are available and differ from the primary address.