Merge pull request #10 from EspressoSystems/ma/fix-faucet-unfunded-at…

…-startup

Enable funding after startup

src/faucet.rs

@@ -11,7 +11,7 @@ use ethers::{
     prelude::SignerMiddleware,
     providers::{Http, Middleware as _, Provider, StreamExt, Ws},
     signers::{coins_bip39::English, LocalWallet, MnemonicBuilder, Signer},
-    types::{Address, TransactionRequest, H256, U256, U512},
+    types::{Address, TransactionReceipt, TransactionRequest, H256, U256, U512},
     utils::{parse_ether, ConversionError},
 };
 use std::{
@@ -26,6 +26,9 @@ use url::Url;
 
 pub type Middleware = SignerMiddleware<Provider<Http>, LocalWallet>;
 
+pub(crate) const TEST_MNEMONIC: &str =
+    "test test test test test test test test test test test junk";
+
 #[derive(Parser, Debug, Clone)]
 pub struct Options {
     /// Number of Ethereum accounts to use for the faucet.
@@ -92,7 +95,7 @@ impl Default for Options {
     fn default() -> Self {
         Self {
             num_clients: 10,
-            mnemonic: "test test test test test test test test test test test junk".to_string(),
+            mnemonic: TEST_MNEMONIC.to_string(),
             port: 8111,
             faucet_grant_amount: parse_ether("100").unwrap(),
             transaction_timeout: Duration::from_secs(300),
@@ -103,6 +106,15 @@ impl Default for Options {
     }
 }
 
+impl Options {
+    /// Returns the minimum balance required to consider a client funded.
+    ///
+    /// Set to 2 times the faucet grant amount to be on the safe side regarding gas.
+    fn min_funding_balance(&self) -> U256 {
+        self.faucet_grant_amount * 2
+    }
+}
+
 #[derive(Debug, Clone, Copy)]
 pub enum TransferRequest {
     Faucet {
@@ -271,7 +283,10 @@ impl Faucet {
             clients.push((balance, client));
         }
 
-        let desired_balance = total_balance / options.num_clients * 8 / 10;
+        let desired_balance = std::cmp::max(
+            total_balance / options.num_clients * 8 / 10,
+            options.min_funding_balance().into(),
+        );
         // At this point, `desired_balance` is less than the average of all the clients' balances,
         // each of which was a `U256`, so we can safely cast back into a `U256`.
         let desired_balance =
@@ -411,19 +426,46 @@ impl Faucet {
         }
     }
 
-    async fn handle_receipt(&self, tx_hash: H256) -> Result<()> {
-        tracing::debug!("Got tx hash {:?}", tx_hash);
-
-        let Transfer {
-            sender, request, ..
-        } = {
-            if let Some(inflight) = self.state.read().await.inflight.get(&tx_hash) {
-                inflight.clone()
+    /// Handle external incoming transfers to faucet accounts
+    async fn handle_non_faucet_transfer(&self, receipt: &TransactionReceipt) -> Result<()> {
+        tracing::debug!("Handling external incoming transfer to {:?}", receipt.to);
+        if let Some(receiver) = receipt.to {
+            if self
+                .state
+                .read()
+                .await
+                .clients_being_funded
+                .contains_key(&receiver)
+            {
+                let balance = self.balance(receiver).await?;
+                if balance >= self.config.min_funding_balance() {
+                    tracing::info!("Funded client {:?} with external transfer", receiver);
+                    let mut state = self.state.write().await;
+                    if let Some(transfer_index) =
+                        state.transfer_queue.iter().position(|r| r.to() == receiver)
+                    {
+                        tracing::info!("Removing funding request from queue");
+                        state.transfer_queue.remove(transfer_index);
+                    } else {
+                        tracing::warn!("Funding request not found in queue");
+                    }
+                    tracing::info!("Making client {receiver:?} available");
+                    let client = state.clients_being_funded.remove(&receiver).unwrap();
+                    state.clients.push(balance, client);
+                } else {
+                    tracing::warn!(
+                        "Balance for client {receiver:?} {balance:?} too low to make it available"
+                    );
+                }
             } else {
-                // Not a transaction we are monitoring.
-                return Ok(());
+                tracing::debug!("Irrelevant transaction {:?}", receipt.transaction_hash);
             }
-        };
+        }
+        Ok(())
+    }
+
+    async fn handle_receipt(&self, tx_hash: H256) -> Result<()> {
+        tracing::debug!("Got tx hash {:?}", tx_hash);
 
         // In case there is a race condition and the receipt is not yet available, wait for it.
         let receipt = loop {
@@ -434,8 +476,21 @@ impl Faucet {
             async_std::task::sleep(Duration::from_secs(1)).await;
         };
 
-        tracing::info!("Received receipt for {:?}", request);
+        tracing::debug!("Got receipt {:?}", receipt);
 
+        // Using `cloned` here to avoid borrow
+        let inflight = self.state.read().await.inflight.get(&tx_hash).cloned();
+        if inflight.is_none() {
+            // Not a transaction we are monitoring but the recipient could
+            // be a faucet account that is waiting for funding.
+            return self.handle_non_faucet_transfer(&receipt).await;
+        }
+
+        let Transfer {
+            sender, request, ..
+        } = inflight.unwrap();
+
+        tracing::info!("Received receipt for {request:?}");
         // Do all external calls before state modifications
         let new_sender_balance = self.balance(sender.address()).await?;
 
@@ -505,6 +560,11 @@ impl Faucet {
                 }
             };
 
+            // There is some room for optimization here because we are fetching
+            // every transaction receipt. We could use the `get_block_with_txs` of
+            // the ethers-rs provider to avoid fetching individual transaction
+            // receipts.
+
             let mut stream = provider
                 .subscribe_blocks()
                 .await
@@ -544,6 +604,7 @@ impl Faucet {
     }
 
     async fn process_transaction_timeouts(&self) -> Result<()> {
+        tracing::info!("Processing transaction timeouts");
         let inflight = self.state.read().await.inflight.clone();
 
         for (

src/web.rs

@@ -105,17 +105,18 @@ impl WebState {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::faucet::{Faucet, Options};
+    use crate::faucet::{Faucet, Middleware, Options, TEST_MNEMONIC};
     use anyhow::Result;
     use async_compatibility_layer::logging::{setup_backtrace, setup_logging};
     use async_std::task::spawn;
     use ethers::{
-        providers::{Http, Middleware, Provider},
-        types::U256,
+        providers::{Http, Middleware as _, Provider},
+        signers::{coins_bip39::English, MnemonicBuilder, Signer},
+        types::{TransactionRequest, U256},
         utils::parse_ether,
     };
     use sequencer_utils::AnvilOptions;
-    use std::time::Duration;
+    use std::{sync::Arc, time::Duration};
     use surf_disco::Client;
 
     async fn run_faucet_test(options: Options, num_transfers: usize) -> Result<()> {
@@ -224,4 +225,68 @@ mod test {
 
         Ok(())
     }
+
+    // A test to verify that the faucet functions if it's funded only after startup.
+    #[async_std::test]
+    async fn test_unfunded_faucet() -> Result<()> {
+        setup_logging();
+        setup_backtrace();
+
+        let anvil_opts = AnvilOptions::default();
+        let anvil = anvil_opts.clone().spawn().await;
+
+        let mut ws_url = anvil.url();
+        ws_url.set_scheme("ws").unwrap();
+
+        let provider = Provider::<Http>::try_from(anvil.url().to_string())?;
+        let chain_id = provider.get_chainid().await?.as_u64();
+
+        let funded_wallet = MnemonicBuilder::<English>::default()
+            .phrase(TEST_MNEMONIC)
+            .index(0u32)?
+            .build()?
+            .with_chain_id(chain_id);
+        let funded_client = Arc::new(Middleware::new(provider.clone(), funded_wallet));
+
+        // An unfunded mnemonic
+        let mnemonic =
+            "obvious clean kidney better photo young sun similar unit home half rough".to_string();
+        let faucet_wallet = MnemonicBuilder::<English>::default()
+            .phrase(mnemonic.as_str())
+            .index(0u32)?
+            .build()?
+            .with_chain_id(chain_id);
+
+        let options = Options {
+            num_clients: 2,
+            faucet_grant_amount: parse_ether(1).unwrap(),
+            provider_url_ws: ws_url,
+            provider_url_http: anvil.url(),
+            port: portpicker::pick_unused_port().unwrap(),
+            mnemonic,
+            ..Default::default()
+        };
+
+        let (sender, receiver) = async_std::channel::unbounded();
+
+        // Start the faucet
+        let faucet = Faucet::create(options.clone(), receiver).await?;
+        let _handle = faucet.start().await;
+
+        // Start the web server
+        spawn(async move { serve(options.port, WebState::new(sender)).await });
+
+        // Transfer some funds to the faucet
+        funded_client
+            .send_transaction(
+                TransactionRequest::pay(faucet_wallet.address(), options.faucet_grant_amount * 100),
+                None,
+            )
+            .await?
+            .await?;
+
+        run_faucet_test(options, 3).await?;
+
+        Ok(())
+    }
 }