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

TriggerOrder close order logic changes #514

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

TriggerOrder close order logic changes #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5998909
prevent close order from executing if account has no position
EdNoepel Dec 17, 2024
b50253b
move logic out of type, handle pending orders
EdNoepel Dec 17, 2024
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
16 changes: 13 additions & 3 deletions packages/perennial-order/contracts/Manager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import { Fixed6, Fixed6Lib } from "@equilibria/root/number/types/Fixed6.sol";
import { UFixed6, UFixed6Lib } from "@equilibria/root/number/types/UFixed6.sol";
import { UFixed18, UFixed18Lib } from "@equilibria/root/number/types/UFixed18.sol";
import { Token6 } from "@equilibria/root/token/types/Token6.sol";
import { IMarket, IMarketFactory } from "@perennial/core/contracts/interfaces/IMarketFactory.sol";
import { IMarket, Order, Position } from "@perennial/core/contracts/interfaces/IMarket.sol";
import { IMarketFactory } from "@perennial/core/contracts/interfaces/IMarketFactory.sol";

import { IManager } from "./interfaces/IManager.sol";
import { IOrderVerifier } from "./interfaces/IOrderVerifier.sol";
import { Action } from "./types/Action.sol";
import { CancelOrderAction } from "./types/CancelOrderAction.sol";
import { InterfaceFee } from "./types/InterfaceFee.sol";
import { TriggerOrder, TriggerOrderStorage } from "./types/TriggerOrder.sol";
import { TriggerOrder, TriggerOrderLib, TriggerOrderStorage } from "./types/TriggerOrder.sol";
import { PlaceOrderAction } from "./types/PlaceOrderAction.sol";

/// @notice Base class with business logic to store and execute trigger orders.
Expand Down Expand Up @@ -131,7 +132,16 @@ abstract contract Manager is IManager, Kept {
order = _orders[market][account][orderId].read();
// prevent calling canExecute on a spent or empty order
if (order.isSpent || order.isEmpty()) revert ManagerInvalidOrderNonceError();
canExecute = order.canExecute(market.oracle().latest());
Position memory position;
if (order.delta.eq(TriggerOrderLib.MAGIC_VALUE_CLOSE_POSITION)) {
position = market.positions(account);
Order memory pending = market.pendings(account);
position.update(pending);
} else {
position = Position(0, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO);
}
Copy link
Contributor Author

@EdNoepel EdNoepel Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This feels awkward, but avoids encumbering every order execution with three additional storage slot reads.


canExecute = order.canExecute(market.oracle().latest(), position);
}

/// @inheritdoc IManager
Expand Down
6 changes: 3 additions & 3 deletions packages/perennial-order/contracts/test/TriggerOrderTester.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.13;

import { UFixed6, UFixed6Lib } from "@equilibria/root/number/types/UFixed6.sol";
import { IMarket, OracleVersion } from "@perennial/core/contracts/interfaces/IMarket.sol";
import { IMarket, OracleVersion, Position } from "@perennial/core/contracts/interfaces/IMarket.sol";
import {
TriggerOrder,
TriggerOrderLib,
Expand All @@ -21,8 +21,8 @@ contract TriggerOrderTester {
order.store(newOrder);
}

function canExecute(TriggerOrder calldata order_, OracleVersion calldata version) external pure returns (bool) {
return order_.canExecute(version);
function canExecute(TriggerOrder calldata order_, OracleVersion calldata version, Position calldata position) external pure returns (bool) {
return order_.canExecute(version, position);
}

function notionalValue(TriggerOrder calldata order_, IMarket market, address user) external view returns (UFixed6) {
Expand Down
9 changes: 7 additions & 2 deletions packages/perennial-order/contracts/types/TriggerOrder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ using TriggerOrderLib for TriggerOrder global;
/// @notice Logic for interacting with trigger orders
/// @dev (external-unsafe): this library must be used internally only
library TriggerOrderLib {
Fixed6 private constant MAGIC_VALUE_CLOSE_POSITION = Fixed6.wrap(type(int64).min);
Fixed6 public constant MAGIC_VALUE_CLOSE_POSITION = Fixed6.wrap(type(int64).min);

// sig: 0x5b8c7e99
/// @custom:error side or comparison is not supported
Expand All @@ -40,8 +40,13 @@ library TriggerOrderLib {
/// @param self Trigger order
/// @param latestVersion Latest oracle version
/// @return Whether the trigger order is fillable
function canExecute(TriggerOrder memory self, OracleVersion memory latestVersion) internal pure returns (bool) {
function canExecute(
TriggerOrder memory self,
OracleVersion memory latestVersion,
Position memory position
) internal pure returns (bool) {
if (!latestVersion.valid) return false;
if (self.delta.eq(MAGIC_VALUE_CLOSE_POSITION) && position.empty()) return false;
if (self.comparison == 1) return latestVersion.price.gte(self.price);
if (self.comparison == -1) return latestVersion.price.lte(self.price);
return false;
Expand Down
14 changes: 14 additions & 0 deletions packages/perennial-order/test/integration/Manager_Arbitrum.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,11 +707,25 @@ describe('Manager_Arbitrum', () => {
await executeOrder(userA, 509)
await commitPrice()

// cannot close with pending closed position
orderId = await placeOrder(userB, Side.LONG, Compare.LTE, parse6decimal('4001'), MAGIC_VALUE_CLOSE_POSITION)
expect(orderId).to.equal(BigNumber.from(505))
await expect(
manager.connect(keeper).executeOrder(market.address, userB.address, orderId, TX_OVERRIDES),
).to.be.revertedWithCustomError(manager, 'ManagerCannotExecuteError')

// settle and confirm positions are closed
await market.settle(userA.address, TX_OVERRIDES)
await ensureNoPosition(userA)
await market.settle(userB.address, TX_OVERRIDES)
await ensureNoPosition(userB)

// cannot close with no position
orderId = await placeOrder(userB, Side.LONG, Compare.LTE, parse6decimal('4002'), MAGIC_VALUE_CLOSE_POSITION)
expect(orderId).to.equal(BigNumber.from(506))
await expect(
manager.connect(keeper).executeOrder(market.address, userB.address, orderId, TX_OVERRIDES),
).to.be.revertedWithCustomError(manager, 'ManagerCannotExecuteError')
})
})

Expand Down
72 changes: 55 additions & 17 deletions packages/perennial-order/test/unit/TriggerOrder.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@ import { parse6decimal } from '../../../common/testutil/types'
import { expect } from 'chai'
import { FakeContract, smock } from '@defi-wonderland/smock'

import {
IMarket,
IOracleProvider,
TriggerOrderTester,
TriggerOrderTester__factory,
TriggerOrderStruct,
} from '../../types/generated'
import { IMarket, IOracleProvider, TriggerOrderTester, TriggerOrderTester__factory } from '../../types/generated'
import { Compare, compareOrders, DEFAULT_TRIGGER_ORDER, MAGIC_VALUE_CLOSE_POSITION, Side } from '../helpers/order'
import { TriggerOrderStruct } from '../../types/generated/contracts/Manager'
import { OracleVersionStruct } from '../../types/generated/contracts/test/TriggerOrderTester'
import { PositionStruct } from '../../types/generated/@perennial/core/contracts/interfaces/IMarket'

const { ethers } = HRE

Expand Down Expand Up @@ -42,16 +38,24 @@ const ORDER_SHORT: TriggerOrderStruct = {
maxFee: parse6decimal('0.66'),
}

const EMPTY_POSITION: PositionStruct = {
timestamp: BigNumber.from(0),
maker: BigNumber.from(0),
long: BigNumber.from(0),
short: BigNumber.from(0),
}

function now(): BigNumber {
return BigNumber.from(Math.floor(Date.now() / 1000))
}

describe('TriggerOrder', () => {
let owner: SignerWithAddress
let user: SignerWithAddress
let orderTester: TriggerOrderTester

before(async () => {
;[owner] = await ethers.getSigners()
;[owner, user] = await ethers.getSigners()
orderTester = await new TriggerOrderTester__factory(owner).deploy()
})

Expand All @@ -63,29 +67,45 @@ describe('TriggerOrder', () => {
}
}

function createMakerPosition(size: BigNumber): PositionStruct {
return {
timestamp: now(),
maker: size,
long: constants.Zero,
short: constants.Zero,
}
}

describe('#logic', () => {
it('handles invalid oracle version', async () => {
const invalidVersion = createOracleVersion(parse6decimal('2444.66'), false)
expect(await orderTester.canExecute(ORDER_SHORT, invalidVersion)).to.be.false
expect(
await orderTester.canExecute(ORDER_SHORT, createOracleVersion(parse6decimal('2444.66'), false), EMPTY_POSITION),
).to.be.false
})

it('compares greater than', async () => {
// ORDER_SHORT price is 2444.55
expect(await orderTester.canExecute(ORDER_SHORT, createOracleVersion(parse6decimal('3000')))).to.be.true
expect(await orderTester.canExecute(ORDER_SHORT, createOracleVersion(parse6decimal('2000')))).to.be.false
expect(await orderTester.canExecute(ORDER_SHORT, createOracleVersion(parse6decimal('3000')), EMPTY_POSITION)).to
.be.true
expect(await orderTester.canExecute(ORDER_SHORT, createOracleVersion(parse6decimal('2000')), EMPTY_POSITION)).to
.be.false
})

it('compares less than', async () => {
// ORDER_LONG price is 1999.88
expect(await orderTester.canExecute(ORDER_LONG, createOracleVersion(parse6decimal('1800')))).to.be.true
expect(await orderTester.canExecute(ORDER_LONG, createOracleVersion(parse6decimal('2000')))).to.be.false
expect(await orderTester.canExecute(ORDER_LONG, createOracleVersion(parse6decimal('1800')), EMPTY_POSITION)).to.be
.true
expect(await orderTester.canExecute(ORDER_LONG, createOracleVersion(parse6decimal('2000')), EMPTY_POSITION)).to.be
.false
})

it('handles invalid comparison', async () => {
const badOrder = { ...ORDER_SHORT }
badOrder.comparison = 0
expect(await orderTester.canExecute(badOrder, createOracleVersion(parse6decimal('1800')))).to.be.false
expect(await orderTester.canExecute(badOrder, createOracleVersion(parse6decimal('2000')))).to.be.false
expect(await orderTester.canExecute(badOrder, createOracleVersion(parse6decimal('1800')), EMPTY_POSITION)).to.be
.false
expect(await orderTester.canExecute(badOrder, createOracleVersion(parse6decimal('2000')), EMPTY_POSITION)).to.be
.false
})

it('allows execution greater than 0 trigger price', async () => {
Expand All @@ -97,7 +117,25 @@ describe('TriggerOrder', () => {
delta: parse6decimal('200'),
maxFee: parse6decimal('0.55'),
}
expect(await orderTester.canExecute(zeroPriceOrder, createOracleVersion(parse6decimal('1')))).to.be.true

expect(await orderTester.canExecute(zeroPriceOrder, createOracleVersion(parse6decimal('1')), EMPTY_POSITION)).to
.be.true
})

it('can execute close position order', async () => {
const position = parse6decimal('12.2')
const closeOrder = {
...DEFAULT_TRIGGER_ORDER,
comparison: Compare.GTE,
price: 0,
delta: MAGIC_VALUE_CLOSE_POSITION,
maxFee: parse6decimal('0.56'),
}
const oracleVersion = createOracleVersion(parse6decimal('123'))
// can execute with position
expect(await orderTester.canExecute(closeOrder, oracleVersion, createMakerPosition(position))).to.be.true
// cannot execute without position
expect(await orderTester.canExecute(closeOrder, oracleVersion, createMakerPosition(constants.Zero))).to.be.false
})
})

Expand Down
Loading