Improve sTPS calculation (ignore non-call txs, ignore last block) (pa…

…ritytech#58)

.github/workflows/benchmark.yml

@@ -1,4 +1,4 @@
-name: Schedule based benchmark of pallet-contracts and pallet-evm
+name: Benchmark of pallet-contracts and pallet-evm
 
 on:
   schedule:
@@ -11,7 +11,7 @@ env:
   MOONBEAM_BIN: moonbeam_release/*/target/release/moonbeam
   MOONBEAM_VERSION: version
   BENCHMARK_DIR: stats
-  TEST_PARAMS: --instance-count 1 --call-count 1000
+  TEST_PARAMS: --instance-count 1 --call-count 2000
   BENCHMARK_URI: https://raw.githubusercontent.com/paritytech/smart-bench/gh-pages
 
 jobs:
@@ -70,9 +70,9 @@ jobs:
     strategy:
       matrix:
         type: [ink-wasm, sol-wasm, evm]
-        contract: [erc20]
+        contract: [erc20, flipper, triangle-number, storage-read, storage-write]
     env:
-      BENCHMARK_FILE: benchmark-${{ matrix.type }}-${{ matrix.contract }}.csv
+      BENCHMARK_FILE: benchmark_${{ matrix.type }}_${{ matrix.contract }}.csv
     needs: build_dev_moonbeam
     runs-on: ubuntu-latest
     steps:
@@ -204,8 +204,6 @@ jobs:
   collect:
     runs-on: ubuntu-latest
     needs: [smart_contract_benchmark]
-    env:
-      BENCHMARK_FILE: benchmark-results.csv
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -217,13 +215,24 @@ jobs:
 
       - name: Merge CSV
         run: |
-          curl -L -o ${{ env.BENCHMARK_DIR }}/${{ env.BENCHMARK_FILE }}  ${{ env.BENCHMARK_URI }}/${{ env.BENCHMARK_FILE }}
-          cat ${{ env.BENCHMARK_DIR }}/*/*.csv >> ${{ env.BENCHMARK_DIR }}/${{ env.BENCHMARK_FILE }}
+          for file in ${{ env.BENCHMARK_DIR }}/*/*.csv; do
+            # Extract contract name
+            contract_name=$(basename "$file" | sed 's/^.*_\(.*\)\.csv$/\1/')
+            benchmark_file=bench_${contract_name}.csv
+            if [ ! -f ${{ env.BENCHMARK_DIR }}/${benchmark_file} ]; then
+              curl -L -o ${{ env.BENCHMARK_DIR }}/${benchmark_file}  ${{ env.BENCHMARK_URI }}/${benchmark_file} || exit 1
+            fi
+            cat $file >> ${{ env.BENCHMARK_DIR }}/${benchmark_file}
+          done
 
       - name: Generate graph
         run: |
-          cd stats
-          ./get_graph.sh --panel-id=2 --csv-data=../${{ env.BENCHMARK_DIR }}/${{ env.BENCHMARK_FILE }} --output=../${{ env.BENCHMARK_DIR }}/tps.png
+          for file in ${{ env.BENCHMARK_DIR }}/bench_*.csv; do
+            contract_name=$(basename "$file" | sed 's/^.*_\(.*\)\.csv$/\1/')
+            pushd stats
+            ./get_graph.sh --panel-id=2 --csv-data=../${file} --output=../${{ env.BENCHMARK_DIR }}/stps_${contract_name}.png
+            popd
+          done
 
       - name: Commit benchmark stats
         run: |
@@ -234,12 +243,12 @@ jobs:
           git fetch origin gh-pages
           # saving stats
           mkdir /tmp/stats
-          mv ${{ env.BENCHMARK_DIR }}/${{ env.BENCHMARK_FILE }} /tmp/stats
-          mv ${{ env.BENCHMARK_DIR }}/tps.png /tmp/stats
+          mv ${{ env.BENCHMARK_DIR }}/bench_*.csv /tmp/stats
+          mv ${{ env.BENCHMARK_DIR }}/stps_*.png /tmp/stats
           git checkout gh-pages
           mv /tmp/stats/* .
           # Upload files
-          git add ${{ env.BENCHMARK_FILE }} tps.png --force
+          git add *.csv *.png --force
           git status
           git commit -m "Updated stats in ${CURRENT_DATE} and pushed to gh-pages"
           git push origin gh-pages --force

README.md

@@ -117,3 +117,20 @@ Before running tests, smart-bench needs to be build using `cargo build` command.
 Integration tests requires two types of nodes to be installed and available on `PATH`.
 - [`moonbeam`](https://github.com/PureStake/moonbeam/) with enabled [`dev RPC`](https://github.com/paritytech/substrate-contracts-node/blob/539cf0271090f406cb3337e4d97680a6a63bcd2f/node/src/rpc.rs#L60) for Solidity/EVM contracts
 - [`substrate-contracts-node`](https://github.com/paritytech/substrate-contracts-node/) for Ink! and Solang (Solidity/Wasm) contracts
+
+### Benchmarks
+
+## Erc20
+![Erc20](https://github.com/paritytech/smart-bench/blob/gh-pages/stps_erc20.png?raw=true)
+
+## Flipper
+![Flipper](https://github.com/paritytech/smart-bench/blob/gh-pages/stps_flipper.png?raw=true)
+
+## Storage Read
+![Storage Read](https://github.com/paritytech/smart-bench/blob/gh-pages/stps_storage-read.png?raw=true)
+
+## Storage Write
+![Storage Write](https://github.com/paritytech/smart-bench/blob/gh-pages/stps_storage-write.png?raw=true)
+
+## Triangle Number
+![Triangle Number](https://github.com/paritytech/smart-bench/blob/gh-pages/stps_triangle-number.png?raw=true)

src/evm/runner.rs

@@ -3,7 +3,9 @@ use std::collections::HashSet;
 use super::xts::{
     api::{
         self,
+        ethereum::calls::types::Transact,
         ethereum::events::Executed,
+        runtime_types::ethereum::transaction::{TransactionAction, TransactionV2},
         runtime_types::evm_core::error::{ExitReason, ExitSucceed},
     },
     MoonbeamApi,
@@ -25,7 +27,7 @@ pub struct MoonbeamRunner {
     pub api: MoonbeamApi,
     signer: SecretKey,
     address: Address,
-    calls: Vec<(String, Vec<Call>)>,
+    calls: Vec<(String, Vec<RunnerCall>)>,
 }
 
 impl MoonbeamRunner {
@@ -93,7 +95,7 @@ impl MoonbeamRunner {
                 .estimate_gas(self.address, Some(contract), &data)
                 .await
                 .note("Error estimating gas")?;
-            calls.push(Call {
+            calls.push(RunnerCall {
                 name: name.to_string(),
                 contract,
                 data,
@@ -210,15 +212,30 @@ impl MoonbeamRunner {
         client: OnlineClient<DefaultConfig>,
         block_hash: sp_core::H256,
     ) -> color_eyre::Result<(u64, Vec<sp_core::H256>)> {
-        let events = client.events().at(block_hash).await?;
+        let block = client.blocks().at(block_hash).await?;
         let mut tx_hashes = Vec::new();
-        for event in events.iter() {
-            let event = event?;
-            if let Some(Executed {
-                transaction_hash, ..
-            }) = event.as_event::<Executed>()?
-            {
-                tx_hashes.push(transaction_hash);
+        let extrinsics_details = block
+            .extrinsics()
+            .await?
+            .iter()
+            .collect::<Result<Vec<_>, _>>()?;
+
+        for extrinsic_detail in extrinsics_details {
+            if let Some(Transact { transaction }) = extrinsic_detail.as_extrinsic::<Transact>()? {
+                if let TransactionV2::Legacy(tx) = transaction {
+                    if let TransactionAction::Call(_) = tx.action {
+                        let events = extrinsic_detail.events().await?;
+                        for event in events.iter() {
+                            let event = event?;
+                            if let Some(Executed {
+                                transaction_hash, ..
+                            }) = event.as_event::<Executed>()?
+                            {
+                                tx_hashes.push(transaction_hash);
+                            }
+                        }
+                    }
+                }
             }
         }
         let storage_timestamp_storage_addr = api::storage().timestamp().now();
@@ -293,7 +310,7 @@ impl MoonbeamRunner {
     }
 }
 
-struct Call {
+struct RunnerCall {
     name: String,
     contract: Address,
     data: Vec<u8>,

src/integration_tests.rs

@@ -164,7 +164,7 @@ async fn test_ink_contract_success() {
         .arg("ink-wasm")
         .arg("flipper")
         .args(["--instance-count", "1"])
-        .args(["--call-count", "1"])
+        .args(["--call-count", "10"])
         .args(["--url", "ws://localhost:9944"])
         .timeout(std::time::Duration::from_secs(5))
         .output()
@@ -209,7 +209,7 @@ async fn test_solidity_wasm_contract_success() {
         .arg("sol-wasm")
         .arg("flipper")
         .args(["--instance-count", "1"])
-        .args(["--call-count", "1"])
+        .args(["--call-count", "10"])
         .args(["--url", "ws://localhost:9944"])
         .timeout(std::time::Duration::from_secs(5))
         .output()
@@ -255,7 +255,7 @@ async fn test_solidity_evm_contract_success() {
         .arg("evm")
         .arg("flipper")
         .args(["--instance-count", "1"])
-        .args(["--call-count", "1"])
+        .args(["--call-count", "10"])
         .args(["--url", "ws://localhost:9944"])
         .timeout(std::time::Duration::from_secs(5))
         .output()

src/stats.rs

@@ -8,7 +8,7 @@ pub struct BlockInfo {
     pub time_stamp: u64,
     pub stats: blockstats::BlockStats,
     // list of hashes to look for
-    pub hashes: Vec<sp_core::H256>,
+    pub contract_call_hashes: Vec<sp_core::H256>,
 }
 
 /// Subscribes to block stats. Completes once *all* hashes in `remaining_hashes` have been received.
@@ -48,27 +48,45 @@ where
             }
             Ok(BlockInfo {
                 time_stamp,
-                hashes,
+                contract_call_hashes: hashes,
                 stats,
             })
         }
     })
 }
 
-///  Print the block info stats to the console
+/// This function prints statistics to the standard output.
+
+/// The TPS calculation is based on the following assumptions about smart-bench:
+/// - smart-bench instantiates smart contracts on the chain and waits for the completion of these transactions.
+/// - Starting from some future block (after creation), smart-bench uploads transactions related to contract calls to the node.
+/// - Sending contract call transactions to the node is continuous once started and is not mixed with any other type of transactions.
+/// - Smart-bench finishes benchmarking at the block that contains the last contract call from the set.
+
+/// TPS calculation is exclusively concerned with contract calls, disregarding any system or contract-creating transactions.
+
+/// TPS calculation excludes the last block of the benchmark, as its full utilization is not guaranteed. In other words, only blocks in the middle will consist entirely of contract calls.
 pub async fn print_block_info(
     block_info: impl TryStream<Ok = BlockInfo, Error = color_eyre::Report>,
 ) -> color_eyre::Result<()> {
-    let mut total_extrinsics = 0u64;
-    let mut total_blocks = 0u64;
+    let mut call_extrinsics_per_block: Vec<u64> = Vec::new();
+    let mut call_block_expected = false;
     let mut time_stamp = None;
     let mut time_diff = None;
     println!();
     block_info
         .try_for_each(|block| {
             println!("{}", block.stats);
-            total_extrinsics += block.stats.num_extrinsics;
-            total_blocks += 1;
+            let contract_calls_count = block.contract_call_hashes.len() as u64;
+            // Skip blocks at the beggining until we see first call related transaction
+            // Once first call is seen, we expect all further blocks to contain calls until all calls are covered
+            if !call_block_expected && contract_calls_count > 0 {
+                call_block_expected = true;
+            }
+            if call_block_expected {
+                call_extrinsics_per_block.push(contract_calls_count);
+            }
+
             if time_diff.is_none() {
                 if let Some(ts) = time_stamp {
                     time_diff = Some((block.time_stamp - ts) as f64 / 1000.0)
@@ -79,20 +97,33 @@ pub async fn print_block_info(
             future::ready(Ok(()))
         })
         .await?;
+
+    // Skip the last block as it's not stressed to its full capabilities, 
+    // since there is a very low chance of hitting that exact amount of transactions 
+    // (it will contain as many transactions as there are left to execute).
+    let call_extrinsics_per_block =
+        &call_extrinsics_per_block[0..call_extrinsics_per_block.len() - 1];
+
+    let tps_blocks = call_extrinsics_per_block.len();
+    let tps_total_extrinsics = call_extrinsics_per_block.iter().sum::<u64>();
     println!("\nSummary:");
-    println!("Total Blocks: {total_blocks}");
-    println!("Total Extrinsics: {total_extrinsics}");
+    println!("Total Blocks: {tps_blocks}");
+    println!("Total Extrinsics: {tps_total_extrinsics}");
     let diff = time_diff.unwrap_or_else(|| {
         // default block build time
         let default = 12.0;
         println!("Warning: Could not calculate block build time, assuming {default}");
         default
     });
     println!("Block Build Time: {diff}");
-    println!("sTPS - Standard Transaction per Second");
-    println!(
-        "sTPS: {}",
-        total_extrinsics as f64 / (total_blocks as f64 * diff)
-    );
+    if tps_blocks > 0 {
+        println!("sTPS - Standard Transaction Per Second");
+        println!(
+            "sTPS: {:.2}",
+            tps_total_extrinsics as f64 / (tps_blocks as f64 * diff)
+        );
+    } else {
+        println!("sTPS - Error - not enough data to calculate sTPS, consider increasing --call-count value")
+    }
     Ok(())
 }