feat: FlashMintDex #181

edkim · 2024-08-02T20:42:10Z

Contract that allows FlashMint and FlashRedeem using any DEX supported by the DEXAdapterV2 library.
Supports SetTokens on Index Protocol or Set Protocol to avoid needing two deployments.

Frontends can use the getIssueExactSet function to return the amount of ETH/ERC20 required to Flash Mint, using on-chain DEX prices. Likewise getRedeemExactSet returns how much ETH/ERC20 the user will receive after Flash Redeeming.

To run tests:

yarn chain:fork:ethereum
INTEGRATIONTEST=true VERBOSE=true npx hardhat test ./test/integration/ethereum/flashMintDex.spec.ts --network localhost

edkim · 2024-08-13T19:27:41Z

test/integration/ethereum/flashMintDex.spec.ts

+          expect(inputTokenBalanceAfter).to.gt(inputTokenBalanceBefore.sub(paymentInfo.limitAmt));
+        });
+
+        it("Can issue set token from USDC", async () => {


@janndriessen see this test for how to call from the SDK.

ckoopmann

Great work.
Mostly looks good to me.
Only major points I made are regarding the necessity of calling getWethReceived/Costs in the actual issuance process, that I think we should review

Rest of the comments are mostly just me thinking aloud or minor potential adjustments (like a token whitelist and avoid sending unnecessary calldata)

ckoopmann · 2024-08-14T02:20:04Z

utils/deploys/deployExtensions.ts

+  ) {
+    const dexAdapter = await this.deployDEXAdapterV2();
+
+    const linkId = convertLibraryNameToLinkId(


Is there a reason why we will need to redeploy the DexAdapterV2 library ? (i.e. did any of the router addresses / configuration change )
If not we could reuse the existing library address that is already deployed, which you might also want to in the tests then.

Fixed in a40e46a

ckoopmann · 2024-08-14T02:26:31Z

contracts/exchangeIssuance/FlashMintDex.sol

+
+    modifier isValidModuleAndSet(address _issuanceModule, address _setToken) {
+        require(
+            setController.isModule(_issuanceModule) && setController.isSet(_setToken) ||


We might also want to check that the given issuance module is actually associated to that set token.
Otherwise if a user specifies the basic issuance module for a dim set token, it might result in weird behaviour ? (with regards to getting the redeem components further down for example)

Also not sure if we really want to have all legacy set tokens (including those not managed by us) be enabled by default.

Maybe we keep a whitelist instead ?

what user case is this input sanitization guarding against?

One scenario:
A user using this to issue some weird set token of their own creation, which might not actually be compatible with our assumptions and then blaming us for something going wrong.

ckoopmann · 2024-08-14T02:29:05Z

contracts/exchangeIssuance/FlashMintDex.sol

+     * @param _setToken          Set token to issue
+     * @param _amountSetToken    Amount of set token to issue
+     */
+    function getRequiredIssuanceComponents(address _issuanceModule, bool _isDebtIssuance, ISetToken _setToken, uint256 _amountSetToken) public view returns(address[] memory components, uint256[] memory positions) {


Not super important, but I was wondering what happens if a user tries to issue a leverage token via this contract. I guess the debt tokens returned will be stranded in the flash mint contract ?
In that case maybe we want to have an allow list of tokens that can be issued / redeemed with this contract ?

I ended up enforcing no external positions in dc7a943.

I preferred that to having a privileged user maintain a whitelist, but it does add gas on every issue/redeem.

ckoopmann · 2024-08-14T02:32:32Z

contracts/exchangeIssuance/FlashMintDex.sol

+        require(ethUsedForIssuance <= msg.value, "FlashMint: OVERSPENT ETH");
+
+        uint256 ethReturned = msg.value.sub(ethUsedForIssuance);
+        if (ethReturned > 0) {


I think there might be scenarios where ethReturned is 0.
Theoretically I can call this from a smart contract which then should be able to exactly calculate the eth to be spent beforehand in the same transaction.

ckoopmann · 2024-08-14T02:33:06Z

contracts/exchangeIssuance/FlashMintDex.sol

+        uint256 leftoverWeth = wethReceived.sub(wethSpent);
+        uint256 paymentTokenReturned = 0;
+
+        if (leftoverWeth > 0) {


As mentioned above I think there are scenarios where the caller might be able to exactly predict the amount spent, so I'd keep this check.

contracts/exchangeIssuance/FlashMintDex.sol

ckoopmann · 2024-08-14T02:41:50Z

contracts/exchangeIssuance/FlashMintDex.sol

+        uint256 leftoverWeth = wethReceived.sub(wethSpent);
+        uint256 paymentTokenReturned = 0;
+
+        if (leftoverWeth > 0) {


We might think about letting the caller specify a minimum amount of leftoverWeth at which he wants to swap back though.
If the amount is 1 wei, it is definetely not worth the gas fees.

Alternatively maybe the caller could signal opting out of this by sending an empty swapDataWetToToken.

ckoopmann · 2024-08-14T02:44:18Z

contracts/exchangeIssuance/FlashMintDex.sol

+        _redeem(_redeemParams.setToken, _redeemParams.amountSetToken, _redeemParams.issuanceModule);
+
+        uint256 wethReceived = _sellComponentsForWeth(_redeemParams);
+        outputTokenReceived = _swapWethForPaymentToken(wethReceived, _paymentInfo.token, _paymentInfo.swapDataWethToToken);


Note that in this instance we never use paymentInfo.swapDataTokenToWeth but still force the caller to send this.

You might want to create different versions of the PaymentInfo struct so that we don't force unnecessary calldata with the associated gas costs.

Yeah I went with simplicity here rather than trying to optimize for the case that gas costs more than the value of token returned.

I'll see if I can split out that unused var (I was hitting stack too deep at one point).

ckoopmann · 2024-08-14T02:55:10Z

contracts/exchangeIssuance/FlashMintDex.sol

+            address[] memory components, 
+            uint256[] memory componentUnits,
+            uint256[] memory wethCosts
+        ) = _getWethCostsPerComponent(_issueParams);


Why do we need to calculate the weth costs here ?
We could just check wether the totalWethSpent is less that what the user provided no ?
Should save us some gas. Also there might be scenarios where the swap paths of the components overlap resulting in changing prices / weth costs between this call and the actual swaps.

You're right, we shouldn't have to worry about checking exact amounts here. And I didn't think about the overlapping swaps!

Removed in 8963206 and refactored in 9b1708b

ckoopmann · 2024-08-14T02:55:46Z

contracts/exchangeIssuance/FlashMintDex.sol

+            address[] memory components, 
+            uint256[] memory componentUnits,
+            uint256[] memory wethReceived
+        ) = _getWethReceivedPerComponent(_redeemParams);


Same question here. Why do we need to calculate wethReceived beforehand ?
We can just verify totalWethBought, no ?

janndriessen · 2024-08-14T07:01:44Z

test/integration/ethereum/flashMintDex.spec.ts

+  fees: [500],
+  path: [addresses.tokens.weth, addresses.tokens.USDC],
+  pool: ADDRESS_ZERO,
+};


So this swap data would have to be constructed for any ERC-20 input/output token. And I assume it would be best to find the best path dynamically or could it be hard-coded to paths [inputToken, WETH] or [WETH, outputToken]? @edkim

Correct, we'd need these paths defined for all input/output tokens we let users choose. I was imagining these would be hard-coded. The best priced path can change, but not often.

janndriessen · 2024-08-14T07:07:30Z

contracts/exchangeIssuance/FlashMintDex.sol

+            totalEthNeeded += wethCosts[i];
+        }
+        return dexAdapter.getAmountIn(_swapDataInputTokenToWeth, totalEthNeeded);
+    }


These getter functions are nice 🌟

pblivin0x · 2024-08-14T18:17:37Z

contracts/exchangeIssuance/FlashMintDex.sol

+import { DEXAdapterV2 } from "./DEXAdapterV2.sol";
+
+/**
+ * @title FlashMintDex


can we add an @author and @notice here, highlighting the SDK/API benefits in the notice

Added in 562f2d6

looks good, key note imo is that no 3rd party quote provider is necessary (0x)

Added a mention of this to the notice.

contracts/exchangeIssuance/FlashMintDex.sol

pblivin0x

looks good. couple comments on adding documentation and making sure any reverts in the source are hit by sensible test cases

edkim · 2024-08-19T20:04:23Z

Almost ready for 2nd review. Last thing is to let the user specify a minimum threshold for leftover dust to be swapped and returned.

ckoopmann

Great work. LGTM.
In particular the following changes since my last review look good:

Minimum weth threshold for swapping back left over weth
Avoid calculating weth costs for each component
External position checks

ckoopmann · 2024-08-21T10:48:58Z

contracts/exchangeIssuance/FlashMintDex.sol

-            }
+        for (uint256 i = 0; i < components.length; i++) {
+            require(_issueParams.setToken.getExternalPositionModules(components[i]).length == 0, "FlashMint: EXTERNAL POSITION MODULES NOT SUPPORTED");
+            uint256 wethSold = dexAdapter.swapTokensForExactTokens(componentUnits[i], type(uint256).max, _issueParams.componentSwapData[i]);


Technically here instead of max_uint256 you could specify the remaining weth available to be spent. That way you would get an earlier revert in case it overspends.

Won't make much difference in practice though so feel free to disregard.

edkim added 30 commits July 24, 2024 01:08

swap data instead of component quotes

263d307

stack still too deep

6aaaa31

refactor issuance functions

34583af

refactor redeem functions

63295ff

clean up issue and redeem with eth

8c8f6b1

WIP FlashMintDex integration tests

a1c7f3e

add test file

c111735

refactor tests wip

aa2f5ad

flashmint dex issuance test for 1 component

70a8915

univ3 tests pass

06beea4

test sushiswap

71af8c0

wip usdc support

b038f67

need to change buycomponents function to use weth only

a8783e7

return excess input token

e35ba96

issue from erc20

5d7af6c

internal issue exact set from weth function

4735018

failing redeem for eth test

8836faa

redeem for eth test passes

8d2ce4c

redeem to usdc test

7b889e2

issue and redeem from weth

59d4412

1 deploymnent 2 controllers

e5f0e74

update tests for combined params

dd16c56

cleanup

bd749d2

finish separating payment info struct

ce837f9

refactor tests for separate payment struct

6e2a27a

cleanup console logs

c646fca

issue legacy set token with eth

fb38aeb

erc20 tests for legacy set token

65d5ab3

tests for redeem legacy set tokens

79b5fe7

add getIssueExactSet functions return input token amounts

a307e11

edkim added 2 commits August 13, 2024 15:05

cleanup for pr

4b38df6

cleanup for pr

c90f6ea

edkim marked this pull request as ready for review August 13, 2024 19:08

edkim requested review from ckoopmann, janndriessen and pblivin0x August 13, 2024 19:08

edkim commented Aug 13, 2024

View reviewed changes

ckoopmann requested changes Aug 14, 2024

View reviewed changes

janndriessen reviewed Aug 14, 2024

View reviewed changes

pblivin0x reviewed Aug 14, 2024

View reviewed changes

contracts/exchangeIssuance/FlashMintDex.sol Show resolved Hide resolved

pblivin0x reviewed Aug 14, 2024

View reviewed changes

contracts/exchangeIssuance/FlashMintDex.sol Outdated Show resolved Hide resolved

pblivin0x reviewed Aug 14, 2024

View reviewed changes

contracts/exchangeIssuance/FlashMintDex.sol Show resolved Hide resolved

pblivin0x requested changes Aug 14, 2024

View reviewed changes

edkim added 5 commits August 15, 2024 12:39

no need to calc exact component prices when issuing/redeeming

8963206

reuse deployed dexadapterv2 library; add natspec to constructor

a40e46a

refactor getter functions

9b1708b

add contract-level notice and reference FlashMint SDK

562f2d6

remove require for overspent eth; enforce no external positions

dc7a943

Add minimum threshold for returning unused funds when issuing

7b4605b

edkim requested review from ckoopmann and pblivin0x August 20, 2024 18:33

ckoopmann approved these changes Aug 21, 2024

View reviewed changes

edkim added 2 commits August 21, 2024 10:27

add note about not requiring off-chain quotes

0440755

update struct comment

de26022

pblivin0x approved these changes Aug 22, 2024

View reviewed changes

edkim merged commit 3847aab into master Aug 22, 2024
5 checks passed

edkim deleted the ek/flashmint-dex branch August 22, 2024 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: FlashMintDex #181

feat: FlashMintDex #181

edkim commented Aug 2, 2024 •

edited

Loading

edkim Aug 13, 2024

ckoopmann left a comment

ckoopmann Aug 14, 2024

edkim Aug 19, 2024

ckoopmann Aug 14, 2024

ckoopmann Aug 14, 2024

pblivin0x Aug 14, 2024

ckoopmann Aug 15, 2024

ckoopmann Aug 14, 2024

edkim Aug 19, 2024

ckoopmann Aug 14, 2024

ckoopmann Aug 14, 2024

ckoopmann Aug 14, 2024

ckoopmann Aug 14, 2024

edkim Aug 14, 2024

ckoopmann Aug 14, 2024

edkim Aug 14, 2024

edkim Aug 19, 2024 •

edited

Loading

ckoopmann Aug 14, 2024

janndriessen Aug 14, 2024

edkim Aug 14, 2024

janndriessen Aug 14, 2024

pblivin0x Aug 14, 2024

edkim Aug 19, 2024

pblivin0x Aug 20, 2024

edkim Aug 21, 2024

pblivin0x left a comment

edkim commented Aug 19, 2024

ckoopmann left a comment

ckoopmann Aug 21, 2024

feat: FlashMintDex #181

feat: FlashMintDex #181

Conversation

edkim commented Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

ckoopmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edkim Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pblivin0x left a comment

Choose a reason for hiding this comment

edkim commented Aug 19, 2024

ckoopmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edkim commented Aug 2, 2024 •

edited

Loading

edkim Aug 19, 2024 •

edited

Loading