Skip to content

Commit 58113aa

Browse files
cgeweckecgewecke
authored
fix(tests): Small fixes for BasisTradingModule audit changes (#243)
* Add fix for _updateExternalPositionUnit * Add test for negative balance check in _calculateNetFeesPositionUnit * Fix externalPositionUnit test in perpV2LeverageV2SlippageIssuance tests * Use `tradeAndTrackFunding` everywhere in BasisTrading integration tests
1 parent 56bfe82 commit 58113aa

File tree

6 files changed

+67
-32
lines changed

6 files changed

+67
-32
lines changed

‎contracts/protocol/modules/v2/PerpV2LeverageModuleV2.sol

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { PreciseUnitMath } from "../../../lib/PreciseUnitMath.sol";
4747
import { AddressArrayUtils } from "../../../lib/AddressArrayUtils.sol";
4848
import { UnitConversionUtils } from "../../../lib/UnitConversionUtils.sol";
4949

50+
5051
/**
5152
* @title PerpV2LeverageModuleV2
5253
* @author Set Protocol
@@ -1053,25 +1054,28 @@ contract PerpV2LeverageModuleV2 is ModuleBaseV2, ReentrancyGuard, Ownable, SetTo
10531054
}
10541055

10551056
/**
1056-
* @dev Sets the external position unit based on PerpV2 account value.
1057+
* @dev Sets the external position unit based on PerpV2 Vault collateral balance. If there more than 1
1058+
* position unit of USDC of account value (allowing for PRECISE UNIT rounding errors) adds the component
1059+
* and/or the external position to the SetToken. Else, untracks the component and/or removes external
1060+
* position from the SetToken. Refer to PositionV2#editExternalPosition for detailed flow.
1061+
*
1062+
* NOTE: Setting external position unit to the collateral balance is acceptable because, as noted above, the
1063+
* external position unit is only updated on an as-needed basis during issuance, redemption, deposit and
1064+
* withdraw. It does not reflect the current value of the Set's perpetual position. The true current value can
1065+
* be calculated from getPositionNotionalInfo.
10571066
*
10581067
* @param _setToken Instance of SetToken
10591068
*/
10601069
function _updateExternalPositionUnit(ISetToken _setToken) internal {
1061-
int256 accountValueUnit = perpClearingHouse.getAccountValue(address(_setToken))
1070+
int256 collateralValueUnit = perpVault.getBalance(address(_setToken))
1071+
.toPreciseUnitsFromDecimals(collateralDecimals)
10621072
.preciseDiv(_setToken.totalSupply().toInt256())
10631073
.fromPreciseUnitToDecimals(collateralDecimals);
10641074

1065-
// Set the external position unit based on PerpV2 Account value; If there is more than 1 position unit of USDC of account
1066-
// value (to tolerate PRECISE_UNIT math rounding errors), add the component and/or the external position to the SetToken.
1067-
// Else, untrack the component and/or remove external position from the SetToken. Refer to PositionV2#editExternalPosition for detailed flow.
1068-
// Setting external posiiton unit to 1 or 0 is acceptable because, As noted above, the external position unit is only updated on an as-needed
1069-
// basis during issuance, redemption, deposit and withdraw. It does not reflect the current value of the Set's perpetual position.
1070-
// The current value can be calculated from getPositionNotionalInfo.
10711075
_setToken.editExternalPosition(
10721076
address(collateralToken),
10731077
address(this),
1074-
accountValueUnit > 1 ? 1 : 0,
1078+
collateralValueUnit,
10751079
""
10761080
);
10771081
}

‎test/integration/perpV2BasisTradingSlippageIssuance.spec.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
254254
setToken.address,
255255
{
256256
feeRecipient: owner.address,
257-
performanceFeePercentage: ether(.0),
257+
performanceFeePercentage: ether(.1),
258258
maxPerformanceFeePercentage: ether(.2)
259259
}
260260
);
@@ -294,6 +294,7 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
294294
baseToken,
295295
2,
296296
ether(.02),
297+
true,
297298
true
298299
);
299300

@@ -508,6 +509,7 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
508509
baseToken,
509510
2,
510511
ether(.02),
512+
true,
511513
true
512514
);
513515

@@ -855,6 +857,7 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
855857
baseToken,
856858
6,
857859
ether(.02),
860+
true,
858861
true
859862
);
860863

@@ -909,7 +912,12 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
909912

910913
describe("when liquidation results in negative account value", () => {
911914
beforeEach(async () => {
915+
// Move oracle price down, wait one day
916+
await perpSetup.setBaseTokenOraclePrice(vETH, usdcUnits(10.5));
917+
await increaseTimeAsync(ONE_DAY_IN_SECONDS);
918+
912919
// Calculated leverage = ~8.5X = 8_654_438_822_995_683_587
920+
// Lever again to track funding as `settled`
913921
await leverUp(
914922
setToken,
915923
perpBasisTradingModule,
@@ -918,9 +926,13 @@ describe("PerpV2BasisTradingSlippageIssuance", () => {
918926
baseToken,
919927
6,
920928
ether(.02),
929+
true,
921930
true
922931
);
923932

933+
// Freeze funding changes
934+
await perpSetup.clearingHouseConfig.setMaxFundingRate(ZERO);
935+
924936
// Move oracle price down to 5 USDC to enable liquidation
925937
await perpSetup.setBaseTokenOraclePrice(vETH, usdcUnits(5.0));
926938

‎test/integration/perpV2LeverageV2SlippageIssuance.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {
3434
} from "@utils/test/index";
3535
import { PerpV2Fixture, SystemFixture } from "@utils/fixtures";
3636
import { BigNumber } from "ethers";
37-
import { ONE, ADDRESS_ZERO, ZERO, MAX_UINT_256, ZERO_BYTES } from "@utils/constants";
37+
import { ADDRESS_ZERO, ZERO, MAX_UINT_256, ZERO_BYTES } from "@utils/constants";
3838

3939
const expect = getWaffleExpect();
4040

@@ -373,7 +373,7 @@ describe("PerpV2LeverageSlippageIssuance", () => {
373373
);
374374

375375
expect(defaultPositionUnit).eq(ZERO);
376-
expect(externalPositionUnit).eq(ONE);
376+
expect(externalPositionUnit).eq(depositQuantityUnit);
377377
});
378378
});
379379

‎test/protocol/modules/v2/perpV2BasisTradingModule.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,8 @@ describe("PerpV2BasisTradingModule", () => {
779779
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpBasisTradingModule.address);
780780
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
781781
subjectSetToken,
782-
perpSetup
782+
perpSetup,
783+
perpBasisTradingModule
783784
);
784785
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
785786
});
@@ -870,7 +871,8 @@ describe("PerpV2BasisTradingModule", () => {
870871
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpBasisTradingModule.address);
871872
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
872873
subjectSetToken,
873-
perpSetup
874+
perpSetup,
875+
perpBasisTradingModule
874876
);
875877
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
876878
});
@@ -1020,7 +1022,8 @@ describe("PerpV2BasisTradingModule", () => {
10201022
const externalPositionUnit = await setToken.getExternalPositionRealUnit(usdc.address, perpBasisTradingModule.address);
10211023
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
10221024
setToken,
1023-
perpSetup
1025+
perpSetup,
1026+
perpBasisTradingModule
10241027
);
10251028
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
10261029
});

‎test/protocol/modules/v2/perpV2LeverageModuleV2.spec.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,8 @@ describe("PerpV2LeverageModuleV2", () => {
12051205
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
12061206
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
12071207
subjectSetToken,
1208-
perpSetup
1208+
perpSetup,
1209+
perpLeverageModule,
12091210
);
12101211
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
12111212
});
@@ -1261,7 +1262,8 @@ describe("PerpV2LeverageModuleV2", () => {
12611262
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
12621263
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
12631264
subjectSetToken,
1264-
perpSetup
1265+
perpSetup,
1266+
perpLeverageModule
12651267
);
12661268

12671269
// Deposit notional amount = specified position unit * totalSupply = 1 * 2 = $2
@@ -1344,7 +1346,8 @@ describe("PerpV2LeverageModuleV2", () => {
13441346
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
13451347
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
13461348
subjectSetToken,
1347-
perpSetup
1349+
perpSetup,
1350+
perpLeverageModule
13481351
);
13491352

13501353
// Deposit amount = $1 * 2 (two deposits)
@@ -1438,7 +1441,8 @@ describe("PerpV2LeverageModuleV2", () => {
14381441
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
14391442
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
14401443
subjectSetToken,
1441-
perpSetup
1444+
perpSetup,
1445+
perpLeverageModule
14421446
);
14431447

14441448
// Deposit amount = $1 * 2 (two deposits)
@@ -1579,7 +1583,8 @@ describe("PerpV2LeverageModuleV2", () => {
15791583
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
15801584
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
15811585
subjectSetToken,
1582-
perpSetup
1586+
perpSetup,
1587+
perpLeverageModule
15831588
);
15841589
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
15851590
});
@@ -1657,7 +1662,8 @@ describe("PerpV2LeverageModuleV2", () => {
16571662
const externalPositionUnit = await subjectSetToken.getExternalPositionRealUnit(usdc.address, perpLeverageModule.address);
16581663
const expectedExternalPositionUnit = await calculateExternalPositionUnit(
16591664
subjectSetToken,
1660-
perpSetup
1665+
perpSetup,
1666+
perpLeverageModule
16611667
);
16621668
expect(externalPositionUnit).eq(expectedExternalPositionUnit);
16631669
});

‎utils/common/perpV2Utils.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
preciseMul
99
} from "../index";
1010

11-
import { ONE, TWO, ZERO, ONE_DAY_IN_SECONDS } from "../constants";
11+
import { TWO, ZERO, ONE_DAY_IN_SECONDS } from "../constants";
1212
import { PerpV2BasisTradingModule, PerpV2LeverageModuleV2, SetToken } from "../contracts";
1313
import { PerpV2Fixture } from "../fixtures";
1414

@@ -27,7 +27,8 @@ export async function leverUp(
2727
baseToken: Address,
2828
leverageRatio: number,
2929
slippagePercentage: BigNumber,
30-
isLong: boolean
30+
isLong: boolean,
31+
trackFunding?: boolean,
3132
): Promise<BigNumber>{
3233
const spotPrice = await fixture.getSpotPrice(baseToken);
3334
const totalSupply = await setToken.totalSupply();
@@ -50,12 +51,21 @@ export async function leverUp(
5051
totalSupply
5152
);
5253

53-
await module.connect(owner.wallet).trade(
54-
setToken.address,
55-
baseToken,
56-
baseTradeQuantityUnit,
57-
receiveQuoteQuantityUnit
58-
);
54+
if (trackFunding) {
55+
await (module as PerpV2BasisTradingModule).connect(owner.wallet).tradeAndTrackFunding(
56+
setToken.address,
57+
baseToken,
58+
baseTradeQuantityUnit,
59+
receiveQuoteQuantityUnit
60+
);
61+
} else {
62+
await module.connect(owner.wallet).trade(
63+
setToken.address,
64+
baseToken,
65+
baseTradeQuantityUnit,
66+
receiveQuoteQuantityUnit
67+
);
68+
}
5969

6070
return baseTradeQuantityUnit;
6171
}
@@ -180,10 +190,10 @@ export async function calculateUSDCTransferOutPreciseUnits(
180190
export async function calculateExternalPositionUnit(
181191
setToken: SetToken,
182192
fixture: PerpV2Fixture,
193+
module: PerpV2LeverageModuleV2 | PerpV2BasisTradingModule
183194
): Promise<BigNumber> {
184-
const totalPositionValue = await fixture.clearingHouse.getAccountValue(setToken.address);
185-
// return toUSDCDecimals(preciseDiv(totalPositionValue, await setToken.totalSupply()));
186-
return totalPositionValue.gt(ZERO) ? ONE : ZERO;
195+
const accountInfo = await module.getAccountInfo(setToken.address);
196+
return toUSDCDecimals(preciseDiv(accountInfo.collateralBalance, await setToken.totalSupply()));
187197
}
188198

189199
// On every interaction with perpV2, it settles funding for a trader into owed realized pnl

0 commit comments

Comments
 (0)