Skip to content

Commit

Permalink
Order book sorting problem with small amounts (#82)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
amarinkovic authored Oct 18, 2023
1 parent 1d62edb commit ebd7833
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 6 deletions.
8 changes: 4 additions & 4 deletions src/diamonds/nayms/libs/LibMarket.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
104 changes: 103 additions & 1 deletion test/T04Market.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ struct TestInfo {
}

contract T04MarketTest is D03ProtocolDefaults, MockAccounts {
using StdStyle for string;
using StdStyle for *;
using LibHelpers for *;

bytes32 internal dividendBankId;
Expand Down Expand Up @@ -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)));
}
}
1 change: 1 addition & 0 deletions test/defaults/D02TestSetup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@ abstract contract D02TestSetup is D01Deployment {

vm.label(wethAddress, "WETH");
vm.label(wbtcAddress, "WBTC");
vm.label(usdcAddress, "USDC");
}
}
33 changes: 32 additions & 1 deletion test/defaults/D03ProtocolDefaults.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -213,6 +243,7 @@ contract D03ProtocolDefaults is T02AccessHelpers {

changePrank(systemAdmin);
nayms.addSupportedExternalToken(wethAddress);
nayms.addSupportedExternalToken(usdcAddress);

entity = Entity({
assetId: LibHelpers._getIdForAddress(wethAddress),
Expand Down

0 comments on commit ebd7833

Please sign in to comment.