checkpoint

svm/src/account_loader.rs

@@ -97,6 +97,11 @@ pub struct FeesOnlyTransaction {
     pub fee_details: FeeDetails,
 }
 
+// XXX im going to go insane how the fuck is this supposed to work
+// we cant let the program cache preempt the account cache if accounts are closed
+// that means we want to go with starrys idea of creating a new program cache for every tx
+// so we want to impl the trait for
+
 #[cfg_attr(feature = "dev-context-only-utils", derive(Clone))]
 pub(crate) struct AccountLoader<'a, CB: TransactionProcessingCallback> {
     pub(crate) program_cache: ProgramCacheForTxBatch,
@@ -131,11 +136,33 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
             .account_overrides
             .and_then(|overrides| overrides.get(account_key))
         {
-            Some(LoadedTransactionAccount {
-                loaded_size: account_override.data().len(),
-                account: account_override.clone(),
-                rent_collected: 0,
-            })
+            // Ensure account overrides preempt program cache. This is only for
+            // abundance of caution: account overrides is an overly general mechanism
+            // to replace a specific sysvar during simulation. It may be removed in
+            // the future.
+            if let Some(cache_item) = self.account_cache.get(account_key) {
+                Some(LoadedTransactionAccount {
+                    loaded_size: cache_item.account.data().len(),
+                    account: cache_item.account.clone(),
+                    rent_collected: 0,
+                })
+            } else {
+                // Account overrides never need to be inspected. It would be a grave error for
+                // the runtime to pass a writable override for a non-simulated transaction.
+                self.account_cache.insert(
+                    *account_key,
+                    AccountCacheItem {
+                        account: account_override.clone(),
+                        inspected_as_writable: true,
+                    },
+                );
+
+                Some(LoadedTransactionAccount {
+                    loaded_size: account_override.data().len(),
+                    account: account_override.clone(),
+                    rent_collected: 0,
+                })
+            }
         } else if let Some(program) = is_invisible_read
             .then_some(())
             .and_then(|_| self.program_cache.find(account_key))
@@ -148,22 +175,13 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
                 account: account_shared_data_from_program(&program),
                 rent_collected: 0,
             })
-        } else if let Some(account) = self.callbacks.get_account_shared_data(account_key) {
-            // Inspect the account prior to collecting rent, since
-            // rent collection can modify the account.
-            self.callbacks
-                .inspect_account(account_key, AccountState::Alive(&account), is_writable);
-
-            Some(LoadedTransactionAccount {
-                loaded_size: account.data().len(),
-                account,
-                rent_collected: 0,
-            })
         } else {
-            self.callbacks
-                .inspect_account(account_key, AccountState::Dead, is_writable);
-
-            None
+            self.get_and_inspect_account_shared_data(account_key, is_writable)
+                .map(|account| LoadedTransactionAccount {
+                    loaded_size: account.data().len(),
+                    account,
+                    rent_collected: 0,
+                })
         }
     }
 
@@ -211,6 +229,49 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
         }
     }
 
+    fn get_and_inspect_account_shared_data(
+        &mut self,
+        account_key: &Pubkey,
+        is_writable: bool,
+    ) -> Option<AccountSharedData> {
+        if let Some(cache_item) = self.account_cache.get_mut(account_key) {
+            if is_writable && !cache_item.inspected_as_writable {
+                // Inspecting a read-only account is a no-op. But inspecting a writable
+                // account must be done before rent collection, and must never be done
+                // again in-batch, because inspection is intended to preserve the
+                // state prior to any writes.
+                self.callbacks.inspect_account(
+                    account_key,
+                    AccountState::Alive(&cache_item.account),
+                    is_writable,
+                );
+                cache_item.inspected_as_writable = cache_item.inspected_as_writable || is_writable;
+            }
+
+            Some(cache_item.account.clone())
+        } else if let Some(account) = self.callbacks.get_account_shared_data(account_key) {
+            // Inspect the account prior to collecting rent, since
+            // rent collection can modify the account.
+            self.callbacks
+                .inspect_account(account_key, AccountState::Alive(&account), is_writable);
+
+            self.account_cache.insert(
+                *account_key,
+                AccountCacheItem {
+                    account: account.clone(),
+                    inspected_as_writable: is_writable,
+                },
+            );
+
+            Some(account)
+        } else {
+            self.callbacks
+                .inspect_account(account_key, AccountState::Dead, is_writable);
+
+            None
+        }
+    }
+
     fn update_accounts_for_successful_tx(
         &mut self,
         message: &impl SVMMessage,