Don't use global storage account

Other accounts would not be able to modify the system accounts userdata.
solana-labs · Feb 22, 2019 · 39632c1 · 39632c1
1 parent f5400cc
commit 39632c1
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 115 deletions.
diff --git a/programs/native/storage/src/lib.rs b/programs/native/storage/src/lib.rs
@@ -33,7 +33,7 @@ fn entrypoint(
 ) -> Result<(), ProgramError> {
     solana_logger::setup();
 
-    if keyed_accounts.len() != 2 {
+    if keyed_accounts.len() != 1 {
         // keyed_accounts[1] should be the main storage key
         // to access its userdata
         Err(ProgramError::InvalidArgument)?;
@@ -45,26 +45,9 @@ fn entrypoint(
         Err(ProgramError::GenericError)?;
     }
 
-    // Following https://github.com/solana-labs/solana/pull/2773,
-    // Modifications to userdata can only be made by accounts owned
-    // by this program. TODO: Add this check:
-    //if !check_id(&keyed_accounts[1].account.owner) {
-    //    error!("account[1] is not assigned to the STORAGE_PROGRAM");
-    //    Err(ProgramError::InvalidArgument)?;
-    //}
-
-    if *keyed_accounts[1].unsigned_key() != system_id() {
-        info!(
-            "invalid account id owner: {:?} system_id: {:?}",
-            keyed_accounts[1].unsigned_key(),
-            system_id()
-        );
-        Err(ProgramError::InvalidArgument)?;
-    }
-
     if let Ok(syscall) = bincode::deserialize(data) {
         let mut storage_account_state = if let Ok(storage_account_state) =
-            bincode::deserialize(&keyed_accounts[1].account.userdata)
+            bincode::deserialize(&keyed_accounts[0].account.userdata)
         {
             storage_account_state
         } else {
@@ -176,7 +159,7 @@ fn entrypoint(
         }
 
         if bincode::serialize_into(
-            &mut keyed_accounts[1].account.userdata[..],
+            &mut keyed_accounts[0].account.userdata[..],
             &storage_account_state,
         )
         .is_err()
@@ -197,7 +180,6 @@ mod test {
     use solana_sdk::account::{create_keyed_accounts, Account};
     use solana_sdk::hash::Hash;
     use solana_sdk::signature::{Keypair, KeypairUtil, Signature};
-    use solana_sdk::storage_program;
     use solana_sdk::storage_program::ProofStatus;
     use solana_sdk::storage_program::StorageTransaction;
     use solana_sdk::transaction::{Instruction, Transaction};
@@ -244,11 +226,8 @@ mod test {
         let keypair = Keypair::new();
         let mut keyed_accounts = Vec::new();
         let mut user_account = Account::default();
-        let mut system_account = Account::default();
         let pubkey = keypair.pubkey();
-        let system_key = storage_program::system_id();
         keyed_accounts.push(KeyedAccount::new(&pubkey, true, &mut user_account));
-        keyed_accounts.push(KeyedAccount::new(&system_key, false, &mut system_account));
 
         let tx = StorageTransaction::new_advertise_last_id(
             &keypair,
@@ -306,7 +285,7 @@ mod test {
         solana_logger::setup();
         let keypair = Keypair::new();
         let mut accounts = [Account::default(), Account::default()];
-        accounts[1].userdata.resize(16 * 1024, 0);
+        accounts[0].userdata.resize(16 * 1024, 0);
 
         let tx = StorageTransaction::new_advertise_last_id(
             &keypair,
@@ -333,7 +312,7 @@ mod test {
         solana_logger::setup();
         let keypair = Keypair::new();
         let mut accounts = [Account::default(), Account::default()];
-        accounts[1].userdata.resize(16 * 1024, 0);
+        accounts[0].userdata.resize(16 * 1024, 0);
 
         let entry_height = 0;
 

diff --git a/programs/native/storage/tests/storage.rs b/programs/native/storage/tests/storage.rs
@@ -3,12 +3,14 @@ use log::info;
 use solana_runtime::bank::Bank;
 use solana_sdk::genesis_block::GenesisBlock;
 use solana_sdk::hash::{hash, Hash};
+use solana_sdk::pubkey::Pubkey;
 use solana_sdk::signature::{Keypair, KeypairUtil, Signature};
 use solana_sdk::storage_program;
 use solana_sdk::storage_program::{StorageTransaction, ENTRIES_PER_SEGMENT};
+use solana_sdk::system_transaction::SystemTransaction;
 
-fn get_storage_entry_height(bank: &Bank) -> u64 {
-    match bank.get_account(&storage_program::system_id()) {
+fn get_storage_entry_height(bank: &Bank, account: Pubkey) -> u64 {
+    match bank.get_account(&account) {
         Some(storage_system_account) => {
             let state = deserialize(&storage_system_account.userdata);
             if let Ok(state) = state {
@@ -23,8 +25,8 @@ fn get_storage_entry_height(bank: &Bank) -> u64 {
     0
 }
 
-fn get_storage_last_id(bank: &Bank) -> Hash {
-    if let Some(storage_system_account) = bank.get_account(&storage_program::system_id()) {
+fn get_storage_last_id(bank: &Bank, account: Pubkey) -> Hash {
+    if let Some(storage_system_account) = bank.get_account(&account) {
         let state = deserialize(&storage_system_account.userdata);
         if let Ok(state) = state {
             let state: storage_program::StorageProgramState = state;
@@ -35,7 +37,6 @@ fn get_storage_last_id(bank: &Bank) -> Hash {
 }
 
 #[test]
-#[ignore]
 fn test_bank_storage() {
     let (genesis_block, alice) = GenesisBlock::new(1000);
     let bank = Bank::new(&genesis_block);
@@ -56,6 +57,18 @@ fn test_bank_storage() {
     bank.transfer(10, &alice, bob.pubkey(), last_id).unwrap();
     bank.transfer(10, &alice, jack.pubkey(), last_id).unwrap();
 
+    let tx = SystemTransaction::new_program_account(
+        &alice,
+        bob.pubkey(),
+        last_id,
+        1,
+        4 * 1024,
+        storage_program::id(),
+        0,
+    );
+
+    bank.process_transaction(&tx).unwrap();
+
     let tx = StorageTransaction::new_advertise_last_id(
         &bob,
         storage_last_id,
@@ -77,6 +90,9 @@ fn test_bank_storage() {
 
     bank.process_transaction(&tx).unwrap();
 
-    assert_eq!(get_storage_entry_height(&bank), ENTRIES_PER_SEGMENT);
-    assert_eq!(get_storage_last_id(&bank), storage_last_id);
+    assert_eq!(
+        get_storage_entry_height(&bank, bob.pubkey()),
+        ENTRIES_PER_SEGMENT
+    );
+    assert_eq!(get_storage_last_id(&bank, bob.pubkey()), storage_last_id);
 }
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -7,7 +7,7 @@ use crate::accounts::{Accounts, ErrorCounters, InstructionAccounts, InstructionL
 use crate::last_id_queue::LastIdQueue;
 use crate::runtime::{self, RuntimeError};
 use crate::status_cache::StatusCache;
-use bincode::{deserialize, serialize};
+use bincode::serialize;
 use hashbrown::HashMap;
 use log::{debug, info, Level};
 use solana_metrics::counter::Counter;
@@ -219,13 +219,6 @@ impl Bank {
         self.add_native_program("solana_bpf_loader", &bpf_loader::id());
         self.add_native_program("solana_budget_program", &budget_program::id());
         self.add_native_program("solana_erc20", &token_program::id());
-
-        let storage_system_account = Account::new(1, 16 * 1024, storage_program::system_id());
-        self.accounts.store_slow(
-            self.is_root(),
-            &storage_program::system_id(),
-            &storage_system_account,
-        );
     }
 
     /// Return the last entry ID registered.
@@ -237,33 +230,6 @@ impl Bank {
             .expect("no last_id has been set")
     }
 
-    pub fn get_storage_entry_height(&self) -> u64 {
-        match self.get_account(&storage_program::system_id()) {
-            Some(storage_system_account) => {
-                let state = deserialize(&storage_system_account.userdata);
-                if let Ok(state) = state {
-                    let state: storage_program::StorageProgramState = state;
-                    return state.entry_height;
-                }
-            }
-            None => {
-                info!("error in reading entry_height");
-            }
-        }
-        0
-    }
-
-    pub fn get_storage_last_id(&self) -> Hash {
-        if let Some(storage_system_account) = self.get_account(&storage_program::system_id()) {
-            let state = deserialize(&storage_system_account.userdata);
-            if let Ok(state) = state {
-                let state: storage_program::StorageProgramState = state;
-                return state.id;
-            }
-        }
-        Hash::default()
-    }
-
     /// Forget all signatures. Useful for benchmarking.
     pub fn clear_signatures(&self) {
         self.status_cache.write().unwrap().clear();
@@ -1232,10 +1198,6 @@ mod tests {
             132, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
             0, 0, 0, 0,
         ]);
-        let storage_system = Pubkey::new(&[
-            133, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 0, 0, 0,
-        ]);
 
         assert_eq!(system_program::id(), system);
         assert_eq!(native_loader::id(), native);
@@ -1244,7 +1206,6 @@ mod tests {
         assert_eq!(storage_program::id(), storage);
         assert_eq!(token_program::id(), token);
         assert_eq!(vote_program::id(), vote);
-        assert_eq!(storage_program::system_id(), storage_system);
     }
 
     #[test]
@@ -1258,7 +1219,6 @@ mod tests {
             storage_program::id(),
             token_program::id(),
             vote_program::id(),
-            storage_program::system_id(),
         ];
         assert!(ids.into_iter().all(move |id| unique.insert(id)));
     }

diff --git a/sdk/src/storage_program.rs b/sdk/src/storage_program.rs
@@ -66,11 +66,6 @@ pub const STORAGE_PROGRAM_ID: [u8; 32] = [
     0,
 ];
 
-pub const STORAGE_SYSTEM_ACCOUNT_ID: [u8; 32] = [
-    133, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-    0,
-];
-
 pub fn check_id(program_id: &Pubkey) -> bool {
     program_id.as_ref() == STORAGE_PROGRAM_ID
 }
@@ -79,10 +74,6 @@ pub fn id() -> Pubkey {
     Pubkey::new(&STORAGE_PROGRAM_ID)
 }
 
-pub fn system_id() -> Pubkey {
-    Pubkey::new(&STORAGE_SYSTEM_ACCOUNT_ID)
-}
-
 pub struct StorageTransaction {}
 
 impl StorageTransaction {
@@ -98,14 +89,7 @@ impl StorageTransaction {
             entry_height,
             signature,
         };
-        Transaction::new(
-            from_keypair,
-            &[Pubkey::new(&STORAGE_SYSTEM_ACCOUNT_ID)],
-            id(),
-            &program,
-            last_id,
-            0,
-        )
+        Transaction::new(from_keypair, &[], id(), &program, last_id, 0)
     }
 
     pub fn new_advertise_last_id(
@@ -118,14 +102,7 @@ impl StorageTransaction {
             id: storage_id,
             entry_height,
         };
-        Transaction::new(
-            from_keypair,
-            &[Pubkey::new(&STORAGE_SYSTEM_ACCOUNT_ID)],
-            id(),
-            &program,
-            last_id,
-            0,
-        )
+        Transaction::new(from_keypair, &[], id(), &program, last_id, 0)
     }
 
     pub fn new_proof_validation(
@@ -138,14 +115,7 @@ impl StorageTransaction {
             entry_height,
             proof_mask,
         };
-        Transaction::new(
-            from_keypair,
-            &[Pubkey::new(&STORAGE_SYSTEM_ACCOUNT_ID)],
-            id(),
-            &program,
-            last_id,
-            0,
-        )
+        Transaction::new(from_keypair, &[], id(), &program, last_id, 0)
     }
 
     pub fn new_reward_claim(
@@ -154,13 +124,6 @@ impl StorageTransaction {
         entry_height: u64,
     ) -> Transaction {
         let program = StorageProgram::ClaimStorageReward { entry_height };
-        Transaction::new(
-            from_keypair,
-            &[Pubkey::new(&STORAGE_SYSTEM_ACCOUNT_ID)],
-            id(),
-            &program,
-            last_id,
-            0,
-        )
+        Transaction::new(from_keypair, &[], id(), &program, last_id, 0)
     }
 }