Skip to content

Commit

Permalink
Update isBadPrice to check for 0 amounts
Browse files Browse the repository at this point in the history
  • Loading branch information
sisuresh committed Oct 4, 2021
1 parent f095e2d commit 0906667
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/transactions/LiquidityPoolDepositOpFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ isBadPrice(int64_t amountA, int64_t amountB, Price const& minPrice,
Price const& maxPrice)
{
// a * d < b * n is equivalent to a/b < n/d but avoids rounding.
if (bigMultiply(amountA, minPrice.d) < bigMultiply(amountB, minPrice.n) ||
if (amountA == 0 || amountB == 0 ||
bigMultiply(amountA, minPrice.d) < bigMultiply(amountB, minPrice.n) ||
bigMultiply(amountA, maxPrice.d) > bigMultiply(amountB, maxPrice.n))
{
return true;
Expand Down
23 changes: 23 additions & 0 deletions src/transactions/test/LiquidityPoolDepositTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,4 +511,27 @@ TEST_CASE("liquidity pool deposit", "[tx][liquiditypool]")
}
});
}

SECTION("bad price in existing pool due to 0 calculated deposit amounts "
"for both assets")
{
for_versions_from(18, *app, [&] {
auto a1 = root.create("a1", minBal(10));

a1.changeTrust(cur1, 10000);
a1.changeTrust(cur2, 10000);
a1.changeTrust(share12, 1000);

root.pay(a1, cur1, 10000);
root.pay(a1, cur2, 10000);

a1.liquidityPoolDeposit(pool12, 500, 1000, Price{1, 2},
Price{1, 2});
checkLiquidityPool(*app, pool12, 500, 1000, 707, 1);

REQUIRE_THROWS_AS(a1.liquidityPoolDeposit(pool12, 100, 1,
Price{2, 3}, Price{4, 1}),
ex_LIQUIDITY_POOL_DEPOSIT_BAD_PRICE);
});
}
}

5 comments on commit 0906667

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from MonsieurNicolas
at sisuresh@0906667

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging sisuresh/stellar-core/deposit-fix = 0906667 into auto

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sisuresh/stellar-core/deposit-fix = 0906667 merged ok, testing candidate = 5bec96c

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 5bec96c

Please sign in to comment.