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.

Sign up for GitHub

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

Change output format for price feed check #52

Merged
merged 5 commits into from
Dec 30, 2024
Merged

Change output format for price feed check #52

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f7bca6
Change output format for price feed check
nivcertora Dec 29, 2024
51e9b73
Address case where proxy address being used in the contract
nivcertora Dec 29, 2024
fdb39fc
Merge branch 'main' into niv/CERT-7876-Coingecko-Output
nivcertora Dec 29, 2024
221631c
Prettyfiy
nivcertora Dec 29, 2024
8570213
Change to symbol
nivcertora Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Quorum/apis/price_feeds/price_feed_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class Config:

def __str__(self) -> str:
s = ""
s += f"Address: {self.address}\n"
if self.name:
s += f"Name: {self.name}\n"
if self.pair:
Expand All @@ -39,8 +38,6 @@ def __str__(self) -> str:
s += f"Pairs: {pair_str}\n"
else:
s += f"Pair: {self.pair}\n"
yoav-el-certora marked this conversation as resolved.
Show resolved Hide resolved
if self.proxy_address:
s += f"Proxy Address: {self.proxy_address}\n"
if self.decimals:
s += f"Decimals: {self.decimals}\n"
return s
Expand Down
16 changes: 13 additions & 3 deletions Quorum/checks/price_feed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,20 @@ def __check_price_feed_address(self, address: str, file_name: str) -> dict | Non
"""
for provider in self.providers:
if (price_feed := provider.get_price_feed(self.chain, address)):

color = pp.Colors.SUCCESS
message = f"Found {address} on {provider.get_name()}\n"
message += f"info: {price_feed}"
if price_feed.proxy_address and price_feed.proxy_address.lower() != address.lower():
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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only this sentence should be printed in red

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not possible to do. and better to put the entire message to not miss it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But everything in red makes you think this is a failure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is a failure according to the ticket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the contract uses the proxy address instead of the original they want it in red

Copy link
Collaborator

Choose a reason for hiding this comment

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

From ticket description, they want only the last line.
Please check this with Michael.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ill merge it and if have a issue ill talk to him.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Check with @MichaelMorami before merge.

message += f"Origin Address: {price_feed.address}\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was not requested

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is an implementation contract with a proxy address </in red>

Copy link
Collaborator

Choose a reason for hiding this comment

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

This line - message += f"Origin Address: {price_feed.address}\n"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.


pp.pretty_print(
f"Found {address} on {provider.get_name()}\n"
f"info: {price_feed}",
pp.Colors.SUCCESS
message,
color
)
return price_feed.model_dump()

Expand Down
Loading