Advance test-vectors commit in SVM conformance (#3898)

Advance test-vectors commit

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))

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;
-}
+  fixed64 slot = 1;
+}

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;
-}
+  FixtureMetadata metadata = 1;
+  InstrContext input = 2;
+  InstrEffects output = 3;
+}

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;
+}

svm-conformance/proto/sysvar.proto

@@ -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;
-}

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;
-}
+  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;
+}

svm/src/transaction_processor.rs

@@ -1196,6 +1196,11 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             .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<SysvarCache> {
+        &self.sysvar_cache
+    }
 }
 
 #[cfg(test)]

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,

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<u8>,
     filename: OsString,
 ) {
-    let idx_map: HashMap<Pubkey, usize> = 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<Pubkey, Vec<usize>> = 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<String> = 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<String> {
+    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
+}

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<SanitizedTransaction, TransactionError> {
         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<InnerInstruction> {
-        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;