Merge #970

970: Fix outdated schema and correctly handle temporary contract id r=da-kami a=da-kami Unfortunately #966 merged with an outdated `schema.rs` version in the coordinator. This PR fixes this! Co-authored-by: Daniel Karzel <[email protected]>
get10101 · Jul 21, 2023 · 8d33367 · 8d33367
2 parents 6b05827 + 0520259
commit 8d33367
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 10 deletions.
diff --git a/coordinator/src/db/positions.rs b/coordinator/src/db/positions.rs
@@ -31,7 +31,7 @@ pub struct Position {
     pub expiry_timestamp: OffsetDateTime,
     pub update_timestamp: OffsetDateTime,
     pub trader_pubkey: String,
-    pub temporary_contract_id: String,
+    pub temporary_contract_id: Option<String>,
     pub realized_pnl: Option<i64>,
 }
 
@@ -158,8 +158,9 @@ impl From<Position> for crate::position::models::Position {
             expiry_timestamp: value.expiry_timestamp,
             update_timestamp: value.update_timestamp,
             trader: value.trader_pubkey.parse().expect("to be valid public key"),
-            temporary_contract_id: ContractId::from_hex(value.temporary_contract_id.as_str())
-                .expect("contract id to decode"),
+            temporary_contract_id: value.temporary_contract_id.map(|contract_id| {
+                ContractId::from_hex(contract_id.as_str()).expect("contract id to decode")
+            }),
         }
     }
 }

diff --git a/coordinator/src/node/closed_positions.rs b/coordinator/src/node/closed_positions.rs
@@ -11,10 +11,15 @@ pub fn sync(node: Node) -> Result<()> {
             .context("Failed to load open and closing positions")?;
 
     for position in open_and_closing_positions {
-        let contract = match node
-            .inner
-            .get_closed_contract(position.temporary_contract_id)
-        {
+        let temporary_contract_id = match position.temporary_contract_id {
+            None => {
+                tracing::trace!(position_id=%position.id, "Position does not have temporary contract id, skipping");
+                continue;
+            }
+            Some(temporary_contract_id) => temporary_contract_id,
+        };
+
+        let contract = match node.inner.get_closed_contract(temporary_contract_id) {
             Ok(Some(closed_contract)) => closed_contract,
             Ok(None) => {
                 tracing::trace!(position_id=%position.id, "Position not closed yet, skipping");
@@ -29,7 +34,7 @@ pub fn sync(node: Node) -> Result<()> {
         if let Err(e) =
             db::positions::Position::set_position_to_closed(&mut conn, position.id, contract.pnl)
         {
-            tracing::error!(position_id=%position.id, temporary_contract_id=%position.temporary_contract_id.to_hex(), pnl=%contract.pnl, "Failed to set position to closed: {e:#}")
+            tracing::error!(position_id=%position.id, temporary_contract_id=%temporary_contract_id.to_hex(), pnl=%contract.pnl, "Failed to set position to closed: {e:#}")
         }
     }
 

diff --git a/coordinator/src/position/models.rs b/coordinator/src/position/models.rs
@@ -46,5 +46,7 @@ pub struct Position {
     /// We use the temporary contract id because the actual contract id might not be known at that
     /// point. The temporary contract id is propagated to all states until the contract is
     /// closed.
-    pub temporary_contract_id: ContractId,
+    /// This field is optional for backwards compatibility because we cannot deterministically
+    /// associate already existing contracts with positions.
+    pub temporary_contract_id: Option<ContractId>,
 }
diff --git a/coordinator/src/schema.rs b/coordinator/src/schema.rs
@@ -107,7 +107,7 @@ diesel::table! {
         expiry_timestamp -> Timestamptz,
         update_timestamp -> Timestamptz,
         trader_pubkey -> Text,
-        temporary_contract_id -> Text,
+        temporary_contract_id -> Nullable<Text>,
         realized_pnl -> Nullable<Int8>,
     }
 }