From 67549f4349e3bdac821f3ffe5067c188dec0ffe0 Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Fri, 11 Oct 2024 02:11:16 +0530 Subject: [PATCH 1/3] fix(test): use separate wallets to prevent payment failures --- evmlib/src/contract/network_token.rs | 52 ++++++++++++++++----------- sn_node/tests/common/client.rs | 53 ++++++++++++++++++++-------- sn_node/tests/data_with_churn.rs | 15 +++++--- test_utils/src/evm.rs | 21 ++++++++--- 4 files changed, 97 insertions(+), 44 deletions(-) diff --git a/evmlib/src/contract/network_token.rs b/evmlib/src/contract/network_token.rs index 361c87e340..be9d2bca7f 100644 --- a/evmlib/src/contract/network_token.rs +++ b/evmlib/src/contract/network_token.rs @@ -54,6 +54,7 @@ where /// Get the raw token balance of an address. pub async fn balance_of(&self, account: Address) -> Result { + debug!("Getting balance of account: {account:?}"); let balance = self .contract .balanceOf(account) @@ -61,37 +62,48 @@ where .await .inspect_err(|err| error!("Error getting balance of account: {err:?}"))? ._0; + debug!("Balance of account: {account} is {balance}"); Ok(balance) } /// Approve spender to spend a raw amount of tokens. pub async fn approve(&self, spender: Address, value: U256) -> Result { - let tx_hash = self - .contract - .approve(spender, value) - .send() - .await - .inspect_err(|err| { - error!("Error approving spender to spend raw amt of tokens: {err:?}") - })? - .watch() - .await - .inspect_err(|err| error!("Error watching approve tx: {err:?}"))?; + debug!("Approving spender to spend raw amt of tokens: {value}"); + let call = self.contract.approve(spender, value); + let pending_tx_builder = call.send().await.inspect_err(|err| { + error!( + "Error approving spender {spender:?} to spend raw amt of tokens {value}: {err:?}" + ) + })?; + + let pending_tx_hash = *pending_tx_builder.tx_hash(); + debug!("The approval from sender {spender:?} is pending with tx_hash: {pending_tx_hash:?}",); + let tx_hash = pending_tx_builder.watch().await.inspect_err(|err| { + error!("Error watching approve tx with hash {pending_tx_hash:?}: {err:?}") + })?; + + debug!("Approve tx with hash {tx_hash:?} is successful"); Ok(tx_hash) } /// Transfer a raw amount of tokens. pub async fn transfer(&self, receiver: Address, amount: U256) -> Result { - let tx_hash = self - .contract - .transfer(receiver, amount) - .send() - .await - .inspect_err(|err| error!("Error transferring raw amt of tokens: {err:?}"))? - .watch() - .await - .inspect_err(|err| error!("Error watching transfer tx: {err:?}"))?; + debug!("Transferring raw amt of tokens: {amount} to {receiver:?}"); + let call = self.contract.transfer(receiver, amount); + let pending_tx_builder = call.send().await.inspect_err(|err| { + error!("Error transferring raw amt of tokens to {receiver:?}: {err:?}") + })?; + + let pending_tx_hash = *pending_tx_builder.tx_hash(); + debug!( + "The transfer to receiver {receiver:?} is pending with tx_hash: {pending_tx_hash:?}" + ); + let tx_hash = pending_tx_builder.watch().await.inspect_err(|err| { + error!("Error watching transfer tx with hash {pending_tx_hash:?}: {err:?}") + })?; + + debug!("Transfer tx with hash {tx_hash:?} is successful"); Ok(tx_hash) } diff --git a/sn_node/tests/common/client.rs b/sn_node/tests/common/client.rs index 106a13567e..513fc46a95 100644 --- a/sn_node/tests/common/client.rs +++ b/sn_node/tests/common/client.rs @@ -7,10 +7,14 @@ // permissions and limitations relating to use of the SAFE Network Software. use autonomi::Client; +use evmlib::wallet::Wallet; use eyre::Result; +use sn_evm::Amount; use sn_protocol::safenode_proto::{NodeInfoRequest, RestartRequest}; use sn_service_management::{get_local_node_registry_path, NodeRegistry}; +use std::str::FromStr; use std::{net::SocketAddr, path::Path}; +use test_utils::evm::get_new_wallet; use test_utils::testnet::DeploymentInventory; use test_utils::{evm::get_funded_wallet, peers_from_env}; use tokio::sync::Mutex; @@ -35,7 +39,7 @@ const LOAD_FAUCET_WALLET_RETRIES: usize = 6; // mutex to restrict access to faucet wallet from concurrent tests static FAUCET_WALLET_MUTEX: Mutex<()> = Mutex::const_new(()); -pub async fn get_client_and_funded_wallet() -> (Client, evmlib::wallet::Wallet) { +pub async fn get_client_and_funded_wallet() -> (Client, Wallet) { match DeploymentInventory::load() { Ok(_inventory) => { todo!("Not implemented yet for WanNetwork"); @@ -107,20 +111,17 @@ pub fn get_all_rpc_addresses(_skip_genesis_for_droplet: bool) -> Result Result { -// match DeploymentInventory::load() { -// Ok(inventory) => { -// Droplet::get_funded_wallet(client, to_wallet_dir, inventory.faucet_address, false).await -// } -// Err(_) => NonDroplet::get_funded_wallet(client, to_wallet_dir, false).await, -// } -// } +/// Transfer tokens from the provided wallet to a newly created wallet +/// Returns the newly created wallet +pub async fn transfer_to_new_wallet(from: &Wallet, amount: usize) -> Result { + match DeploymentInventory::load() { + Ok(_inventory) => { + todo!("Not implemented yet for WanNetwork"); + // Droplet::get_funded_wallet(client, to_wallet_dir, inventory.faucet_address, false).await + } + Err(_) => LocalNetwork::transfer_to_new_wallet(from, amount).await, + } +} pub struct LocalNetwork; impl LocalNetwork { @@ -139,6 +140,28 @@ impl LocalNetwork { get_funded_wallet() } + /// Transfer tokens from the provided wallet to a newly created wallet + /// Returns the newly created wallet + async fn transfer_to_new_wallet(from: &Wallet, amount: usize) -> Result { + let wallet_balance = from.balance_of_tokens().await?; + let gas_balance = from.balance_of_gas_tokens().await?; + + debug!("Wallet balance: {wallet_balance}, Gas balance: {gas_balance}"); + + let new_wallet = get_new_wallet()?; + + from.transfer_tokens(new_wallet.address(), Amount::from(amount)) + .await?; + + from.transfer_gas_tokens( + new_wallet.address(), + Amount::from_str("10000000000000000000")?, + ) + .await?; + + Ok(new_wallet) + } + // Restart a local node by sending in the SafenodeRpcCmd::Restart to the node's RPC endpoint. pub async fn restart_node(rpc_endpoint: SocketAddr, retain_peer_id: bool) -> Result<()> { let mut rpc_client = get_safenode_rpc_client(rpc_endpoint).await?; diff --git a/sn_node/tests/data_with_churn.rs b/sn_node/tests/data_with_churn.rs index 51a2a32803..dcc4e077ae 100644 --- a/sn_node/tests/data_with_churn.rs +++ b/sn_node/tests/data_with_churn.rs @@ -13,6 +13,7 @@ use crate::common::{ NodeRestart, }; use autonomi::{Client, Wallet}; +use common::client::transfer_to_new_wallet; use eyre::{bail, ErrReport, Result}; use rand::Rng; use self_encryption::MAX_CHUNK_SIZE; @@ -31,6 +32,8 @@ use tokio::{sync::RwLock, task::JoinHandle, time::sleep}; use tracing::{debug, error, info, trace, warn}; use xor_name::XorName; +const TOKENS_TO_TRANSFER: usize = 10000000; + const EXTRA_CHURN_COUNT: u32 = 5; const CHURN_CYCLES: u32 = 2; const CHUNK_CREATION_RATIO_TO_CHURN: u32 = 15; @@ -108,11 +111,11 @@ async fn data_availability_during_churn() -> Result<()> { if chunks_only { " (Chunks only)" } else { "" } ); - let (client, wallet) = get_client_and_funded_wallet().await; + let (client, main_wallet) = get_client_and_funded_wallet().await; info!( - "Client and wallet created. Wallet address: {:?}", - wallet.address() + "Client and wallet created. Main wallet address: {:?}", + main_wallet.address() ); // Shared bucket where we keep track of content created/stored on the network @@ -121,9 +124,10 @@ async fn data_availability_during_churn() -> Result<()> { // Spawn a task to create Registers and CashNotes at random locations, // at a higher frequency than the churning events let create_register_handle = if !chunks_only { + let register_wallet = transfer_to_new_wallet(&main_wallet, TOKENS_TO_TRANSFER).await?; let create_register_handle = create_registers_task( client.clone(), - wallet.clone(), + register_wallet, Arc::clone(&content), churn_period, ); @@ -135,10 +139,11 @@ async fn data_availability_during_churn() -> Result<()> { println!("Uploading some chunks before carry out node churning"); info!("Uploading some chunks before carry out node churning"); + let chunk_wallet = transfer_to_new_wallet(&main_wallet, TOKENS_TO_TRANSFER).await?; // Spawn a task to store Chunks at random locations, at a higher frequency than the churning events let store_chunks_handle = store_chunks_task( client.clone(), - wallet.clone(), + chunk_wallet, Arc::clone(&content), churn_period, ); diff --git a/test_utils/src/evm.rs b/test_utils/src/evm.rs index 2c85c0d90a..b7443df991 100644 --- a/test_utils/src/evm.rs +++ b/test_utils/src/evm.rs @@ -6,10 +6,14 @@ // KIND, either express or implied. Please review the Licences for the specific language governing // permissions and limitations relating to use of the SAFE Network Software. -use evmlib::{utils::network_from_env, Network}; +use color_eyre::{ + eyre::{bail, Context}, + Result, +}; +use evmlib::{utils::network_from_env, wallet::Wallet, Network}; use std::env; -pub fn get_funded_wallet() -> evmlib::wallet::Wallet { +pub fn get_funded_wallet() -> Wallet { let network = network_from_env().expect("Failed to get EVM network from environment variables"); if matches!(network, Network::ArbitrumOne) { panic!("You're trying to use ArbitrumOne network. Use a custom network for testing."); @@ -20,6 +24,15 @@ pub fn get_funded_wallet() -> evmlib::wallet::Wallet { let private_key = env::var("EVM_PRIVATE_KEY").unwrap_or(DEFAULT_WALLET_PRIVATE_KEY.to_string()); - evmlib::wallet::Wallet::new_from_private_key(network, &private_key) - .expect("Invalid private key") + Wallet::new_from_private_key(network, &private_key).expect("Invalid private key") +} + +pub fn get_new_wallet() -> Result { + let network = + network_from_env().wrap_err("Failed to get EVM network from environment variables")?; + if matches!(network, Network::ArbitrumOne) { + bail!("You're trying to use ArbitrumOne network. Use a custom network for testing."); + } + + Ok(Wallet::new_with_random_wallet(network)) } From b532cc43e0a61e425e0733c6f1b8197adbfba42c Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Fri, 11 Oct 2024 02:12:56 +0530 Subject: [PATCH 2/3] fix(ci): re enable churn test --- .github/workflows/merge.yml | 233 ++++++++++++++++++------------------ 1 file changed, 116 insertions(+), 117 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 1e22700f58..fd11bce681 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -556,135 +556,134 @@ jobs: # log_file_prefix: safe_test_logs_token_distribution # platform: ${{ matrix.os }} - # churn: - # if: "!startsWith(github.event.head_commit.message, 'chore(release):')" - # name: Network churning tests - # runs-on: ${{ matrix.os }} - # strategy: - # matrix: - # include: - # - os: ubuntu-latest - # node_data_path: /home/runner/.local/share/safe/node - # safe_path: /home/runner/.local/share/safe - # - os: windows-latest - # node_data_path: C:\\Users\\runneradmin\\AppData\\Roaming\\safe\\node - # safe_path: C:\\Users\\runneradmin\\AppData\\Roaming\\safe - # - os: macos-latest - # node_data_path: /Users/runner/Library/Application Support/safe/node - # safe_path: /Users/runner/Library/Application Support/safe - # steps: - # - uses: actions/checkout@v4 - - # - uses: dtolnay/rust-toolchain@stable + churn: + if: "!startsWith(github.event.head_commit.message, 'chore(release):')" + name: Network churning tests + runs-on: ${{ matrix.os }} + strategy: + matrix: + include: + - os: ubuntu-latest + node_data_path: /home/runner/.local/share/safe/node + safe_path: /home/runner/.local/share/safe + - os: windows-latest + node_data_path: C:\\Users\\runneradmin\\AppData\\Roaming\\safe\\node + safe_path: C:\\Users\\runneradmin\\AppData\\Roaming\\safe + - os: macos-latest + node_data_path: /Users/runner/Library/Application Support/safe/node + safe_path: /Users/runner/Library/Application Support/safe + steps: + - uses: actions/checkout@v4 - # - uses: Swatinem/rust-cache@v2 + - uses: dtolnay/rust-toolchain@stable - # - name: Build binaries - # run: cargo build --release --features local --bin safenode - # timeout-minutes: 30 + - uses: Swatinem/rust-cache@v2 - # - name: Build faucet binaries - # run: cargo build --release --features="local,gifting" --bin faucet - # timeout-minutes: 30 + - name: Build binaries + run: cargo build --release --features local --bin safenode + timeout-minutes: 30 - # - name: Build churn tests - # run: cargo test --release -p sn_node --features=local --test data_with_churn --no-run - # env: - # # only set the target dir for windows to bypass the linker issue. - # # happens if we build the node manager via testnet action - # CARGO_TARGET_DIR: ${{ matrix.os == 'windows-latest' && './test-target' || '.' }} - # timeout-minutes: 30 + - name: Build churn tests + run: cargo test --release -p sn_node --features=local --test data_with_churn --no-run + env: + # only set the target dir for windows to bypass the linker issue. + # happens if we build the node manager via testnet action + CARGO_TARGET_DIR: ${{ matrix.os == 'windows-latest' && './test-target' || '.' }} + timeout-minutes: 30 - # - name: Start a local network - # uses: maidsafe/sn-local-testnet-action@main - # with: - # action: start - # interval: 2000 - # node-path: target/release/safenode - # faucet-path: target/release/faucet - # platform: ${{ matrix.os }} - # build: true + - name: Start a local network + uses: maidsafe/sn-local-testnet-action@evm-dev + with: + action: start + enable-evm-testnet: true + node-path: target/release/safenode + platform: ${{ matrix.os }} + build: true - # - name: Check SAFE_PEERS was set - # shell: bash - # run: | - # if [[ -z "$SAFE_PEERS" ]]; then - # echo "The SAFE_PEERS variable has not been set" - # exit 1 - # else - # echo "SAFE_PEERS has been set to $SAFE_PEERS" - # fi + - name: Check if SAFE_PEERS and EVM_NETWORK are set + shell: bash + run: | + if [[ -z "$SAFE_PEERS" ]]; then + echo "The SAFE_PEERS variable has not been set" + exit 1 + elif [[ -z "$EVM_NETWORK" ]]; then + echo "The EVM_NETWORK variable has not been set" + exit 1 + else + echo "SAFE_PEERS has been set to $SAFE_PEERS" + echo "EVM_NETWORK has been set to $EVM_NETWORK" + fi - # - name: Chunks data integrity during nodes churn - # run: cargo test --release -p sn_node --features="local" --test data_with_churn -- --nocapture - # env: - # TEST_DURATION_MINS: 5 - # TEST_TOTAL_CHURN_CYCLES: 15 - # SN_LOG: "all" - # CARGO_TARGET_DIR: ${{ matrix.os == 'windows-latest' && './test-target' || '.' }} - # timeout-minutes: 30 + - name: Chunks data integrity during nodes churn + run: cargo test --release -p sn_node --features=local --test data_with_churn -- --nocapture + env: + TEST_DURATION_MINS: 5 + TEST_TOTAL_CHURN_CYCLES: 15 + SN_LOG: "all" + CARGO_TARGET_DIR: ${{ matrix.os == 'windows-latest' && './test-target' || '.' }} + timeout-minutes: 30 - # - name: Stop the local network and upload logs - # if: always() - # uses: maidsafe/sn-local-testnet-action@main - # with: - # action: stop - # log_file_prefix: safe_test_logs_churn - # platform: ${{ matrix.os }} + - name: Stop the local network and upload logs + if: always() + uses: maidsafe/sn-local-testnet-action@evm-dev + with: + action: stop + log_file_prefix: safe_test_logs_churn + platform: ${{ matrix.os }} - # - name: Verify restart of nodes using rg - # shell: bash - # timeout-minutes: 1 - # # get the counts, then the specific line, and then the digit count only - # # then check we have an expected level of restarts - # # TODO: make this use an env var, or relate to testnet size - # run: | - # restart_count=$(rg "Node is restarting in" "${{ matrix.node_data_path }}" -c --stats | \ - # rg "(\d+) matches" | rg "\d+" -o) - # echo "Restart $restart_count nodes" - # peer_removed=$(rg "PeerRemovedFromRoutingTable" "${{ matrix.node_data_path }}" -c --stats | \ - # rg "(\d+) matches" | rg "\d+" -o) - # echo "PeerRemovedFromRoutingTable $peer_removed times" - # if [ $peer_removed -lt $restart_count ]; then - # echo "PeerRemovedFromRoutingTable times of: $peer_removed is less than the restart count of: $restart_count" - # exit 1 - # fi - # node_count=$(ls "${{ matrix.node_data_path }}" | wc -l) - # echo "Node dir count is $node_count" + - name: Verify restart of nodes using rg + shell: bash + timeout-minutes: 1 + # get the counts, then the specific line, and then the digit count only + # then check we have an expected level of restarts + # TODO: make this use an env var, or relate to testnet size + run: | + restart_count=$(rg "Node is restarting in" "${{ matrix.node_data_path }}" -c --stats | \ + rg "(\d+) matches" | rg "\d+" -o) + echo "Restart $restart_count nodes" + peer_removed=$(rg "PeerRemovedFromRoutingTable" "${{ matrix.node_data_path }}" -c --stats | \ + rg "(\d+) matches" | rg "\d+" -o) + echo "PeerRemovedFromRoutingTable $peer_removed times" + if [ $peer_removed -lt $restart_count ]; then + echo "PeerRemovedFromRoutingTable times of: $peer_removed is less than the restart count of: $restart_count" + exit 1 + fi + node_count=$(ls "${{ matrix.node_data_path }}" | wc -l) + echo "Node dir count is $node_count" - # # TODO: reenable this once the testnet dir creation is tidied up to avoid a large count here - # # if [ $restart_count -lt $node_count ]; then - # # echo "Restart count of: $restart_count is less than the node count of: $node_count" - # # exit 1 - # # fi + # TODO: reenable this once the testnet dir creation is tidied up to avoid a large count here + # if [ $restart_count -lt $node_count ]; then + # echo "Restart count of: $restart_count is less than the node count of: $node_count" + # exit 1 + # fi - # - name: Verify data replication using rg - # shell: bash - # timeout-minutes: 1 - # # get the counts, then the specific line, and then the digit count only - # # then check we have an expected level of replication - # # TODO: make this use an env var, or relate to testnet size - # run: | - # fetching_attempt_count=$(rg "FetchingKeysForReplication" "${{ matrix.node_data_path }}" -c --stats | \ - # rg "(\d+) matches" | rg "\d+" -o) - # echo "Carried out $fetching_attempt_count fetching attempts" - # node_count=$(ls "${{ matrix.node_data_path }}" | wc -l) - # if [ $fetching_attempt_count -lt $node_count ]; then - # echo "Replication fetching attempts of: $fetching_attempt_count is less than the node count of: $node_count" - # exit 1 - # fi + - name: Verify data replication using rg + shell: bash + timeout-minutes: 1 + # get the counts, then the specific line, and then the digit count only + # then check we have an expected level of replication + # TODO: make this use an env var, or relate to testnet size + run: | + fetching_attempt_count=$(rg "FetchingKeysForReplication" "${{ matrix.node_data_path }}" -c --stats | \ + rg "(\d+) matches" | rg "\d+" -o) + echo "Carried out $fetching_attempt_count fetching attempts" + node_count=$(ls "${{ matrix.node_data_path }}" | wc -l) + if [ $fetching_attempt_count -lt $node_count ]; then + echo "Replication fetching attempts of: $fetching_attempt_count is less than the node count of: $node_count" + exit 1 + fi - # # Only error out after uploading the logs - # - name: Don't log raw data - # if: matrix.os != 'windows-latest' # causes error - # shell: bash - # timeout-minutes: 10 - # run: | - # if ! rg '^' "${{ matrix.safe_path }}"/*/*/logs | awk 'length($0) > 15000 { print; exit 1 }' - # then - # echo "We are logging an extremely large data" - # exit 1 - # fi + # Only error out after uploading the logs + - name: Don't log raw data + if: matrix.os != 'windows-latest' # causes error + shell: bash + timeout-minutes: 10 + run: | + if ! rg '^' "${{ matrix.safe_path }}"/*/*/logs | awk 'length($0) > 15000 { print; exit 1 }' + then + echo "We are logging an extremely large data" + exit 1 + fi verify_data_location_routing_table: if: "!startsWith(github.event.head_commit.message, 'chore(release):')" From 8af00bb5d2a983810eeaf608b91378f07612b9dc Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Fri, 11 Oct 2024 17:25:59 +0530 Subject: [PATCH 3/3] fix(test): retry during PUT to fix failures due to node restarts --- sn_node/tests/data_with_churn.rs | 47 ++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/sn_node/tests/data_with_churn.rs b/sn_node/tests/data_with_churn.rs index dcc4e077ae..c372fc0331 100644 --- a/sn_node/tests/data_with_churn.rs +++ b/sn_node/tests/data_with_churn.rs @@ -320,21 +320,40 @@ fn store_chunks_task( loop { let random_data = gen_random_data(*DATA_SIZE); - let data_map = client - .data_put(random_data, &wallet) - .await - .inspect_err(|err| { - println!("Error to put chunk: {err:?}"); - error!("Error to put chunk: {err:?}") - })?; - - println!("Stored Chunk/s at {data_map:?} after a delay of: {delay:?}"); - info!("Stored Chunk/s at {data_map:?} after a delay of: {delay:?}"); + // FIXME: The client does not have the retry repay to different payee feature yet. + // Retry here for now + let mut retries = 1; + loop { + match client + .data_put(random_data.clone(), &wallet) + .await + .inspect_err(|err| { + println!("Error to put chunk: {err:?}"); + error!("Error to put chunk: {err:?}") + }) { + Ok(data_map) => { + println!("Stored Chunk/s at {data_map:?} after a delay of: {delay:?}"); + info!("Stored Chunk/s at {data_map:?} after a delay of: {delay:?}"); + + content + .write() + .await + .push_back(NetworkAddress::ChunkAddress(ChunkAddress::new(data_map))); + break; + } + Err(err) => { + println!("Failed to store chunk: {err:?}. Retrying ..."); + error!("Failed to store chunk: {err:?}. Retrying ..."); + if retries >= 3 { + println!("Failed to store chunk after 3 retries: {err}"); + error!("Failed to store chunk after 3 retries: {err}"); + bail!("Failed to store chunk after 3 retries: {err}"); + } + retries += 1; + } + } + } - content - .write() - .await - .push_back(NetworkAddress::ChunkAddress(ChunkAddress::new(data_map))); sleep(delay).await; } });