This repository has been archived by the owner on Oct 6, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation of the solution
This PR encapsulates two changes that are blocking for strategy development and flux deployment:
Refactor
I found that the Flux implementation was duplicating a lot of generic strategy-specific logic and genercized it further and moved it into
APStrategy_V1
. This part of the PR should be a no-op change, just a refactor.Flux Strategy -> SafeERC20
I also changed the Flux strategy to using
SafeERC20
instead of in-line approve/transfer checks. This should help with readability and gas. Additionally, I was using the Flux interface for interfacing with USDC. Though technically valid, that wasn't a good choice for readability... I'm not really sure why I made it in the first place. I've switched to using our standardIERC20
with SafeERC20 in its stead.Instructions on making this work
yarn
oryarn install
to install npm dependencies