Support Curve stableswap pools #253

FlattestWhite · 2022-05-13T15:12:15Z

Code Review Processes

Curve pools can have 2, 3 or even 4 coins in a pool. However, from an exchange perspective, we'd only ever be swapping two coins at one time. Furthermore, most coins in all pools are ERC20 tokens. Hence the eth/steth pool needed some extra logic around wrapping and unwrapping for eth/weth. The approach taken with this adapter is to assume we're only going to be trading ERC20 tokens.

It assumes both tokens that are being traded are ERC20 tokens and it requires the index on of the coin on the curve pool to be passed in at construction. The rest is just a simple encoding of the exchange function.

Newer curve pools also have an exchange function that can take a destination address as the last argument. This means we can save an extra contract call. However, most existing curve pools don't support that new exchange method. Hence, I've decided to use the older more widesperad "exchange" method, but it requires doing an extra transfer of tokens back to the user.

New Feature Review

Before submitting a pull request for new review, make sure the following is done:

Design doc is created and posted here: [Insert Link]
Code cleanliness and completeness is addressed via guidelines

README Checks

[] README has proper context for the reviewer to understand what the code includes, any important design considerations, and areas to pay more attention to

Code Checks

[] Add explanatory comments. If there is complex code that requires specific context or understanding, note that in a comment
[] Remove unncessary comments. Any comments that do not add additional context, information, etc. should be removed
[] Add javadocs.
[] Scrub through the code for inconsistencies (e.g. removing extra spaces)
[] Ensure there are not any .onlys in spec files

Broader Considerations

[] Ensure variable, function and event naming is clear, consistent, and reflective for the scope of the code.
[] Consider if certain pieces of logic should be placed in a different library, module

FlattestWhite · 2022-05-14T00:48:38Z

contracts/mocks/external/CurveStableswapMock.sol

-        IERC20(tokens[1]).safeTransferFrom(msg.sender, address(this), _dx);
-        Address.sendValue(msg.sender, _dx);
+
+        if (tokens[uint256(_i)] == ETH_TOKEN_ADDRESS) {


Support eth transfers for mocking in unit tests.

cgewecke

@FlattestWhite This looks really good.

Will this also integrate with the AaveLeverageModule? If so is it possible to copy and adapt the existing forked integration tests for the CurveStEth adapter to validate the behavior of this contract?

Have tried this locally as part of the review and a couple of its tests are failing and giving results that I don't understand (probably my mistake). The lever tests for:

"should update the collateral position on the SetToken correctly"
"should transfer the correct components from the exchange"

have broken expectations.

That doesn't mean anything's incorrect here ... would just like the test suite to have a good description of how the adapter is expected to behave in the wild.

You'll need to:

update the hardhat config's forking block number to something more recent since the WETH stableswap pool you've added to the dependencies is newish
make an adjustment to the existing Uniswap adapter forked integration tests to reflect current price conditions as Christian has done in his notional PR here)

FlattestWhite · 2022-05-17T16:13:48Z

Added forked integration tests - and fixed up the assertions so they're passing. Also added comments to explain why the assertions are the way they are.

cgewecke

LGTM! Thanks for the additional tests and the clarifying comments - very helpful.

FlattestWhite · 2022-05-18T05:31:33Z

LGTM! Thanks for the additional tests and the clarifying comments - very helpful.

Thanks - will your team merge and write the deploy script? Or will you need me to do so?

cgewecke · 2022-05-18T14:30:49Z

@FlattestWhite Just merged - we will deploy the adapter. I've asked some questions about how to parameterize the deployment in a slack channel Set shares with IC which Ed added you to yesterday.

Will continue discussion there :)

cgewecke · 2022-05-18T14:37:08Z

🎉 This PR is included in version 0.10.3-hhat.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

## [0.10.3](v0.10.2...v0.10.3) (2022-05-18) ### Bug Fixes * **adapter:** Support Curve stableswap pools ([#253](#253)) ([777ed94](777ed94))

# 1.0.0 (2022-10-04) ### Bug Fixes * **adapter:** Support Curve stableswap pools ([SetProtocol#253](https://github.com/IndexCoop/index-protocol/issues/253)) ([777ed94](777ed94)) * add proper tests, require statements for sellTokenForToken ([5c84995](5c84995)) * address review feedback ([5c6fcfc](5c6fcfc)) * **build:** Upgrade eslint-plugin-jsdoc to latest ([SetProtocol#228](https://github.com/IndexCoop/index-protocol/issues/228)) ([f5758d2](f5758d2)) * **interfaces:** add IAirdropModule, IClaimModule, and IWrapModuleV2 ([SetProtocol#252](https://github.com/IndexCoop/index-protocol/issues/252)) ([6999e84](6999e84)) * **interfaces:** Add protocol interfaces for set-v2-strategies [SIM-124] ([SetProtocol#226](https://github.com/IndexCoop/index-protocol/issues/226)) ([0bd5ca4](0bd5ca4)) * **IPerpV2BasisTradingModule:** Improve javadocs [SetProtocol#246](https://github.com/IndexCoop/index-protocol/issues/246) ([3129b37](3129b37)) * **NotionalTradeModule:** audit adjustments ([de39ee1](de39ee1)) * **PerpV2BasisTradingModule:** bytecode size limit bug ([SetProtocol#200](https://github.com/IndexCoop/index-protocol/issues/200)) ([ddfab38](ddfab38)) * **PerpV2BasisTradingModule:** Expose getter to fetch updated settled funding ([SetProtocol#244](https://github.com/IndexCoop/index-protocol/issues/244)) ([f215147](f215147)) * **PerpV2BasisTradingModule:** External audit fixes for low-risk findings and Internal audit fixes ([SetProtocol#232](https://github.com/IndexCoop/index-protocol/issues/232)) ([d7d37da](d7d37da)), closes [SetProtocol#243](https://github.com/IndexCoop/index-protocol/issues/243) * **PerpV2LeverageModuleV2:** Round up during withdraw ([SetProtocol#249](https://github.com/IndexCoop/index-protocol/issues/249)) ([7780963](7780963)) * remove incorrect require check for sellEthForTokenToUniswapV3 ([7f8048d](7f8048d)) * sellEthForTokenToUniswapV3 impl and address review comments ([73ea7b3](73ea7b3)) * **SlippageIssuanceModule:** Fix issuance & redemption flows to not update position units ([SetProtocol#254](https://github.com/IndexCoop/index-protocol/issues/254)) ([9be3d1b](9be3d1b)) * **tests:** Add Hardhat deployment subgraph test scripts ([SetProtocol#242](https://github.com/IndexCoop/index-protocol/issues/242)) ([0aa0d14](0aa0d14)) * **utils:** Port StringArrayUtils from set-v2-strategies [SIM-124] ([SetProtocol#227](https://github.com/IndexCoop/index-protocol/issues/227)) ([dbf6c04](dbf6c04)) ### Features * **adapter:** Add Curve trade adapter [SIM-173] ([SetProtocol#238](https://github.com/IndexCoop/index-protocol/issues/238)) ([f697783](f697783)) * **adapter:** Add RGT to Tribe migration adapter [SIM-52] ([SetProtocol#188](https://github.com/IndexCoop/index-protocol/issues/188)) ([ca5c2ab](ca5c2ab)) * add direct RFQT fill suport to ZeroExApiAdapter [SIM-230] ([SetProtocol#241](https://github.com/IndexCoop/index-protocol/issues/241)) ([f4b976e](f4b976e)) * **BytesArrayUtils:** Add BytesArrayUtils library ([SetProtocol#248](https://github.com/IndexCoop/index-protocol/issues/248)) ([7b35392](7b35392)) * **deprecation:** Consolidate or remove deprecated contracts [SIM-124] ([SetProtocol#225](https://github.com/IndexCoop/index-protocol/issues/225)) ([579852a](579852a)) * implement sellTokenForEthToUniswapV3 and sellEthForTokenToUniswapV3 ([2296f6e](2296f6e)) * initial stab at Uniswap V3 adaptor ([c25665b](c25665b)) * **Intefaces:** Add basis trading module interfaces ([SetProtocol#237](https://github.com/IndexCoop/index-protocol/issues/237)) ([c7af322](c7af322)) * **module:** Notional Trade Module ([SetProtocol#251](https://github.com/IndexCoop/index-protocol/issues/251)) ([49bdf46](49bdf46)) * **publishing:** Auto publish repository to npm in CI [SIM-123] ([SetProtocol#203](https://github.com/IndexCoop/index-protocol/issues/203)) ([8ddbbf1](8ddbbf1)) * **reorg:** Re-org modules into v1 and v2 directories ([SetProtocol#230](https://github.com/IndexCoop/index-protocol/issues/230)) ([d43df0c](d43df0c)) * **UniswapV3ExchangeAdapterV2:** Add UniswapV3ExchangeAdapterV2 exchange adapter ([SetProtocol#240](https://github.com/IndexCoop/index-protocol/issues/240)) ([b4d95ca](b4d95ca)) * verify path for multiple hops ([06ca9b6](06ca9b6)) * verify uniswap path for single hops WIP ([7905d73](7905d73))

# 1.0.0 (2022-10-05) ### Bug Fixes * **adapter:** Support Curve stableswap pools ([SetProtocol#253](https://github.com/IndexCoop/index-protocol/issues/253)) ([777ed94](777ed94)) * add proper tests, require statements for sellTokenForToken ([5c84995](5c84995)) * address review feedback ([5c6fcfc](5c6fcfc)) * **build:** Upgrade eslint-plugin-jsdoc to latest ([SetProtocol#228](https://github.com/IndexCoop/index-protocol/issues/228)) ([f5758d2](f5758d2)) * **interfaces:** add IAirdropModule, IClaimModule, and IWrapModuleV2 ([SetProtocol#252](https://github.com/IndexCoop/index-protocol/issues/252)) ([6999e84](6999e84)) * **interfaces:** Add protocol interfaces for set-v2-strategies [SIM-124] ([SetProtocol#226](https://github.com/IndexCoop/index-protocol/issues/226)) ([0bd5ca4](0bd5ca4)) * **IPerpV2BasisTradingModule:** Improve javadocs [SetProtocol#246](https://github.com/IndexCoop/index-protocol/issues/246) ([3129b37](3129b37)) * **NotionalTradeModule:** audit adjustments ([de39ee1](de39ee1)) * **PerpV2BasisTradingModule:** bytecode size limit bug ([SetProtocol#200](https://github.com/IndexCoop/index-protocol/issues/200)) ([ddfab38](ddfab38)) * **PerpV2BasisTradingModule:** Expose getter to fetch updated settled funding ([SetProtocol#244](https://github.com/IndexCoop/index-protocol/issues/244)) ([f215147](f215147)) * **PerpV2BasisTradingModule:** External audit fixes for low-risk findings and Internal audit fixes ([SetProtocol#232](https://github.com/IndexCoop/index-protocol/issues/232)) ([d7d37da](d7d37da)), closes [SetProtocol#243](https://github.com/IndexCoop/index-protocol/issues/243) * **PerpV2LeverageModuleV2:** Round up during withdraw ([SetProtocol#249](https://github.com/IndexCoop/index-protocol/issues/249)) ([7780963](7780963)) * remove incorrect require check for sellEthForTokenToUniswapV3 ([7f8048d](7f8048d)) * sellEthForTokenToUniswapV3 impl and address review comments ([73ea7b3](73ea7b3)) * **SlippageIssuanceModule:** Fix issuance & redemption flows to not update position units ([SetProtocol#254](https://github.com/IndexCoop/index-protocol/issues/254)) ([9be3d1b](9be3d1b)) * **tests:** Add Hardhat deployment subgraph test scripts ([SetProtocol#242](https://github.com/IndexCoop/index-protocol/issues/242)) ([0aa0d14](0aa0d14)) * **utils:** Port StringArrayUtils from set-v2-strategies [SIM-124] ([SetProtocol#227](https://github.com/IndexCoop/index-protocol/issues/227)) ([dbf6c04](dbf6c04)) ### Features * **adapter:** Add Curve trade adapter [SIM-173] ([SetProtocol#238](https://github.com/IndexCoop/index-protocol/issues/238)) ([f697783](f697783)) * **adapter:** Add RGT to Tribe migration adapter [SIM-52] ([SetProtocol#188](https://github.com/IndexCoop/index-protocol/issues/188)) ([ca5c2ab](ca5c2ab)) * add direct RFQT fill suport to ZeroExApiAdapter [SIM-230] ([SetProtocol#241](https://github.com/IndexCoop/index-protocol/issues/241)) ([f4b976e](f4b976e)) * **BytesArrayUtils:** Add BytesArrayUtils library ([SetProtocol#248](https://github.com/IndexCoop/index-protocol/issues/248)) ([7b35392](7b35392)) * **deprecation:** Consolidate or remove deprecated contracts [SIM-124] ([SetProtocol#225](https://github.com/IndexCoop/index-protocol/issues/225)) ([579852a](579852a)) * implement sellTokenForEthToUniswapV3 and sellEthForTokenToUniswapV3 ([2296f6e](2296f6e)) * initial stab at Uniswap V3 adaptor ([c25665b](c25665b)) * **Intefaces:** Add basis trading module interfaces ([SetProtocol#237](https://github.com/IndexCoop/index-protocol/issues/237)) ([c7af322](c7af322)) * **module:** Notional Trade Module ([SetProtocol#251](https://github.com/IndexCoop/index-protocol/issues/251)) ([49bdf46](49bdf46)) * **publishing:** Auto publish repository to npm in CI [SIM-123] ([SetProtocol#203](https://github.com/IndexCoop/index-protocol/issues/203)) ([8ddbbf1](8ddbbf1)) * **reorg:** Re-org modules into v1 and v2 directories ([SetProtocol#230](https://github.com/IndexCoop/index-protocol/issues/230)) ([d43df0c](d43df0c)) * **UniswapV3ExchangeAdapterV2:** Add UniswapV3ExchangeAdapterV2 exchange adapter ([SetProtocol#240](https://github.com/IndexCoop/index-protocol/issues/240)) ([b4d95ca](b4d95ca)) * verify path for multiple hops ([06ca9b6](06ca9b6)) * verify uniswap path for single hops WIP ([7905d73](7905d73))

FlattestWhite added 7 commits May 14, 2022 01:11

.

bde3128

.

8167c8f

fix test

ab431d2

.

67dd890

add adapter

519ee67

.

1022fbf

.

d064c51

FlattestWhite commented May 14, 2022

View reviewed changes

FlattestWhite marked this pull request as ready for review May 14, 2022 01:06

cgewecke self-requested a review May 16, 2022 20:45

cgewecke reviewed May 16, 2022

View reviewed changes

FlattestWhite added 3 commits May 18, 2022 01:21

integration test

8b48a18

get forked tests passing

f4b8bae

fix steth alm test

011b53a

FlattestWhite requested a review from cgewecke May 17, 2022 16:13

cgewecke approved these changes May 17, 2022

View reviewed changes

cgewecke merged commit 777ed94 into SetProtocol:master May 18, 2022

cgewecke added the released on @hardhat label May 18, 2022

cgewecke pushed a commit that referenced this pull request May 18, 2022

chore(release): 0.10.3 [skip ci]

c04e399

## [0.10.3](v0.10.2...v0.10.3) (2022-05-18) ### Bug Fixes * **adapter:** Support Curve stableswap pools ([#253](#253)) ([777ed94](777ed94))

cgewecke mentioned this pull request May 18, 2022

Curve amm adapter #155

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Curve stableswap pools #253

Support Curve stableswap pools #253

FlattestWhite commented May 13, 2022 •

edited

Loading

FlattestWhite May 14, 2022 •

edited

Loading

cgewecke left a comment •

edited

Loading

FlattestWhite commented May 17, 2022

cgewecke left a comment

FlattestWhite commented May 18, 2022

cgewecke commented May 18, 2022

cgewecke commented May 18, 2022

Support Curve stableswap pools #253

Support Curve stableswap pools #253

Conversation

FlattestWhite commented May 13, 2022 • edited Loading

Code Review Processes

New Feature Review

FlattestWhite May 14, 2022 • edited Loading

Choose a reason for hiding this comment

cgewecke left a comment • edited Loading

Choose a reason for hiding this comment

FlattestWhite commented May 17, 2022

cgewecke left a comment

Choose a reason for hiding this comment

FlattestWhite commented May 18, 2022

cgewecke commented May 18, 2022

cgewecke commented May 18, 2022

FlattestWhite commented May 13, 2022 •

edited

Loading

FlattestWhite May 14, 2022 •

edited

Loading

cgewecke left a comment •

edited

Loading