From a1c41f4ba61813148f7c76b83b92b3193d207606 Mon Sep 17 00:00:00 2001 From: Kevin Heavey Date: Sat, 23 Mar 2024 17:35:23 +0400 Subject: [PATCH] Include sigverify in the BanksClient benchmark to make it apples-to-apples (#30) * add test to confirm that process_transaction_with_metadata skips sigverify * add .verify() and .verify_precompiles() to banks client benchmark to make it apples-to-apples * put get_feature_set behind a feature * add bench.sh --- Cargo.toml | 4 ++ bench.sh | 2 + benches/banks_client_comparison.rs | 19 +++++-- src/bank.rs | 5 ++ tests/counter_test.rs | 83 +++++++++++++++++++++++++++++- 5 files changed, 107 insertions(+), 6 deletions(-) create mode 100755 bench.sh diff --git a/Cargo.toml b/Cargo.toml index 2689bc9..e733d41 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,8 +22,12 @@ solana-program-test = "=1.17.18" criterion = "0.5" tokio = "1.35" +[features] +internal-test = [] + [[bench]] name = "banks_client_comparison" +required-features = ["internal-test"] harness = false [[bench]] diff --git a/bench.sh b/bench.sh new file mode 100755 index 0000000..15d2feb --- /dev/null +++ b/bench.sh @@ -0,0 +1,2 @@ +#!/usr/bin/bash +RUST_LOG= cargo bench --features internal-test diff --git a/benches/banks_client_comparison.rs b/benches/banks_client_comparison.rs index bafaa78..379b8ef 100644 --- a/benches/banks_client_comparison.rs +++ b/benches/banks_client_comparison.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::{path::PathBuf, sync::Arc}; use criterion::{criterion_group, criterion_main, Criterion}; use litesvm::LiteSVM; @@ -8,8 +8,8 @@ use solana_program::{ rent::Rent, }; use solana_sdk::{ - account::Account, message::Message, signature::Keypair, signer::Signer, - transaction::Transaction, + account::Account, feature_set::FeatureSet, message::Message, signature::Keypair, + signer::Signer, transaction::Transaction, }; fn read_counter_program() -> Vec { @@ -53,7 +53,11 @@ fn make_tx( Transaction::new(&[payer_kp], msg, blockhash) } -async fn do_program_test(program_id: Pubkey, counter_address: Pubkey) { +async fn do_program_test( + program_id: Pubkey, + counter_address: Pubkey, + feature_set: Arc, +) { let mut pt = solana_program_test::ProgramTest::default(); add_program(&read_counter_program(), program_id, &mut pt); let mut ctx = pt.start_with_context().await; @@ -68,6 +72,10 @@ async fn do_program_test(program_id: Pubkey, counter_address: Pubkey) { &ctx.payer, deduper, ); + // We verify the transaction to align the benchmark + // as LiteSVM also verifies the transaction by default. + tx.verify().unwrap(); + tx.verify_precompiles(&feature_set).unwrap(); let tx_res = ctx .banks_client .process_transaction_with_metadata(tx) @@ -93,6 +101,7 @@ fn criterion_benchmark(c: &mut Criterion) { svm.store_program(program_id, &read_counter_program()); svm.airdrop(&payer_pk, 1000000000).unwrap(); + let feature_set = svm.get_feature_set(); let counter_address = Pubkey::new_unique(); let mut group = c.benchmark_group("comparison"); group.bench_function("litesvm_bench", |b| { @@ -121,7 +130,7 @@ fn criterion_benchmark(c: &mut Criterion) { b.iter(|| { let rt = tokio::runtime::Runtime::new().unwrap(); rt.block_on(async { - do_program_test(program_id, counter_address).await; + do_program_test(program_id, counter_address, feature_set.clone()).await; }); }) }); diff --git a/src/bank.rs b/src/bank.rs index 9d673bb..dec8c27 100644 --- a/src/bank.rs +++ b/src/bank.rs @@ -517,4 +517,9 @@ impl LiteSVM { self.block_height = slot; self.accounts.programs_cache.set_slot_for_tests(slot); } + + #[cfg(feature = "internal-test")] + pub fn get_feature_set(&self) -> Arc { + self.feature_set.clone() + } } diff --git a/tests/counter_test.rs b/tests/counter_test.rs index 9014853..e8674f2 100644 --- a/tests/counter_test.rs +++ b/tests/counter_test.rs @@ -7,7 +7,12 @@ use solana_program::{ pubkey::Pubkey, rent::Rent, }; -use solana_sdk::{account::Account, signature::Keypair, signer::Signer, transaction::Transaction}; +use solana_sdk::{ + account::Account, + signature::{Keypair, Signature}, + signer::Signer, + transaction::Transaction, +}; const NUM_GREETINGS: u8 = 255; @@ -155,3 +160,79 @@ fn banks_client_test() { let rt = tokio::runtime::Runtime::new().unwrap(); rt.block_on(async { do_program_test(program_id, counter_address).await }); } + +fn make_tx_wrong_signature( + program_id: Pubkey, + counter_address: Pubkey, + payer_pk: &Pubkey, + blockhash: solana_program::hash::Hash, + payer_kp: &Keypair, +) -> Transaction { + let msg = Message::new_with_blockhash( + &[Instruction { + program_id, + accounts: vec![AccountMeta::new(counter_address, false)], + data: vec![0, 0], + }], + Some(payer_pk), + &blockhash, + ); + let mut tx = Transaction::new(&[&payer_kp], msg, blockhash); + tx.signatures[0] = Signature::new_unique(); + tx +} + +async fn do_program_test_wrong_signature(program_id: Pubkey, counter_address: Pubkey) { + let mut pt = solana_program_test::ProgramTest::default(); + add_program(&read_counter_program(), program_id, &mut pt); + let mut ctx = pt.start_with_context().await; + ctx.set_account(&counter_address, &counter_acc(program_id).into()); + assert_eq!( + ctx.banks_client + .get_account(counter_address) + .await + .unwrap() + .unwrap() + .data, + 0u32.to_le_bytes().to_vec() + ); + assert!(ctx + .banks_client + .get_account(program_id) + .await + .unwrap() + .is_some()); + + let tx = make_tx_wrong_signature( + program_id, + counter_address, + &ctx.payer.pubkey(), + ctx.last_blockhash, + &ctx.payer, + ); + let tx_res = ctx + .banks_client + .process_transaction_with_metadata(tx) + .await + .unwrap(); + tx_res.result.unwrap(); + let fetched = ctx + .banks_client + .get_account(counter_address) + .await + .unwrap() + .unwrap() + .data[0]; + assert_eq!(fetched, 1); +} + +/// Confirm that process_transaction_with_metadata +/// does not do sigverify. +#[test] +fn test_process_transaction_with_metadata_wrong_signature() { + let program_id = Pubkey::new_unique(); + + let counter_address = Pubkey::new_unique(); + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { do_program_test_wrong_signature(program_id, counter_address).await }); +}