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

🔍 review contract #1

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

🔍 review contract #1

wants to merge 1 commit into from

Conversation

GabrielAstieres
Copy link
Collaborator

Had a quick look at the contract:

  • Nice re-implem of owner, but careful when not using known libs.
  • With payable functions, you are in a scenario where re-entrency could be an issue if you had public facing function. Best to keep that in mind.
  • Minor comments in the contract.
  • An issue with the withdraw function, not able to handle every type of ERC-20, that could lead to a loss of funds.

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