From 7489c53ebdd4dd8bbcb3b45490cccee6426fc7c5 Mon Sep 17 00:00:00 2001 From: JinHwan Date: Wed, 8 Nov 2023 15:40:50 +0900 Subject: [PATCH] Separated signer from core components. (#174) * feat: added mutex lock to signer; used it as ref * fix: mutex deadlock in signer * chore: minor updates * refactor: signer spawn provider internally; updated comments --- backend/examples/summa_solvency_flow.rs | 28 ++--- backend/src/apis/address_ownership.rs | 22 ++-- backend/src/apis/round.rs | 22 ++-- backend/src/contracts/signer.rs | 46 +++++--- backend/src/tests.rs | 149 ++++++++++++++++++------ 5 files changed, 170 insertions(+), 97 deletions(-) diff --git a/backend/examples/summa_solvency_flow.rs b/backend/examples/summa_solvency_flow.rs index 172ccb74..be26bb2b 100644 --- a/backend/examples/summa_solvency_flow.rs +++ b/backend/examples/summa_solvency_flow.rs @@ -9,7 +9,7 @@ use summa_backend::{ address_ownership::AddressOwnership, round::{MstInclusionProof, Round}, }, - contracts::signer::AddressInput, + contracts::signer::{AddressInput, SummaSigner}, tests::initialize_test_env, }; use summa_solvency::merkle_sum_tree::utils::generate_leaf_hash; @@ -20,24 +20,26 @@ const USER_INDEX: usize = 0; #[tokio::main] async fn main() -> Result<(), Box> { // Initialize test environment without `address_ownership` instance from `initialize_test_env` function. - let (anvil, _, _, _, summa_contract) = initialize_test_env().await; + let (anvil, _, _, _, summa_contract) = initialize_test_env(None).await; // 1. Submit ownership proof // // Each CEX prepares its own `signature` CSV file. let signature_csv_path = "src/apis/csv/signatures.csv"; + // The signer instance would be shared with `address_ownership` and `round` instances + // // Using AddressInput::Address to directly provide the summa_contract's address. // For deployed contracts, if the address is stored in a config file, // you can alternatively use AddressInput::Path to specify the file's path. - let mut address_ownership_client = AddressOwnership::new( + let signer = SummaSigner::new( "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", - anvil.chain_id(), anvil.endpoint().as_str(), AddressInput::Address(summa_contract.address()), - signature_csv_path, ) - .unwrap(); + .await?; + + let mut address_ownership_client = AddressOwnership::new(&signer, signature_csv_path).unwrap(); // Dispatch the proof of address ownership. // the `dispatch_proof_of_address_ownership` function sends a transaction to the Summa contract. @@ -55,17 +57,9 @@ async fn main() -> Result<(), Box> { let params_path = "ptau/hermez-raw-11"; // Using the `round` instance, the solvency proof is dispatched to the Summa contract with the `dispatch_solvency_proof` method. - let mut round = Round::<4, 2, 14>::new( - "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", // anvil account [0] - anvil.chain_id(), - anvil.endpoint().as_str(), - AddressInput::Address(summa_contract.address()), - entry_csv, - asset_csv, - params_path, - 1, - ) - .unwrap(); + let timestamp = 1u64; + let mut round = + Round::<4, 2, 14>::new(&signer, entry_csv, asset_csv, params_path, timestamp).unwrap(); // Sends the solvency proof, which should ideally complete without errors. round.dispatch_solvency_proof().await?; diff --git a/backend/src/apis/address_ownership.rs b/backend/src/apis/address_ownership.rs index 54233e86..42a405d1 100644 --- a/backend/src/apis/address_ownership.rs +++ b/backend/src/apis/address_ownership.rs @@ -1,29 +1,23 @@ -use crate::contracts::{ - generated::summa_contract::AddressOwnershipProof, - signer::{AddressInput, SummaSigner}, -}; +use crate::contracts::{generated::summa_contract::AddressOwnershipProof, signer::SummaSigner}; use std::{error::Error, result::Result}; use super::csv_parser::parse_signature_csv; -pub struct AddressOwnership { +pub struct AddressOwnership<'a> { address_ownership_proofs: Vec, - signer: SummaSigner, + signer: &'a SummaSigner, } -impl AddressOwnership { - pub fn new( - signer_key: &str, - chain_id: u64, - rpc_url: &str, - summa_address_input: AddressInput, +impl AddressOwnership<'_> { + pub fn new<'a>( + signer: &'a SummaSigner, signature_csv_path: &str, - ) -> Result> { + ) -> Result, Box> { let address_ownership_proofs = parse_signature_csv(signature_csv_path)?; Ok(AddressOwnership { address_ownership_proofs, - signer: SummaSigner::new(signer_key, chain_id, rpc_url, summa_address_input), + signer, }) } diff --git a/backend/src/apis/round.rs b/backend/src/apis/round.rs index eba011d3..d986bb9c 100644 --- a/backend/src/apis/round.rs +++ b/backend/src/apis/round.rs @@ -8,10 +8,7 @@ use serde::{Deserialize, Serialize}; use std::error::Error; use super::csv_parser::parse_asset_csv; -use crate::contracts::{ - generated::summa_contract::summa::Asset, - signer::{AddressInput, SummaSigner}, -}; +use crate::contracts::{generated::summa_contract::summa::Asset, signer::SummaSigner}; use summa_solvency::{ circuits::{ merkle_sum_tree::MstInclusionCircuit, @@ -65,28 +62,25 @@ pub struct Snapshot { +pub struct Round<'a, const LEVELS: usize, const N_ASSETS: usize, const N_BYTES: usize> { timestamp: u64, snapshot: Snapshot, - signer: SummaSigner, + signer: &'a SummaSigner, } impl - Round + Round<'_, LEVELS, N_ASSETS, N_BYTES> where [usize; N_ASSETS + 1]: Sized, [usize; 2 * (1 + N_ASSETS)]: Sized, { - pub fn new( - signer_key: &str, - chain_id: u64, - rpc_url: &str, - summa_address_input: AddressInput, + pub fn new<'a>( + signer: &'a SummaSigner, entry_csv_path: &str, asset_csv_path: &str, params_path: &str, timestamp: u64, - ) -> Result, Box> { + ) -> Result, Box> { Ok(Round { timestamp, snapshot: Snapshot::::new( @@ -95,7 +89,7 @@ where params_path, ) .unwrap(), - signer: SummaSigner::new(signer_key, chain_id, rpc_url, summa_address_input), + signer: &signer, }) } diff --git a/backend/src/contracts/signer.rs b/backend/src/contracts/signer.rs index 633b2ed6..3d1fd129 100644 --- a/backend/src/contracts/signer.rs +++ b/backend/src/contracts/signer.rs @@ -1,13 +1,12 @@ use ethers::{ prelude::SignerMiddleware, - providers::{Http, Provider}, + providers::{Http, Middleware, Provider}, signers::{LocalWallet, Signer}, types::Address, }; use serde_json::Value; -use std::{ - error::Error, fs::File, io::BufReader, path::Path, str::FromStr, sync::Arc, time::Duration, -}; +use std::{error::Error, fs::File, io::BufReader, path::Path, str::FromStr, sync::Arc}; +use tokio::sync::Mutex; use super::generated::summa_contract::{AddressOwnershipProof, Asset}; use crate::contracts::generated::summa_contract::Summa; @@ -19,27 +18,25 @@ pub enum AddressInput { #[derive(Debug)] pub struct SummaSigner { - summa_contract: Summa, LocalWallet>>, + nonce_lock: Mutex<()>, // To prevent running `submit` methods concurrently + summa_contract: Summa>, LocalWallet>>, } impl SummaSigner { /// Creates a new SummaSigner instance /// # Arguments /// * `signer_key` - The private key of wallet that will interact with the chain on behalf of the exchange - /// * `chain_id` - The chain id of the network - /// * `rpc_url` - The RPC URL of the network - /// * `address_input` - Either the contract's direct address or a path to its config file. - pub fn new( + /// * `url` - The endpoint for connecting to the node + /// * `address` - The address of the Summa contract + pub async fn new( signer_key: &str, - chain_id: u64, - rpc_url: &str, + url: &str, address_input: AddressInput, - ) -> Self { + ) -> Result> { let wallet: LocalWallet = LocalWallet::from_str(signer_key).unwrap(); - let provider = Provider::::try_from(rpc_url) - .unwrap() - .interval(Duration::from_millis(10u64)); + let provider = Arc::new(Provider::try_from(url)?); + let chain_id = provider.get_chainid().await?.as_u64(); let client = Arc::new(SignerMiddleware::new( provider, wallet.with_chain_id(chain_id), @@ -53,10 +50,10 @@ impl SummaSigner { } }; - let contract = Summa::new(address, client); - Self { - summa_contract: contract, - } + Ok(Self { + nonce_lock: Mutex::new(()), + summa_contract: Summa::new(address, client), + }) } pub fn get_summa_address(&self) -> Address { @@ -91,14 +88,19 @@ impl SummaSigner { &self, address_ownership_proofs: Vec, ) -> Result<(), Box> { + let lock_guard = self.nonce_lock.lock().await; + let submit_proof_of_address_ownership = &self .summa_contract .submit_proof_of_address_ownership(address_ownership_proofs); + + // To prevent nonce collision, we lock the nonce before sending the transaction let tx = submit_proof_of_address_ownership.send().await?; // Wait for the pending transaction to be mined tx.await?; + drop(lock_guard); Ok(()) } @@ -109,14 +111,20 @@ impl SummaSigner { proof: ethers::types::Bytes, timestamp: ethers::types::U256, ) -> Result<(), Box> { + let lock_guard = self.nonce_lock.lock().await; + let submit_proof_of_solvency_call = &self .summa_contract .submit_proof_of_solvency(mst_root, assets, proof, timestamp); + + // To prevent nonce collision, we lock the nonce before sending the transaction let tx = submit_proof_of_solvency_call.send().await?; // Wait for the pending transaction to be mined tx.await?; + drop(lock_guard); + Ok(()) } } diff --git a/backend/src/tests.rs b/backend/src/tests.rs index 7098c8e6..aac59da5 100644 --- a/backend/src/tests.rs +++ b/backend/src/tests.rs @@ -16,16 +16,25 @@ use crate::contracts::generated::{ use crate::contracts::mock::mock_erc20::{MockERC20, MOCKERC20_ABI, MOCKERC20_BYTECODE}; // Setup test environment on the anvil instance -pub async fn initialize_test_env() -> ( +pub async fn initialize_test_env( + block_time: Option, +) -> ( AnvilInstance, H160, H160, Arc, LocalWallet>>, Summa, LocalWallet>>, ) { - let anvil: ethers::utils::AnvilInstance = Anvil::new() - .mnemonic("test test test test test test test test test test test junk") - .spawn(); + // Initiate anvil by following assign block time or instant mining + let anvil = match block_time { + Some(interval) => Anvil::new() + .mnemonic("test test test test test test test test test test test junk") + .block_time(interval) + .spawn(), + None => Anvil::new() + .mnemonic("test test test test test test test test test test test junk") + .spawn(), + }; // Extracting two exchange addresses from the Anvil instance let cex_addr_1 = anvil.addresses()[1]; @@ -43,13 +52,6 @@ pub async fn initialize_test_env() -> ( wallet.with_chain_id(anvil.chain_id()), )); - // Creating a factory to deploy a mock ERC20 contract - let factory = ContractFactory::new( - MOCKERC20_ABI.to_owned(), - MOCKERC20_BYTECODE.to_owned(), - Arc::clone(&client), - ); - // Send RPC requests with `anvil_setBalance` method via provider to set ETH balance of `cex_addr_1` and `cex_addr_2` // This is for meeting `proof_of_solvency` test conditions for addr in [cex_addr_1, cex_addr_2].iter().copied() { @@ -59,6 +61,14 @@ pub async fn initialize_test_env() -> ( .await; } + // Mock ERC20 contract deployment + // Creating a factory to deploy a mock ERC20 contract + let factory = ContractFactory::new( + MOCKERC20_ABI.to_owned(), + MOCKERC20_BYTECODE.to_owned(), + Arc::clone(&client), + ); + // Deploy Mock ERC20 contract let mock_erc20_deployment = factory.deploy(()).unwrap().send().await.unwrap(); @@ -71,6 +81,10 @@ pub async fn initialize_test_env() -> ( time::sleep(Duration::from_millis(500)).await; + if block_time != None { + time::sleep(Duration::from_secs(block_time.unwrap())).await; + }; + // Deploy verifier contracts before deploy Summa contract let solvency_verifer_contract = SolvencyVerifier::deploy(Arc::clone(&client), ()) .unwrap() @@ -78,12 +92,20 @@ pub async fn initialize_test_env() -> ( .await .unwrap(); + if block_time != None { + time::sleep(Duration::from_secs(block_time.unwrap())).await; + }; + let inclusion_verifer_contract = InclusionVerifier::deploy(Arc::clone(&client), ()) .unwrap() .send() .await .unwrap(); + if block_time != None { + time::sleep(Duration::from_secs(block_time.unwrap())).await; + }; + // Deploy Summa contract let summa_contract = Summa::deploy( Arc::clone(&client), @@ -97,13 +119,24 @@ pub async fn initialize_test_env() -> ( .await .unwrap(); + time::sleep(Duration::from_secs(3)).await; + (anvil, cex_addr_1, cex_addr_2, client, summa_contract) } #[cfg(test)] mod test { - use ethers::{abi::AbiEncode, types::U256, utils::to_checksum}; - use std::error::Error; + use ethers::{ + abi::AbiEncode, + providers::{Http, Middleware, Provider}, + types::{U256, U64}, + utils::to_checksum, + }; + use std::{convert::TryFrom, error::Error}; + use tokio::{ + join, + time::{sleep, Duration}, + }; use crate::apis::{address_ownership::AddressOwnership, round::Round}; use crate::contracts::{ @@ -117,37 +150,96 @@ mod test { #[tokio::test] async fn test_deployed_address() -> Result<(), Box> { - let (anvil, _, _, _, summa_contract) = initialize_test_env().await; + let (anvil, _, _, _, summa_contract) = initialize_test_env(None).await; // Hardhat development environment, usually updates the address of a deployed contract in the `artifacts` directory. // However, in our custom deployment script, `contracts/scripts/deploy.ts`, // the address gets updated in `backend/src/contracts/deployments.json`. let contract_address = summa_contract.address(); - let summa_signer = SummaSigner::new( + let signer = SummaSigner::new( "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", - anvil.chain_id(), anvil.endpoint().as_str(), AddressInput::Path("./src/contracts/deployments.json".into()), // the file contains the address of the deployed contract + ) + .await?; + + assert_eq!(contract_address, signer.get_summa_address()); + + Ok(()) + } + + #[tokio::test] + async fn test_concurrent_proof_submissions() -> Result<(), Box> { + let (anvil, _, _, _, summa_contract) = initialize_test_env(Some(1)).await; + + // This test ensures that two proofs, when dispatched concurrently, do not result in nonce collisions. + // It checks that both proofs are processed and mined within a reasonable timeframe, + // indicating that there's no interference or delay when the two are submitted simultaneously. + let signer = SummaSigner::new( + "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", + anvil.endpoint().as_str(), + AddressInput::Address(summa_contract.address()), + ) + .await?; + + // At least one address ownership proof should be submitted before submitting solvency proof + let mut address_ownership_client = + AddressOwnership::new(&signer, "src/apis/csv/signatures.csv").unwrap(); + + address_ownership_client + .dispatch_proof_of_address_ownership() + .await?; + + // Do sumbit solvency proofs simultaneously + let asset_csv = "src/apis/csv/assets.csv"; + let entry_csv = "../zk_prover/src/merkle_sum_tree/csv/entry_16.csv"; + let params_path = "ptau/hermez-raw-11"; + + let mut round_one = + Round::<4, 2, 14>::new(&signer, entry_csv, asset_csv, params_path, 1).unwrap(); + let mut round_two = + Round::<4, 2, 14>::new(&signer, entry_csv, asset_csv, params_path, 2).unwrap(); + + // Checking block number before sending transaction of proof of solvency + let outer_provider: Provider = Provider::try_from(anvil.endpoint().as_str())?; + let start_block_number = outer_provider.get_block_number().await?; + + // Send two solvency proofs simultaneously + let (round_one_result, round_two_result) = join!( + round_one.dispatch_solvency_proof(), + round_two.dispatch_solvency_proof() ); - assert_eq!(contract_address, summa_signer.get_summa_address()); + // Check two blocks has been mined + for _ in 0..5 { + sleep(Duration::from_millis(500)).await; + let updated_block_number = outer_provider.get_block_number().await?; + if (updated_block_number - start_block_number) > U64::from(2) { + break; + } + } + + // Check two rounds' result are both Ok + assert!(round_one_result.is_ok()); + assert!(round_two_result.is_ok()); Ok(()) } #[tokio::test] async fn test_round_features() -> Result<(), Box> { - let (anvil, cex_addr_1, cex_addr_2, _, summa_contract) = initialize_test_env().await; + let (anvil, cex_addr_1, cex_addr_2, _, summa_contract) = initialize_test_env(None).await; - let mut address_ownership_client = AddressOwnership::new( + let signer = SummaSigner::new( "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", - anvil.chain_id(), anvil.endpoint().as_str(), AddressInput::Address(summa_contract.address()), - "src/apis/csv/signatures.csv", ) - .unwrap(); + .await?; + + let mut address_ownership_client = + AddressOwnership::new(&signer, "src/apis/csv/signatures.csv").unwrap(); address_ownership_client .dispatch_proof_of_address_ownership() @@ -184,17 +276,8 @@ mod test { let entry_csv = "../zk_prover/src/merkle_sum_tree/csv/entry_16.csv"; let params_path = "ptau/hermez-raw-11"; - let mut round = Round::<4, 2, 14>::new( - "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", // anvil account [0] - anvil.chain_id(), - anvil.endpoint().as_str(), - AddressInput::Address(summa_contract.address()), - entry_csv, - asset_csv, - params_path, - 1, - ) - .unwrap(); + let mut round = + Round::<4, 2, 14>::new(&signer, entry_csv, asset_csv, params_path, 1).unwrap(); // Verify solvency proof let mut solvency_proof_logs = summa_contract