fix(SlippageIssuanceModule): Fix issuance & redemption flows to not update position units #254

0xSachinK · 2022-05-19T10:05:07Z

Bug Analysis

Triage Basis Trading: Delta Neutrality Deviation Spike

Virtual position units update during issuance and redemption.

Requirement

Update calculations to

Ensure position units don't change due to issuance and redemption.
Ensure we aren't double charging fees in the new flow.

Solution

Sheet explaining new calculations which fulfill the requirements

Tasks

Implement Fix
Fix existing tests
Add new test cases to ensure position units don't update

contracts/protocol/modules/v1/SlippageIssuanceModule.sol

test/integration/perpV2BasisTradingSlippageIssuance.spec.ts

bweick · 2022-05-19T15:06:32Z

test/integration/perpV2BasisTradingSlippageIssuance.spec.ts

+        expect(finalDefaultPositionUnit).to.eq(initialDefaultPositionUnit);
+      });
+
+      it("should NOT update the virtual quote token position unit", async () => {


Were you able to verify the position unit change without the contract change?

Yes, on tenderly by inspecting transactions on the old SIM contract.

…nce and redemption

cgewecke · 2022-05-20T15:00:09Z

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

The release is available on:

Your semantic-release bot 📦🚀

## [0.10.4](v0.10.3...v0.10.4) (2022-05-20) ### Bug Fixes * **SlippageIssuanceModule:** Fix issuance & redemption flows to not update position units ([#254](#254)) ([9be3d1b](9be3d1b))

# 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))

bweick reviewed May 19, 2022

View reviewed changes

contracts/protocol/modules/v1/SlippageIssuanceModule.sol Outdated Show resolved Hide resolved

bweick reviewed May 19, 2022

View reviewed changes

test/integration/perpV2BasisTradingSlippageIssuance.spec.ts Show resolved Hide resolved

bweick reviewed May 19, 2022

View reviewed changes

Reapply all fixes by Brain; Skip package.json changes

e8e07d9

0xSachinK force-pushed the sachin/bt-triage-sim-fix branch from c1ab8b3 to e8e07d9 Compare May 20, 2022 08:39

0xSachinK added 6 commits May 20, 2022 15:25

Fix SIM

f80c927

Fix PerpV2LeverageModule<>SIM integration tests

ebf53c3

Fix BTM <> SIM integration tests

60334bd

Add tests to ensure base token position unit does not update on issua…

a341dbd

…nce and redemption

Bump package to 0.10.3-basis.0 and publish

c2842b8

Override issue and redeem; Add tests

6c2c4ab

bweick approved these changes May 20, 2022

View reviewed changes

Reset package version to 0.10.3

3813824

0xSachinK merged commit 9be3d1b into master May 20, 2022

0xSachinK deleted the sachin/bt-triage-sim-fix branch May 20, 2022 14:52

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(SlippageIssuanceModule): Fix issuance & redemption flows to not update position units #254

fix(SlippageIssuanceModule): Fix issuance & redemption flows to not update position units #254

0xSachinK commented May 19, 2022 •

edited

Loading

bweick May 19, 2022

0xSachinK May 20, 2022

cgewecke commented May 20, 2022

fix(SlippageIssuanceModule): Fix issuance & redemption flows to not update position units #254

fix(SlippageIssuanceModule): Fix issuance & redemption flows to not update position units #254

Conversation

0xSachinK commented May 19, 2022 • edited Loading

Bug Analysis

Requirement

Solution

Tasks

bweick May 19, 2022

Choose a reason for hiding this comment

0xSachinK May 20, 2022

Choose a reason for hiding this comment

cgewecke commented May 20, 2022

0xSachinK commented May 19, 2022 •

edited

Loading