Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to update an existing order to provide more liquidity #19

Open
wants to merge 1 commit into
base: partial_order_fills
Choose a base branch
from
Open

Add the ability to update an existing order to provide more liquidity #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions packages/portal-contracts/src/Portal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ contract Portal is Owned {
address maker
);

event OrderUpdated(
uint256 orderID,
int128 amountSats,
uint256 makerStakedTok,
address maker
);

event OrderCancelled(uint256 orderID);

event OrderMatched(
Expand Down Expand Up @@ -414,6 +421,42 @@ contract Portal is Owned {
_transferToSender(tokToSend);
}

// Update an existing bid or ask order. amountSats is negative if the order is an ask.
function updateOrder(uint256 orderID, int128 amountSats) public payable {
Order storage o = orderbook[orderID];

require(o.amountSats != 0, "Order not found");
require(o.maker == msg.sender, "Order not yours");

if (o.amountSats < 0) {
// This is an ask. Receive the ether to be sold and update the order.
require(amountSats < 0 && uint256(int256(-(o.amountSats + amountSats))) <= MAX_SATS, "Must be adding a valid amount of liquidity");
Copy link
Owner

Choose a reason for hiding this comment

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

I'd calculate the new amount o.amountSats + amountSats first.

Gotta check that it's still negative, otherwise this function lets a user convert a bid into an ask.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If 0.amountSats and amountSats are the same sign, is that something we have to worry about?

Maybe in the overflow case, but since we're on solidity 0.8+ that's checked automatically AFAIK

Copy link
Owner

Choose a reason for hiding this comment

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

I see--so the goal is to only let people add liquidity?

Then, for an existing order, you can increase the size via updateOrder, but if you want to decrease the size or change the price, you have to cancel + place a new order.

LMK if I understood it right


uint256 additionalValue = uint256(int256(-amountSats * int128(o.priceTokPerSat)));
o.amountSats += amountSats;
Copy link
Owner

Choose a reason for hiding this comment

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

👍


_transferFromSender(additionalValue);
} else {
// This is a bid, update the order and take the additional stake
require(amountSats > 0 && uint256(int256(o.amountSats)) + uint256(int256(amountSats)) <= MAX_SATS, "Must be adding a valid amount of liquidity");
uint256 expectedStakeTok = (uint256(int256(amountSats)) * uint256(o.priceTokPerSat) * stakePercent) / 100;

// Update the order
o.amountSats += amountSats;
o.stakedTok += expectedStakeTok;

// Receive the additional stake
_transferFromSender(expectedStakeTok);
}
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if this is worth the complexity.

The API, as it stands, is a big gnarly:

  • For a bid, passing positive amountSats increases the size of your bid
  • For an ask, passing positive amountSats decreases the size of your ask

In either case, we'd have to add a check to ensure bids are not turning into asks or vice versa.

I'm down to revisit later once we have testnet MMs. If it turns out that modifying bid size is a common need, we ship this.

My guess is that at least initially, immutable orders (that can only be cancelled and recreated) might be good enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I should probably have added more clarity:

This only lets you add additional liquidity, not remove it. The removing liquidity case strikes me as less likely, and therefore ok to go through the: cancel order -> place order flow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The adding liquidity case I think will be common. Lets take a WBTC <-> BTC portal where the spreads are tight and non-volatile. MM can have large liquidity, and most orders won't take the whole order from the consumer side. Being able to in place update your order to add additional liquidity strikes me as easier to manage than needing to place a separate order, which would then also fragment liquidity and make it harder to match orders.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That being said, I don't think it'd be too hard to add later so we can punt on this for now


emit OrderUpdated(
orderID,
o.amountSats,
o.stakedTok,
o.maker
);
}

function _transferFromSender(uint256 tok) private {
if (address(token) == address(0)) {
// Receive wei
Expand Down
53 changes: 53 additions & 0 deletions packages/portal-contracts/src/test/Portal.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ contract PortalTest is Test {
uint256 makerStakedWei,
address maker
);

event OrderUpdated(
uint256 orderID,
int128 amountSats,
uint256 makerStakedTok,
address maker
);

event OrderCancelled(uint256 orderID);

Expand Down Expand Up @@ -360,6 +367,52 @@ contract PortalTest is Test {
emit EscrowSettled(2, 1e8-1e7, address(this), 18.9 ether);
p.proveSettlement(2, 123, proof, 12);
}

function testUpdateOrderNotMaker() public {
Portal p = testBid();
vm.prank(address(0x42));
vm.expectRevert(bytes("Order not yours"));
p.updateOrder(1, 1e8);
}

function testUpdateBid() public {
Portal p = testBid();

uint256 stakeWei = 1 ether; /* 1 ETH = 5% of 1 * 20 ETH */
vm.expectEmit(true, true, true, true);
emit OrderUpdated(1, 2e8, 2e18, address(this));
p.updateOrder{value: stakeWei}(1, 1e8);

uint256 orderID = 1;
bytes20 destScriptHash = hex"0011223344556677889900112233445566778899";
uint256 escrowID = p.initiateSell{value: 40 ether}(
orderID,
2e8,
destScriptHash
);
assertEq(escrowID, 1);

vm.expectRevert(bytes("Order already filled"));
p.initiateSell{value: 20 ether}(orderID, 1e8, destScriptHash);
}

function testUpdateAsk() public {
Portal p = testAsk();

vm.expectEmit(true, true, true, true);
emit OrderUpdated(1, -2e8, 0, address(this));
p.updateOrder{value: 20 ether}(1, -1e8);

uint256 orderID = 1;
uint256 escrowID = p.initiateBuy{value: 2 ether}(
orderID,
2e8
);
assertEq(escrowID, 1);

vm.expectRevert(bytes("Order already filled"));
p.initiateBuy{value: 20 ether}(orderID, 1e8);
}
}

contract StubBtcTxVerifier is IBtcTxVerifier {
Expand Down