Skip to content

Commit

Permalink
feat: Allow resizing positions
Browse files Browse the repository at this point in the history
- I fixed calculation bugs in our original implementation of position
resizing that we used for LN-DLC channels. This is why some assert
values changed.

- I reintroduced the resizing e2e tests, which are now much more
helpful because they assert on the specific values of position and
trade for the app. This can be further improved by asserting on the
coordinator side. The same pattern should eventually be applied to
most e2e tests.

- I had to add an `is_complete` column to the coordinator's `trade`
table to be able to record the `trader_realized_pnl_sat` ahead of
time.

Limitations:

- The app must have an order in `Filling` to be able to update the
position after we get a resizing channel renewal offer. This pattern
has caused problems in the past. We will probably fix this my sending
extra metadata with the `rust-dlc` offer message.
  • Loading branch information
luckysori committed Apr 9, 2024
1 parent fa9b5e6 commit 703fd0f
Show file tree
Hide file tree
Showing 29 changed files with 1,594 additions and 259 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions coordinator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ url = "2.3.1"
uuid = { version = "1.3.0", features = ["v4", "serde"] }

[dev-dependencies]
insta = "1"
proptest = "1"
rust_decimal_macros = "1"
testcontainers = "0.14.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE trades DROP COLUMN IF EXISTS "is_complete";
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE trades
ADD COLUMN is_complete BOOLEAN NOT NULL DEFAULT true;
67 changes: 61 additions & 6 deletions coordinator/src/db/positions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anyhow::ensure;
use anyhow::Result;
use bitcoin::secp256k1::PublicKey;
use bitcoin::Amount;
use bitcoin::SignedAmount;
use diesel::prelude::*;
use diesel::query_builder::QueryId;
use diesel::result::QueryResult;
Expand Down Expand Up @@ -136,18 +137,27 @@ impl Position {
Ok(positions)
}

/// sets the status of the position in state `Proposed` to a new state
pub fn update_proposed_position(
/// Set the `position_state` column from to `updated`. This will only succeed if the column does
/// match one of the values contained in `original`.
pub fn update_position_state(
conn: &mut PgConnection,
trader_pubkey: String,
state: crate::position::models::PositionState,
original: Vec<crate::position::models::PositionState>,
updated: crate::position::models::PositionState,
) -> QueryResult<crate::position::models::Position> {
let state = PositionState::from(state);
if original.is_empty() {
// It is not really a `NotFound` error, but `diesel` does not make it easy to build
// other variants.
return QueryResult::Err(diesel::result::Error::NotFound);
}

let updated = PositionState::from(updated);

let position: Position = diesel::update(positions::table)
.filter(positions::trader_pubkey.eq(trader_pubkey.clone()))
.filter(positions::position_state.eq(PositionState::Proposed))
.filter(positions::position_state.eq_any(original.into_iter().map(PositionState::from)))
.set((
positions::position_state.eq(state),
positions::position_state.eq(updated),
positions::update_timestamp.eq(OffsetDateTime::now_utc()),
))
.get_result(conn)?;
Expand Down Expand Up @@ -230,6 +240,51 @@ impl Position {
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub fn set_position_to_resizing(
conn: &mut PgConnection,
trader_pubkey: PublicKey,
temporary_contract_id: ContractId,
quantity: Decimal,
trader_direction: trade::Direction,
trader_margin: Amount,
coordinator_margin: Amount,
average_entry_price: Decimal,
expiry: OffsetDateTime,
coordinator_liquidation_price: Decimal,
trader_liquidation_price: Decimal,
// Reducing or changing direction may generate PNL.
realized_pnl: Option<SignedAmount>,
order_matching_fee: Amount,
) -> QueryResult<usize> {
let resize_trader_realized_pnl_sat = realized_pnl.unwrap_or_default().to_sat();

diesel::update(positions::table)
.filter(positions::trader_pubkey.eq(trader_pubkey.to_string()))
.filter(positions::position_state.eq(PositionState::Open))
.set((
positions::position_state.eq(PositionState::Resizing),
positions::temporary_contract_id.eq(hex::encode(temporary_contract_id)),
positions::quantity.eq(quantity.to_f32().expect("to fit")),
positions::trader_direction.eq(Direction::from(trader_direction)),
positions::average_entry_price.eq(average_entry_price.to_f32().expect("to fit")),
positions::trader_liquidation_price
.eq(trader_liquidation_price.to_f32().expect("to fit")),
positions::coordinator_liquidation_price
.eq(coordinator_liquidation_price.to_f32().expect("to fit")),
positions::coordinator_margin.eq(coordinator_margin.to_sat() as i64),
positions::expiry_timestamp.eq(expiry),
positions::trader_realized_pnl_sat
.eq(positions::trader_realized_pnl_sat + resize_trader_realized_pnl_sat),
positions::trader_unrealized_pnl_sat.eq(0),
positions::trader_margin.eq(trader_margin.to_sat() as i64),
positions::update_timestamp.eq(OffsetDateTime::now_utc()),
positions::order_matching_fees
.eq(positions::order_matching_fees + order_matching_fee.to_sat() as i64),
))
.execute(conn)
}

pub fn set_position_to_open(
conn: &mut PgConnection,
trader_pubkey: String,
Expand Down
24 changes: 24 additions & 0 deletions coordinator/src/db/trades.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct Trade {
timestamp: OffsetDateTime,
order_matching_fee_sat: i64,
trader_realized_pnl_sat: Option<i64>,
is_complete: bool,
}

#[derive(Insertable, Debug, Clone)]
Expand All @@ -38,6 +39,7 @@ struct NewTrade {
average_price: f32,
order_matching_fee_sat: i64,
trader_realized_pnl_sat: Option<i64>,
is_complete: bool,
}

pub fn insert(
Expand All @@ -51,6 +53,26 @@ pub fn insert(
Ok(trade.into())
}

pub fn mark_as_completed(conn: &mut PgConnection, position_id: i32) -> QueryResult<()> {
let trade = trades::table
.filter(trades::position_id.eq(position_id))
.order_by(trades::id.desc())
.first::<Trade>(conn)
.optional()?
.ok_or(diesel::result::Error::NotFound)?;

let affected_rows = diesel::update(trades::table)
.filter(trades::id.eq(trade.id))
.set(trades::is_complete.eq(true))
.execute(conn)?;

if affected_rows == 0 {
return Err(diesel::result::Error::NotFound);
}

Ok(())
}

pub fn get_latest_for_position(
conn: &mut PgConnection,
position_id: i32,
Expand Down Expand Up @@ -93,6 +115,7 @@ impl From<crate::trade::models::NewTrade> for NewTrade {
average_price: value.average_price,
order_matching_fee_sat: value.order_matching_fee.to_sat() as i64,
trader_realized_pnl_sat: value.trader_realized_pnl_sat,
is_complete: value.is_complete,
}
}
}
Expand All @@ -113,6 +136,7 @@ impl From<Trade> for crate::trade::models::Trade {
timestamp: value.timestamp,
order_matching_fee: Amount::from_sat(value.order_matching_fee_sat as u64),
trader_realized_pnl_sat: value.trader_realized_pnl_sat,
is_complete: value.is_complete,
}
}
}
69 changes: 42 additions & 27 deletions coordinator/src/dlc_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl DlcProtocolExecutor {
let contract_id = contract_id
.context("missing contract id")
.map_err(|_| RollbackTransaction)?;
self.finish_open_trade_dlc_protocol(
self.finish_open_or_resize_trade_dlc_protocol(
conn,
trade_params,
protocol_id,
Expand Down Expand Up @@ -401,9 +401,6 @@ impl DlcProtocolExecutor {

let order_matching_fee = trade_params.matching_fee;

// TODO(holzeis): Add optional pnl to trade.
// Instead of tracking pnl on the position we want to track pnl on the trade. e.g. Long
// -> Short or Short -> Long.
let new_trade = NewTrade {
position_id: position.id,
contract_symbol: position.contract_symbol,
Expand All @@ -415,19 +412,22 @@ impl DlcProtocolExecutor {
average_price: trade_params.average_price,
order_matching_fee,
trader_realized_pnl_sat: Some(trader_realized_pnl_sat),
is_complete: true,
};

db::trades::insert(conn, new_trade)?;

Ok(())
}

/// Completes the open trade dlc protocol as successful and updates the 10101 meta data
/// accordingly in a single database transaction.
/// - Set dlc protocol to success
/// - Updates the `[PostionState::Proposed`] position state to `[PostionState::Open]`
/// - Creates and inserts the new trade
fn finish_open_trade_dlc_protocol(
/// Complete an open or resize trade DLC protocol as successful and update the 10101 metadata
/// accordingly, in a single database transaction.
///
/// - Set DLC protocol to success.
/// - Update the [`PositionState::Proposed`] or [`PositionState::Resizing`] position state to
/// [`PositionState::Open`].
/// - Create and insert the new trade.
fn finish_open_or_resize_trade_dlc_protocol(
&self,
conn: &mut PgConnection,
trade_params: &TradeParams,
Expand All @@ -442,12 +442,21 @@ impl DlcProtocolExecutor {
channel_id,
)?;

let original = db::positions::Position::get_position_by_trader(
conn,
trade_params.trader,
vec![PositionState::Proposed, PositionState::Resizing],
)?
.context("Can't finish open or resize protocol without position")
.map_err(|_| RollbackTransaction)?;

// TODO(holzeis): We are still updating the position based on the position state. This
// will change once we only have a single position per user and representing
// the position only as view on multiple trades.
let position = db::positions::Position::update_proposed_position(
let position = db::positions::Position::update_position_state(
conn,
trade_params.trader.to_string(),
vec![PositionState::Proposed, PositionState::Resizing],
PositionState::Open,
)?;

Expand All @@ -460,23 +469,29 @@ impl DlcProtocolExecutor {

let order_matching_fee = trade_params.matching_fee;

// TODO(holzeis): Add optional pnl to trade.
// Instead of tracking pnl on the position we want to track pnl on the trade. e.g. Long
// -> Short or Short -> Long.
let new_trade = NewTrade {
position_id: position.id,
contract_symbol: position.contract_symbol,
trader_pubkey: trade_params.trader,
quantity: trade_params.quantity,
trader_leverage: trade_params.leverage,
coordinator_margin: coordinator_margin as i64,
trader_direction: trade_params.direction,
average_price: trade_params.average_price,
order_matching_fee,
trader_realized_pnl_sat: None,
};
if matches!(original.position_state, PositionState::Resizing) {
// A resizing protocol may generate PNL, but here we are not able to calculate it, so
// the `Trade` has to be inserted before it is complete. So here we just have to mark
// the `Trade` as complete.

db::trades::mark_as_completed(conn, position.id)?;
} else {
let new_trade = NewTrade {
position_id: position.id,
contract_symbol: position.contract_symbol,
trader_pubkey: trade_params.trader,
quantity: trade_params.quantity,
trader_leverage: trade_params.leverage,
coordinator_margin: coordinator_margin as i64,
trader_direction: trade_params.direction,
average_price: trade_params.average_price,
order_matching_fee,
trader_realized_pnl_sat: None,
is_complete: true,
};

db::trades::insert(conn, new_trade)?;
db::trades::insert(conn, new_trade)?;
};

Ok(())
}
Expand Down
6 changes: 4 additions & 2 deletions coordinator/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,10 @@ impl Node {
"DLC Channel offer has been rejected. Setting position to failed."
);

db::positions::Position::update_proposed_position(
db::positions::Position::update_position_state(
&mut connection,
node_id.to_string(),
vec![PositionState::Proposed],
PositionState::Failed,
)?;
}
Expand Down Expand Up @@ -442,9 +443,10 @@ impl Node {
"DLC Channel renew offer has been rejected. Setting position to failed."
);

db::positions::Position::update_proposed_position(
db::positions::Position::update_position_state(
&mut connection,
node_id.to_string(),
vec![PositionState::Proposed],
PositionState::Failed,
)?;
}
Expand Down
2 changes: 2 additions & 0 deletions coordinator/src/payout_curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub fn build_contract_descriptor(
/// Additionally returns the [`RoundingIntervals`] to indicate how it should be discretized.
#[allow(clippy::too_many_arguments)]
fn build_inverse_payout_function(
// TODO: The `coordinator_margin` and `trader_margin` are _not_ orthogonal to the other
// arguments passed in.
coordinator_margin: u64,
trader_margin: u64,
initial_price: Decimal,
Expand Down
Loading

0 comments on commit 703fd0f

Please sign in to comment.