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.

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

Periodically update unrealized pnl for position in database #967

Merged
merged 4 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
-- However, there is no proper way to replace the values to be removed where they are used (i.e. referenced in `positions` table)
-- We opt to NOT remove enum values that were added at a later point.
ALTER TABLE
"positions" DROP COLUMN "realized_pnl";
"positions" DROP COLUMN "realized_pnl_sat";
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ ADD
ALTER TABLE
positions
ADD
COLUMN "realized_pnl" BIGINT;
COLUMN "realized_pnl_sat" BIGINT;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- This file should undo anything in `up.sql`
ALTER TABLE
"positions" DROP COLUMN "unrealized_pnl_sat";
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Your SQL goes here
ALTER TABLE
positions
ADD
COLUMN "unrealized_pnl_sat" BIGINT;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- This file should undo anything in `up.sql`
ALTER TABLE
"positions" DROP COLUMN "closing_price";
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Your SQL goes here
ALTER TABLE
positions
ADD
COLUMN "closing_price" REAL;
16 changes: 16 additions & 0 deletions coordinator/src/bin/coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use coordinator::node::closed_positions;
use coordinator::node::connection;
use coordinator::node::expired_positions;
use coordinator::node::storage::NodeStorage;
use coordinator::node::unrealized_pnl;
use coordinator::node::Node;
use coordinator::routes::router;
use coordinator::run_migration;
Expand All @@ -34,6 +35,7 @@ const PROCESS_PROMETHEUS_METRICS: Duration = Duration::from_secs(10);
const PROCESS_INCOMING_DLC_MESSAGES_INTERVAL: Duration = Duration::from_secs(5);
const EXPIRED_POSITION_SYNC_INTERVAL: Duration = Duration::from_secs(300);
const CLOSED_POSITION_SYNC_INTERVAL: Duration = Duration::from_secs(30);
const UNREALIZED_PNL_SYNC_INTERVAL: Duration = Duration::from_secs(600);
const CONNECTION_CHECK_INTERVAL: Duration = Duration::from_secs(30);

#[tokio::main]
Expand Down Expand Up @@ -162,6 +164,20 @@ async fn main() -> Result<()> {
}
});

tokio::spawn({
let node = node.clone();
async move {
loop {
tokio::time::sleep(UNREALIZED_PNL_SYNC_INTERVAL).await;
if let Err(e) = unrealized_pnl::sync(node.clone()).await {
tracing::error!(
"Failed to sync unrealized PnL with positions in database: {e:#}"
);
}
}
}
});

tokio::spawn({
let node = node.clone();
async move {
Expand Down
27 changes: 24 additions & 3 deletions coordinator/src/db/positions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ pub struct Position {
pub update_timestamp: OffsetDateTime,
pub trader_pubkey: String,
pub temporary_contract_id: Option<String>,
pub realized_pnl: Option<i64>,
pub realized_pnl_sat: Option<i64>,
pub unrealized_pnl_sat: Option<i64>,
pub closing_price: Option<f32>,
}

impl Position {
Expand Down Expand Up @@ -91,12 +93,14 @@ impl Position {
pub fn set_open_position_to_closing(
conn: &mut PgConnection,
trader_pubkey: String,
closing_price: f32,
) -> Result<()> {
let effected_rows = diesel::update(positions::table)
.filter(positions::trader_pubkey.eq(trader_pubkey.clone()))
.filter(positions::position_state.eq(PositionState::Open))
.set((
positions::position_state.eq(PositionState::Closing),
positions::closing_price.eq(Some(closing_price)),
positions::update_timestamp.eq(OffsetDateTime::now_utc()),
))
.execute(conn)?;
Expand All @@ -113,7 +117,7 @@ impl Position {
.filter(positions::id.eq(id))
.set((
positions::position_state.eq(PositionState::Closed),
positions::realized_pnl.eq(Some(pnl)),
positions::realized_pnl_sat.eq(Some(pnl)),
positions::update_timestamp.eq(OffsetDateTime::now_utc()),
))
.execute(conn)?;
Expand All @@ -125,6 +129,22 @@ impl Position {
Ok(())
}

pub fn update_unrealized_pnl(conn: &mut PgConnection, id: i32, pnl: i64) -> Result<()> {
let effected_rows = diesel::update(positions::table)
.filter(positions::id.eq(id))
.set((
positions::unrealized_pnl_sat.eq(Some(pnl)),
positions::update_timestamp.eq(OffsetDateTime::now_utc()),
))
.execute(conn)?;

if effected_rows == 0 {
luckysori marked this conversation as resolved.
Show resolved Hide resolved
bail!("Could not update unrealized pnl {pnl} for position {id}")
}

Ok(())
}

/// inserts the given position into the db. Returns the position if successful
#[autometrics]
pub fn insert(
Expand All @@ -151,7 +171,7 @@ impl From<Position> for crate::position::models::Position {
liquidation_price: value.liquidation_price,
position_state: crate::position::models::PositionState::from((
value.position_state,
value.realized_pnl,
value.realized_pnl_sat,
)),
collateral: value.collateral,
creation_timestamp: value.creation_timestamp,
Expand All @@ -161,6 +181,7 @@ impl From<Position> for crate::position::models::Position {
temporary_contract_id: value.temporary_contract_id.map(|contract_id| {
ContractId::from_hex(contract_id.as_str()).expect("contract id to decode")
}),
closing_price: value.closing_price,
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion coordinator/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ pub mod expired_positions;
pub mod order_matching_fee;
pub mod routing_fees;
pub mod storage;
pub mod unrealized_pnl;

/// The leverage used by the coordinator for all trades.
const COORDINATOR_LEVERAGE: f32 = 1.0;
pub const COORDINATOR_LEVERAGE: f32 = 1.0;

#[derive(Debug, Clone)]
pub struct NodeSettings {
Expand Down Expand Up @@ -332,6 +333,9 @@ impl Node {
db::positions::Position::set_open_position_to_closing(
&mut connection,
position.trader.to_string(),
closing_price
.to_f32()
.expect("Closing price to fit into f32"),
)
}

Expand Down
5 changes: 1 addition & 4 deletions coordinator/src/node/expired_positions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ pub async fn close(node: Node) {
};

let closing_price = match BitmexClient::get_quote(&position.expiry_timestamp).await {
Copy link
Contributor

@bonomat bonomat Jul 24, 2023

Choose a reason for hiding this comment

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

I know it's not your code, but this price should not be from bitmex but from our orderbook/counterparty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see where you are going, but for me using a price from the orderbook only makes sense if there is also an order that is eventually executed. We don't do this at the moment.
The coordinator can still use an orderbook price and just simulate the match - but I find this a bit odd.
I opened #988 to capture this; feel free to change the description

Copy link
Contributor

Choose a reason for hiding this comment

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

I see where you are going, but for me using a price from the orderbook only makes sense if there is also an order that is eventually executed. We don't do this at the moment.

Totally agree. We should be doing this though.

Ok(quote) => match position.direction {
trade::Direction::Long => quote.bid_price,
trade::Direction::Short => quote.ask_price,
},
Ok(quote) => quote.get_price_for_direction(position.direction.opposite()),
Err(e) => {
tracing::warn!(
"Failed to get quote from bitmex for {} at {}. Error: {e:?}",
Expand Down
40 changes: 40 additions & 0 deletions coordinator/src/node/unrealized_pnl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use crate::db;
use crate::node::Node;
use crate::position::models::Position;
use anyhow::Context;
use anyhow::Result;
use diesel::r2d2::ConnectionManager;
use diesel::r2d2::PooledConnection;
use diesel::PgConnection;
use time::OffsetDateTime;
use trade::bitmex_client::BitmexClient;
use trade::bitmex_client::Quote;

pub async fn sync(node: Node) -> Result<()> {
let mut conn = node.pool.get()?;

let positions = db::positions::Position::get_all_open_or_closing_positions(&mut conn)?;
let current_quote = BitmexClient::get_quote(&OffsetDateTime::now_utc())
da-kami marked this conversation as resolved.
Show resolved Hide resolved
.await
.context("Failed to fetch quote from BitMEX")?;

for position in positions.iter() {
if let Err(e) = sync_position(&mut conn, position, current_quote.clone()) {
tracing::error!(position_id=%position.id, ?current_quote, "Failed to update position's unrealized pnl in database: {e:#}")
}
}

Ok(())
}

fn sync_position(
luckysori marked this conversation as resolved.
Show resolved Hide resolved
conn: &mut PooledConnection<ConnectionManager<PgConnection>>,
position: &Position,
quote: Quote,
) -> Result<()> {
let pnl = position.calculate_coordinator_pnl(quote)?;
db::positions::Position::update_unrealized_pnl(conn, position.id, pnl)
.context("Failed to update unrealized pnl in db")?;

Ok(())
}
Loading