Add new TransactionError

solana-labs · Sep 7, 2021 · 9321e88 · 9321e88
1 parent b0e1db9
commit 9321e88
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 12 deletions.
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
@@ -281,8 +281,8 @@ impl Accounts {
                             && message.is_writable(i, demote_program_write_locks)
                             && !is_upgradeable_loader_present
                         {
-                            error_counters.invalid_account_index += 1;
-                            return Err(TransactionError::InvalidAccountIndex);
+                            error_counters.invalid_writable_account += 1;
+                            return Err(TransactionError::InvalidWritableAccount);
                         }
 
                         if bpf_loader_upgradeable::check_id(account.owner()) {
@@ -316,8 +316,8 @@ impl Accounts {
                                         && message.is_writable(i, demote_program_write_locks)
                                         && !is_upgradeable_loader_present
                                     {
-                                        error_counters.invalid_account_index += 1;
-                                        return Err(TransactionError::InvalidAccountIndex);
+                                        error_counters.invalid_writable_account += 1;
+                                        return Err(TransactionError::InvalidWritableAccount);
                                     }
                                 }
                             }
@@ -1746,11 +1746,11 @@ mod tests {
         let tx = Transaction::new(&[&keypair], message.clone(), Hash::default());
         let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         assert_eq!(
             loaded_accounts[0],
-            (Err(TransactionError::InvalidAccountIndex), None)
+            (Err(TransactionError::InvalidWritableAccount), None)
         );
 
         // Solution 1: include bpf_loader_upgradeable account
@@ -1767,7 +1767,7 @@ mod tests {
         let loaded_accounts =
             load_accounts(tx, &accounts_with_upgradeable_loader, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         let result = loaded_accounts[0].0.as_ref().unwrap();
         assert_eq!(result.accounts[..2], accounts_with_upgradeable_loader[..2]);
@@ -1782,7 +1782,7 @@ mod tests {
         let tx = Transaction::new(&[&keypair], message, Hash::default());
         let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         let result = loaded_accounts[0].0.as_ref().unwrap();
         assert_eq!(result.accounts[..2], accounts[..2]);
@@ -1829,11 +1829,12 @@ mod tests {
         let tx = Transaction::new(&[&keypair], message.clone(), Hash::default());
         let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        println!("{:?}", error_counters);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         assert_eq!(
             loaded_accounts[0],
-            (Err(TransactionError::InvalidAccountIndex), None)
+            (Err(TransactionError::InvalidWritableAccount), None)
         );
 
         // Solution 1: include bpf_loader_upgradeable account
@@ -1850,7 +1851,7 @@ mod tests {
         let loaded_accounts =
             load_accounts(tx, &accounts_with_upgradeable_loader, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         let result = loaded_accounts[0].0.as_ref().unwrap();
         assert_eq!(result.accounts[..2], accounts_with_upgradeable_loader[..2]);
@@ -1865,7 +1866,7 @@ mod tests {
         let tx = Transaction::new(&[&keypair], message, Hash::default());
         let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_account_index, 1);
+        assert_eq!(error_counters.invalid_writable_account, 1);
         assert_eq!(loaded_accounts.len(), 1);
         let result = loaded_accounts[0].0.as_ref().unwrap();
         assert_eq!(result.accounts[..2], accounts[..2]);

diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs
@@ -175,6 +175,7 @@ pub struct ErrorCounters {
     pub invalid_account_index: usize,
     pub invalid_program_for_execution: usize,
     pub not_allowed_during_cluster_maintenance: usize,
+    pub invalid_writable_account: usize,
 }
 
 #[derive(Default, Debug)]

diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -3119,6 +3119,12 @@ impl Bank {
                 error_counters.not_allowed_during_cluster_maintenance
             );
         }
+        if 0 != error_counters.invalid_writable_account {
+            inc_new_counter_info!(
+                "bank-process_transactions-error-invalid_writable_account",
+                error_counters.invalid_writable_account
+            );
+        }
     }
 
     /// Converts Accounts into RefCell<AccountSharedData>, this involves moving

diff --git a/sdk/src/transaction/mod.rs b/sdk/src/transaction/mod.rs
@@ -116,6 +116,10 @@ pub enum TransactionError {
     /// Transaction version is unsupported
     #[error("Transaction version is unsupported")]
     UnsupportedVersion,
+
+    /// Transaction loads a writable account that cannot be written
+    #[error("Transaction loads a writable account that cannot be written")]
+    InvalidWritableAccount,
 }
 
 pub type Result<T> = result::Result<T, TransactionError>;

diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto
@@ -43,6 +43,7 @@ enum TransactionErrorType {
     ACCOUNT_BORROW_OUTSTANDING_TX = 16;
     WOULD_EXCEED_MAX_BLOCK_COST_LIMIT = 17;
     UNSUPPORTED_VERSION = 18;
+    INVALID_WRITABLE_ACCOUNT = 19;
 }
 
 message InstructionError {

diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs
@@ -550,6 +550,7 @@ impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
             16 => TransactionError::AccountBorrowOutstanding,
             17 => TransactionError::WouldExceedMaxBlockCostLimit,
             18 => TransactionError::UnsupportedVersion,
+            19 => TransactionError::InvalidWritableAccount,
             _ => return Err("Invalid TransactionError"),
         })
     }
@@ -614,6 +615,9 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
                 TransactionError::UnsupportedVersion => {
                     tx_by_addr::TransactionErrorType::UnsupportedVersion
                 }
+                TransactionError::InvalidWritableAccount => {
+                    tx_by_addr::TransactionErrorType::InvalidWritableAccount
+                }
             } as i32,
             instruction_error: match transaction_error {
                 TransactionError::InstructionError(index, ref instruction_error) => {