From 4e8026ee4f7e7c406d26ab1e97bbfea1dac5540b Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Mon, 13 Dec 2021 13:43:15 +1030 Subject: [PATCH 1/4] Use the same settlement interval in tests as in production This hardcoded value got out of sync recently --- daemon/tests/harness/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/tests/harness/mod.rs b/daemon/tests/harness/mod.rs index 745fd59..6ed29dc 100644 --- a/daemon/tests/harness/mod.rs +++ b/daemon/tests/harness/mod.rs @@ -24,6 +24,7 @@ use daemon::MakerActorSystem; use daemon::Tasks; use daemon::HEARTBEAT_INTERVAL; use daemon::N_PAYOUTS; +use daemon::SETTLEMENT_INTERVAL; use rust_decimal_macros::dec; use sqlx::SqlitePool; use std::net::SocketAddr; @@ -149,7 +150,7 @@ impl Maker { let (wallet_addr, wallet_fut) = wallet.create(None).run(); tasks.add(wallet_fut); - let settlement_interval = time::Duration::hours(24); + let settlement_interval = SETTLEMENT_INTERVAL; let (identity_pk, identity_sk) = config.seed.derive_identity(); From 5e0ff5b05d8b5b7a75dd92abc45ca8edbc478f92 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Mon, 13 Dec 2021 13:44:19 +1030 Subject: [PATCH 2/4] Use a macro for delivering events in tests Reduces boilerplate considerably when adding new events for delivery. --- daemon/tests/happy_path.rs | 10 +++++----- daemon/tests/harness/mod.rs | 38 ++++++++----------------------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/daemon/tests/happy_path.rs b/daemon/tests/happy_path.rs index 7c60860..5122be1 100644 --- a/daemon/tests/happy_path.rs +++ b/daemon/tests/happy_path.rs @@ -1,7 +1,5 @@ use std::time::Duration; -use crate::harness::deliver_close_finality_event; -use crate::harness::deliver_lock_finality_event; use crate::harness::dummy_new_order; use crate::harness::flow::is_next_none; use crate::harness::flow::next; @@ -17,6 +15,7 @@ use crate::harness::TakerConfig; use daemon::connection::ConnectionStatus; use daemon::model::cfd::OrderId; use daemon::model::Usd; +use daemon::monitor::Event; use daemon::projection::CfdState; use daemon::projection::Identity; use maia::secp256k1_zkp::schnorrsig; @@ -129,7 +128,7 @@ async fn taker_takes_order_and_maker_accepts_and_contract_setup() { assert!(matches!(taker_cfd.state, CfdState::PendingOpen { .. })); assert!(matches!(maker_cfd.state, CfdState::PendingOpen { .. })); - deliver_lock_finality_event(&maker, &taker, received.id).await; + deliver_event!(maker, taker, Event::LockFinality(received.id)); let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); assert!(matches!(taker_cfd.state, CfdState::Open { .. })); @@ -163,7 +162,8 @@ async fn collaboratively_close_an_open_cfd() { assert!(matches!(taker_cfd.state, CfdState::PendingClose { .. })); assert!(matches!(maker_cfd.state, CfdState::PendingClose { .. })); - deliver_close_finality_event(&maker, &taker, order_id).await; + deliver_event!(maker, taker, Event::CloseFinality(order_id)); + sleep(Duration::from_secs(5)).await; // need to wait a bit until both transition let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); @@ -290,7 +290,7 @@ async fn start_from_open_cfd_state() -> (Maker, Taker, OrderId) { assert!(matches!(taker_cfd.state, CfdState::PendingOpen { .. })); assert!(matches!(maker_cfd.state, CfdState::PendingOpen { .. })); - deliver_lock_finality_event(&maker, &taker, received.id).await; + deliver_event!(maker, taker, Event::LockFinality(received.id)); let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); assert!(matches!(taker_cfd.state, CfdState::Open { .. })); diff --git a/daemon/tests/harness/mod.rs b/daemon/tests/harness/mod.rs index 6ed29dc..3c4295b 100644 --- a/daemon/tests/harness/mod.rs +++ b/daemon/tests/harness/mod.rs @@ -354,36 +354,14 @@ impl Taker { } } -/// Deliver the event that provokes the transition to cfd's "Open" state -pub async fn deliver_lock_finality_event(maker: &Maker, taker: &Taker, id: OrderId) { - maker - .system - .cfd_actor_addr - .send(daemon::monitor::Event::LockFinality(id)) - .await - .unwrap(); - taker - .system - .cfd_actor_addr - .send(daemon::monitor::Event::LockFinality(id)) - .await - .unwrap(); -} - -/// Deliver the event that provokes the transition to cfd's "Close" state -pub async fn deliver_close_finality_event(maker: &Maker, taker: &Taker, id: OrderId) { - taker - .system - .cfd_actor_addr - .send(daemon::monitor::Event::CloseFinality(id)) - .await - .unwrap(); - maker - .system - .cfd_actor_addr - .send(daemon::monitor::Event::CloseFinality(id)) - .await - .unwrap(); +/// Deliver monitor event to both actor systems +#[macro_export] +macro_rules! deliver_event { + ($maker:expr, $taker:expr, $event:expr) => { + tracing::debug!("Delivering event: {:?}", $event); + $taker.system.cfd_actor_addr.send($event).await.unwrap(); + $maker.system.cfd_actor_addr.send($event).await.unwrap(); + }; } async fn in_memory_db() -> SqlitePool { From eda4aea3d77980519de4a17ea267489935393a34 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Mon, 13 Dec 2021 13:55:57 +1030 Subject: [PATCH 3/4] Improve dummy oracle announcement Use strong types, and use the announcement time from the example data. --- daemon/tests/harness/maia.rs | 14 ++++++++------ daemon/tests/harness/mocks/oracle.rs | 10 +--------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/daemon/tests/harness/maia.rs b/daemon/tests/harness/maia.rs index 19ddd95..797f0e8 100644 --- a/daemon/tests/harness/maia.rs +++ b/daemon/tests/harness/maia.rs @@ -1,11 +1,12 @@ +use daemon::model::BitMexPriceEventId; +use daemon::oracle::{self}; use maia::secp256k1_zkp::schnorrsig; use maia::secp256k1_zkp::SecretKey; -use maia::Announcement; use std::str::FromStr; #[allow(dead_code)] pub struct OliviaData { - id: String, + id: BitMexPriceEventId, pk: schnorrsig::PublicKey, nonce_pks: Vec, price: u64, @@ -29,7 +30,7 @@ impl OliviaData { fn example(id: &str, price: u64, nonce_pks: &[&str], attestations: &[&str]) -> Self { let oracle_pk = schnorrsig::PublicKey::from_str(Self::OLIVIA_PK).unwrap(); - let id = id.to_string(); + let id = id.parse().unwrap(); let nonce_pks = nonce_pks .iter() @@ -50,9 +51,10 @@ impl OliviaData { } } - pub fn announcement(&self) -> Announcement { - Announcement { - id: self.id.clone(), + pub fn announcement(&self) -> oracle::Announcement { + oracle::Announcement { + id: self.id, + expected_outcome_time: self.id.timestamp(), nonce_pks: self.nonce_pks.clone(), } } diff --git a/daemon/tests/harness/mocks/oracle.rs b/daemon/tests/harness/mocks/oracle.rs index e64d959..a55dc9a 100644 --- a/daemon/tests/harness/mocks/oracle.rs +++ b/daemon/tests/harness/mocks/oracle.rs @@ -1,9 +1,7 @@ use crate::harness::maia::OliviaData; -use daemon::model::BitMexPriceEventId; use daemon::oracle; use mockall::*; use std::sync::Arc; -use time::OffsetDateTime; use tokio::sync::Mutex; use xtra_productivity::xtra_productivity; @@ -53,11 +51,5 @@ pub trait Oracle { } pub fn dummy_announcement() -> oracle::Announcement { - let announcement = OliviaData::example_0().announcement(); - - oracle::Announcement { - id: BitMexPriceEventId::new(OffsetDateTime::UNIX_EPOCH, 0), - expected_outcome_time: OffsetDateTime::now_utc(), - nonce_pks: announcement.nonce_pks, - } + OliviaData::example_0().announcement() } From 99c52733173afc2d3638dd65e625f2ad53229bfa Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Mon, 13 Dec 2021 13:58:16 +1030 Subject: [PATCH 4/4] Simplify Cfd state assertions in tests Add a macro to simplify assertions on Cfd state in both maker and taker, which significantly reduces boilerplate in tests. Macro was used for brevity (we can hide `.await.unwrap()`), when a function was used it would take 4 lines after formatting. --- daemon/src/projection.rs | 2 +- daemon/tests/happy_path.rs | 85 ++++++++++++++------------------------ 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/daemon/src/projection.rs b/daemon/src/projection.rs index d63cc9b..9275ece 100644 --- a/daemon/src/projection.rs +++ b/daemon/src/projection.rs @@ -354,7 +354,7 @@ impl From for Identity { } } -#[derive(Debug, Clone, Serialize)] +#[derive(Debug, Clone, PartialEq, Serialize)] pub enum CfdState { OutgoingOrderRequest, IncomingOrderRequest, diff --git a/daemon/tests/happy_path.rs b/daemon/tests/happy_path.rs index 5122be1..4e43be5 100644 --- a/daemon/tests/happy_path.rs +++ b/daemon/tests/happy_path.rs @@ -1,5 +1,3 @@ -use std::time::Duration; - use crate::harness::dummy_new_order; use crate::harness::flow::is_next_none; use crate::harness::flow::next; @@ -20,9 +18,25 @@ use daemon::projection::CfdState; use daemon::projection::Identity; use maia::secp256k1_zkp::schnorrsig; use rust_decimal_macros::dec; +use std::time::Duration; use tokio::time::sleep; mod harness; +/// Assert the next state of the single cfd present at both maker and taker +macro_rules! assert_next_state { + ($state:expr, $maker:expr, $taker:expr, $id:expr) => { + // TODO: Allow fetching cfd with the specified id if there is more than + // one on the cfd feed + let (taker_cfd, maker_cfd) = next_cfd($taker.cfd_feed(), $maker.cfd_feed()) + .await + .unwrap(); + assert_eq!(taker_cfd.order_id, $id); + assert_eq!(maker_cfd.order_id, $id); + assert_eq!(taker_cfd.state, $state); + assert_eq!(maker_cfd.state, $state); + }; +} + #[tokio::test] async fn taker_receives_order_from_maker_on_publication() { let _guard = init_tracing(); @@ -58,23 +72,12 @@ async fn taker_takes_order_and_maker_rejects() { let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); assert_eq!(taker_cfd.order_id, received.id); assert_eq!(maker_cfd.order_id, received.id); - assert!(matches!( - taker_cfd.state, - CfdState::OutgoingOrderRequest { .. } - )); - assert!(matches!( - maker_cfd.state, - CfdState::IncomingOrderRequest { .. } - )); + assert_eq!(taker_cfd.state, CfdState::OutgoingOrderRequest); + assert_eq!(maker_cfd.state, CfdState::IncomingOrderRequest); maker.reject_take_request(received.clone()).await; - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - // TODO: More elaborate Cfd assertions - assert_eq!(taker_cfd.order_id, received.id); - assert_eq!(maker_cfd.order_id, received.id); - assert!(matches!(taker_cfd.state, CfdState::Rejected { .. })); - assert!(matches!(maker_cfd.state, CfdState::Rejected { .. })); + assert_next_state!(CfdState::Rejected, maker, taker, received.id); } #[tokio::test] @@ -111,28 +114,16 @@ async fn taker_takes_order_and_maker_accepts_and_contract_setup() { maker.accept_take_request(received.clone()).await; - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - // TODO: More elaborate Cfd assertions - assert_eq!(taker_cfd.order_id, received.id); - assert_eq!(maker_cfd.order_id, received.id); - assert!(matches!(taker_cfd.state, CfdState::ContractSetup { .. })); - assert!(matches!(maker_cfd.state, CfdState::ContractSetup { .. })); + assert_next_state!(CfdState::ContractSetup, maker, taker, received.id); maker.mocks.mock_wallet_sign_and_broadcast().await; taker.mocks.mock_wallet_sign_and_broadcast().await; - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - // TODO: More elaborate Cfd assertions - assert_eq!(taker_cfd.order_id, received.id); - assert_eq!(maker_cfd.order_id, received.id); - assert!(matches!(taker_cfd.state, CfdState::PendingOpen { .. })); - assert!(matches!(maker_cfd.state, CfdState::PendingOpen { .. })); + assert_next_state!(CfdState::PendingOpen, maker, taker, received.id); deliver_event!(maker, taker, Event::LockFinality(received.id)); - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::Open { .. })); - assert!(matches!(maker_cfd.state, CfdState::Open { .. })); + assert_next_state!(CfdState::Open, maker, taker, received.id); } #[tokio::test] @@ -143,14 +134,8 @@ async fn collaboratively_close_an_open_cfd() { taker.propose_settlement(order_id).await; let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!( - taker_cfd.state, - CfdState::OutgoingSettlementProposal { .. } - )); - assert!(matches!( - maker_cfd.state, - CfdState::IncomingSettlementProposal { .. } - )); + assert_eq!(taker_cfd.state, CfdState::OutgoingSettlementProposal); + assert_eq!(maker_cfd.state, CfdState::IncomingSettlementProposal); maker.mocks.mock_monitor_collaborative_settlement().await; taker.mocks.mock_monitor_collaborative_settlement().await; @@ -158,17 +143,13 @@ async fn collaboratively_close_an_open_cfd() { maker.accept_settlement_proposal(order_id).await; sleep(Duration::from_secs(5)).await; // need to wait a bit until both transition - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::PendingClose { .. })); - assert!(matches!(maker_cfd.state, CfdState::PendingClose { .. })); + assert_next_state!(CfdState::PendingClose, maker, taker, order_id); deliver_event!(maker, taker, Event::CloseFinality(order_id)); sleep(Duration::from_secs(5)).await; // need to wait a bit until both transition - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::Closed { .. })); - assert!(matches!(maker_cfd.state, CfdState::Closed { .. })); + assert_next_state!(CfdState::Closed, maker, taker, order_id); } #[tokio::test] @@ -279,21 +260,15 @@ async fn start_from_open_cfd_state() -> (Maker, Taker, OrderId) { maker.accept_take_request(received.clone()).await; - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::ContractSetup { .. })); - assert!(matches!(maker_cfd.state, CfdState::ContractSetup { .. })); - + assert_next_state!(CfdState::ContractSetup, maker, taker, received.id); maker.mocks.mock_wallet_sign_and_broadcast().await; taker.mocks.mock_wallet_sign_and_broadcast().await; - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::PendingOpen { .. })); - assert!(matches!(maker_cfd.state, CfdState::PendingOpen { .. })); + assert_next_state!(CfdState::PendingOpen, maker, taker, received.id); deliver_event!(maker, taker, Event::LockFinality(received.id)); - let (taker_cfd, maker_cfd) = next_cfd(taker.cfd_feed(), maker.cfd_feed()).await.unwrap(); - assert!(matches!(taker_cfd.state, CfdState::Open { .. })); - assert!(matches!(maker_cfd.state, CfdState::Open { .. })); + assert_next_state!(CfdState::Open, maker, taker, received.id); + (maker, taker, received.id) }