fix: better transaction diagnostics & rent exemption check

magicblock-aperture/src/tests.rs

@@ -27,6 +27,8 @@ use crate::{
     EventProcessor,
 };
 
+const LAMPORTS_PER_SOL: u64 = 1_000_000_000;
+
 /// A test helper to create a unique WebSocket connection channel pair.
 fn ws_channel() -> (WsConnectionChannel, Receiver<Bytes>) {
     static CHAN_ID: AtomicU32 = AtomicU32::new(0);
@@ -100,7 +102,9 @@ mod event_processor {
     #[tokio::test]
     async fn test_account_update() {
         let (state, env) = setup();
-        let acc = env.create_account_with_config(1, 1, guinea::ID).pubkey();
+        let acc = env
+            .create_account_with_config(LAMPORTS_PER_SOL, 1, guinea::ID)
+            .pubkey();
         let (tx, mut rx) = ws_channel();
 
         // Subscribe to both the specific account and the program that owns it.
@@ -140,7 +144,9 @@ mod event_processor {
     #[tokio::test]
     async fn test_transaction_update() {
         let (state, env) = setup();
-        let acc = env.create_account_with_config(1, 42, guinea::ID).pubkey();
+        let acc = env
+            .create_account_with_config(LAMPORTS_PER_SOL, 42, guinea::ID)
+            .pubkey();
         let (tx, mut rx) = ws_channel();
 
         let ix = Instruction::new_with_bincode(
@@ -201,7 +207,9 @@ mod event_processor {
         let (state, env) = setup();
 
         // Test multiple subscriptions to the same ACCOUNT.
-        let acc1 = env.create_account_with_config(1, 1, guinea::ID).pubkey();
+        let acc1 = env
+            .create_account_with_config(LAMPORTS_PER_SOL, 1, guinea::ID)
+            .pubkey();
         let (acc_tx1, mut acc_rx1) = ws_channel();
         let (acc_tx2, mut acc_rx2) = ws_channel();
 
@@ -227,7 +235,9 @@ mod event_processor {
         assert_receives_update(&mut acc_rx2, "second account subscriber").await;
 
         // Test multiple subscriptions to the same PROGRAM.
-        let acc2 = env.create_account_with_config(1, 1, guinea::ID).pubkey();
+        let acc2 = env
+            .create_account_with_config(LAMPORTS_PER_SOL, 1, guinea::ID)
+            .pubkey();
         let (prog_tx1, mut prog_rx1) = ws_channel();
         let (prog_tx2, mut prog_rx2) = ws_channel();
         let prog_encoder = ProgramAccountEncoder {

magicblock-api/src/magic_validator.rs

@@ -157,7 +157,6 @@ impl MagicValidator {
         let GenesisConfigInfo {
             genesis_config,
             validator_pubkey,
-            ..
         } = create_genesis_config_with_leader(
             u64::MAX,
             &validator_pubkey,
@@ -274,7 +273,7 @@ impl MagicValidator {
             });
 
         validator::init_validator_authority(identity_keypair);
-
+        let base_fee = config.validator.base_fees.unwrap_or_default();
         let txn_scheduler_state = TransactionSchedulerState {
             accountsdb: accountsdb.clone(),
             ledger: ledger.clone(),
@@ -303,7 +302,7 @@ impl MagicValidator {
         let node_context = NodeContext {
             identity: validator_pubkey,
             faucet,
-            base_fee: config.validator.base_fees.unwrap_or_default(),
+            base_fee,
             featureset: txn_scheduler_state.environment.feature_set.clone(),
         };
         let transaction_scheduler =

magicblock-config/src/validator.rs

@@ -35,7 +35,6 @@ pub struct ValidatorConfig {
     #[derive_env_var]
     #[clap_from_serde_skip] // Skip because it defaults to None
     #[arg(help = "The base fees to use for the validator.")]
-    #[serde(default = "default_base_fees")]
     pub base_fees: Option<u64>,
 
     /// Uses alpha2 country codes following https://en.wikipedia.org/wiki/ISO_3166-1
@@ -65,7 +64,7 @@ impl Default for ValidatorConfig {
             millis_per_slot: default_millis_per_slot(),
             sigverify: default_sigverify(),
             fqdn: default_fqdn(),
-            base_fees: default_base_fees(),
+            base_fees: None,
             country_code: default_country_code(),
             claim_fees_interval_secs: default_claim_fees_interval_secs(),
         }
@@ -84,10 +83,6 @@ fn default_fqdn() -> Option<String> {
     None
 }
 
-fn default_base_fees() -> Option<u64> {
-    None
-}
-
 fn default_country_code() -> CountryCode {
     CountryCode::for_alpha2("US").unwrap()
 }

magicblock-processor/Cargo.toml

@@ -20,14 +20,14 @@ magicblock-metrics = { workspace = true }
 magicblock-program = { workspace = true }
 
 solana-account = { workspace = true }
+solana-address-lookup-table-program = { workspace = true }
 solana-bpf-loader-program = { workspace = true }
 solana-compute-budget-program = { workspace = true }
 solana-feature-set = { workspace = true }
 solana-fee = { workspace = true }
 solana-fee-structure = { workspace = true }
-solana-address-lookup-table-program = { workspace = true }
-solana-program = { workspace = true }
 solana-loader-v4-program = { workspace = true }
+solana-program = { workspace = true }
 solana-program-runtime = { workspace = true }
 solana-pubkey = { workspace = true }
 solana-rent-collector = { workspace = true }
@@ -36,8 +36,8 @@ solana-svm = { workspace = true }
 solana-svm-transaction = { workspace = true }
 solana-system-program = { workspace = true }
 solana-transaction = { workspace = true }
-solana-transaction-status = { workspace = true }
 solana-transaction-error = { workspace = true }
+solana-transaction-status = { workspace = true }
 
 [dev-dependencies]
 guinea = { workspace = true }

magicblock-processor/src/executor/mod.rs

@@ -57,8 +57,10 @@ pub(super) struct TransactionExecutor {
     /// A read lock held during a slot's processing to synchronize with critical global
     /// operations like `AccountsDb` snapshots.
     sync: StWLock,
+    /// Hacky temporary solution to allow automatic airdrops, the flag
+    /// is tightly contolled and will be removed in the nearest future
     /// True when auto airdrop for fee payers is enabled (auto_airdrop_lamports > 0).
-    pub is_auto_airdrop_lamports_enabled: bool,
+    is_auto_airdrop_lamports_enabled: bool,
 }
 
 impl TransactionExecutor {

magicblock-processor/src/executor/processing.rs

@@ -61,28 +61,31 @@ impl super::TransactionExecutor {
                 self.commit_failed_transaction(txn, status.clone());
                 FAILED_TRANSACTIONS_COUNT.inc();
                 tx.map(|tx| tx.send(status));
+                // NOTE:
+                // Transactions that failed to load, cannot have touched the thread
+                // local storage, thus there's no need to clear it before returning
                 return;
             }
         };
 
         // The transaction has been processed, we can commit the account state changes
-        // Failed transactions still pay fees, so we need to commit the accounts even if the transaction failed
+        // NOTE:
+        // Failed transactions still pay fees, so we need to
+        // commit the accounts even if the transaction failed
         let feepayer = *txn.fee_payer();
         self.commit_accounts(feepayer, &processed, is_replay);
 
         let result = processed.status();
-        if result.is_ok() {
+        if result.is_ok() && !is_replay {
             // If the transaction succeeded, check for potential tasks
             // that may have been scheduled during the transaction execution
             // TODO: send intents here as well once implemented
-            if !is_replay {
-                while let Some(task) = ExecutionTlsStash::next_task() {
-                    // This is a best effort send, if the tasks service has terminated
-                    // for some reason, logging is the best we can do at this point
-                    let _ = self.tasks_tx.send(task).inspect_err(|_|
-                            error!("Scheduled tasks service has hung up and is no longer running")
-                        );
-                }
+            while let Some(task) = ExecutionTlsStash::next_task() {
+                // This is a best effort send, if the tasks service has terminated
+                // for some reason, logging is the best we can do at this point
+                let _ = self.tasks_tx.send(task).inspect_err(|_|
+                    error!("Scheduled tasks service has hung up and is no longer running")
+                );
             }
         }
 
@@ -109,9 +112,6 @@ impl super::TransactionExecutor {
         transaction: [SanitizedTransaction; 1],
         tx: TxnSimulationResultTx,
     ) {
-        // Defensively clear any stale data from previous calls
-        ExecutionTlsStash::clear();
-
         let (result, _) = self.process(&transaction);
         let result = match result {
             Ok(processed) => {
@@ -171,44 +171,11 @@ impl super::TransactionExecutor {
         let mut result = output.processing_results.pop().expect(
             "single transaction result is always present in the output",
         );
-
-        let gasless = self.environment.fee_lamports_per_signature == 0;
-        // If we are running in the gasless mode, we should not allow
-        // any mutation of the feepayer account, since that would make
-        // it possible for malicious actors to perform transfer operations
-        // from undelegated feepayers to delegated accounts, which would
-        // result in validator losing funds upon balance settling.
-        if gasless {
-            let undelegated_feepayer_was_modified = result
-                .as_ref()
-                .ok()
-                .and_then(|r| r.executed_transaction())
-                .and_then(|txn| {
-                    let first_acc = txn.loaded_transaction.accounts.first();
-                    let rollback_lamports = rollback_feepayer_lamports(
-                        &txn.loaded_transaction.rollback_accounts,
-                    );
-                    first_acc.map(|acc| (acc, rollback_lamports))
-                })
-                .map(|(acc, rollback_lamports)| {
-                    (acc.1.is_dirty()
-                        && (acc.1.lamports() != 0 || rollback_lamports != 0))
-                        && !acc.1.delegated()
-                        && !acc.1.privileged()
-                })
-                .unwrap_or(false);
-
-            if undelegated_feepayer_was_modified
-                && !self.is_auto_airdrop_lamports_enabled
-            {
-                if let Ok(ProcessedTransaction::Executed(ref mut executed)) =
-                    &mut result
-                {
-                    executed.execution_details.status =
-                        Err(TransactionError::InvalidAccountForFee)
-                }
-            }
+        // Verify that account state invariants haven't been violated
+        if let Ok(ref mut processed) = result {
+            self.verify_account_states(processed);
         }
+
         (result, output.balances)
     }
 
@@ -368,9 +335,82 @@ impl super::TransactionExecutor {
             let _ = self.accounts_tx.send(account);
         }
     }
+
+    /// Ensure that no post execution account state violations occurred:
+    /// 1. No modification of the non-delegated feepayer in gasless mode
+    /// 2. No illegal account resizing when the balance is zero
+    fn verify_account_states(&self, processed: &mut ProcessedTransaction) {
+        let ProcessedTransaction::Executed(executed) = processed else {
+            return;
+        };
+        let txn = &executed.loaded_transaction;
+        let feepayer = txn.accounts.first();
+        let rollback_lamports =
+            rollback_feepayer_lamports(&txn.rollback_accounts);
+
+        let gasless = self.environment.fee_lamports_per_signature == 0;
+        if gasless {
+            // If we are running in the gasless mode, we should not allow
+            // any mutation of the feepayer account, since that would make
+            // it possible for malicious actors to peform transfer operations
+            // from undelegated feepayers to delegated accounts, which would
+            // result in validator loosing funds upon balance settling.
+            let undelegated_feepayer_was_modified = feepayer
+                .map(|acc| {
+                    (acc.1.is_dirty()
+                        && !self.is_auto_airdrop_lamports_enabled
+                        && (acc.1.lamports() != 0 || rollback_lamports != 0))
+                        && !acc.1.delegated()
+                        && !acc.1.privileged()
+                })
+                .unwrap_or_default();
+            if undelegated_feepayer_was_modified {
+                executed.execution_details.status =
+                    Err(TransactionError::InvalidAccountForFee);
+                let logs = executed
+                    .execution_details
+                    .log_messages
+                    .get_or_insert_default();
+                let msg = "Feepayer balance has been modified illegally".into();
+                logs.push(msg);
+                return;
+            }
+        }
+        // SVM ignores rent exemption enforcement for accounts, which have
+        // 0 lamports, so it's possible to call realloc on account with zero
+        // balance bypassing the runtime checks. In order to prevent this
+        // edge case we perform explicit post execution check here.
+        for (i, (pubkey, acc)) in txn.accounts.iter().enumerate() {
+            if !acc.is_dirty() {
+                continue;
+            }
+            let Some(rent) = self.environment.rent_collector else {
+                continue;
+            };
+            if acc.lamports() == 0 && acc.data().is_empty() {
+                continue;
+            }
+            let rent_exemption_balance =
+                rent.get_rent().minimum_balance(acc.data().len());
+            if acc.lamports() >= rent_exemption_balance {
+                continue;
+            }
+            let error = Err(TransactionError::InsufficientFundsForRent {
+                account_index: i as u8,
+            });
+            executed.execution_details.status = error;
+            let logs = executed
+                .execution_details
+                .log_messages
+                .get_or_insert_default();
+            let msg = format!("Account {pubkey} has violated rent exemption");
+            logs.push(msg);
+            return;
+        }
+    }
 }
 
-// A utils to extract the rollback lamports of the feepayer
+// A utility to extract the rollback lamports of the feepayer
 fn rollback_feepayer_lamports(rollback: &RollbackAccounts) -> u64 {
     match rollback {
         RollbackAccounts::FeePayerOnly { fee_payer_account } => {

magicblock-processor/src/lib.rs

@@ -46,9 +46,10 @@ pub fn build_svm_env(
     }
 
     // We have a static rent which is setup once at startup,
-    // and never changes afterwards. For now we use the same
-    // values as the vanila solana validator (default())
-    let rent_collector = Box::leak(Box::new(RentCollector::default()));
+    // and never changes afterwards, so we just extend the
+    // lifetime to 'static by leaking the allocation.
+    let rent_collector = RentCollector::default();
+    let rent_collector = Box::leak(Box::new(rent_collector));
 
     TransactionProcessingEnvironment {
         blockhash,