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

checksum flashloan and target tokens #638

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

NIXBNT
Copy link
Collaborator

@NIXBNT NIXBNT commented May 9, 2024

Improve the user experience by checksumming input tokens so they arent dropped later

@NIXBNT NIXBNT requested a review from barakman May 9, 2024 00:24
@@ -233,7 +233,6 @@ def get_tkn_symbols(flashloan_tokens, tokens: pd.DataFrame) -> List:

returns: list
"""
flashloan_tokens = flashloan_tokens.split(",")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

drop this because its done before this function

flashloan_tokens = flashloan_tokens.split(",")
flashloan_tokens = [cfg.w3.to_checksum_address(x) for x in flashloan_tokens]
flt_symbols = get_tkn_symbols(flashloan_tokens=flashloan_tokens, tokens=tokens)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to change the order here else the token symbols are fetched on the checksummed addresses

cfg.NATIVE_GAS_TOKEN_ADDRESS in flashloan_tokens
or cfg.WRAPPED_GAS_TOKEN_ADDRESS in flashloan_tokens
cfg.NATIVE_GAS_TOKEN_ADDRESS.lower() in flashloan_tokens.lower()
or cfg.WRAPPED_GAS_TOKEN_ADDRESS.lower() in flashloan_tokens.lower()
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the w3 module is required for checksum, and the cfg is required to be initialized before using w3, its not as clean to checksum the flashloan_tokens AND target_tokens at this point.
i.e. if we want to checksum flashloan tokens here it can go after w3 intialized but then we also need to pass the target tokens - when we have specific functions called handle_flashloan_tokens() etc.
So if the native or wrapped gas token are not checksummed we can check for their presence with lower() instead without modifying

@NIXBNT NIXBNT requested a review from zavelevsky May 9, 2024 00:30
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.

1 participant