-
Notifications
You must be signed in to change notification settings - Fork 47
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
FlashMintLeveraged: Support for fixed input Issuance #169
Conversation
7520d41
to
28d49af
Compare
_maxDust, | ||
outputTokenBalanceBefore | ||
); | ||
uint256 outputTokenObtained = IERC20(_outputToken).balanceOf(address(this)).sub(outputTokenBalanceBefore); |
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.
should we also check maxSetAmount
here?
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.
This should already be checked in the existing logic, so that another check here would be duplication.
Will double check to verify this though.
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.
No need to check this here. Since we only ever redeem maxSetAmount and then reissue from the excess output tokens, there should be no way for the user to spent more than that set amount.
_minSetAmount = _inputTokenAmount.mul(priceEstimate).div(1 ether); | ||
} | ||
|
||
// TODO: Decide what to do with the dust (maybe swap to eth and return as gas subsidy) |
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.
i think its ideal to return dust in the form of ETH rebate here. we can also add a check that this ETH value is not too large
3 checks
- input token amount matches exactly
- set token amount is at least the minimum amount
- eth gas rebate does not exceed contract setting (if it did, that would mean misparameterization is likely)
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.
the maximum amount that will be rebated is already implictly limited by "maxDust" so I don't think we necessarily need that check.
For the setTokenAmount we are doing a fixedIssuance of minSetAmount in the first step, (and then only adding to it by incrementally issuing more), so I don't think this is necessary either, but I can add that check to be double / triple sure.
Similar reasoning will apply for the inputTokenAmount. (if we do an exactInput swap on the excess input token it will by definition resulting in us having spent the correct amount of the inputToken, assuming the DEX and our integration works as designed). Again gas is cheap though, so I can add another check here.
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.
Generally agreed though on doing the gas rebate though. Probably means that the frontend needs to pass along another "SwapData" struct though to swap from the input / output token to eth.
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.
Eth Gas rebate is now implemented including contract level check on the maximum eth amount rebated.
* Copy paste FlashMintLeveraged as starting point * Fix tests * Remove debug logs * Correct wrong eth logic, still failing because eth is sent back prematurely * Fix lint failure * Fix eth issuance by adjusting handling of input token, only paying back at the end * Fix fixedSetAmount issuance method and add related tests * Add redemExactSet tests back in * Adjust redemption methods * First version of exact output redemption * Add redemSetForExactETH method * Fix fixedOutputRedeem methods * copy paste ethereum integration test to new arbitrum integration test folder * Adjust arbitrum integration test addresses * Fix arbitrum integration test setup * More test fixes * Arbitrum tests green * Add arbitrum integration tests to ci * Set Arbitrum rpc url in env variables * update solidity-coverage * Remove ethereum integration tests for flashmintextended * Return excess eth in issueSetFromExactETH * Swap Input Token to ETH * Add swap data to ETH for redeem method * Temporarily comment out non-integration tests * Contract fixes * Fix test * More contract changes * Return amount of set token obtained from fixed input / output methods * Add maxIterations check * Add docstring comments and fix maxIterations check * more docstrings * Test issuance / redemption from / to usdc * Temporarily comment out polygon tests * Add maxGasRebate to protect against misconfiguration * Fix contract bug regarding eth rebate opt out * Extend tests * Re-Trigger CI * Use node 20 in github actions * Bump hardhat version * Update circleci node version * fix tests * fix tests * Deactivate optimism tests * only run optimistic auction rebalance tests in eth integration pipeline * fix tests * Inline doc comment * Remove test step from publish workflow * Fix tests
No description provided.