From 95d89100132cf3ab5c3509146c2aabc714f1aa2f Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 14 Oct 2021 16:13:02 +1100 Subject: [PATCH 1/2] Monitor oracle attestation after completed setup Does not make sense to start this earlier and the code is only logical right after triggering the blockchain monitoring. --- daemon/src/maker_cfd.rs | 14 ++++++-------- daemon/src/taker_cfd.rs | 14 ++++++-------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs index aa80248..ce4d749 100644 --- a/daemon/src/maker_cfd.rs +++ b/daemon/src/maker_cfd.rs @@ -442,8 +442,6 @@ impl Actor { tracing::info!("Lock transaction published with txid {}", txid); - // TODO: It's a bit suspicious to load this just to get the - // refund timelock let cfd = load_cfd_by_order_id(order_id, &mut conn).await?; self.monitor_actor @@ -453,6 +451,12 @@ impl Actor { }) .await?; + self.oracle_actor + .do_send_async(oracle::MonitorAttestation { + event_id: cfd.order.oracle_event_id, + }) + .await?; + Ok(()) } @@ -602,12 +606,6 @@ impl Actor { .await? .with_context(|| format!("Announcement {} not found", cfd.order.oracle_event_id))?; - self.oracle_actor - .do_send_async(oracle::MonitorAttestation { - event_id: offer_announcement.id, - }) - .await?; - let contract_future = setup_contract::new( self.takers.clone().into_sink().with(move |msg| { future::ok(maker_inc_connections::TakerMessage { diff --git a/daemon/src/taker_cfd.rs b/daemon/src/taker_cfd.rs index 5870670..115b5f1 100644 --- a/daemon/src/taker_cfd.rs +++ b/daemon/src/taker_cfd.rs @@ -407,12 +407,6 @@ impl Actor { .await? .with_context(|| format!("Announcement {} not found", oracle_event_id))?; - self.oracle_actor - .do_send_async(oracle::MonitorAttestation { - event_id: announcement.id, - }) - .await?; - let contract_future = setup_contract::roll_over( self.send_to_maker .clone() @@ -522,8 +516,6 @@ impl Actor { tracing::info!("Lock transaction published with txid {}", txid); - // TODO: It's a bit suspicious to load this just to get the - // refund timelock let cfd = load_cfd_by_order_id(order_id, &mut conn).await?; self.monitor_actor @@ -533,6 +525,12 @@ impl Actor { }) .await?; + self.oracle_actor + .do_send_async(oracle::MonitorAttestation { + event_id: cfd.order.oracle_event_id, + }) + .await?; + Ok(()) } From 9c6511a141b63e46ae1ecc504b8ddfa79ea7035b Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 14 Oct 2021 16:19:03 +1100 Subject: [PATCH 2/2] Re-order the contract setup for resilient error handling Anything that can fail (e.g. fetching oracle stuff) has to be tried before we change any state or notify the taker, otherwise it can happen that we remain in a failed state that causes very weird side effects. --- daemon/src/maker_cfd.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs index ce4d749..5b5ddaa 100644 --- a/daemon/src/maker_cfd.rs +++ b/daemon/src/maker_cfd.rs @@ -564,7 +564,7 @@ impl Actor { let mut conn = self.db.acquire().await?; - // Validate if order is still valid + // 1. Validate if order is still valid let cfd = load_cfd_by_order_id(order_id, &mut conn).await?; let taker_id = match cfd { Cfd { @@ -576,8 +576,15 @@ impl Actor { } }; - let (sender, receiver) = mpsc::unbounded(); + // 2. Try to get the oracle announcement, if that fails we should exit prior to changing any + // state + let offer_announcement = self + .oracle_actor + .send(oracle::GetAnnouncement(cfd.order.oracle_event_id)) + .await? + .with_context(|| format!("Announcement {} not found", cfd.order.oracle_event_id))?; + // 3. Insert that we are in contract setup and refresh our own feed insert_new_cfd_state_by_order_id( order_id, &CfdState::ContractSetup { @@ -589,7 +596,13 @@ impl Actor { ) .await?; - // use `.send` here to ensure we only continue once the message has been sent + self.cfd_feed_actor_inbox + .send(load_all_cfds(&mut conn).await?)?; + + // 4. Notify the taker that we are ready for contract setup + // Use `.send` here to ensure we only continue once the message has been sent + // Nothing done after this call should be able to fail, otherwise we notified the taker, but + // might not transition to `Active` ourselves! self.takers .send(maker_inc_connections::TakerMessage { taker_id, @@ -597,15 +610,8 @@ impl Actor { }) .await?; - self.cfd_feed_actor_inbox - .send(load_all_cfds(&mut conn).await?)?; - - let offer_announcement = self - .oracle_actor - .send(oracle::GetAnnouncement(cfd.order.oracle_event_id)) - .await? - .with_context(|| format!("Announcement {} not found", cfd.order.oracle_event_id))?; - + // 5. Spawn away the contract setup + let (sender, receiver) = mpsc::unbounded(); let contract_future = setup_contract::new( self.takers.clone().into_sink().with(move |msg| { future::ok(maker_inc_connections::TakerMessage { @@ -631,6 +637,7 @@ impl Actor { .await }); + // 6. Record that we are in an active contract setup self.setup_state = SetupState::Active { sender, taker: taker_id,