From de5cad921102aa8c0e67cadecd47f1cfa758fad6 Mon Sep 17 00:00:00 2001 From: Sagar Dhawan Date: Tue, 4 Jun 2019 14:52:52 -0700 Subject: [PATCH] Add account owner to Storage Accounts (#4537) * Add account owner to Storage Accounts * Fix tests --- core/src/local_cluster.rs | 5 +- core/src/replicator.rs | 1 + genesis/src/main.rs | 5 +- multinode-demo/fullnode.sh | 7 +- programs/storage_api/src/storage_contract.rs | 16 +++- .../storage_api/src/storage_instruction.rs | 18 ++++- programs/storage_api/src/storage_processor.rs | 75 +++++++++++++++++-- .../storage_program/src/genesis_block_util.rs | 10 ++- wallet/src/wallet.rs | 53 ++++++++++--- 9 files changed, 161 insertions(+), 29 deletions(-) diff --git a/core/src/local_cluster.rs b/core/src/local_cluster.rs index e0181585b..1871e931f 100644 --- a/core/src/local_cluster.rs +++ b/core/src/local_cluster.rs @@ -131,7 +131,7 @@ impl LocalCluster { config.node_stakes[0], ); let storage_keypair = Keypair::new(); - genesis_block.add_storage_program(&storage_keypair.pubkey()); + genesis_block.add_storage_program(&leader_pubkey, &storage_keypair.pubkey()); genesis_block.ticks_per_slot = config.ticks_per_slot; genesis_block.slots_per_epoch = config.slots_per_epoch; genesis_block.stakers_slot_offset = config.stakers_slot_offset; @@ -479,6 +479,7 @@ impl LocalCluster { )) } + /// Sets up the storage account for validators/replicators and assumes the funder is the owner fn setup_storage_account( client: &ThinClient, storage_keypair: &Keypair, @@ -488,12 +489,14 @@ impl LocalCluster { let message = Message::new_with_payer( if replicator { storage_instruction::create_replicator_storage_account( + &from_keypair.pubkey(), &from_keypair.pubkey(), &storage_keypair.pubkey(), 1, ) } else { storage_instruction::create_validator_storage_account( + &from_keypair.pubkey(), &from_keypair.pubkey(), &storage_keypair.pubkey(), 1, diff --git a/core/src/replicator.rs b/core/src/replicator.rs index ee03026b8..5a642d1ef 100644 --- a/core/src/replicator.rs +++ b/core/src/replicator.rs @@ -444,6 +444,7 @@ impl Replicator { let (blockhash, _fee_calculator) = client.get_recent_blockhash().expect("blockhash"); let ix = storage_instruction::create_replicator_storage_account( + &keypair.pubkey(), &keypair.pubkey(), &storage_keypair.pubkey(), 1, diff --git a/genesis/src/main.rs b/genesis/src/main.rs index fefbf1005..720a73e78 100644 --- a/genesis/src/main.rs +++ b/genesis/src/main.rs @@ -228,7 +228,10 @@ fn main() -> Result<(), Box> { solana_exchange_program!(), ], ); - genesis_block.add_storage_program(&bootstrap_storage_keypair.pubkey()); + genesis_block.add_storage_program( + &bootstrap_leader_keypair.pubkey(), + &bootstrap_storage_keypair.pubkey(), + ); genesis_block.fee_calculator.lamports_per_signature = value_t_or_exit!(matches, "lamports_per_signature", u64); diff --git a/multinode-demo/fullnode.sh b/multinode-demo/fullnode.sh index 68792d64b..585592af1 100755 --- a/multinode-demo/fullnode.sh +++ b/multinode-demo/fullnode.sh @@ -116,7 +116,7 @@ setup_validator_accounts() { # Setup validator storage account $solana_wallet --keypair "$node_keypair_path" --url "http://$entrypoint_ip:8899" \ - create-validator-storage-account "$storage_pubkey" || return $? + create-validator-storage-account "$node_pubkey" "$storage_pubkey" || return $? touch "$node_keypair_path".configured fi @@ -140,6 +140,9 @@ setup_replicator_account() { declare storage_keypair_path=$3 declare node_lamports=$4 + declare node_pubkey + node_pubkey=$($solana_keygen pubkey "$node_keypair_path") + declare storage_pubkey storage_pubkey=$($solana_keygen pubkey "$storage_keypair_path") @@ -150,7 +153,7 @@ setup_replicator_account() { # Setup replicator storage account $solana_wallet --keypair "$node_keypair_path" --url "http://$entrypoint_ip:8899" \ - create-replicator-storage-account "$storage_pubkey" || return $? + create-replicator-storage-account "$node_pubkey" "$storage_pubkey" || return $? touch "$node_keypair_path".configured fi diff --git a/programs/storage_api/src/storage_contract.rs b/programs/storage_api/src/storage_contract.rs index e5e2082ae..6bba0f769 100644 --- a/programs/storage_api/src/storage_contract.rs +++ b/programs/storage_api/src/storage_contract.rs @@ -45,6 +45,7 @@ pub enum StorageContract { Uninitialized, // Must be first (aka, 0) ValidatorStorage { + owner: Pubkey, // Most recently advertised slot slot: u64, // Most recently advertised blockhash @@ -53,6 +54,7 @@ pub enum StorageContract { reward_validations: HashMap>, }, ReplicatorStorage { + owner: Pubkey, /// Map of Proofs per segment, in a HashMap based on the sha_state proofs: HashMap>, /// Map of Rewards per segment, in a HashMap based on the sha_state @@ -64,11 +66,12 @@ pub enum StorageContract { } // utility function, used by Bank, tests, genesis -pub fn create_validator_storage_account(lamports: u64) -> Account { +pub fn create_validator_storage_account(owner: Pubkey, lamports: u64) -> Account { let mut storage_account = Account::new(lamports, STORAGE_ACCOUNT_SPACE as usize, &crate::id()); storage_account .set_state(&StorageContract::ValidatorStorage { + owner, slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), @@ -98,10 +101,11 @@ impl<'a> StorageAccount<'a> { } } - pub fn initialize_replicator_storage(&mut self) -> Result<(), InstructionError> { + pub fn initialize_replicator_storage(&mut self, owner: Pubkey) -> Result<(), InstructionError> { let storage_contract = &mut self.account.state()?; if let StorageContract::Uninitialized = storage_contract { *storage_contract = StorageContract::ReplicatorStorage { + owner, proofs: HashMap::new(), reward_validations: HashMap::new(), }; @@ -111,10 +115,11 @@ impl<'a> StorageAccount<'a> { } } - pub fn initialize_validator_storage(&mut self) -> Result<(), InstructionError> { + pub fn initialize_validator_storage(&mut self, owner: Pubkey) -> Result<(), InstructionError> { let storage_contract = &mut self.account.state()?; if let StorageContract::Uninitialized = storage_contract { *storage_contract = StorageContract::ValidatorStorage { + owner, slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), @@ -179,6 +184,7 @@ impl<'a> StorageAccount<'a> { hash: state_hash, reward_validations, lockout_validations, + .. } = &mut storage_contract { let current_segment = get_segment_from_slot(current_slot); @@ -315,6 +321,7 @@ impl<'a> StorageAccount<'a> { } else if let StorageContract::ReplicatorStorage { proofs, reward_validations, + .. } = &mut storage_contract { // if current tick height is a full segment away, allow reward collection @@ -449,6 +456,7 @@ mod tests { } contract = StorageContract::ValidatorStorage { + owner: Pubkey::default(), slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), @@ -459,6 +467,7 @@ mod tests { panic!("Wrong contract type"); } contract = StorageContract::ReplicatorStorage { + owner: Pubkey::default(), proofs: HashMap::new(), reward_validations: HashMap::new(), }; @@ -502,6 +511,7 @@ mod tests { let mut proofs = HashMap::new(); proofs.insert(0, proof_map); *storage_contract = StorageContract::ReplicatorStorage { + owner: Pubkey::default(), proofs, reward_validations: HashMap::new(), }; diff --git a/programs/storage_api/src/storage_instruction.rs b/programs/storage_api/src/storage_instruction.rs index e261913bc..53ca6634f 100644 --- a/programs/storage_api/src/storage_instruction.rs +++ b/programs/storage_api/src/storage_instruction.rs @@ -16,8 +16,12 @@ pub enum StorageInstruction { /// Expects 1 Account: /// 0 - Account to be initialized InitializeMiningPool, - InitializeValidatorStorage, - InitializeReplicatorStorage, + InitializeValidatorStorage { + owner: Pubkey, + }, + InitializeReplicatorStorage { + owner: Pubkey, + }, SubmitMiningProof { sha_state: Hash, @@ -44,6 +48,7 @@ pub enum StorageInstruction { pub fn create_validator_storage_account( from_pubkey: &Pubkey, + storage_owner: &Pubkey, storage_pubkey: &Pubkey, lamports: u64, ) -> Vec { @@ -57,7 +62,9 @@ pub fn create_validator_storage_account( ), Instruction::new( id(), - &StorageInstruction::InitializeValidatorStorage, + &StorageInstruction::InitializeValidatorStorage { + owner: *storage_owner, + }, vec![AccountMeta::new(*storage_pubkey, false)], ), ] @@ -65,6 +72,7 @@ pub fn create_validator_storage_account( pub fn create_replicator_storage_account( from_pubkey: &Pubkey, + storage_owner: &Pubkey, storage_pubkey: &Pubkey, lamports: u64, ) -> Vec { @@ -78,7 +86,9 @@ pub fn create_replicator_storage_account( ), Instruction::new( id(), - &StorageInstruction::InitializeReplicatorStorage, + &StorageInstruction::InitializeReplicatorStorage { + owner: *storage_owner, + }, vec![AccountMeta::new(*storage_pubkey, false)], ), ] diff --git a/programs/storage_api/src/storage_processor.rs b/programs/storage_api/src/storage_processor.rs index 298efa116..f7194c0ee 100644 --- a/programs/storage_api/src/storage_processor.rs +++ b/programs/storage_api/src/storage_processor.rs @@ -27,17 +27,17 @@ pub fn process_instruction( } storage_account.initialize_mining_pool() } - StorageInstruction::InitializeReplicatorStorage => { + StorageInstruction::InitializeReplicatorStorage { owner } => { if !rest.is_empty() { Err(InstructionError::InvalidArgument)?; } - storage_account.initialize_replicator_storage() + storage_account.initialize_replicator_storage(owner) } - StorageInstruction::InitializeValidatorStorage => { + StorageInstruction::InitializeValidatorStorage { owner } => { if !rest.is_empty() { Err(InstructionError::InvalidArgument)?; } - storage_account.initialize_validator_storage() + storage_account.initialize_validator_storage(owner) } StorageInstruction::SubmitMiningProof { sha_state, @@ -109,6 +109,7 @@ mod tests { use solana_runtime::bank::Bank; use solana_runtime::bank_client::BankClient; use solana_sdk::account::{create_keyed_accounts, Account}; + use solana_sdk::account_utils::State; use solana_sdk::client::SyncClient; use solana_sdk::genesis_block::create_genesis_block; use solana_sdk::hash::{hash, Hash}; @@ -140,8 +141,61 @@ mod tests { ret } + #[test] + fn test_account_owner() { + let account_owner = Pubkey::new_rand(); + let validator_storage_pubkey = Pubkey::new_rand(); + let replicator_storage_pubkey = Pubkey::new_rand(); + + let (genesis_block, mint_keypair) = create_genesis_block(1000); + let mut bank = Bank::new(&genesis_block); + let mint_pubkey = mint_keypair.pubkey(); + bank.add_instruction_processor(id(), process_instruction); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + let message = Message::new(storage_instruction::create_validator_storage_account( + &mint_pubkey, + &account_owner, + &validator_storage_pubkey, + 1, + )); + bank_client + .send_message(&[&mint_keypair], message) + .expect("failed to create account"); + let account = bank + .get_account(&validator_storage_pubkey) + .expect("account not found"); + let storage_contract = account.state().expect("couldn't unpack account data"); + if let StorageContract::ValidatorStorage { owner, .. } = storage_contract { + assert_eq!(owner, account_owner); + } else { + assert!(false, "wrong account type found") + } + + let message = Message::new(storage_instruction::create_replicator_storage_account( + &mint_pubkey, + &account_owner, + &replicator_storage_pubkey, + 1, + )); + bank_client + .send_message(&[&mint_keypair], message) + .expect("failed to create account"); + let account = bank + .get_account(&replicator_storage_pubkey) + .expect("account not found"); + let storage_contract = account.state().expect("couldn't unpack account data"); + if let StorageContract::ReplicatorStorage { owner, .. } = storage_contract { + assert_eq!(owner, account_owner); + } else { + assert!(false, "wrong account type found") + } + } + #[test] fn test_proof_bounds() { + let account_owner = Pubkey::new_rand(); let pubkey = Pubkey::new_rand(); let mut account = Account { data: vec![0; STORAGE_ACCOUNT_SPACE as usize], @@ -149,7 +203,9 @@ mod tests { }; { let mut storage_account = StorageAccount::new(&mut account); - storage_account.initialize_replicator_storage().unwrap(); + storage_account + .initialize_replicator_storage(account_owner) + .unwrap(); } let ix = storage_instruction::mining_proof( @@ -233,12 +289,15 @@ mod tests { #[test] fn test_submit_mining_ok() { solana_logger::setup(); + let account_owner = Pubkey::new_rand(); let pubkey = Pubkey::new_rand(); let mut account = Account::default(); account.data.resize(STORAGE_ACCOUNT_SPACE as usize, 0); { let mut storage_account = StorageAccount::new(&mut account); - storage_account.initialize_replicator_storage().unwrap(); + storage_account + .initialize_replicator_storage(account_owner) + .unwrap(); } let ix = @@ -448,6 +507,7 @@ mod tests { .flat_map(|account| { storage_instruction::create_validator_storage_account( &mint.pubkey(), + &Pubkey::default(), account, lamports, ) @@ -458,6 +518,7 @@ mod tests { .for_each(|account| { ixs.append(&mut storage_instruction::create_replicator_storage_account( &mint.pubkey(), + &Pubkey::default(), account, lamports, )) @@ -559,6 +620,7 @@ mod tests { let message = Message::new(storage_instruction::create_replicator_storage_account( &mint_pubkey, + &Pubkey::default(), &replicator_pubkey, 1, )); @@ -566,6 +628,7 @@ mod tests { let message = Message::new(storage_instruction::create_validator_storage_account( &mint_pubkey, + &Pubkey::default(), &validator_pubkey, 1, )); diff --git a/programs/storage_program/src/genesis_block_util.rs b/programs/storage_program/src/genesis_block_util.rs index c5c35b52e..5ddd0eb99 100644 --- a/programs/storage_program/src/genesis_block_util.rs +++ b/programs/storage_program/src/genesis_block_util.rs @@ -4,14 +4,18 @@ use solana_sdk::pubkey::Pubkey; use solana_storage_api::storage_contract; pub trait GenesisBlockUtil { - fn add_storage_program(&mut self, validator_storage_pubkey: &Pubkey); + fn add_storage_program(&mut self, validator_pubkey: &Pubkey, validator_storage_pubkey: &Pubkey); } impl GenesisBlockUtil for GenesisBlock { - fn add_storage_program(&mut self, validator_storage_pubkey: &Pubkey) { + fn add_storage_program( + &mut self, + validator_pubkey: &Pubkey, + validator_storage_pubkey: &Pubkey, + ) { self.accounts.push(( *validator_storage_pubkey, - storage_contract::create_validator_storage_account(1), + storage_contract::create_validator_storage_account(*validator_pubkey, 1), )); self.native_instruction_processors .push(solana_storage_program!()); diff --git a/wallet/src/wallet.rs b/wallet/src/wallet.rs index c6db33efa..fed7331bf 100644 --- a/wallet/src/wallet.rs +++ b/wallet/src/wallet.rs @@ -55,8 +55,8 @@ pub enum WalletCommand { RedeemVoteCredits(Pubkey, Pubkey, Pubkey), ShowStakeAccount(Pubkey), CreateStorageMiningPoolAccount(Pubkey, u64), - CreateReplicatorStorageAccount(Pubkey), - CreateValidatorStorageAccount(Pubkey), + CreateReplicatorStorageAccount(Pubkey, Pubkey), + CreateValidatorStorageAccount(Pubkey, Pubkey), ClaimStorageReward(Pubkey, Pubkey, u64), ShowStorageAccount(Pubkey), Deploy(String), @@ -269,14 +269,18 @@ pub fn parse_command( )) } ("create-replicator-storage-account", Some(matches)) => { + let account_owner = pubkey_of(matches, "storage_account_owner").unwrap(); let storage_account_pubkey = pubkey_of(matches, "storage_account_pubkey").unwrap(); Ok(WalletCommand::CreateReplicatorStorageAccount( + account_owner, storage_account_pubkey, )) } ("create-validator-storage-account", Some(matches)) => { + let account_owner = pubkey_of(matches, "storage_account_owner").unwrap(); let storage_account_pubkey = pubkey_of(matches, "storage_account_pubkey").unwrap(); Ok(WalletCommand::CreateValidatorStorageAccount( + account_owner, storage_account_pubkey, )) } @@ -670,11 +674,13 @@ fn process_create_storage_mining_pool_account( fn process_create_replicator_storage_account( rpc_client: &RpcClient, config: &WalletConfig, + account_owner: &Pubkey, storage_account_pubkey: &Pubkey, ) -> ProcessResult { let (recent_blockhash, _fee_calculator) = rpc_client.get_recent_blockhash()?; let ixs = storage_instruction::create_replicator_storage_account( &config.keypair.pubkey(), + &account_owner, storage_account_pubkey, 1, ); @@ -686,11 +692,13 @@ fn process_create_replicator_storage_account( fn process_create_validator_storage_account( rpc_client: &RpcClient, config: &WalletConfig, + account_owner: &Pubkey, storage_account_pubkey: &Pubkey, ) -> ProcessResult { let (recent_blockhash, _fee_calculator) = rpc_client.get_recent_blockhash()?; let ixs = storage_instruction::create_validator_storage_account( &config.keypair.pubkey(), + account_owner, storage_account_pubkey, 1, ); @@ -1073,12 +1081,23 @@ pub fn process_command(config: &WalletConfig) -> ProcessResult { ) } - WalletCommand::CreateReplicatorStorageAccount(storage_account_pubkey) => { - process_create_replicator_storage_account(&rpc_client, config, &storage_account_pubkey) - } + WalletCommand::CreateReplicatorStorageAccount( + storage_account_owner, + storage_account_pubkey, + ) => process_create_replicator_storage_account( + &rpc_client, + config, + &storage_account_owner, + &storage_account_pubkey, + ), - WalletCommand::CreateValidatorStorageAccount(storage_account_pubkey) => { - process_create_validator_storage_account(&rpc_client, config, &storage_account_pubkey) + WalletCommand::CreateValidatorStorageAccount(account_owner, storage_account_pubkey) => { + process_create_validator_storage_account( + &rpc_client, + config, + &account_owner, + &storage_account_pubkey, + ) } WalletCommand::ClaimStorageReward( @@ -1500,25 +1519,41 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' SubCommand::with_name("create-replicator-storage-account") .about("Create a replicator storage account") .arg( - Arg::with_name("storage_account_pubkey") + Arg::with_name("storage_account_owner") .index(1) .value_name("PUBKEY") .takes_value(true) .required(true) .validator(is_pubkey) ) + .arg( + Arg::with_name("storage_account_pubkey") + .index(2) + .value_name("PUBKEY") + .takes_value(true) + .required(true) + .validator(is_pubkey) + ) ) .subcommand( SubCommand::with_name("create-validator-storage-account") .about("Create a validator storage account") .arg( - Arg::with_name("storage_account_pubkey") + Arg::with_name("storage_account_owner") .index(1) .value_name("PUBKEY") .takes_value(true) .required(true) .validator(is_pubkey) ) + .arg( + Arg::with_name("storage_account_pubkey") + .index(2) + .value_name("PUBKEY") + .takes_value(true) + .required(true) + .validator(is_pubkey) + ) ) .subcommand( SubCommand::with_name("claim-storage-reward")