Bank::check_reserved_keys (#3100)

core/src/banking_stage/consumer.rs

@@ -440,35 +440,29 @@ impl Consumer {
         // This means that the transaction may cross and epoch boundary (not allowed),
         //  or account lookup tables may have been closed.
         let pre_results = txs.iter().zip(max_ages).map(|(tx, max_age)| {
+            // If the transaction was sanitized before this bank's epoch,
+            // additional checks are necessary.
             if bank.slot() > max_age.epoch_invalidation_slot {
-                // Epoch has rolled over. Need to fully re-verify the transaction.
-                // Pre-compiles are verified here.
-                // Attempt re-sanitization after epoch-cross.
-                // Re-sanitized transaction should be equal to the original transaction,
-                // but whether it will pass sanitization needs to be checked.
-                let resanitized_tx =
-                    bank.fully_verify_transaction(tx.to_versioned_transaction())?;
-                if resanitized_tx != *tx {
-                    // Sanitization before/after epoch give different transaction data - do not execute.
-                    return Err(TransactionError::ResanitizationNeeded);
-                }
-            } else {
-                if bank.slot() > max_age.alt_invalidation_slot {
-                    // The address table lookup **may** have expired, but the
-                    // expiration is not guaranteed since there may have been
-                    // skipped slot.
-                    // If the addresses still resolve here, then the transaction is still
-                    // valid, and we can continue with processing.
-                    // If they do not, then the ATL has expired and the transaction
-                    // can be dropped.
-                    let (_addresses, _deactivation_slot) =
-                        bank.load_addresses_from_ref(tx.message_address_table_lookups())?;
-                }
+                // Reserved key set may have cahnged, so we must verify that
+                // no writable keys are reserved.
+                bank.check_reserved_keys(tx)?;
+            }
 
-                // Verify pre-compiles.
-                if !move_precompile_verification_to_svm {
-                    verify_precompiles(tx, &bank.feature_set)?;
-                }
+            if bank.slot() > max_age.alt_invalidation_slot {
+                // The address table lookup **may** have expired, but the
+                // expiration is not guaranteed since there may have been
+                // skipped slot.
+                // If the addresses still resolve here, then the transaction is still
+                // valid, and we can continue with processing.
+                // If they do not, then the ATL has expired and the transaction
+                // can be dropped.
+                let (_addresses, _deactivation_slot) =
+                    bank.load_addresses_from_ref(tx.message_address_table_lookups())?;
+            }
+
+            // Verify pre-compiles.
+            if !move_precompile_verification_to_svm {
+                verify_precompiles(tx, &bank.feature_set)?;
             }
 
             Ok(())

runtime/src/bank.rs

@@ -5870,6 +5870,22 @@ impl Bank {
         self.verify_transaction(tx, TransactionVerificationMode::FullVerification)
     }
 
+    /// Checks if the transaction violates the bank's reserved keys.
+    /// This needs to be checked upon epoch boundary crosses because the
+    /// reserved key set may have changed since the initial sanitization.
+    pub fn check_reserved_keys(&self, tx: &impl SVMMessage) -> Result<()> {
+        // Check keys against the reserved set - these failures simply require us
+        // to re-sanitize the transaction. We do not need to drop the transaction.
+        let reserved_keys = self.get_reserved_account_keys();
+        for (index, key) in tx.account_keys().iter().enumerate() {
+            if tx.is_writable(index) && reserved_keys.contains(key) {
+                return Err(TransactionError::ResanitizationNeeded);
+            }
+        }
+
+        Ok(())
+    }
+
     /// only called from ledger-tool or tests
     fn calculate_capitalization(&self, debug_verify: bool) -> u64 {
         let is_startup = true;

runtime/src/bank/tests.rs

@@ -10046,6 +10046,31 @@ fn test_verify_transactions_packet_data_size() {
     }
 }
 
+#[test]
+fn test_check_reserved_keys() {
+    let (genesis_config, _mint_keypair) = create_genesis_config(1);
+    let bank = Bank::new_for_tests(&genesis_config);
+    let mut bank = Bank::new_from_parent(Arc::new(bank), &Pubkey::new_unique(), 1);
+
+    let transaction =
+        SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
+            &Keypair::new(),
+            &Pubkey::new_unique(),
+            1,
+            genesis_config.hash(),
+        ));
+
+    assert_eq!(bank.check_reserved_keys(&transaction), Ok(()));
+
+    Arc::make_mut(&mut bank.reserved_account_keys)
+        .active
+        .insert(transaction.account_keys()[1]);
+    assert_eq!(
+        bank.check_reserved_keys(&transaction),
+        Err(TransactionError::ResanitizationNeeded)
+    );
+}
+
 #[test]
 fn test_call_precomiled_program() {
     let GenesisConfigInfo {