From df27fb3a7386f0d2cc64df81c46a09ab79b18ba0 Mon Sep 17 00:00:00 2001 From: Lucas Ste <38472950+LucasSte@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:09:51 -0300 Subject: [PATCH] Advance test-vectors commit in SVM conformance (#3898) Advance test-vectors commit --- svm-conformance/build.rs | 7 +- svm-conformance/proto/context.proto | 11 +- svm-conformance/proto/invoke.proto | 10 +- svm-conformance/proto/metadata.proto | 7 ++ svm-conformance/proto/sysvar.proto | 32 ----- svm-conformance/proto/txn.proto | 75 +++++++----- svm/src/transaction_processor.rs | 5 + svm/tests/concurrent_tests.rs | 8 +- svm/tests/conformance.rs | 177 +++++++++++++++++++-------- svm/tests/transaction_builder.rs | 15 ++- 10 files changed, 216 insertions(+), 131 deletions(-) create mode 100644 svm-conformance/proto/metadata.proto delete mode 100644 svm-conformance/proto/sysvar.proto diff --git a/svm-conformance/build.rs b/svm-conformance/build.rs index b3fb66548a03b5..09f10dfc4cd342 100644 --- a/svm-conformance/build.rs +++ b/svm-conformance/build.rs @@ -1,6 +1,11 @@ fn main() { let proto_base_path = std::path::PathBuf::from("proto"); - let protos = ["context.proto", "invoke.proto", "sysvar.proto", "txn.proto"]; + let protos = [ + "context.proto", + "invoke.proto", + "metadata.proto", + "txn.proto", + ]; let protos_path: Vec<_> = protos .iter() .map(|name| proto_base_path.join(name)) diff --git a/svm-conformance/proto/context.proto b/svm-conformance/proto/context.proto index cd196a3bcfc843..d13568c6002924 100644 --- a/svm-conformance/proto/context.proto +++ b/svm-conformance/proto/context.proto @@ -1,8 +1,6 @@ syntax = "proto3"; package org.solana.sealevel.v1; -import "sysvar.proto"; - // A set of feature flags. message FeatureSet { // Every item in this list marks an enabled feature. The value of @@ -55,14 +53,9 @@ message EpochContext { FeatureSet features = 1; } - // SlotContext includes context scoped to a block. // On "real" ledgers, it is created during the slot boundary. message SlotContext { - repeated bytes recent_block_hashes = 1; - // public key for the leader - bytes leader = 2; // Slot number - fixed64 slot = 3; - SysvarCache sysvar_cache = 4; -} \ No newline at end of file + fixed64 slot = 1; +} diff --git a/svm-conformance/proto/invoke.proto b/svm-conformance/proto/invoke.proto index 63d0d31985f188..1ea2f383ff3a02 100644 --- a/svm-conformance/proto/invoke.proto +++ b/svm-conformance/proto/invoke.proto @@ -2,7 +2,7 @@ syntax = "proto3"; package org.solana.sealevel.v1; import "context.proto"; -import "txn.proto"; +import "metadata.proto"; message InstrAcct { // Selects an account in an external list @@ -30,7 +30,6 @@ message InstrContext { uint64 cu_avail = 6; - TxnContext txn_context = 7; SlotContext slot_context = 8; EpochContext epoch_context = 9; } @@ -60,6 +59,7 @@ message InstrEffects { // An instruction processing test fixture. message InstrFixture { - InstrContext input = 1; - InstrEffects output = 2; -} \ No newline at end of file + FixtureMetadata metadata = 1; + InstrContext input = 2; + InstrEffects output = 3; +} diff --git a/svm-conformance/proto/metadata.proto b/svm-conformance/proto/metadata.proto new file mode 100644 index 00000000000000..e790c0463d0974 --- /dev/null +++ b/svm-conformance/proto/metadata.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; +package org.solana.sealevel.v1; + +// FixtureMetadata includes the metadata for the fixture +message FixtureMetadata { + string fn_entrypoint = 1; +} diff --git a/svm-conformance/proto/sysvar.proto b/svm-conformance/proto/sysvar.proto deleted file mode 100644 index a0139b61622104..00000000000000 --- a/svm-conformance/proto/sysvar.proto +++ /dev/null @@ -1,32 +0,0 @@ -syntax = "proto3"; -package org.solana.sealevel.v1; - -// The clock account data -message Clock { - uint64 slot = 1; - uint64 epoch_start_timestamp = 2; - uint64 epoch = 3; - uint64 leader_schedule_epoch = 4; - uint64 unix_timestamp = 5; -} - -// The data for the Rent account -message Rent { - uint64 lamports_per_byte_year = 1; - double exemption_threshold = 2; - uint64 burn_percent = 3; -} - -// The recent slot hash vector contents -message SlotHash { - uint64 slot = 1; - bytes hash = 2; -} - -// The sysvar cache for a transaction execution -message SysvarCache { - Clock clock = 1; - Rent rent = 2; - // Slot hashes sysvar: SysvarS1otHashes111111111111111111111111111 - repeated SlotHash slot_hash = 3; -} \ No newline at end of file diff --git a/svm-conformance/proto/txn.proto b/svm-conformance/proto/txn.proto index b71e524fc80926..811f32dfc9ee89 100644 --- a/svm-conformance/proto/txn.proto +++ b/svm-conformance/proto/txn.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package org.solana.sealevel.v1; import "context.proto"; +import "metadata.proto"; // Message header contains the counts of required readonly and signatures message MessageHeader { @@ -10,7 +11,6 @@ message MessageHeader { uint32 num_readonly_unsigned_accounts = 3; } - // The instruction a transaction executes message CompiledInstruction { // Index into the message pubkey array @@ -27,31 +27,27 @@ message MessageAddressTableLookup { repeated uint32 readonly_indexes = 3; } -// Addresses loaded with on-chain lookup tables -message LoadedAddresses { - repeated bytes writable = 1; - repeated bytes readonly = 2; -} - // Message contains the transaction data message TransactionMessage { // Whether this is a legacy message or not bool is_legacy = 1; MessageHeader header = 2; + // Vector of pubkeys repeated bytes account_keys = 3; + // Data associated with the accounts referred above. Not all accounts need to be here. repeated AcctState account_shared_data = 4; - // The block hash contains 32-bytes + + // Recent blockhash provided in message bytes recent_blockhash = 5; + // The instructions this transaction executes repeated CompiledInstruction instructions = 6; // Not available in legacy message repeated MessageAddressTableLookup address_table_lookups = 7; - // Not available in legacy messages - LoadedAddresses loaded_addresses = 8; } // A valid verified transaction @@ -72,10 +68,9 @@ message SanitizedTransaction { message TxnContext { // The transaction data SanitizedTransaction tx = 1; - // The maximum age allowed for this transaction - uint64 max_age = 2; - // The limit of bytes allowed for this transaction to load - uint64 log_messages_byte_limit = 3; + + // Up to 300 (actually 301) most recent blockhashes (ordered from oldest to newest) + repeated bytes blockhash_queue = 3; EpochContext epoch_ctx = 4; SlotContext slot_ctx = 5; @@ -83,33 +78,57 @@ message TxnContext { // The resulting state of an account after a transaction message ResultingState { - AcctState state = 1; - uint64 transaction_rent = 2; - RentDebits rent_debit = 3; + repeated AcctState acct_states = 1; + repeated RentDebits rent_debits = 2; + uint64 transaction_rent = 3; } // The rent state for an account after a transaction message RentDebits { - uint64 rent_collected = 1; - uint64 post_balance = 2; + bytes pubkey = 1; + int64 rent_collected = 2; +} + +message FeeDetails { + uint64 transaction_fee = 1; + uint64 prioritization_fee = 2; } // The execution results for a transaction message TxnResult { // Whether this transaction was executed bool executed = 1; + // Whether there was a sanitization error + bool sanitization_error = 2; // The state of each account after the transaction - repeated ResultingState states = 2; - uint64 rent = 3; + ResultingState resulting_state = 3; + uint64 rent = 4; // If an executed transaction has no error - bool is_ok = 4; + bool is_ok = 5; // The transaction status (error code) - uint32 status = 5; + uint32 status = 6; + // The instruction error, if any + uint32 instruction_error = 7; + // The instruction error index, if any + uint32 instruction_error_index = 8; + // Custom error, if any + uint32 custom_error = 9; + + // The return data from this transaction, if any - bytes return_data = 6; + bytes return_data = 10; // Number of executed compute units - uint64 executed_units = 7; - // The change in accounts data len for this transaction - uint64 accounts_data_len_delta = 8; -} \ No newline at end of file + uint64 executed_units = 11; + // The collected fees in this transaction + FeeDetails fee_details = 12; +} + +// Txn fixtures +message TxnFixture { + FixtureMetadata metadata = 1; + // Context + TxnContext input = 2; + // Effects + TxnResult output = 3; +} diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index fee2678ad1dbb0..bff7560a96d7b0 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1196,6 +1196,11 @@ impl TransactionBatchProcessor { .assign_program(program_id, Arc::new(builtin)); debug!("Added program {} under {:?}", name, program_id); } + + #[cfg(feature = "dev-context-only-utils")] + pub fn writable_sysvar_cache(&self) -> &RwLock { + &self.sysvar_cache + } } #[cfg(test)] diff --git a/svm/tests/concurrent_tests.rs b/svm/tests/concurrent_tests.rs index bcc8d8cc788b51..5725a895d8b3c0 100644 --- a/svm/tests/concurrent_tests.rs +++ b/svm/tests/concurrent_tests.rs @@ -219,8 +219,12 @@ fn svm_concurrent() { vec![0], ); - let sanitized_transaction = - transaction_builder.build(Hash::default(), (fee_payer, Signature::new_unique()), true); + let sanitized_transaction = transaction_builder.build( + Hash::default(), + (fee_payer, Signature::new_unique()), + true, + false, + ); transactions[idx % THREADS].push(sanitized_transaction.unwrap()); check_data[idx % THREADS].push(CheckTxData { fee_payer, diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index 3264c3efc5a861..45fec05a1abb59 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -15,12 +15,15 @@ use { }, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, + clock::Clock, + epoch_schedule::EpochSchedule, hash::Hash, instruction::AccountMeta, message::SanitizedMessage, pubkey::Pubkey, rent::Rent, signature::Signature, + sysvar::{last_restart_slot, SysvarId}, transaction_context::{ ExecutionRecord, IndexOfAccount, InstructionAccount, TransactionAccount, TransactionContext, @@ -30,10 +33,10 @@ use { program_loader, transaction_processing_callback::TransactionProcessingCallback, transaction_processor::TransactionBatchProcessor, }, - solana_svm_conformance::proto::{InstrEffects, InstrFixture}, + solana_svm_conformance::proto::{AcctState, InstrEffects, InstrFixture}, solana_timings::ExecuteTimings, std::{ - collections::HashMap, + collections::{hash_map::Entry, HashMap}, env, ffi::OsString, fs::{self, File}, @@ -82,10 +85,10 @@ fn setup() -> PathBuf { .status() .expect("Failed to download test-vectors"); - std::println!("Checking out commit 90a8ad069f8a07d2fdad3cf03b3fb486a00fe988"); + std::println!("Checking out commit 4abb2046cf51efe809498f4fd717023684050d2f"); Command::new("git") .current_dir(&dir) - .args(["checkout", "90a8ad069f8a07d2fdad3cf03b3fb486a00fe988"]) + .args(["checkout", "4abb2046cf51efe809498f4fd717023684050d2f"]) .status() .expect("Failed to checkout to proper test-vector commit"); @@ -156,7 +159,6 @@ fn run_fixture(fixture: InstrFixture, filename: OsString) { is_signer: item.is_signer, is_writable: item.is_writable, }); - if item.is_signer { signatures.insert(pubkey, Signature::new_unique()); } @@ -182,7 +184,9 @@ fn run_fixture(fixture: InstrFixture, filename: OsString) { let mut account_data = AccountSharedData::default(); account_data.set_lamports(item.lamports); account_data.set_data(item.data); - account_data.set_owner(Pubkey::new_from_array(item.owner.try_into().unwrap())); + account_data.set_owner(Pubkey::new_from_array( + item.owner.clone().try_into().unwrap(), + )); account_data.set_executable(item.executable); account_data.set_rent_epoch(item.rent_epoch); @@ -198,9 +202,12 @@ fn run_fixture(fixture: InstrFixture, filename: OsString) { account_data_map.insert(fee_payer, account_data); } - let Ok(transaction) = - transaction_builder.build(Hash::default(), (fee_payer, Signature::new_unique()), false) - else { + let Ok(transaction) = transaction_builder.build( + Hash::default(), + (fee_payer, Signature::new_unique()), + false, + true, + ) else { // If we can't build a sanitized transaction, // the output must be a failed instruction as well assert_ne!(output.result, 0); @@ -228,7 +235,34 @@ fn run_fixture(fixture: InstrFixture, filename: OsString) { None, ); - batch_processor.fill_missing_sysvar_cache_entries(&mock_bank); + batch_processor + .writable_sysvar_cache() + .write() + .unwrap() + .fill_missing_entries(|pubkey, callbackback| { + if let Some(account) = mock_bank.get_account_shared_data(pubkey) { + if account.lamports() > 0 { + callbackback(account.data()); + return; + } + } + + if *pubkey == Clock::id() { + let default_clock = Clock { + slot: 10, + ..Default::default() + }; + let clock_data = bincode::serialize(&default_clock).unwrap(); + callbackback(&clock_data); + } else if *pubkey == EpochSchedule::id() { + callbackback(&bincode::serialize(&EpochSchedule::default()).unwrap()); + } else if *pubkey == Rent::id() { + callbackback(&bincode::serialize(&Rent::default()).unwrap()); + } else if *pubkey == last_restart_slot::id() { + let slot_val = 5000_u64; + callbackback(&bincode::serialize(&slot_val).unwrap()); + } + }); execute_fixture_as_instr( &mock_bank, @@ -268,6 +302,7 @@ fn execute_fixture_as_instr( compute_budget.max_instruction_stack_depth, compute_budget.max_instruction_trace_length, ); + transaction_context.set_remove_accounts_executable_flag_checks(false); let mut loaded_programs = ProgramCacheForTxBatch::new( 42, @@ -407,53 +442,43 @@ fn verify_accounts_and_data( return_data: &Vec, filename: OsString, ) { - let idx_map: HashMap = accounts - .iter() - .enumerate() - .map(|(idx, item)| (item.0, idx)) - .collect(); + // The input created by firedancer is malformed in that there may be repeated accounts in the + // instruction execution output. This happens because the set system program as the program ID, + // as pass it as an account to be modified in the instruction. + let mut idx_map: HashMap> = HashMap::new(); + for (idx, item) in accounts.iter().enumerate() { + match idx_map.entry(item.0) { + Entry::Occupied(mut this) => { + this.get_mut().push(idx); + } + Entry::Vacant(this) => { + this.insert(vec![idx]); + } + } + } for item in &output.modified_accounts { let pubkey = Pubkey::new_from_array(item.address.clone().try_into().unwrap()); - let index = *idx_map + let indexes = *idx_map .get(&pubkey) + .as_ref() .expect("Account not in expected results"); - let received_data = &accounts[index].1; + let mut error: Option = Some("err".to_string()); + for idx in indexes { + let received_data = &accounts[*idx].1; + let check_result = check_account(received_data, item, &filename); - assert_eq!( - received_data.lamports(), - item.lamports, - "Lamports differ in case: {:?}", - filename - ); - assert_eq!( - received_data.data(), - item.data.as_slice(), - "Account data differs in case: {:?}", - filename - ); - assert_eq!( - received_data.owner(), - &Pubkey::new_from_array(item.owner.clone().try_into().unwrap()), - "Account owner differs in case: {:?}", - filename - ); - assert_eq!( - received_data.executable(), - item.executable, - "Executable boolean differs in case: {:?}", - filename - ); + if error.is_some() && check_result.is_none() { + // If at least one of the accounts pass the check, we have no error. + error = None; + } else if error.is_some() && check_result.is_some() { + error = check_result; + } + } - // u64::MAX means we are not considering the epoch - if item.rent_epoch != u64::MAX && received_data.rent_epoch() != u64::MAX { - assert_eq!( - received_data.rent_epoch(), - item.rent_epoch, - "Rent epoch differs in case: {:?}", - filename - ); + if let Some(error) = error { + panic!("{}", error); } } @@ -470,3 +495,57 @@ fn verify_accounts_and_data( assert_eq!(&output.return_data, return_data); } } + +fn check_account( + received: &AccountSharedData, + expected: &AcctState, + filename: &OsString, +) -> Option { + macro_rules! format_args { + ($received:expr, $expected:expr) => { + format!("received: {:?}\nexpected: {:?}", $received, $expected).as_str() + }; + } + + if received.lamports() != expected.lamports { + return Some( + format!("Lamports differ in case: {:?}\n", filename) + + format_args!(received.lamports(), expected.lamports), + ); + } + + if received.data() != expected.data.as_slice() { + return Some( + format!("Account data differs in case: {:?}\n", filename) + + format_args!(received.data(), expected.data.as_slice()), + ); + } + + let expected_owner = Pubkey::new_from_array(expected.owner.clone().try_into().unwrap()); + if received.owner() != &expected_owner { + return Some( + format!("Account owner differs in case: {:?}\n", filename) + + format_args!(received.owner(), expected_owner), + ); + } + + if received.executable() != expected.executable { + return Some( + format!("Executable boolean differs in case: {:?}\n", filename) + + format_args!(received.executable(), expected.executable), + ); + } + + // u64::MAX means we are not considering the epoch + if received.rent_epoch() != u64::MAX + && expected.rent_epoch != u64::MAX + && received.rent_epoch() != expected.rent_epoch + { + return Some( + format!("Rent epoch differs in case: {:?}\n", filename) + + format_args!(received.rent_epoch(), expected.rent_epoch), + ); + } + + None +} diff --git a/svm/tests/transaction_builder.rs b/svm/tests/transaction_builder.rs index 664ea6237120cb..b3b1757cfa6abd 100644 --- a/svm/tests/transaction_builder.rs +++ b/svm/tests/transaction_builder.rs @@ -14,7 +14,7 @@ use { VersionedTransaction, }, }, - std::collections::HashMap, + std::collections::{HashMap, HashSet}, }; #[derive(Default)] @@ -112,6 +112,7 @@ impl SanitizedTransactionBuilder { block_hash: Hash, fee_payer: (Pubkey, Signature), v0_message: bool, + ignore_reserved_accounts: bool, ) -> Result { let mut account_keys = Vec::with_capacity( self.signed_mutable_accounts @@ -214,19 +215,23 @@ impl SanitizedTransactionBuilder { let loader = MockLoader {}; + let reserved_active = &ReservedAccountKeys::new_all_activated().active; + let all_inactive = HashSet::new(); SanitizedTransaction::try_new( sanitized_versioned_transaction, Hash::new_unique(), false, loader, - &ReservedAccountKeys::new_all_activated().active, + if ignore_reserved_accounts { + &all_inactive + } else { + reserved_active + }, ) } fn clean_up(&mut self) -> Vec { - let mut instructions = Vec::new(); - - std::mem::swap(&mut instructions, &mut self.instructions); + let instructions = std::mem::take(&mut self.instructions); self.num_required_signatures = 0; self.num_readonly_signed_accounts = 0; self.num_readonly_unsigned_accounts = 0;