From ebd7833daed9bdc3e8b6baf22c9a49d1716cf70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksandar=20Marinkovi=C4=87?= Date: Wed, 18 Oct 2023 11:20:28 +0200 Subject: [PATCH] Order book sorting problem with small amounts (#82) * fix: order sorting for small amounts * chore: label USDC and make it supported by default * fix: test prank * chore: clean up comments and dead code --- src/diamonds/nayms/libs/LibMarket.sol | 8 +- test/T04Market.t.sol | 104 +++++++++++++++++++++++++- test/defaults/D02TestSetup.sol | 1 + test/defaults/D03ProtocolDefaults.sol | 33 +++++++- 4 files changed, 140 insertions(+), 6 deletions(-) diff --git a/src/diamonds/nayms/libs/LibMarket.sol b/src/diamonds/nayms/libs/LibMarket.sol index 1e2e8122..5805fded 100644 --- a/src/diamonds/nayms/libs/LibMarket.sol +++ b/src/diamonds/nayms/libs/LibMarket.sol @@ -127,11 +127,11 @@ library LibMarket { function _isOfferPricedLtOrEq(uint256 _lowOfferId, uint256 _highOfferId) internal view returns (bool) { AppStorage storage s = LibAppStorage.diamondStorage(); - uint256 lowSellAmount = s.offers[_lowOfferId].sellAmount; - uint256 lowBuyAmount = s.offers[_lowOfferId].buyAmount; + uint256 lowSellAmount = s.offers[_lowOfferId].sellAmountInitial; + uint256 lowBuyAmount = s.offers[_lowOfferId].buyAmountInitial; - uint256 highSellAmount = s.offers[_highOfferId].sellAmount; - uint256 highBuyAmount = s.offers[_highOfferId].buyAmount; + uint256 highSellAmount = s.offers[_highOfferId].sellAmountInitial; + uint256 highBuyAmount = s.offers[_highOfferId].buyAmountInitial; return lowBuyAmount * highSellAmount >= highBuyAmount * lowSellAmount; } diff --git a/test/T04Market.t.sol b/test/T04Market.t.sol index 81031009..8d21c96a 100644 --- a/test/T04Market.t.sol +++ b/test/T04Market.t.sol @@ -32,7 +32,7 @@ struct TestInfo { } contract T04MarketTest is D03ProtocolDefaults, MockAccounts { - using StdStyle for string; + using StdStyle for *; using LibHelpers for *; bytes32 internal dividendBankId; @@ -806,4 +806,106 @@ contract T04MarketTest is D03ProtocolDefaults, MockAccounts { changePrank(signer1); nayms.executeLimitOffer(e1Id, salePrice, wethId, saleAmount); } + + function testMessUpOrderSorting_IM24299() public { + assertEq(nayms.getBestOfferId(usdcId, entity1), 0, "invalid best offer, when no offer exists"); + + vm.startPrank(sm.addr); + nayms.createEntity(entity1, signer1Id, initEntity(usdcId, collateralRatio_500, maxCapital_2000eth, true), "entity test hash"); + vm.stopPrank(); + + // mint usdc for account0 + vm.startPrank(account0); + writeTokenBalance(account0, naymsAddress, usdcAddress, dt.entity1StartingBal); + nayms.externalDeposit(usdcAddress, dt.entity1ExternalDepositAmt); + vm.stopPrank(); + + // deposit into nayms vaults + // note: the entity creator can deposit funds into an entity + vm.startPrank(signer1); + writeTokenBalance(signer1, naymsAddress, usdcAddress, dt.entity1StartingBal); + nayms.externalDeposit(usdcAddress, dt.entity1ExternalDepositAmt); + vm.stopPrank(); + + vm.startPrank(sm.addr); + nayms.enableEntityTokenization(entity1, "E1", "Entity1"); + nayms.startTokenSale(entity1, 550, 550); + + // init entities + nayms.createEntity(entity2, signer2Id, initEntity(usdcId, collateralRatio_500, maxCapital_2000eth, true), "entity test hash"); + nayms.createEntity(entity3, signer3Id, initEntity(usdcId, collateralRatio_500, maxCapital_2000eth, true), "entity test hash"); + nayms.createEntity(entity4, signer4Id, initEntity(usdcId, collateralRatio_500, maxCapital_2000eth, true), "entity test hash"); + vm.stopPrank(); + + // fund taker entity + vm.startPrank(signer2); // honest user + writeTokenBalance(signer2, naymsAddress, usdcAddress, 1 ether); + nayms.externalDeposit(usdcAddress, 1 ether); + vm.stopPrank(); + + vm.startPrank(signer3); // attacker + writeTokenBalance(signer3, naymsAddress, usdcAddress, 1 ether); + nayms.externalDeposit(usdcAddress, 1 ether); + vm.stopPrank(); + + vm.startPrank(signer4); // another entity of attacker + writeTokenBalance(signer4, naymsAddress, usdcAddress, 1 ether); + nayms.externalDeposit(usdcAddress, 1 ether); + vm.stopPrank(); + + // unassign entity admin role + vm.startPrank(sa.addr); + hUnassignRole(signer2Id, entity2); + hUnassignRole(signer3Id, entity3); + hUnassignRole(signer4Id, entity4); + vm.stopPrank(); + + // assign entity cp role + vm.startPrank(sm.addr); + hAssignRole(signer2Id, entity2, LC.ROLE_ENTITY_CP); + hAssignRole(signer3Id, entity3, LC.ROLE_ENTITY_CP); + hAssignRole(signer4Id, entity4, LC.ROLE_ENTITY_CP); + vm.stopPrank(); + + // signer 2 & 3 take all the ptokens + vm.startPrank(signer2); + nayms.executeLimitOffer(usdcId, 200, entity1, 200); // it will be the best offer + vm.stopPrank(); + + vm.startPrank(signer3); + nayms.executeLimitOffer(usdcId, 350, entity1, 350); // it will be the best offer + vm.stopPrank(); + + vm.startPrank(signer2); //honest user's offer (sellAmount1, buyAmount1) = (200, 101) + nayms.executeLimitOffer(entity1, 200, usdcId, 101); // it will be the best offer + vm.stopPrank(); + + vm.startPrank(signer3); //attacker adds a better offer (sellAmount2, buyAmount2) = (200, 100) + nayms.executeLimitOffer(entity1, 200, usdcId, 100); // it will be the best offer now + vm.stopPrank(); + + // this one causes rounding isseue and is incorrectly added to the order book + vm.startPrank(signer4); + nayms.executeLimitOffer(usdcId, 100, entity1, 199); + vm.stopPrank(); + + vm.startPrank(signer3); + nayms.executeLimitOffer(entity1, 150, usdcId, 100); + vm.stopPrank(); + + uint256 bestId = nayms.getBestOfferId(entity1, usdcId); + uint256 prev1 = nayms.getOffer(bestId).rankPrev; + uint256 prev2 = nayms.getOffer(prev1).rankPrev; + + MarketInfo memory o1 = nayms.getOffer(bestId); + MarketInfo memory o2 = nayms.getOffer(prev1); + MarketInfo memory o3 = nayms.getOffer(prev2); + + uint256 price1 = (o1.buyAmountInitial * 1000) / o1.sellAmountInitial; + uint256 price2 = (o2.buyAmountInitial * 1000) / o2.sellAmountInitial; + uint256 price3 = (o3.buyAmountInitial * 1000) / o3.sellAmountInitial; + + require(price1 < price2, string.concat("best order incorrect: ", vm.toString(price1))); + require(price2 < price3, string.concat("second best order incorrect: ", vm.toString(price2))); + } } diff --git a/test/defaults/D02TestSetup.sol b/test/defaults/D02TestSetup.sol index 07077e1e..380cc4d2 100644 --- a/test/defaults/D02TestSetup.sol +++ b/test/defaults/D02TestSetup.sol @@ -36,5 +36,6 @@ abstract contract D02TestSetup is D01Deployment { vm.label(wethAddress, "WETH"); vm.label(wbtcAddress, "WBTC"); + vm.label(usdcAddress, "USDC"); } } diff --git a/test/defaults/D03ProtocolDefaults.sol b/test/defaults/D03ProtocolDefaults.sol index 30bac5dc..9c1c9e06 100644 --- a/test/defaults/D03ProtocolDefaults.sol +++ b/test/defaults/D03ProtocolDefaults.sol @@ -2,8 +2,9 @@ pragma solidity 0.8.20; import { D02TestSetup, LibHelpers, c } from "./D02TestSetup.sol"; -import { Entity, SimplePolicy, Stakeholders, FeeSchedule } from "src/diamonds/nayms/interfaces/FreeStructs.sol"; +import { Entity, SimplePolicy, MarketInfo, Stakeholders, FeeSchedule } from "src/diamonds/nayms/interfaces/FreeStructs.sol"; import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import { IERC20 } from "src/erc20/IERC20.sol"; import { LibAdmin } from "src/diamonds/nayms/libs/LibAdmin.sol"; import { LibConstants as LC } from "src/diamonds/nayms/libs/LibConstants.sol"; @@ -85,6 +86,10 @@ abstract contract T02AccessHelpers is D02TestSetup { } } + function hUnassignRole(bytes32 _objectId, bytes32 _contextId) internal { + nayms.unassignRole(_objectId, _contextId); + } + function hCreateEntity( bytes32 _entityId, bytes32 _entityAdmin, @@ -131,6 +136,31 @@ abstract contract T02AccessHelpers is D02TestSetup { nayms.setEntity(acc.id, entityId); acc.entityId = entityId; } + + function logOfferDetails(uint256 offerId) public view { + MarketInfo memory m = nayms.getOffer(offerId); + string memory offerState; + if (m.state == 1) offerState = "Active".green(); + if (m.state == 2) offerState = "Cancelled".red(); + if (m.state == 3) offerState = "Fulfilled".blue(); + + string memory sellSymbol; + string memory buySymbol; + if (nayms.isSupportedExternalToken(m.sellToken)) { + sellSymbol = IERC20(LibHelpers._getAddressFromId(m.sellToken)).symbol(); + (, , buySymbol, , ) = nayms.getObjectMeta(m.buyToken); + } else { + (, , sellSymbol, , ) = nayms.getObjectMeta(m.sellToken); + buySymbol = IERC20(LibHelpers._getAddressFromId(m.buyToken)).symbol(); + } + + c.log(string.concat("ID: ", vm.toString(offerId), " (", offerState, ")")); + c.log(string.concat(sellSymbol.red(), ":\t ", vm.toString(m.sellAmount), " (", vm.toString(m.sellAmountInitial), ")")); + c.log(string.concat(buySymbol.green(), ":\t ", vm.toString(m.buyAmount), " (", vm.toString(m.buyAmountInitial), ")")); + + // price is multiplied by 1000 to prevent rounding loss for small amounts in tests + c.log(string.concat("Price: ", vm.toString((m.buyAmount * 1000) / m.sellAmount).blue(), "(", vm.toString((m.buyAmountInitial * 1000) / m.sellAmountInitial).blue(), ")\n")); + } } /// @notice Default test setup part 03 @@ -213,6 +243,7 @@ contract D03ProtocolDefaults is T02AccessHelpers { changePrank(systemAdmin); nayms.addSupportedExternalToken(wethAddress); + nayms.addSupportedExternalToken(usdcAddress); entity = Entity({ assetId: LibHelpers._getIdForAddress(wethAddress),