randomized reorg check timer & swap_tx_not_found test

doc/swap_api.md

@@ -47,6 +47,7 @@ In case of errors, the API will return a `SwapError` type with one of the follow
 - `FeeTooLow`: The provided fee is too low.
 - `StoreError`: An error occurred when saving swap to the data store.
 - `ClientError`: An error occurred during client communication.
+- `SwapTxNotFound`: The previous swap transaction was not found in data store.
 - `UnknownError`: An unknown error occurred.
 
 ### Example

src/bin/mwixnet.rs

@@ -14,12 +14,12 @@ use grin_util::{StopState, ZeroingString};
 use mwixnet::client::{MixClient, MixClientImpl};
 use mwixnet::node::GrinNode;
 use mwixnet::store::StoreError;
+use rand::{thread_rng, Rng};
 use rpassword;
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 use std::thread::{sleep, spawn};
 use std::time::Duration;
-use grin_core::core::Transaction;
 
 #[macro_use]
 extern crate clap;
@@ -262,7 +262,10 @@ fn real_main() -> Result<(), Box<dyn std::error::Error>> {
 
         let close_handle = http_server.close_handle();
         let round_handle = spawn(move || {
-            let mut secs = 0;
+            let mut rng = thread_rng();
+            let mut secs = 0u32;
+            let mut reorg_secs = 0u32;
+            let mut reorg_window = rng.gen_range(900u32, 3600u32);
             let prev_tx = Arc::new(Mutex::new(None));
             let server = swap_server.clone();
 
@@ -274,6 +277,7 @@ fn real_main() -> Result<(), Box<dyn std::error::Error>> {
 
                 sleep(Duration::from_secs(1));
                 secs = (secs + 1) % server_config.interval_s;
+                reorg_secs = (reorg_secs + 1) % reorg_window;
 
                 if secs == 0 {
                     let prev_tx_clone = prev_tx.clone();
@@ -286,7 +290,9 @@ fn real_main() -> Result<(), Box<dyn std::error::Error>> {
                             _ => None,
                         };
                     });
-                } else if secs % 30 == 0 {
+                    reorg_secs = 0;
+                    reorg_window = rng.gen_range(900u32, 3600u32);
+                } else if reorg_secs == 0 {
                     let prev_tx_clone = prev_tx.clone();
                     let server_clone = server.clone();
                     rt.spawn(async move {
@@ -304,6 +310,7 @@ fn real_main() -> Result<(), Box<dyn std::error::Error>> {
                             };
                         }
                     });
+                    reorg_window = rng.gen_range(900u32, 3600u32);
                 }
             }
         });

src/servers/swap.rs

@@ -45,6 +45,8 @@ pub enum SwapError {
     NodeError(String),
     #[error("Client communication error: {0:?}")]
     ClientError(String),
+    #[error("Swap transaction not found: {0:?}")]
+    SwapTxNotFound(Commitment),
     #[error("{0}")]
     UnknownError(String),
 }
@@ -327,7 +329,8 @@ impl SwapServer for SwapServerImpl {
 
     async fn check_reorg(&self, tx: Arc<Transaction>) -> Result<Option<Arc<Transaction>>, SwapError> {
         let excess = tx.kernels().first().unwrap().excess;
-        if let Ok(swap_tx) = self.store.lock().await.get_swap_tx(&excess) {
+        let locked_store = self.store.lock().await;
+        if let Ok(swap_tx) = locked_store.get_swap_tx(&excess) {
             // If kernel is in active chain, return tx
             if self.node.async_get_kernel(&excess, Some(swap_tx.chain_tip.0), None).await?.is_some() {
                 return Ok(Some(tx));
@@ -341,7 +344,6 @@ impl SwapServer for SwapServerImpl {
 
             // Collect all swaps based on tx's inputs, and execute_round with those swaps
             let next_block_height = self.node.async_get_chain_tip().await?.0 + 1;
-            let locked_store = self.store.lock().await;
             let mut swaps = Vec::new();
             for input_commit in &tx.inputs_committed() {
                 if let Ok(swap) = locked_store.get_swap(&input_commit) {
@@ -353,7 +355,7 @@ impl SwapServer for SwapServerImpl {
 
             self.async_execute_round(&locked_store, swaps).await
         } else {
-            Err(SwapError::UnknownError("Swap transaction not found".to_string())) // TODO: Create SwapError enum value
+            Err(SwapError::SwapTxNotFound(excess))
         }
     }
 }
@@ -453,7 +455,7 @@ mod tests {
     use crate::tx::TxComponents;
 
     use ::function_name::named;
-    use grin_core::core::{Committed, Input, Output, OutputFeatures, Transaction, Weighting};
+    use grin_core::core::{Committed, Input, Inputs, Output, OutputFeatures, Transaction, Weighting};
     use grin_onion::crypto::comsig::ComSignature;
     use grin_onion::crypto::secp;
     use grin_onion::onion::Onion;
@@ -833,6 +835,26 @@ mod tests {
         Ok(())
     }
 
+    /// Returns SwapTxNotFound when trying to check_reorg with a transaction not found in the store.
+    #[tokio::test]
+    #[named]
+    async fn swap_tx_not_found() -> Result<(), Box<dyn std::error::Error>> {
+        let test_dir = init_test!();
+
+        let server_key = secp::random_secret();
+        let node: Arc<MockGrinNode> = Arc::new(MockGrinNode::new());
+        let (server, _) = super::test_util::new_swapper(&test_dir, &server_key, None, node.clone());
+        let kern = tx::build_kernel(&secp::random_secret(), 1000u64)?;
+        let tx: Arc<Transaction> = Arc::new(Transaction::new(Inputs::default(), &[], &[kern.clone()]));
+        let result = server.check_reorg(tx).await;
+        assert_eq!(
+            Err(SwapError::SwapTxNotFound(kern.excess())),
+            result
+        );
+
+        Ok(())
+    }
+
     /// Returns PeelOnionFailure when a failure occurs trying to decrypt the onion payload.
     #[tokio::test]
     #[named]

src/store.rs

@@ -236,7 +236,7 @@ impl SwapStore {
 
     /// Reads a single value by key
     fn read<K: AsRef<[u8]> + Copy, V: Readable>(&self, prefix: u8, k: K) -> Result<V, StoreError> {
-        store::option_to_not_found(self.db.get_ser(&store::to_key(prefix, k)[..], None), || {
+        store::option_to_not_found(self.db.get_ser(&store::to_key(prefix, k), None), || {
             format!("{}:{}", prefix, k.to_hex())
         })
             .map_err(StoreError::ReadError)
@@ -279,7 +279,6 @@ impl SwapStore {
     }
 
     /// Reads a swap from the database
-    #[allow(dead_code)]
     pub fn get_swap(&self, input_commit: &Commitment) -> Result<SwapData, StoreError> {
         self.read(SWAP_PREFIX, input_commit)
     }

tests/common/wallet.rs

@@ -8,7 +8,7 @@ use grin_keychain::{BlindingFactor, ExtKeychain, Identifier, Keychain, SwitchCom
 use grin_onion::crypto::comsig::ComSignature;
 use grin_onion::onion::Onion;
 use grin_onion::Hop;
-use grin_util::{Mutex, ToHex, ZeroingString};
+use grin_util::{Mutex, ZeroingString};
 use grin_wallet_api::Owner;
 use grin_wallet_config::WalletConfig;
 use grin_wallet_controller::controller;