Skip to content

Commit a1a3ee1

Browse files
kaze-cowclaude
andcommitted
refactor: improve test suite DRYness and remove redundant tests
Refactor CowEvcOpenPositionWrapper test files to reduce code duplication and improve maintainability: Integration tests (CowEvcOpenPositionWrapper.t.sol): - Add helper functions for common test setup patterns - _createDefaultParams(): standardized param creation - _setupUserSusdsApproval(): SUSDS approval setup - _setupUserPreApprovedFlow(): pre-approved hash flow setup - _createPermitSignature(): permit signature creation - _encodeWrapperData(): wrapper data encoding - _executeWrappedSettlement(): settlement execution - _verifyPositionOpened(): position verification assertions - Add DEFAULT_BORROW_AMOUNT and DEFAULT_BUY_AMOUNT constants - Reduce test function size by ~50% while maintaining clarity Unit tests (CowEvcOpenPositionWrapper.unit.t.sol): - Add helper functions for common patterns - _getDefaultParams(): returns default params (tests modify as needed) - _getEmptySettleData(): creates empty settlement data - _encodeWrapperData(): wrapper data encoding - _setupPreApprovedHash(): pre-approved hash setup - Add DEFAULT_COLLATERAL_AMOUNT and DEFAULT_BORROW_AMOUNT constants - Remove 4 redundant tests with no coverage impact: - test_Constructor_SetsName (trivial getter) - test_GetApprovalHash_Consistency (deterministic function) - test_ParseWrapperData_WithSignature (duplicate code path) - test_GetSignedCalldata_ReturnsCorrectStructure (implicitly covered) - Reduce from 24 to 20 unit tests with no loss in coverage All tests pass: 6/6 integration tests, 20/20 unit tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a4cdeae commit a1a3ee1

File tree

2 files changed

+220
-419
lines changed

2 files changed

+220
-419
lines changed

test/CowEvcOpenPositionWrapper.t.sol

Lines changed: 121 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
2020
SignerECDSA internal ecdsa;
2121

2222
uint256 constant SUSDS_MARGIN = 2000e18;
23+
uint256 constant DEFAULT_BORROW_AMOUNT = 1e18;
24+
uint256 constant DEFAULT_BUY_AMOUNT = 999e18;
2325

2426
function setUp() public override {
2527
super.setUp();
@@ -54,6 +56,91 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
5456
ICowSettlement.Interaction[][3] interactions;
5557
}
5658

59+
/// @notice Create default OpenPositionParams for testing
60+
function _createDefaultParams(address owner, address account)
61+
internal
62+
view
63+
returns (CowEvcOpenPositionWrapper.OpenPositionParams memory)
64+
{
65+
return CowEvcOpenPositionWrapper.OpenPositionParams({
66+
owner: owner,
67+
account: account,
68+
deadline: block.timestamp + 1 hours,
69+
collateralVault: ESUSDS,
70+
borrowVault: EWETH,
71+
collateralAmount: SUSDS_MARGIN,
72+
borrowAmount: DEFAULT_BORROW_AMOUNT
73+
});
74+
}
75+
76+
/// @notice Setup user approvals for SUSDS deposit
77+
function _setupUserSusdsApproval() internal {
78+
vm.prank(user);
79+
IERC20(SUSDS).approve(ESUSDS, type(uint256).max);
80+
}
81+
82+
/// @notice Setup user approvals for pre-approved hash flow
83+
function _setupUserPreApprovedFlow(address account, bytes32 hash) internal {
84+
vm.startPrank(user);
85+
IERC20(SUSDS).approve(ESUSDS, type(uint256).max);
86+
evc.setAccountOperator(user, address(openPositionWrapper), true);
87+
evc.setAccountOperator(account, address(openPositionWrapper), true);
88+
openPositionWrapper.setPreApprovedHash(hash, true);
89+
vm.stopPrank();
90+
}
91+
92+
/// @notice Create permit signature for EVC operator
93+
function _createPermitSignature(CowEvcOpenPositionWrapper.OpenPositionParams memory params)
94+
internal
95+
returns (bytes memory)
96+
{
97+
ecdsa.setPrivateKey(privateKey);
98+
return ecdsa.signPermit(
99+
params.owner,
100+
address(openPositionWrapper),
101+
uint256(uint160(address(openPositionWrapper))),
102+
0,
103+
params.deadline,
104+
0,
105+
openPositionWrapper.getSignedCalldata(params)
106+
);
107+
}
108+
109+
/// @notice Encode wrapper data with length prefix
110+
function _encodeWrapperData(CowEvcOpenPositionWrapper.OpenPositionParams memory params, bytes memory signature)
111+
internal
112+
pure
113+
returns (bytes memory)
114+
{
115+
bytes memory wrapperData = abi.encode(params, signature);
116+
return abi.encodePacked(uint16(wrapperData.length), wrapperData);
117+
}
118+
119+
/// @notice Execute wrapped settlement through solver
120+
function _executeWrappedSettlement(bytes memory settleData, bytes memory wrapperData) internal {
121+
address[] memory targets = new address[](1);
122+
bytes[] memory datas = new bytes[](1);
123+
targets[0] = address(openPositionWrapper);
124+
datas[0] = abi.encodeCall(openPositionWrapper.wrappedSettle, (settleData, wrapperData));
125+
solver.runBatch(targets, datas);
126+
}
127+
128+
/// @notice Verify position was opened successfully
129+
function _verifyPositionOpened(
130+
address account,
131+
uint256 expectedCollateral,
132+
uint256 expectedDebt,
133+
uint256 allowedDelta
134+
) internal view {
135+
assertApproxEqAbs(
136+
IEVault(ESUSDS).convertToAssets(IERC20(ESUSDS).balanceOf(account)),
137+
expectedCollateral,
138+
allowedDelta,
139+
"User should have collateral deposited"
140+
);
141+
assertEq(IEVault(EWETH).debtOf(account), expectedDebt, "User should have debt");
142+
}
143+
57144
/// @notice Create settlement data for opening a leveraged position
58145
/// @dev Sells borrowed WETH to buy SUSDS which gets deposited into the vault
59146
function getOpenPositionSettlement(
@@ -107,71 +194,37 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
107194
function test_OpenPositionWrapper_Success() external {
108195
vm.skip(bytes(forkRpcUrl).length == 0);
109196

110-
uint256 borrowAmount = 1e18; // Borrow 1 WETH
111-
uint256 expectedBuyAmount = 999e18; // Expect to receive 999 eSUSDS
112-
113197
address account = address(uint160(user) ^ 1);
114198

199+
// Create params using helper
200+
CowEvcOpenPositionWrapper.OpenPositionParams memory params = _createDefaultParams(user, account);
201+
115202
// Get settlement data
116203
SettlementData memory settlement =
117-
getOpenPositionSettlement(user, account, WETH, ESUSDS, borrowAmount, expectedBuyAmount);
204+
getOpenPositionSettlement(user, account, WETH, ESUSDS, DEFAULT_BORROW_AMOUNT, DEFAULT_BUY_AMOUNT);
118205

119-
// Prepare OpenPositionParams
120-
uint256 deadline = block.timestamp + 1 hours;
121-
ecdsa.setPrivateKey(privateKey);
122-
123-
CowEvcOpenPositionWrapper.OpenPositionParams memory params = CowEvcOpenPositionWrapper.OpenPositionParams({
124-
owner: user,
125-
account: account,
126-
deadline: deadline,
127-
collateralVault: ESUSDS,
128-
borrowVault: EWETH,
129-
collateralAmount: SUSDS_MARGIN,
130-
borrowAmount: borrowAmount
131-
});
206+
// Setup user approvals
207+
_setupUserSusdsApproval();
132208

133-
vm.startPrank(user);
134-
135-
// User approves SUSDS vault for deposit of the margin. Only required if there is margin to deposit and the user hasn't already approved
136-
IERC20(SUSDS).approve(ESUSDS, type(uint256).max);
137-
138-
// User signs (in this case we use setPreSignature. this is just for local testing purposes. Real flow would be a off-chain signature)
209+
// User signs order (using setPreSignature for testing)
210+
vm.prank(user);
139211
COW_SETTLEMENT.setPreSignature(settlement.orderUid, true);
140212

141-
// Sign permit for EVC operator
142-
bytes memory permitSignature = ecdsa.signPermit(
143-
user,
144-
address(openPositionWrapper),
145-
uint256(uint160(address(openPositionWrapper))),
146-
0,
147-
deadline,
148-
0,
149-
openPositionWrapper.getSignedCalldata(params)
150-
);
151-
152-
vm.stopPrank();
213+
// Create permit signature
214+
bytes memory permitSignature = _createPermitSignature(params);
153215

154216
// Record balances before
155217
uint256 susdsBalanceBefore = IERC20(ESUSDS).balanceOf(user);
156-
uint256 debtBefore = IEVault(EWETH).debtOf(user);
218+
uint256 debtBefore = IEVault(EWETH).debtOf(account);
157219

158-
// Encode settlement data
220+
// Encode settlement and wrapper data
159221
bytes memory settleData = abi.encodeCall(
160222
ICowSettlement.settle,
161223
(settlement.tokens, settlement.clearingPrices, settlement.trades, settlement.interactions)
162224
);
225+
bytes memory wrapperData = _encodeWrapperData(params, permitSignature);
163226

164-
// Encode wrapper data with OpenPositionParams
165-
bytes memory wrapperData = abi.encode(params, permitSignature);
166-
wrapperData = abi.encodePacked(uint16(wrapperData.length), wrapperData);
167-
168-
// Execute wrapped settlement through solver
169-
address[] memory targets = new address[](1);
170-
bytes[] memory datas = new bytes[](1);
171-
targets[0] = address(openPositionWrapper);
172-
datas[0] = abi.encodeCall(openPositionWrapper.wrappedSettle, (settleData, wrapperData));
173-
174-
// Expect the event to be emitted
227+
// Expect event emission
175228
vm.expectEmit(true, true, true, true);
176229
emit CowEvcOpenPositionWrapper.CowEvcPositionOpened(
177230
params.owner,
@@ -182,16 +235,11 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
182235
params.borrowAmount
183236
);
184237

185-
solver.runBatch(targets, datas);
238+
// Execute wrapped settlement
239+
_executeWrappedSettlement(settleData, wrapperData);
186240

187-
// Verify the position was created successfully
188-
assertApproxEqAbs(
189-
IEVault(ESUSDS).convertToAssets(IERC20(ESUSDS).balanceOf(account)),
190-
expectedBuyAmount + SUSDS_MARGIN,
191-
1 ether,
192-
"User should have collateral deposited"
193-
);
194-
assertEq(IEVault(EWETH).debtOf(account), borrowAmount, "User should have debt");
241+
// Verify position was created successfully
242+
_verifyPositionOpened(account, DEFAULT_BUY_AMOUNT + SUSDS_MARGIN, DEFAULT_BORROW_AMOUNT, 1 ether);
195243
assertEq(debtBefore, 0, "User should start with no debt");
196244
assertEq(susdsBalanceBefore, 0, "User should start with no eSUSDS");
197245
}
@@ -222,15 +270,8 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
222270

223271
/// @notice Test parseWrapperData function
224272
function test_OpenPositionWrapper_ParseWrapperData() external view {
225-
CowEvcOpenPositionWrapper.OpenPositionParams memory params = CowEvcOpenPositionWrapper.OpenPositionParams({
226-
owner: user,
227-
account: address(uint160(user) ^ 1),
228-
deadline: block.timestamp + 1 hours,
229-
collateralVault: ESUSDS,
230-
borrowVault: EWETH,
231-
collateralAmount: SUSDS_MARGIN,
232-
borrowAmount: 1e18
233-
});
273+
address account = address(uint160(user) ^ 1);
274+
CowEvcOpenPositionWrapper.OpenPositionParams memory params = _createDefaultParams(user, account);
234275

235276
bytes memory wrapperData = abi.encode(params, new bytes(0));
236277
bytes memory remainingData = openPositionWrapper.parseWrapperData(wrapperData);
@@ -243,16 +284,8 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
243284
function test_OpenPositionWrapper_SetPreApprovedHash() external {
244285
vm.skip(bytes(forkRpcUrl).length == 0);
245286

246-
CowEvcOpenPositionWrapper.OpenPositionParams memory params = CowEvcOpenPositionWrapper.OpenPositionParams({
247-
owner: user,
248-
account: address(uint160(user) ^ 1),
249-
deadline: block.timestamp + 1 hours,
250-
collateralVault: ESUSDS,
251-
borrowVault: EWETH,
252-
collateralAmount: SUSDS_MARGIN,
253-
borrowAmount: 1e18
254-
});
255-
287+
address account = address(uint160(user) ^ 1);
288+
CowEvcOpenPositionWrapper.OpenPositionParams memory params = _createDefaultParams(user, account);
256289
bytes32 hash = openPositionWrapper.getApprovalHash(params);
257290

258291
// Initially hash should not be approved
@@ -281,69 +314,34 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
281314
function test_OpenPositionWrapper_WithPreApprovedHash() external {
282315
vm.skip(bytes(forkRpcUrl).length == 0);
283316

284-
uint256 borrowAmount = 1e18; // Borrow 1 WETH
285-
uint256 expectedBuyAmount = 999e18; // Expect to receive 999 eSUSDS
286-
287317
address account = address(uint160(user) ^ 1);
288318

289-
// Prepare OpenPositionParams
290-
CowEvcOpenPositionWrapper.OpenPositionParams memory params = CowEvcOpenPositionWrapper.OpenPositionParams({
291-
owner: user,
292-
account: account,
293-
deadline: block.timestamp + 1 hours,
294-
collateralVault: ESUSDS,
295-
borrowVault: EWETH,
296-
collateralAmount: SUSDS_MARGIN,
297-
borrowAmount: borrowAmount
298-
});
319+
// Create params using helper
320+
CowEvcOpenPositionWrapper.OpenPositionParams memory params = _createDefaultParams(user, account);
299321

300322
// Get settlement data
301323
SettlementData memory settlement =
302-
getOpenPositionSettlement(user, account, WETH, ESUSDS, borrowAmount, expectedBuyAmount);
324+
getOpenPositionSettlement(user, account, WETH, ESUSDS, DEFAULT_BORROW_AMOUNT, DEFAULT_BUY_AMOUNT);
303325

304-
vm.startPrank(user);
305-
306-
// User approves SUSDS vault for deposit of the margin
307-
// This is only needed if the user is depositing new margin
308-
IERC20(SUSDS).approve(ESUSDS, type(uint256).max);
309-
310-
// User approves the wrapper to be operator (both of the main account and the subaccount)
311-
// This is only needed if its the first time the user/account is using this wrapper
312-
evc.setAccountOperator(user, address(openPositionWrapper), true);
313-
evc.setAccountOperator(account, address(openPositionWrapper), true);
314-
315-
// User pre-approves the hash for the wrapper operation (absolutely required every order)
326+
// Setup user approvals and pre-approve hash
316327
bytes32 hash = openPositionWrapper.getApprovalHash(params);
317-
openPositionWrapper.setPreApprovedHash(hash, true);
328+
_setupUserPreApprovedFlow(account, hash);
318329

319330
// User pre-approves the order on CowSwap
320-
// NOTE: this could technically be exchanged for a Permit2 approve on the wrapper contract and EIP-1271 authentication,
321-
// and that would leave the user with only 1 off-chain Permit2 call
322-
// but combined with the approval txns that are needed above, this flow doesn't seem very viable.
331+
vm.prank(user);
323332
COW_SETTLEMENT.setPreSignature(settlement.orderUid, true);
324333

325-
vm.stopPrank();
326-
327334
// Record balances before
328335
uint256 debtBefore = IEVault(EWETH).debtOf(account);
329336

330-
// Encode settlement data
337+
// Encode settlement and wrapper data (empty signature since pre-approved)
331338
bytes memory settleData = abi.encodeCall(
332339
ICowSettlement.settle,
333340
(settlement.tokens, settlement.clearingPrices, settlement.trades, settlement.interactions)
334341
);
342+
bytes memory wrapperData = _encodeWrapperData(params, new bytes(0));
335343

336-
// Encode wrapper data with OpenPositionParams (empty signature since pre-approved)
337-
bytes memory wrapperData = abi.encode(params, new bytes(0));
338-
wrapperData = abi.encodePacked(uint16(wrapperData.length), wrapperData);
339-
340-
// Execute wrapped settlement through solver
341-
address[] memory targets = new address[](1);
342-
bytes[] memory datas = new bytes[](1);
343-
targets[0] = address(openPositionWrapper);
344-
datas[0] = abi.encodeCall(openPositionWrapper.wrappedSettle, (settleData, wrapperData));
345-
346-
// Expect the event to be emitted
344+
// Expect event emission
347345
vm.expectEmit(true, true, true, true);
348346
emit CowEvcOpenPositionWrapper.CowEvcPositionOpened(
349347
params.owner,
@@ -354,16 +352,11 @@ contract CowEvcOpenPositionWrapperTest is CowBaseTest {
354352
params.borrowAmount
355353
);
356354

357-
solver.runBatch(targets, datas);
355+
// Execute wrapped settlement
356+
_executeWrappedSettlement(settleData, wrapperData);
358357

359358
// Verify the position was created successfully
360-
assertApproxEqAbs(
361-
IEVault(ESUSDS).convertToAssets(IERC20(ESUSDS).balanceOf(account)),
362-
expectedBuyAmount + SUSDS_MARGIN,
363-
1 ether,
364-
"User should have collateral deposited"
365-
);
366-
assertEq(IEVault(EWETH).debtOf(account), borrowAmount, "User should have debt");
359+
_verifyPositionOpened(account, DEFAULT_BUY_AMOUNT + SUSDS_MARGIN, DEFAULT_BORROW_AMOUNT, 1 ether);
367360
assertEq(debtBefore, 0, "User should start with no debt");
368361
}
369362
}

0 commit comments

Comments
 (0)