Skip to content

Commit 48fbfef

Browse files
alex0207salex0207s
committed
refine code based on comment and add some test functions
1 parent 4067352 commit 48fbfef

File tree

4 files changed

+80
-31
lines changed

4 files changed

+80
-31
lines changed

‎contracts/ConditionalSwap.sol

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,29 +82,25 @@ contract ConditionalSwap is IConditionalSwap, Ownable, TokenCollector, EIP712 {
8282
bytes1 settlementType = settlementData[0];
8383
bytes memory strategyData = settlementData[1:];
8484

85-
uint256 returnedAmount;
8685
if (settlementType == 0x0) {
8786
// direct settlement type
8887
_collect(order.takerToken, order.taker, msg.sender, takerTokenAmount, order.takerTokenPermit);
8988
_collect(order.makerToken, msg.sender, order.recipient, makerTokenAmount, order.takerTokenPermit);
9089
} else if (settlementType == 0x01) {
9190
// strategy settlement type
92-
9391
(address strategy, bytes memory data) = abi.decode(strategyData, (address, bytes));
9492
_collect(order.takerToken, order.taker, strategy, takerTokenAmount, order.takerTokenPermit);
9593

9694
uint256 makerTokenBalanceBefore = order.makerToken.getBalance(address(this));
9795
//@todo Create a separate strategy contract specifically for conditionalSwap
9896
IStrategy(strategy).executeStrategy(order.takerToken, order.makerToken, takerTokenAmount, data);
99-
returnedAmount = order.makerToken.getBalance(address(this)) - makerTokenBalanceBefore;
97+
uint256 returnedAmount = order.makerToken.getBalance(address(this)) - makerTokenBalanceBefore;
10098

101-
if (returnedAmount < minMakerTokenAmount) revert InsufficientOutput();
99+
if (returnedAmount < makerTokenAmount) revert InsufficientOutput();
102100
order.makerToken.transferTo(order.recipient, returnedAmount);
103-
} else {
104-
revert(); // specific the error message1
105-
}
101+
} else revert InvalidSettlementType();
106102

107-
_emitConOrderFilled(order, orderHash, takerTokenAmount, returnedAmount);
103+
_emitConOrderFilled(order, orderHash, takerTokenAmount, makerTokenAmount);
108104
}
109105

110106
function _emitConOrderFilled(ConOrder calldata order, bytes32 orderHash, uint256 takerTokenSettleAmount, uint256 makerTokenSettleAmount) internal {

‎contracts/interfaces/IConditionalSwap.sol

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@ pragma solidity 0.8.17;
44
import { ConOrder } from "../libraries/ConditionalOrder.sol";
55

66
interface IConditionalSwap {
7-
// error
87
error ExpiredOrder();
98
error InsufficientTimePassed();
10-
error ZeroTokenAmount();
119
error InvalidSignature();
10+
error ZeroTokenAmount();
1211
error InvalidTakingAmount();
1312
error InvalidMakingAmount();
14-
error InvalidRecipient();
15-
error NotOrderMaker();
1613
error InsufficientOutput();
14+
error NotOrderMaker();
15+
error InvalidRecipient();
16+
error InvalidSettlementType();
1717

18-
// event
18+
/// @notice Emitted when a conditional order is filled
1919
event ConditionalOrderFilled(
2020
bytes32 indexed orderHash,
2121
address indexed taker,

‎test/forkMainnet/ConditionalSwap/Fill.t.sol

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
pragma solidity 0.8.17;
33

44
import { IConditionalSwap } from "contracts/interfaces/IConditionalSwap.sol";
5+
import { getConOrderHash } from "contracts/libraries/ConditionalOrder.sol";
56
import { ConditionalOrderSwapTest } from "test/forkMainnet/ConditionalSwap/Setup.t.sol";
67
import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol";
78

@@ -21,11 +22,23 @@ contract ConFillTest is ConditionalOrderSwapTest {
2122
Snapshot memory recMakerToken = BalanceSnapshot.take({ owner: recipient, token: defaultOrder.makerToken });
2223

2324
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
24-
uint256 flags = 1 << 253;
25+
uint256 flags = FLG_PARTIAL_FILL_MASK;
2526
defaultOrder.flagsAndPeriod = flags;
2627

2728
defaultTakerSig = signConOrder(takerPrivateKey, defaultOrder, address(conditionalSwap));
2829

30+
vm.expectEmit(true, true, true, true);
31+
emit ConditionalOrderFilled(
32+
getConOrderHash(defaultOrder),
33+
defaultOrder.taker,
34+
defaultOrder.maker,
35+
defaultOrder.takerToken,
36+
defaultOrder.takerTokenAmount,
37+
defaultOrder.makerToken,
38+
defaultOrder.makerTokenAmount,
39+
recipient
40+
);
41+
2942
vm.startPrank(maker);
3043
conditionalSwap.fillConOrder(defaultOrder, defaultTakerSig, defaultOrder.takerTokenAmount, defaultOrder.makerTokenAmount, defaultSettlementData);
3144
vm.stopPrank();
@@ -38,7 +51,7 @@ contract ConFillTest is ConditionalOrderSwapTest {
3851
recMakerToken.assertChange(int256(defaultOrder.makerTokenAmount));
3952
}
4053

41-
function testRepayment() external {
54+
function testRepaymentOrDCAOrder() external {
4255
Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: taker, token: defaultOrder.takerToken });
4356
Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: taker, token: defaultOrder.makerToken });
4457
Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: maker, token: defaultOrder.takerToken });
@@ -47,12 +60,26 @@ contract ConFillTest is ConditionalOrderSwapTest {
4760
Snapshot memory recMakerToken = BalanceSnapshot.take({ owner: recipient, token: defaultOrder.makerToken });
4861

4962
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
50-
uint256 flags = 7 << 253;
63+
uint256 flags = FLG_SINGLE_AMOUNT_CAP_MASK | FLG_PERIODIC_MASK | FLG_PARTIAL_FILL_MASK;
5164
uint256 period = 12 hours;
52-
uint256 numberOfCycles = (defaultExpiry - block.timestamp) / period;
5365
defaultOrder.flagsAndPeriod = flags | period;
5466

67+
uint256 numberOfCycles = (defaultExpiry - block.timestamp) / period;
68+
5569
defaultTakerSig = signConOrder(takerPrivateKey, defaultOrder, address(conditionalSwap));
70+
71+
vm.expectEmit(true, true, true, true);
72+
emit ConditionalOrderFilled(
73+
getConOrderHash(defaultOrder),
74+
defaultOrder.taker,
75+
defaultOrder.maker,
76+
defaultOrder.takerToken,
77+
defaultOrder.takerTokenAmount,
78+
defaultOrder.makerToken,
79+
defaultOrder.makerTokenAmount,
80+
recipient
81+
);
82+
5683
vm.startPrank(maker);
5784
for (uint256 i; i < numberOfCycles; ++i) {
5885
conditionalSwap.fillConOrder(defaultOrder, defaultTakerSig, defaultOrder.takerTokenAmount, defaultOrder.makerTokenAmount, defaultSettlementData);
@@ -68,14 +95,6 @@ contract ConFillTest is ConditionalOrderSwapTest {
6895
recMakerToken.assertChange(int256(defaultOrder.makerTokenAmount) * int256(numberOfCycles));
6996
}
7097

71-
function testDCAOrder() public {
72-
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
73-
uint256 flags = 7 << 253;
74-
uint256 period = 1 days;
75-
76-
defaultOrder.flagsAndPeriod = flags | period;
77-
}
78-
7998
function testCannotFillExpiredOrder() public {
8099
vm.warp(defaultOrder.expiry + 1);
81100

@@ -103,7 +122,7 @@ contract ConFillTest is ConditionalOrderSwapTest {
103122

104123
function testCannotFillOrderWithInvalidTotalTakerTokenAmount() public {
105124
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
106-
uint256 flags = 1 << 253;
125+
uint256 flags = FLG_PARTIAL_FILL_MASK;
107126
defaultOrder.flagsAndPeriod = flags;
108127

109128
defaultTakerSig = signConOrder(takerPrivateKey, defaultOrder, address(conditionalSwap));
@@ -120,7 +139,7 @@ contract ConFillTest is ConditionalOrderSwapTest {
120139

121140
function testCannotFillOrderWithInvalidSingleTakerTokenAmount() public {
122141
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
123-
uint256 flags = 7 << 253;
142+
uint256 flags = FLG_SINGLE_AMOUNT_CAP_MASK | FLG_PERIODIC_MASK | FLG_PARTIAL_FILL_MASK;
124143
uint256 period = 12 hours;
125144
defaultOrder.flagsAndPeriod = flags | period;
126145

@@ -153,7 +172,7 @@ contract ConFillTest is ConditionalOrderSwapTest {
153172

154173
function testCannotFillOrderWithinSamePeriod() public {
155174
// craft the `flagAndPeriod` of the defaultOrder for BestBuy case
156-
uint256 flags = 7 << 253;
175+
uint256 flags = FLG_SINGLE_AMOUNT_CAP_MASK | FLG_PERIODIC_MASK | FLG_PARTIAL_FILL_MASK;
157176
uint256 period = 12 hours;
158177
defaultOrder.flagsAndPeriod = flags | period;
159178

@@ -165,4 +184,24 @@ contract ConFillTest is ConditionalOrderSwapTest {
165184
conditionalSwap.fillConOrder(defaultOrder, defaultTakerSig, defaultOrder.takerTokenAmount, defaultOrder.makerTokenAmount, defaultSettlementData);
166185
vm.stopPrank();
167186
}
187+
188+
function testCannotFillOrderWithInvalidSettlementType() public {
189+
bytes memory settlementData = hex"02";
190+
191+
defaultTakerSig = signConOrder(takerPrivateKey, defaultOrder, address(conditionalSwap));
192+
193+
vm.expectRevert(IConditionalSwap.InvalidSettlementType.selector);
194+
vm.startPrank(maker);
195+
conditionalSwap.fillConOrder(defaultOrder, defaultTakerSig, defaultOrder.takerTokenAmount, defaultOrder.makerTokenAmount, settlementData);
196+
vm.stopPrank();
197+
}
198+
199+
function testCannotFillOrderWithInvalidMakingAmount() public {
200+
defaultTakerSig = signConOrder(takerPrivateKey, defaultOrder, address(conditionalSwap));
201+
202+
vm.expectRevert(IConditionalSwap.InvalidMakingAmount.selector);
203+
vm.startPrank(maker);
204+
conditionalSwap.fillConOrder(defaultOrder, defaultTakerSig, defaultOrder.takerTokenAmount, defaultOrder.makerTokenAmount - 1, defaultSettlementData);
205+
vm.stopPrank();
206+
}
168207
}

‎test/forkMainnet/ConditionalSwap/Setup.t.sol

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,27 @@
22
pragma solidity 0.8.17;
33

44
import { Test } from "forge-std/Test.sol";
5-
import { IWETH } from "contracts/interfaces/IWETH.sol";
6-
import { IConditionalSwap } from "contracts/interfaces/IConditionalSwap.sol";
75
import { ConditionalSwap } from "contracts/ConditionalSwap.sol";
86
import { AllowanceTarget } from "contracts/AllowanceTarget.sol";
9-
import { ConOrder, getConOrderHash } from "contracts/libraries/ConditionalOrder.sol";
7+
import { ConOrder } from "contracts/libraries/ConditionalOrder.sol";
108
import { Tokens } from "test/utils/Tokens.sol";
119
import { BalanceUtil } from "test/utils/BalanceUtil.sol";
1210
import { SigHelper } from "test/utils/SigHelper.sol";
1311
import { computeContractAddress } from "test/utils/Addresses.sol";
1412
import { Permit2Helper } from "test/utils/Permit2Helper.sol";
1513

1614
contract ConditionalOrderSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper {
15+
event ConditionalOrderFilled(
16+
bytes32 indexed orderHash,
17+
address indexed taker,
18+
address indexed maker,
19+
address takerToken,
20+
uint256 takerTokenFilledAmount,
21+
address makerToken,
22+
uint256 makerTokenSettleAmount,
23+
address recipient
24+
);
25+
1726
// role
1827
address public conditionalOrderOwner = makeAddr("conditionalOrderOwner");
1928
address public allowanceTargetOwner = makeAddr("allowanceTargetOwner");
@@ -29,6 +38,11 @@ contract ConditionalOrderSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, S
2938
bytes public defaultTakerSig;
3039
bytes public defaultSettlementData;
3140

41+
// mask for triggering different business logic (e.g. BestBuy, Repayment, DCA)
42+
uint256 public constant FLG_SINGLE_AMOUNT_CAP_MASK = 1 << 255; // ConOrder.amount is the cap of single execution, not total cap
43+
uint256 public constant FLG_PERIODIC_MASK = 1 << 254; // ConOrder can be executed periodically
44+
uint256 public constant FLG_PARTIAL_FILL_MASK = 1 << 253; // ConOrder can be fill partially
45+
3246
ConditionalSwap conditionalSwap;
3347
AllowanceTarget allowanceTarget;
3448
ConOrder defaultOrder;

0 commit comments

Comments
 (0)