Skip to content

Commit 273d472

Browse files
NIC619NIC619
committed
Add minPionexTokenAmount check to protect user
1 parent b1e9d4b commit 273d472

File tree

2 files changed

+131
-15
lines changed

2 files changed

+131
-15
lines changed

‎contracts/PionexContract.sol

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,10 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
136136
(uint256 userTokenAmount, uint256 remainingUserTokenAmount) = _quoteOrderFromUserToken(_order, orderHash, _params.userTokenAmount);
137137
// Calculate pionexTokenAmount according to the provided pionexToken/userToken ratio
138138
uint256 pionexTokenAmount = userTokenAmount.mul(_params.pionexTokenAmount).div(_params.userTokenAmount);
139+
// Calculate minimum pionexTokenAmount according to the offer's pionexToken/userToken ratio
140+
uint256 minPionexTokenAmount = userTokenAmount.mul(_order.minPionexTokenAmount).div(_order.userTokenAmount);
139141

140-
uint256 userTokenOut = _settleForTrader(
142+
_settleForTrader(
141143
TraderSettlement({
142144
orderHash: orderHash,
143145
allowFillHash: allowFillHash,
@@ -148,6 +150,7 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
148150
pionexToken: _order.pionexToken,
149151
userTokenAmount: userTokenAmount,
150152
pionexTokenAmount: pionexTokenAmount,
153+
minPionexTokenAmount: minPionexTokenAmount,
151154
remainingUserTokenAmount: remainingUserTokenAmount,
152155
gasFeeFactor: _params.gasFeeFactor,
153156
pionexStrategyFeeFactor: _params.pionexStrategyFeeFactor
@@ -156,7 +159,7 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
156159

157160
_recordUserTokenFilled(orderHash, userTokenAmount);
158161

159-
return (pionexTokenAmount, userTokenOut);
162+
return (pionexTokenAmount, userTokenAmount);
160163
}
161164

162165
function _validateTraderFill(PionexContractLibEIP712.Fill memory _fill, bytes memory _fillTakerSig) internal {
@@ -209,12 +212,13 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
209212
IERC20 pionexToken;
210213
uint256 userTokenAmount;
211214
uint256 pionexTokenAmount;
215+
uint256 minPionexTokenAmount;
212216
uint256 remainingUserTokenAmount;
213217
uint16 gasFeeFactor;
214218
uint16 pionexStrategyFeeFactor;
215219
}
216220

217-
function _settleForTrader(TraderSettlement memory _settlement) internal returns (uint256) {
221+
function _settleForTrader(TraderSettlement memory _settlement) internal {
218222
// memory cache
219223
ISpender _spender = spender;
220224
address _feeCollector = feeCollector;
@@ -225,6 +229,7 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
225229
// 2. Fee for Pionex, including gas fee and strategy fee
226230
uint256 pionexFee = _mulFactor(_settlement.pionexTokenAmount, _settlement.gasFeeFactor + _settlement.pionexStrategyFeeFactor);
227231
uint256 pionexTokenForUser = _settlement.pionexTokenAmount.sub(tokenlonFee).sub(pionexFee);
232+
require(pionexTokenForUser >= _settlement.minPionexTokenAmount, "PionexContract: pionex token amount not enough");
228233

229234
// trader -> user
230235
_spender.spendFromUserTo(_settlement.trader, address(_settlement.pionexToken), _settlement.user, pionexTokenForUser);
@@ -254,8 +259,6 @@ contract PionexContract is IPionexContract, StrategyBase, BaseLibEIP712, Signatu
254259
pionexFee: pionexFee
255260
})
256261
);
257-
258-
return _settlement.userTokenAmount;
259262
}
260263

261264
/// @inheritdoc IPionexContract

‎test/forkMainnet/PionexContract.t.sol

Lines changed: 123 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,56 @@ contract PionexContractTest is StrategySharedSetup {
620620
userProxy.toLimitOrder(payload);
621621
}
622622

623-
function testFullyFillByTrader() public {
623+
function testCannotFullyFillByTraderWithWorseTakerTokenAmountDueToFee() public {
624+
IPionexContract.TraderParams memory traderParams = DEFAULT_TRADER_PARAMS;
625+
traderParams.gasFeeFactor = 50; // gasFeeFactor: 0.5%
626+
traderParams.pionexStrategyFeeFactor = 250; // pionexStrategyFeeFactor: 2.5%
627+
traderParams.pionexSig = _signFill(pionexPrivateKey, DEFAULT_FILL, SignatureValidator.SignatureType.EIP712);
628+
629+
bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, traderParams, DEFAULT_CRD_PARAMS);
630+
vm.expectRevert("PionexContract: pionex token amount not enough");
631+
vm.prank(pionex, pionex); // Only EOA
632+
userProxy.toLimitOrder(payload);
633+
}
634+
635+
function testFullyFillByTraderWithNoFee() public {
636+
BalanceSnapshot.Snapshot memory pionexTakerAsset = BalanceSnapshot.take(pionex, address(DEFAULT_ORDER.pionexToken));
637+
BalanceSnapshot.Snapshot memory receiverMakerAsset = BalanceSnapshot.take(receiver, address(DEFAULT_ORDER.userToken));
638+
BalanceSnapshot.Snapshot memory userTakerAsset = BalanceSnapshot.take(user, address(DEFAULT_ORDER.pionexToken));
639+
BalanceSnapshot.Snapshot memory userMakerAsset = BalanceSnapshot.take(user, address(DEFAULT_ORDER.userToken));
640+
BalanceSnapshot.Snapshot memory fcMakerAsset = BalanceSnapshot.take(feeCollector, address(DEFAULT_ORDER.userToken));
641+
BalanceSnapshot.Snapshot memory fcTakerAsset = BalanceSnapshot.take(feeCollector, address(DEFAULT_ORDER.pionexToken));
642+
643+
bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, DEFAULT_TRADER_PARAMS, DEFAULT_CRD_PARAMS);
644+
vm.expectEmit(true, true, true, true);
645+
emit LimitOrderFilledByTrader(
646+
DEFAULT_ORDER_HASH,
647+
DEFAULT_ORDER.user,
648+
pionex,
649+
getEIP712Hash(pionexContract.EIP712_DOMAIN_SEPARATOR(), PionexContractLibEIP712._getAllowFillStructHash(DEFAULT_ALLOW_FILL)),
650+
DEFAULT_TRADER_PARAMS.recipient,
651+
IPionexContract.FillReceipt(
652+
address(DEFAULT_ORDER.userToken),
653+
address(DEFAULT_ORDER.pionexToken),
654+
DEFAULT_ORDER.userTokenAmount,
655+
DEFAULT_ORDER.minPionexTokenAmount,
656+
0, // remainingUserTokenAmount should be zero after order fully filled
657+
0, // tokenlonFee = 0
658+
0 // pionexStrategyFee = 0
659+
)
660+
);
661+
vm.prank(pionex, pionex); // Only EOA
662+
userProxy.toLimitOrder(payload);
663+
664+
pionexTakerAsset.assertChange(-int256(DEFAULT_ORDER.minPionexTokenAmount));
665+
receiverMakerAsset.assertChange(int256(DEFAULT_ORDER.userTokenAmount));
666+
userTakerAsset.assertChange(int256(DEFAULT_ORDER.minPionexTokenAmount));
667+
userMakerAsset.assertChange(-int256(DEFAULT_ORDER.userTokenAmount));
668+
fcMakerAsset.assertChange(0);
669+
fcTakerAsset.assertChange(0);
670+
}
671+
672+
function testFullyFillByTraderWithAddedTokenlonFee() public {
624673
BalanceSnapshot.Snapshot memory pionexTakerAsset = BalanceSnapshot.take(pionex, address(DEFAULT_ORDER.pionexToken));
625674
BalanceSnapshot.Snapshot memory receiverMakerAsset = BalanceSnapshot.take(receiver, address(DEFAULT_ORDER.userToken));
626675
BalanceSnapshot.Snapshot memory userTakerAsset = BalanceSnapshot.take(user, address(DEFAULT_ORDER.pionexToken));
@@ -635,38 +684,102 @@ contract PionexContractTest is StrategySharedSetup {
635684
pionexContract.activateFactors();
636685
vm.stopPrank();
637686

687+
PionexContractLibEIP712.Fill memory fill = DEFAULT_FILL;
688+
// Increase pionex token amount so the pionexToken/userToken ratio is better than order's pionexToken/userToken ratio
689+
// to account for tokenlon fee
690+
fill.pionexTokenAmount = DEFAULT_FILL.pionexTokenAmount.mul(115).div(100); // 15% more
691+
692+
IPionexContract.TraderParams memory traderParams = DEFAULT_TRADER_PARAMS;
693+
traderParams.pionexTokenAmount = fill.pionexTokenAmount;
694+
traderParams.pionexSig = _signFill(pionexPrivateKey, fill, SignatureValidator.SignatureType.EIP712);
695+
696+
PionexContractLibEIP712.AllowFill memory allowFill = DEFAULT_ALLOW_FILL;
697+
allowFill.fillAmount = traderParams.pionexTokenAmount;
698+
699+
IPionexContract.CoordinatorParams memory crdParams = DEFAULT_CRD_PARAMS;
700+
crdParams.sig = _signAllowFill(coordinatorPrivateKey, allowFill, SignatureValidator.SignatureType.EIP712);
701+
702+
bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, traderParams, crdParams);
703+
vm.expectEmit(true, true, true, true);
704+
emit LimitOrderFilledByTrader(
705+
DEFAULT_ORDER_HASH,
706+
DEFAULT_ORDER.user,
707+
pionex,
708+
getEIP712Hash(pionexContract.EIP712_DOMAIN_SEPARATOR(), PionexContractLibEIP712._getAllowFillStructHash(allowFill)),
709+
DEFAULT_TRADER_PARAMS.recipient,
710+
IPionexContract.FillReceipt(
711+
address(DEFAULT_ORDER.userToken),
712+
address(DEFAULT_ORDER.pionexToken),
713+
DEFAULT_ORDER.userTokenAmount,
714+
traderParams.pionexTokenAmount,
715+
0, // remainingUserTokenAmount should be zero after order fully filled
716+
traderParams.pionexTokenAmount.div(10), // tokenlonFee = 10% pionexTokenAmount
717+
0 // pionexStrategyFee = 0
718+
)
719+
);
720+
vm.prank(pionex, pionex); // Only EOA
721+
userProxy.toLimitOrder(payload);
722+
723+
pionexTakerAsset.assertChange(-int256(traderParams.pionexTokenAmount));
724+
receiverMakerAsset.assertChange(int256(DEFAULT_ORDER.userTokenAmount));
725+
userTakerAsset.assertChange(int256(traderParams.pionexTokenAmount.mul(9).div(10))); // 10% fee for Tokenlon
726+
userMakerAsset.assertChange(-int256(DEFAULT_ORDER.userTokenAmount));
727+
fcMakerAsset.assertChange(0);
728+
fcTakerAsset.assertChange(int256(traderParams.pionexTokenAmount.div(10)));
729+
}
730+
731+
function testFullyFillByTraderWithAddedGasFeeAndStrategyFee() public {
732+
BalanceSnapshot.Snapshot memory pionexTakerAsset = BalanceSnapshot.take(pionex, address(DEFAULT_ORDER.pionexToken));
733+
BalanceSnapshot.Snapshot memory receiverMakerAsset = BalanceSnapshot.take(receiver, address(DEFAULT_ORDER.userToken));
734+
BalanceSnapshot.Snapshot memory userTakerAsset = BalanceSnapshot.take(user, address(DEFAULT_ORDER.pionexToken));
735+
BalanceSnapshot.Snapshot memory userMakerAsset = BalanceSnapshot.take(user, address(DEFAULT_ORDER.userToken));
736+
BalanceSnapshot.Snapshot memory fcMakerAsset = BalanceSnapshot.take(feeCollector, address(DEFAULT_ORDER.userToken));
737+
BalanceSnapshot.Snapshot memory fcTakerAsset = BalanceSnapshot.take(feeCollector, address(DEFAULT_ORDER.pionexToken));
738+
739+
PionexContractLibEIP712.Fill memory fill = DEFAULT_FILL;
740+
// Increase pionex token amount so the pionexToken/userToken ratio is better than order's pionexToken/userToken ratio
741+
// to account for gas fee and pionex strategy fee
742+
fill.pionexTokenAmount = DEFAULT_FILL.pionexTokenAmount.mul(11).div(10); // 10% more
743+
638744
IPionexContract.TraderParams memory traderParams = DEFAULT_TRADER_PARAMS;
639745
traderParams.gasFeeFactor = 50; // gasFeeFactor: 0.5%
640746
traderParams.pionexStrategyFeeFactor = 250; // pionexStrategyFeeFactor: 2.5%
641-
traderParams.pionexSig = _signFill(pionexPrivateKey, DEFAULT_FILL, SignatureValidator.SignatureType.EIP712);
747+
traderParams.pionexTokenAmount = fill.pionexTokenAmount;
748+
traderParams.pionexSig = _signFill(pionexPrivateKey, fill, SignatureValidator.SignatureType.EIP712);
642749

643-
bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, traderParams, DEFAULT_CRD_PARAMS);
750+
PionexContractLibEIP712.AllowFill memory allowFill = DEFAULT_ALLOW_FILL;
751+
allowFill.fillAmount = traderParams.pionexTokenAmount;
752+
753+
IPionexContract.CoordinatorParams memory crdParams = DEFAULT_CRD_PARAMS;
754+
crdParams.sig = _signAllowFill(coordinatorPrivateKey, allowFill, SignatureValidator.SignatureType.EIP712);
755+
756+
bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, traderParams, crdParams);
644757
vm.expectEmit(true, true, true, true);
645758
emit LimitOrderFilledByTrader(
646759
DEFAULT_ORDER_HASH,
647760
DEFAULT_ORDER.user,
648761
pionex,
649-
getEIP712Hash(pionexContract.EIP712_DOMAIN_SEPARATOR(), PionexContractLibEIP712._getAllowFillStructHash(DEFAULT_ALLOW_FILL)),
762+
getEIP712Hash(pionexContract.EIP712_DOMAIN_SEPARATOR(), PionexContractLibEIP712._getAllowFillStructHash(allowFill)),
650763
DEFAULT_TRADER_PARAMS.recipient,
651764
IPionexContract.FillReceipt(
652765
address(DEFAULT_ORDER.userToken),
653766
address(DEFAULT_ORDER.pionexToken),
654767
DEFAULT_ORDER.userTokenAmount,
655-
DEFAULT_ORDER.minPionexTokenAmount,
768+
traderParams.pionexTokenAmount,
656769
0, // remainingUserTokenAmount should be zero after order fully filled
657-
DEFAULT_ORDER.minPionexTokenAmount.mul(10).div(100), // tokenlonFee = 10% pionexTokenAmount
658-
DEFAULT_ORDER.minPionexTokenAmount.mul(3).div(100) // pionexStrategyFee = 0.5% + 2.5% = 3% pionexTokenAmount
770+
0, // tokenlonFee = 0
771+
traderParams.pionexTokenAmount.mul(3).div(100) // pionexStrategyFee = 0.5% + 2.5% = 3% pionexTokenAmount
659772
)
660773
);
661774
vm.prank(pionex, pionex); // Only EOA
662775
userProxy.toLimitOrder(payload);
663776

664-
pionexTakerAsset.assertChange(-int256(DEFAULT_ORDER.minPionexTokenAmount.mul(97).div(100))); // 3% fee for Pionex is deducted from pionexTokenAmount directly
777+
pionexTakerAsset.assertChange(-int256(traderParams.pionexTokenAmount.mul(97).div(100))); // 3% fee for Pionex is deducted from pionexTokenAmount directly
665778
receiverMakerAsset.assertChange(int256(DEFAULT_ORDER.userTokenAmount));
666-
userTakerAsset.assertChange(int256(DEFAULT_ORDER.minPionexTokenAmount.mul(87).div(100))); // 10% fee for Tokenlon and 3% fee for Pionex
779+
userTakerAsset.assertChange(int256(traderParams.pionexTokenAmount.mul(97).div(100))); // 3% fee for Pionex
667780
userMakerAsset.assertChange(-int256(DEFAULT_ORDER.userTokenAmount));
668781
fcMakerAsset.assertChange(0);
669-
fcTakerAsset.assertChange(int256(DEFAULT_ORDER.minPionexTokenAmount.mul(10).div(100)));
782+
fcTakerAsset.assertChange(0);
670783
}
671784

672785
function testFullyFillByTraderWithBetterTakerMakerTokenRatio() public {

0 commit comments

Comments
 (0)