-
Notifications
You must be signed in to change notification settings - Fork 70
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
Refactor current_goal_state #167
Refactor current_goal_state #167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tks @saimeunt for the help, some changes
@bitfalt @EmmanuelAR I just made the requested changes, thanks for reviewing 👌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! i loved how you did the test. Great job! :D
Pull Request
Changes description
This PRs refactors the current_goal_state variable in favor of an ERC20 token balance.
The ERC20 token address has been added as a parameter of the fund to be able to create funds compatible with any ERC20 token.
The ongoing tests for withdraw might be impacted by this refactor.
Time spent breakdown
Half a day spent on reading the codebase, refactoring according to guidance and modifying tests accordingly.