From b1ac8f933b9698398b51bddfc7994d232e07571a Mon Sep 17 00:00:00 2001 From: Sagar Dhawan Date: Thu, 13 Jun 2019 17:53:54 -0700 Subject: [PATCH] Fix storage program space issues and limit storage transaction data (#4677) --- Cargo.lock | 2 + core/src/replicator.rs | 36 +- core/src/storage_stage.rs | 67 ++-- core/src/validator.rs | 2 +- programs/storage_api/Cargo.toml | 2 + programs/storage_api/src/storage_contract.rs | 353 ++++++++++-------- .../storage_api/src/storage_instruction.rs | 68 +++- programs/storage_api/src/storage_processor.rs | 25 +- .../tests/storage_processor.rs | 112 ++++-- replicator/src/main.rs | 5 +- wallet/src/wallet.rs | 33 +- 11 files changed, 456 insertions(+), 249 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7918cda38..9f838adc8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2734,6 +2734,8 @@ dependencies = [ "assert_matches 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", + "num-derive 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)", + "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.92 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.92 (registry+https://github.com/rust-lang/crates.io-index)", "solana-logger 0.16.0", diff --git a/core/src/replicator.rs b/core/src/replicator.rs index f24c94c4d..fb67f5114 100644 --- a/core/src/replicator.rs +++ b/core/src/replicator.rs @@ -52,7 +52,7 @@ pub struct Replicator { ledger_path: String, keypair: Arc, storage_keypair: Arc, - blockhash: String, + blockhash: Hash, signature: ed25519_dalek::Signature, cluster_info: Arc>, ledger_data_file_encrypted: PathBuf, @@ -204,7 +204,7 @@ impl Replicator { let client = crate::gossip_service::get_client(&nodes); let (storage_blockhash, storage_slot) = - match Self::poll_for_blockhash_and_slot(&cluster_info, &Hash::default().to_string()) { + match Self::poll_for_blockhash_and_slot(&cluster_info, &Hash::default()) { Ok(blockhash_and_slot) => blockhash_and_slot, Err(e) => { //shutdown services before exiting @@ -301,9 +301,9 @@ impl Replicator { pub fn run(&mut self) { info!("waiting for ledger download"); self.thread_handles.pop().unwrap().join().unwrap(); + self.encrypt_ledger() + .expect("ledger encrypt not successful"); loop { - self.encrypt_ledger() - .expect("ledger encrypt not successful"); self.create_sampling_offsets(); if let Err(err) = self.sample_file_to_create_mining_hash() { warn!("Error sampling file, exiting: {:?}", err); @@ -311,6 +311,7 @@ impl Replicator { } self.submit_mining_proof(); + // Todo make this a lot more frequent by picking a "new" blockhash instead of picking a storage blockhash // prep the next proof let (storage_blockhash, _) = match Self::poll_for_blockhash_and_slot(&self.cluster_info, &self.blockhash) { @@ -323,7 +324,6 @@ impl Replicator { break; } }; - self.signature = self.storage_keypair.sign(storage_blockhash.as_ref()); self.blockhash = storage_blockhash; } } @@ -473,6 +473,7 @@ impl Replicator { self.sha_state, get_segment_from_slot(self.slot), Signature::new(&self.signature.to_bytes()), + self.blockhash, ); let message = Message::new_with_payer(vec![instruction], Some(&self.keypair.pubkey())); let mut transaction = Transaction::new( @@ -507,8 +508,8 @@ impl Replicator { /// Poll for a different blockhash and associated max_slot than `previous_blockhash` fn poll_for_blockhash_and_slot( cluster_info: &Arc>, - previous_blockhash: &str, - ) -> result::Result<(String, u64), Error> { + previous_blockhash: &Hash, + ) -> result::Result<(Hash, u64), Error> { for _ in 0..10 { let rpc_client = { let cluster_info = cluster_info.read().unwrap(); @@ -517,13 +518,28 @@ impl Replicator { let node_index = thread_rng().gen_range(0, rpc_peers.len()); RpcClient::new_socket(rpc_peers[node_index].rpc) }; - let storage_blockhash = rpc_client + let response = rpc_client .retry_make_rpc_request(&RpcRequest::GetStorageBlockhash, None, 0) .map_err(|err| { warn!("Error while making rpc request {:?}", err); Error::IO(io::Error::new(ErrorKind::Other, "rpc error")) - })? - .to_string(); + })?; + let storage_blockhash = + serde_json::from_value::<(String)>(response).map_err(|err| { + io::Error::new( + io::ErrorKind::Other, + format!("Couldn't parse response: {:?}", err), + ) + })?; + let storage_blockhash = storage_blockhash.parse().map_err(|err| { + io::Error::new( + io::ErrorKind::Other, + format!( + "Blockhash parse failure: {:?} on {:?}", + err, storage_blockhash + ), + ) + })?; if storage_blockhash != *previous_blockhash { let storage_slot = rpc_client .retry_make_rpc_request(&RpcRequest::GetStorageSlot, None, 0) diff --git a/core/src/storage_stage.rs b/core/src/storage_stage.rs index 6c1b37ae1..1f9ae38cc 100644 --- a/core/src/storage_stage.rs +++ b/core/src/storage_stage.rs @@ -21,11 +21,10 @@ use solana_sdk::message::Message; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil, Signature}; use solana_sdk::transaction::Transaction; -use solana_storage_api::storage_contract::{CheckedProof, Proof, ProofStatus, StorageContract}; +use solana_storage_api::storage_contract::{Proof, ProofStatus, StorageContract}; use solana_storage_api::storage_instruction::proof_validation; use solana_storage_api::{get_segment_from_slot, storage_instruction}; use std::collections::HashMap; -use std::io; use std::mem::size_of; use std::net::UdpSocket; use std::sync::atomic::{AtomicBool, Ordering}; @@ -33,6 +32,7 @@ use std::sync::mpsc::{channel, Receiver, RecvTimeoutError, Sender}; use std::sync::{Arc, RwLock}; use std::thread::{self, sleep, Builder, JoinHandle}; use std::time::{Duration, Instant}; +use std::{cmp, io}; // Block of hash answers to validate against // Vec of [ledger blocks] x [keys] @@ -86,7 +86,7 @@ fn get_identity_index_from_signature(key: &Signature) -> usize { } impl StorageState { - pub fn new() -> Self { + pub fn new(hash: &Hash) -> Self { let storage_keys = vec![0u8; KEY_SIZE * NUM_IDENTITIES]; let storage_results = vec![Hash::default(); NUM_IDENTITIES]; let replicator_map = vec![]; @@ -96,7 +96,7 @@ impl StorageState { storage_results, replicator_map, slot: 0, - storage_blockhash: Hash::default(), + storage_blockhash: *hash, }; StorageState { @@ -439,7 +439,7 @@ impl StorageStage { //convert slot to segment let segment = get_segment_from_slot(slot); if let Some(proofs) = proofs.get(&segment) { - for (_, proof) in proofs.iter() { + for proof in proofs.iter() { { debug!( "generating storage_keys from storage txs current_key_idx: {}", @@ -543,6 +543,8 @@ impl StorageStage { // bundle up mining submissions from replicators // and submit them in a tx to the leader to get rewarded. let mut w_state = storage_state.write().unwrap(); + let mut max_proof_mask = 0; + let proof_mask_limit = storage_instruction::proof_mask_limit(); let instructions: Vec<_> = w_state .replicator_map .iter_mut() @@ -552,32 +554,44 @@ impl StorageStage { .iter_mut() .filter_map(|(id, proofs)| { if !proofs.is_empty() { - Some(( - *id, - proofs - .drain(..) - .map(|proof| CheckedProof { - proof, - status: ProofStatus::Valid, - }) - .collect::>(), - )) + if (proofs.len() as u64) >= proof_mask_limit { + proofs.clear(); + None + } else { + max_proof_mask = cmp::max(max_proof_mask, proofs.len()); + Some(( + *id, + proofs + .drain(..) + .map(|_| ProofStatus::Valid) + .collect::>(), + )) + } } else { None } }) - .collect::>(); + .collect::>(); + if !checked_proofs.is_empty() { - let ix = proof_validation( - &storage_keypair.pubkey(), - current_segment as u64, - checked_proofs, - ); - Some(ix) + let max_accounts_per_ix = + storage_instruction::validation_account_limit(max_proof_mask); + let ixs = checked_proofs + .chunks(max_accounts_per_ix as usize) + .map(|checked_proofs| { + proof_validation( + &storage_keypair.pubkey(), + current_segment as u64, + checked_proofs.to_vec(), + ) + }) + .collect::>(); + Some(ixs) } else { None } }) + .flatten() .collect(); let res: std::result::Result<_, _> = instructions .into_iter() @@ -633,9 +647,9 @@ mod tests { let cluster_info = test_cluster_info(&keypair.pubkey()); let GenesisBlockInfo { genesis_block, .. } = create_genesis_block(1000); let bank = Arc::new(Bank::new(&genesis_block)); - let bank_forks = Arc::new(RwLock::new(BankForks::new_from_banks(&[bank], 0))); + let bank_forks = Arc::new(RwLock::new(BankForks::new_from_banks(&[bank.clone()], 0))); let (_slot_sender, slot_receiver) = channel(); - let storage_state = StorageState::new(); + let storage_state = StorageState::new(&bank.last_blockhash()); let storage_stage = StorageStage::new( &storage_state, slot_receiver, @@ -674,7 +688,7 @@ mod tests { let cluster_info = test_cluster_info(&keypair.pubkey()); let (bank_sender, bank_receiver) = channel(); - let storage_state = StorageState::new(); + let storage_state = StorageState::new(&bank.last_blockhash()); let storage_stage = StorageStage::new( &storage_state, bank_receiver, @@ -763,7 +777,7 @@ mod tests { let cluster_info = test_cluster_info(&keypair.pubkey()); let (bank_sender, bank_receiver) = channel(); - let storage_state = StorageState::new(); + let storage_state = StorageState::new(&bank.last_blockhash()); let storage_stage = StorageStage::new( &storage_state, bank_receiver, @@ -808,6 +822,7 @@ mod tests { Hash::default(), 0, keypair.sign_message(b"test"), + bank.last_blockhash(), ); let next_bank = Arc::new(Bank::new_from_parent(&bank, &keypair.pubkey(), 2)); diff --git a/core/src/validator.rs b/core/src/validator.rs index 0841b2566..978b176c6 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -153,7 +153,7 @@ impl Validator { keypair.clone(), ))); - let storage_state = StorageState::new(); + let storage_state = StorageState::new(&bank.last_blockhash()); let rpc_service = if node.info.rpc.port() == 0 { None diff --git a/programs/storage_api/Cargo.toml b/programs/storage_api/Cargo.toml index b631167b3..832c51c8e 100644 --- a/programs/storage_api/Cargo.toml +++ b/programs/storage_api/Cargo.toml @@ -12,6 +12,8 @@ edition = "2018" assert_matches = "1.3.0" bincode = "1.1.4" log = "0.4.2" +num-derive = "0.2" +num-traits = "0.2" serde = "1.0.92" serde_derive = "1.0.92" solana-logger = { path = "../../logger", version = "0.16.0" } diff --git a/programs/storage_api/src/storage_contract.rs b/programs/storage_api/src/storage_contract.rs index 522282fd1..dd0a9617c 100644 --- a/programs/storage_api/src/storage_contract.rs +++ b/programs/storage_api/src/storage_contract.rs @@ -1,5 +1,6 @@ use crate::get_segment_from_slot; use log::*; +use num_derive::FromPrimitive; use serde_derive::{Deserialize, Serialize}; use solana_sdk::account::Account; use solana_sdk::account::KeyedAccount; @@ -10,10 +11,22 @@ use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::Signature; use std::collections::HashMap; -pub const TOTAL_VALIDATOR_REWARDS: u64 = 1; -pub const TOTAL_REPLICATOR_REWARDS: u64 = 1; +pub const VALIDATOR_REWARD: u64 = 25; +pub const REPLICATOR_REWARD: u64 = 25; // Todo Tune this for actual use cases when replicators are feature complete pub const STORAGE_ACCOUNT_SPACE: u64 = 1024 * 8; +pub const MAX_PROOFS_PER_SEGMENT: usize = 80; + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, FromPrimitive)] +pub enum StorageError { + InvalidSegment, + InvalidBlockhash, + InvalidProofMask, + DuplicateProof, + RewardPoolDepleted, + InvalidOwner, + ProofLimitReached, +} #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub enum ProofStatus { @@ -30,18 +43,16 @@ impl Default for ProofStatus { #[derive(Default, Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct Proof { + /// The encryption key the replicator used (also used to generate offsets) pub signature: Signature, + /// A "recent" blockhash used to generate the seed + pub blockhash: Hash, + /// The resulting sampled state pub sha_state: Hash, /// The start index of the segment proof is for pub segment_index: usize, } -#[derive(Default, Debug, Serialize, Deserialize, Clone)] -pub struct CheckedProof { - pub proof: Proof, - pub status: ProofStatus, -} - #[derive(Debug, Serialize, Deserialize)] pub enum StorageContract { Uninitialized, // Must be first (aka, 0) @@ -52,16 +63,20 @@ pub enum StorageContract { slot: u64, // Most recently advertised blockhash hash: Hash, - lockout_validations: HashMap>, - reward_validations: HashMap>, + // Lockouts and Rewards are per segment per replicator. It needs to remain this way until + // the challenge stage is added. Once challenges are in rewards can just be a number + lockout_validations: HashMap>>, + // lamports that are ready to be claimed + pending_lamports: u64, }, 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 - /// Multiple validators can validate the same set of proofs so it needs a Vec - reward_validations: HashMap>>, + // TODO what to do about duplicate proofs across segments? - Check the blockhashes + // Map of Proofs per segment, in a Vec + proofs: HashMap>, + // Map of Rewards per segment, in a HashMap based on the validator account that verified + // the proof. This can be used for challenge stage when its added + reward_validations: HashMap>>, }, MiningPool, @@ -77,7 +92,7 @@ pub fn create_validator_storage_account(owner: Pubkey, lamports: u64) -> Account slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), - reward_validations: HashMap::new(), + pending_lamports: 0, }) .expect("set_state"); @@ -85,12 +100,13 @@ pub fn create_validator_storage_account(owner: Pubkey, lamports: u64) -> Account } pub struct StorageAccount<'a> { + pub(crate) id: Pubkey, account: &'a mut Account, } impl<'a> StorageAccount<'a> { - pub fn new(account: &'a mut Account) -> Self { - Self { account } + pub fn new(id: Pubkey, account: &'a mut Account) -> Self { + Self { id, account } } pub fn initialize_mining_pool(&mut self) -> Result<(), InstructionError> { @@ -125,7 +141,7 @@ impl<'a> StorageAccount<'a> { slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), - reward_validations: HashMap::new(), + pending_lamports: 0, }; self.account.set_state(storage_contract) } else { @@ -138,15 +154,28 @@ impl<'a> StorageAccount<'a> { sha_state: Hash, segment_index: usize, signature: Signature, + blockhash: Hash, current_slot: u64, ) -> Result<(), InstructionError> { let mut storage_contract = &mut self.account.state()?; - if let StorageContract::ReplicatorStorage { proofs, .. } = &mut storage_contract { + if let StorageContract::ReplicatorStorage { + proofs, + reward_validations, + .. + } = &mut storage_contract + { let current_segment = get_segment_from_slot(current_slot); + // clean up the account + // TODO check for time correctness - storage seems to run at a delay of about 3 + proofs.retain(|segment, _| *segment >= current_segment.saturating_sub(5)); + reward_validations.retain(|segment, _| *segment >= current_segment.saturating_sub(10)); + if segment_index >= current_segment { // attempt to submit proof for unconfirmed segment - return Err(InstructionError::InvalidArgument); + return Err(InstructionError::CustomError( + StorageError::InvalidSegment as u32, + )); } debug!( @@ -154,23 +183,33 @@ impl<'a> StorageAccount<'a> { sha_state, segment_index ); + // TODO check that this blockhash is valid and recent + // if !is_valid(&blockhash) { + // // proof isn't using a recent blockhash + // return Err(InstructionError::CustomError(InvalidBlockhash as u32)); + // } + + let proof = Proof { + sha_state, + signature, + blockhash, + segment_index, + }; // store the proofs in the "current" segment's entry in the hash map. let segment_proofs = proofs.entry(current_segment).or_default(); - if segment_proofs.contains_key(&sha_state) { + if segment_proofs.contains(&proof) { // do not accept duplicate proofs - return Err(InstructionError::InvalidArgument); + return Err(InstructionError::CustomError( + StorageError::DuplicateProof as u32, + )); } - segment_proofs.insert( - sha_state, - Proof { - sha_state, - signature, - segment_index, - }, - ); - // TODO check for time correctness - proofs.retain(|segment, _| *segment >= current_segment.saturating_sub(5)); - + if segment_proofs.len() >= MAX_PROOFS_PER_SEGMENT { + // do not accept more than MAX_PROOFS_PER_SEGMENT + return Err(InstructionError::CustomError( + StorageError::ProofLimitReached as u32, + )); + } + segment_proofs.push(proof); self.account.set_state(storage_contract) } else { Err(InstructionError::InvalidArgument)? @@ -187,8 +226,8 @@ impl<'a> StorageAccount<'a> { if let StorageContract::ValidatorStorage { slot: state_slot, hash: state_hash, - reward_validations, lockout_validations, + pending_lamports, .. } = &mut storage_contract { @@ -200,14 +239,27 @@ impl<'a> StorageAccount<'a> { segment, current_segment ); if segment < original_segment || segment >= current_segment { - return Err(InstructionError::InvalidArgument); + return Err(InstructionError::CustomError( + StorageError::InvalidSegment as u32, + )); } *state_slot = slot; *state_hash = hash; - // move storage epoch updated, move the lockout_validations to reward_validations - reward_validations.extend(lockout_validations.drain()); + // storage epoch updated, move the lockout_validations to pending_lamports + let num_validations = count_valid_proofs( + &lockout_validations + .drain() + .flat_map(|(_segment, mut proofs)| { + proofs + .drain() + .flat_map(|(_, proof)| proof) + .collect::>() + }) + .collect::>(), + ); + *pending_lamports += VALIDATOR_REWARD * num_validations; self.account.set_state(storage_contract) } else { Err(InstructionError::InvalidArgument)? @@ -216,8 +268,9 @@ impl<'a> StorageAccount<'a> { pub fn proof_validation( &mut self, + me: &Pubkey, segment: u64, - proofs: Vec<(Pubkey, Vec)>, + proofs_per_account: Vec>, replicator_accounts: &mut [StorageAccount], ) -> Result<(), InstructionError> { let mut storage_contract = &mut self.account.state()?; @@ -231,56 +284,72 @@ impl<'a> StorageAccount<'a> { let state_segment = get_segment_from_slot(*state_slot); if segment_index > state_segment { - return Err(InstructionError::InvalidArgument); + return Err(InstructionError::CustomError( + StorageError::InvalidSegment as u32, + )); } - let accounts_and_proofs = replicator_accounts + let accounts = replicator_accounts .iter_mut() - .filter_map(|account| { - account - .account - .state() - .ok() - .map(move |contract| match contract { - StorageContract::ReplicatorStorage { proofs, .. } => { - if let Some(proofs) = proofs.get(&segment_index).cloned() { - Some((account, proofs)) + .enumerate() + .filter_map(|(i, account)| { + account.account.state().ok().map(|contract| match contract { + StorageContract::ReplicatorStorage { + proofs: account_proofs, + .. + } => { + //TODO do this better + if let Some(segment_proofs) = + account_proofs.get(&segment_index).cloned() + { + if proofs_per_account + .get(i) + .filter(|proofs| proofs.len() == segment_proofs.len()) + .is_some() + { + Some(account) } else { None } + } else { + None } - _ => None, - }) + } + _ => None, + }) }) .flatten() .collect::>(); - if accounts_and_proofs.len() != proofs.len() { - // don't have all the accounts to validate the proofs against - return Err(InstructionError::InvalidArgument); + if accounts.len() != proofs_per_account.len() { + // don't have all the accounts to validate the proofs_per_account against + return Err(InstructionError::CustomError( + StorageError::InvalidProofMask as u32, + )); } - let valid_proofs: Vec<_> = proofs + let stored_proofs: Vec<_> = proofs_per_account .into_iter() - .zip(accounts_and_proofs.into_iter()) - .flat_map(|((_id, checked_proofs), (account, proofs))| { - checked_proofs.into_iter().filter_map(move |checked_proof| { - proofs.get(&checked_proof.proof.sha_state).map(|proof| { - process_validation(account, segment_index, &proof, &checked_proof) - .map(|_| checked_proof) - }) - }) + .zip(accounts.into_iter()) + .filter_map(|(checked_proofs, account)| { + if store_validation_result(me, account, segment_index, &checked_proofs).is_ok() + { + Some((account.id, checked_proofs)) + } else { + None + } }) - .flatten() .collect(); // allow validators to store successful validations - valid_proofs.into_iter().for_each(|proof| { - lockout_validations - .entry(segment_index) - .or_default() - .insert(proof.proof.sha_state, proof.status); - }); + stored_proofs + .into_iter() + .for_each(|(replicator_account_id, proof_mask)| { + lockout_validations + .entry(segment_index) + .or_default() + .insert(replicator_account_id, proof_mask); + }); self.account.set_state(storage_contract) } else { @@ -291,59 +360,59 @@ impl<'a> StorageAccount<'a> { pub fn claim_storage_reward( &mut self, mining_pool: &mut KeyedAccount, + owner: &mut StorageAccount, ) -> Result<(), InstructionError> { let mut storage_contract = &mut self.account.state()?; if let StorageContract::ValidatorStorage { - reward_validations, .. + owner: account_owner, + pending_lamports, + .. } = &mut storage_contract { - let num_validations = count_valid_proofs( - &reward_validations - .drain() - .flat_map(|(_segment, mut proofs)| { - proofs.drain().map(|(_, proof)| proof).collect::>() - }) - .collect::>(), - ); - let reward = TOTAL_VALIDATOR_REWARDS * num_validations; - mining_pool.account.lamports -= reward; - self.account.lamports += reward; + if owner.id != *account_owner { + Err(InstructionError::CustomError( + StorageError::InvalidOwner as u32, + ))? + } + + let pending = *pending_lamports; + if mining_pool.account.lamports < pending { + Err(InstructionError::CustomError( + StorageError::RewardPoolDepleted as u32, + ))? + } + mining_pool.account.lamports -= pending; + owner.account.lamports += pending; + //clear pending_lamports + *pending_lamports = 0; self.account.set_state(storage_contract) } else if let StorageContract::ReplicatorStorage { - proofs, + owner: account_owner, reward_validations, .. } = &mut storage_contract { - // remove proofs for which rewards have already been collected - let segment_proofs = proofs; + if owner.id != *account_owner { + Err(InstructionError::CustomError( + StorageError::InvalidOwner as u32, + ))? + } + let checked_proofs = reward_validations .drain() - .flat_map(|(segment, mut proofs)| { + .flat_map(|(_, mut proofs)| { proofs .drain() - .map(|(sha_state, proof)| { - proof - .into_iter() - .map(|proof| { - segment_proofs.get_mut(&segment).and_then(|segment_proofs| { - segment_proofs.remove(&sha_state) - }); - proof - }) - .collect::>() - }) - .flatten() + .flat_map(|(_, proofs)| proofs) .collect::>() }) .collect::>(); let total_proofs = checked_proofs.len() as u64; let num_validations = count_valid_proofs(&checked_proofs); - let reward = - num_validations * TOTAL_REPLICATOR_REWARDS * (num_validations / total_proofs); + let reward = num_validations * REPLICATOR_REWARD * (num_validations / total_proofs); mining_pool.account.lamports -= reward; - self.account.lamports += reward; + owner.account.lamports += reward; self.account.set_state(storage_contract) } else { Err(InstructionError::InvalidArgument)? @@ -353,9 +422,10 @@ impl<'a> StorageAccount<'a> { /// Store the result of a proof validation into the replicator account fn store_validation_result( + me: &Pubkey, storage_account: &mut StorageAccount, segment: usize, - checked_proof: CheckedProof, + proof_mask: &[ProofStatus], ) -> Result<(), InstructionError> { let mut storage_contract = storage_account.account.state()?; match &mut storage_contract { @@ -368,20 +438,14 @@ fn store_validation_result( return Err(InstructionError::InvalidAccountData); } - if proofs - .get(&segment) - .unwrap() - .contains_key(&checked_proof.proof.sha_state) - { - reward_validations - .entry(segment) - .or_default() - .entry(checked_proof.proof.sha_state) - .or_default() - .push(checked_proof.status); - } else { + if proofs.get(&segment).unwrap().len() != proof_mask.len() { return Err(InstructionError::InvalidAccountData); } + + reward_validations + .entry(segment) + .or_default() + .insert(*me, proof_mask.to_vec()); } _ => return Err(InstructionError::InvalidAccountData), } @@ -398,21 +462,6 @@ fn count_valid_proofs(proofs: &[ProofStatus]) -> u64 { num } -fn process_validation( - account: &mut StorageAccount, - segment_index: usize, - proof: &Proof, - checked_proof: &CheckedProof, -) -> Result<(), InstructionError> { - store_validation_result(account, segment_index, checked_proof.clone())?; - if proof.signature != checked_proof.proof.signature - || checked_proof.status != ProofStatus::Valid - { - return Err(InstructionError::GenericError); - } - Ok(()) -} - #[cfg(test)] mod tests { use super::*; @@ -423,7 +472,7 @@ mod tests { solana_logger::setup(); let mut account = Account::default(); account.data.resize(STORAGE_ACCOUNT_SPACE as usize, 0); - let storage_account = StorageAccount::new(&mut account); + let storage_account = StorageAccount::new(Pubkey::default(), &mut account); // pretend it's a validator op code let mut contract = storage_account.account.state().unwrap(); if let StorageContract::ValidatorStorage { .. } = contract { @@ -438,7 +487,7 @@ mod tests { slot: 0, hash: Hash::default(), lockout_validations: HashMap::new(), - reward_validations: HashMap::new(), + pending_lamports: 0, }; storage_account.account.set_state(&contract).unwrap(); if let StorageContract::ReplicatorStorage { .. } = contract { @@ -458,6 +507,7 @@ mod tests { #[test] fn test_process_validation() { let mut account = StorageAccount { + id: Pubkey::default(), account: &mut Account { lamports: 0, data: vec![], @@ -467,17 +517,18 @@ mod tests { }; let segment_index = 0_usize; let proof = Proof { - signature: Signature::default(), - sha_state: Hash::default(), segment_index, - }; - let mut checked_proof = CheckedProof { - proof: proof.clone(), - status: ProofStatus::Valid, + ..Proof::default() }; // account has no space - process_validation(&mut account, segment_index, &proof, &checked_proof).unwrap_err(); + store_validation_result( + &Pubkey::default(), + &mut account, + segment_index, + &vec![ProofStatus::default(); 1], + ) + .unwrap_err(); account .account @@ -485,10 +536,8 @@ mod tests { .resize(STORAGE_ACCOUNT_SPACE as usize, 0); let storage_contract = &mut account.account.state().unwrap(); if let StorageContract::Uninitialized = storage_contract { - let mut proof_map = HashMap::new(); - proof_map.insert(proof.sha_state, proof.clone()); let mut proofs = HashMap::new(); - proofs.insert(0, proof_map); + proofs.insert(0, vec![proof.clone()]); *storage_contract = StorageContract::ReplicatorStorage { owner: Pubkey::default(), proofs, @@ -498,11 +547,21 @@ mod tests { account.account.set_state(storage_contract).unwrap(); // proof is valid - process_validation(&mut account, segment_index, &proof, &checked_proof).unwrap(); + store_validation_result( + &Pubkey::default(), + &mut account, + segment_index, + &vec![ProofStatus::Valid], + ) + .unwrap(); - checked_proof.status = ProofStatus::NotValid; - - // proof failed verification - process_validation(&mut account, segment_index, &proof, &checked_proof).unwrap_err(); + // proof failed verification but we should still be able to store it + store_validation_result( + &Pubkey::default(), + &mut account, + segment_index, + &vec![ProofStatus::NotValid], + ) + .unwrap(); } } diff --git a/programs/storage_api/src/storage_instruction.rs b/programs/storage_api/src/storage_instruction.rs index bf650c468..3f15c784f 100644 --- a/programs/storage_api/src/storage_instruction.rs +++ b/programs/storage_api/src/storage_instruction.rs @@ -1,5 +1,5 @@ use crate::id; -use crate::storage_contract::{CheckedProof, STORAGE_ACCOUNT_SPACE}; +use crate::storage_contract::{ProofStatus, STORAGE_ACCOUNT_SPACE}; use serde_derive::{Deserialize, Serialize}; use solana_sdk::hash::Hash; use solana_sdk::instruction::{AccountMeta, Instruction}; @@ -7,7 +7,6 @@ use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::Signature; use solana_sdk::syscall::current; use solana_sdk::system_instruction; -use std::collections::HashMap; #[derive(Serialize, Deserialize, Debug, Clone)] pub enum StorageInstruction { @@ -27,6 +26,7 @@ pub enum StorageInstruction { sha_state: Hash, segment_index: usize, signature: Signature, + blockhash: Hash, }, AdvertiseStorageRecentBlockhash { hash: Hash, @@ -37,13 +37,49 @@ pub enum StorageInstruction { /// Expects 1 Account: /// 0 - Storage account with credits to redeem /// 1 - MiningPool account to redeem credits from + /// 2 - Replicator account to credit - this account *must* be the owner ClaimStorageReward, ProofValidation { + /// The segment during which this proof was generated segment: u64, - proofs: Vec<(Pubkey, Vec)>, + /// A Vec of proof masks per keyed replicator account loaded by the instruction + proofs: Vec>, }, } +fn get_ratios() -> (u64, u64) { + // max number bytes available for account metas and proofs + // The maximum transaction size is == `PACKET_DATA_SIZE` (1232 bytes) + // There are approx. 900 bytes left over after the storage instruction is wrapped into + // a signed transaction. + static MAX_BYTES: u64 = 900; + let account_meta_size: u64 = + bincode::serialized_size(&AccountMeta::new(Pubkey::new_rand(), false)).unwrap_or(0); + let proof_size: u64 = bincode::serialized_size(&ProofStatus::default()).unwrap_or(0); + + // the ratio between account meta size and a single proof status + let ratio = (account_meta_size + proof_size - 1) / proof_size; + let bytes = (MAX_BYTES + ratio - 1) / ratio; + (ratio, bytes) +} + +/// Returns how many accounts and their proofs will fit in a single proof validation tx +/// +/// # Arguments +/// +/// * `proof_mask_max` - The largest proof mask across all accounts intended for submission +/// +pub fn validation_account_limit(proof_mask_max: usize) -> u64 { + let (ratio, bytes) = get_ratios(); + // account_meta_count * (ratio + proof_mask_max) = bytes + bytes / (ratio + proof_mask_max as u64) +} + +pub fn proof_mask_limit() -> u64 { + let (ratio, bytes) = get_ratios(); + bytes - ratio +} + pub fn create_validator_storage_account( from_pubkey: &Pubkey, storage_owner: &Pubkey, @@ -118,11 +154,13 @@ pub fn mining_proof( sha_state: Hash, segment_index: usize, signature: Signature, + blockhash: Hash, ) -> Instruction { let storage_instruction = StorageInstruction::SubmitMiningProof { sha_state, segment_index, signature, + blockhash, }; let account_metas = vec![ AccountMeta::new(*storage_pubkey, true), @@ -147,26 +185,42 @@ pub fn advertise_recent_blockhash( Instruction::new(id(), &storage_instruction, account_metas) } -pub fn proof_validation( +pub fn proof_validation( storage_pubkey: &Pubkey, segment: u64, - checked_proofs: HashMap, S>, + checked_proofs: Vec<(Pubkey, Vec)>, ) -> Instruction { let mut account_metas = vec![AccountMeta::new(*storage_pubkey, true)]; let mut proofs = vec![]; checked_proofs.into_iter().for_each(|(id, p)| { - proofs.push((id, p)); + proofs.push(p); account_metas.push(AccountMeta::new(id, false)) }); let storage_instruction = StorageInstruction::ProofValidation { segment, proofs }; Instruction::new(id(), &storage_instruction, account_metas) } -pub fn claim_reward(storage_pubkey: &Pubkey, mining_pool_pubkey: &Pubkey) -> Instruction { +pub fn claim_reward( + owner_pubkey: &Pubkey, + storage_pubkey: &Pubkey, + mining_pool_pubkey: &Pubkey, +) -> Instruction { let storage_instruction = StorageInstruction::ClaimStorageReward; let account_metas = vec![ AccountMeta::new(*storage_pubkey, false), AccountMeta::new(*mining_pool_pubkey, false), + AccountMeta::new(*owner_pubkey, false), ]; Instruction::new(id(), &storage_instruction, account_metas) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_size() { + // check that if there's 50 proof per account, only 1 account can fit in a single tx + assert_eq!(validation_account_limit(50), 1); + } +} diff --git a/programs/storage_api/src/storage_processor.rs b/programs/storage_api/src/storage_processor.rs index c68484727..eb9aeec2c 100644 --- a/programs/storage_api/src/storage_processor.rs +++ b/programs/storage_api/src/storage_processor.rs @@ -17,7 +17,7 @@ pub fn process_instruction( let (me, rest) = keyed_accounts.split_at_mut(1); let me_unsigned = me[0].signer_key().is_none(); - let mut storage_account = StorageAccount::new(&mut me[0].account); + let mut storage_account = StorageAccount::new(*me[0].unsigned_key(), &mut me[0].account); match bincode::deserialize(data).map_err(|_| InstructionError::InvalidInstructionData)? { StorageInstruction::InitializeMiningPool => { @@ -42,13 +42,20 @@ pub fn process_instruction( sha_state, segment_index, signature, + blockhash, } => { if me_unsigned || rest.len() != 1 { // This instruction must be signed by `me` Err(InstructionError::InvalidArgument)?; } let current = Current::from(&rest[0].account).unwrap(); - storage_account.submit_mining_proof(sha_state, segment_index, signature, current.slot) + storage_account.submit_mining_proof( + sha_state, + segment_index, + signature, + blockhash, + current.slot, + ) } StorageInstruction::AdvertiseStorageRecentBlockhash { hash, slot } => { if me_unsigned || rest.len() != 1 { @@ -59,21 +66,27 @@ pub fn process_instruction( storage_account.advertise_storage_recent_blockhash(hash, slot, current.slot) } StorageInstruction::ClaimStorageReward => { - if rest.len() != 1 { + if rest.len() != 2 { Err(InstructionError::InvalidArgument)?; } - storage_account.claim_storage_reward(&mut rest[0]) + let (mining_pool, owner) = rest.split_at_mut(1); + let mut owner = StorageAccount::new(*owner[0].unsigned_key(), &mut owner[0].account); + + storage_account.claim_storage_reward(&mut mining_pool[0], &mut owner) } StorageInstruction::ProofValidation { segment, proofs } => { if me_unsigned || rest.is_empty() { // This instruction must be signed by `me` and `rest` cannot be empty Err(InstructionError::InvalidArgument)?; } + let me_id = storage_account.id; let mut rest: Vec<_> = rest .iter_mut() - .map(|keyed_account| StorageAccount::new(&mut keyed_account.account)) + .map(|keyed_account| { + StorageAccount::new(*keyed_account.unsigned_key(), &mut keyed_account.account) + }) .collect(); - storage_account.proof_validation(segment, proofs, &mut rest) + storage_account.proof_validation(&me_id, segment, proofs, &mut rest) } } } diff --git a/programs/storage_program/tests/storage_processor.rs b/programs/storage_program/tests/storage_processor.rs index 0ff6c1819..c590171b8 100644 --- a/programs/storage_program/tests/storage_processor.rs +++ b/programs/storage_program/tests/storage_processor.rs @@ -14,11 +14,11 @@ use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil, Signature}; use solana_sdk::syscall::current; use solana_sdk::syscall::current::Current; +use solana_sdk::system_instruction; use solana_sdk::timing::DEFAULT_TICKS_PER_SLOT; use solana_storage_api::storage_contract::StorageAccount; use solana_storage_api::storage_contract::{ - CheckedProof, Proof, ProofStatus, StorageContract, STORAGE_ACCOUNT_SPACE, - TOTAL_REPLICATOR_REWARDS, TOTAL_VALIDATOR_REWARDS, + ProofStatus, StorageContract, REPLICATOR_REWARD, STORAGE_ACCOUNT_SPACE, VALIDATOR_REWARD, }; use solana_storage_api::storage_instruction; use solana_storage_api::storage_processor::process_instruction; @@ -108,13 +108,19 @@ fn test_proof_bounds() { ..Account::default() }; { - let mut storage_account = StorageAccount::new(&mut account); + let mut storage_account = StorageAccount::new(pubkey, &mut account); storage_account .initialize_replicator_storage(account_owner) .unwrap(); } - let ix = storage_instruction::mining_proof(&pubkey, Hash::default(), 0, Signature::default()); + let ix = storage_instruction::mining_proof( + &pubkey, + Hash::default(), + 0, + Signature::default(), + Hash::default(), + ); // the proof is for segment 0, need to move the slot into segment 2 let mut current_account = current::create_account(1); Current::to( @@ -167,7 +173,13 @@ fn test_invalid_accounts_len() { let pubkey = Pubkey::new_rand(); let mut accounts = [Account::default()]; - let ix = storage_instruction::mining_proof(&pubkey, Hash::default(), 0, Signature::default()); + let ix = storage_instruction::mining_proof( + &pubkey, + Hash::default(), + 0, + Signature::default(), + Hash::default(), + ); // move tick height into segment 1 let mut current_account = current::create_account(1); Current::to( @@ -194,7 +206,13 @@ fn test_submit_mining_invalid_slot() { accounts[0].data.resize(STORAGE_ACCOUNT_SPACE as usize, 0); accounts[1].data.resize(STORAGE_ACCOUNT_SPACE as usize, 0); - let ix = storage_instruction::mining_proof(&pubkey, Hash::default(), 0, Signature::default()); + let ix = storage_instruction::mining_proof( + &pubkey, + Hash::default(), + 0, + Signature::default(), + Hash::default(), + ); // submitting a proof for a slot in the past, so this should fail assert!(test_instruction(&ix, &mut accounts).is_err()); @@ -208,13 +226,19 @@ fn test_submit_mining_ok() { let mut account = Account::default(); account.data.resize(STORAGE_ACCOUNT_SPACE as usize, 0); { - let mut storage_account = StorageAccount::new(&mut account); + let mut storage_account = StorageAccount::new(pubkey, &mut account); storage_account .initialize_replicator_storage(account_owner) .unwrap(); } - let ix = storage_instruction::mining_proof(&pubkey, Hash::default(), 0, Signature::default()); + let ix = storage_instruction::mining_proof( + &pubkey, + Hash::default(), + 0, + Signature::default(), + Hash::default(), + ); // move slot into segment 1 let mut current_account = current::create_account(1); Current::to( @@ -235,11 +259,13 @@ fn test_submit_mining_ok() { #[test] fn test_validate_mining() { solana_logger::setup(); - let (mut genesis_block, mint_keypair) = create_genesis_block(1000); + let (mut genesis_block, mint_keypair) = create_genesis_block(100_000); genesis_block .native_instruction_processors .push(solana_storage_program::solana_storage_program!()); let mint_pubkey = mint_keypair.pubkey(); + // 1 owner for all replicator and validator accounts for the test + let owner_pubkey = Pubkey::new_rand(); let replicator_1_storage_keypair = Keypair::new(); let replicator_1_storage_id = replicator_1_storage_keypair.pubkey(); @@ -258,6 +284,7 @@ fn test_validate_mining() { let bank_client = BankClient::new_shared(&bank); init_storage_accounts( + &owner_pubkey, &bank_client, &mint_keypair, &[&validator_storage_id], @@ -267,7 +294,7 @@ fn test_validate_mining() { let message = Message::new(storage_instruction::create_mining_pool_account( &mint_pubkey, &mining_pool_pubkey, - 100, + 10_000, )); bank_client.send_message(&[&mint_keypair], message).unwrap(); @@ -342,7 +369,7 @@ fn test_validate_mining() { vec![storage_instruction::proof_validation( &validator_storage_id, proof_segment as u64, - checked_proofs, + checked_proofs.into_iter().map(|entry| entry).collect(), )], Some(&mint_pubkey), ); @@ -378,6 +405,7 @@ fn test_validate_mining() { let message = Message::new_with_payer( vec![storage_instruction::claim_reward( + &owner_pubkey, &validator_storage_id, &mining_pool_pubkey, )], @@ -385,8 +413,8 @@ fn test_validate_mining() { ); assert_matches!(bank_client.send_message(&[&mint_keypair], message), Ok(_)); assert_eq!( - bank_client.get_balance(&validator_storage_id).unwrap(), - 10 + (TOTAL_VALIDATOR_REWARDS * 10) + bank_client.get_balance(&owner_pubkey).unwrap(), + 1 + (VALIDATOR_REWARD * 10) ); // tick the bank into the next storage epoch so that rewards can be claimed @@ -401,15 +429,23 @@ fn test_validate_mining() { let message = Message::new_with_payer( vec![storage_instruction::claim_reward( + &owner_pubkey, &replicator_1_storage_id, &mining_pool_pubkey, )], Some(&mint_pubkey), ); + assert_matches!(bank_client.send_message(&[&mint_keypair], message), Ok(_)); + assert_eq!( + bank_client.get_balance(&owner_pubkey).unwrap(), + 1 + (VALIDATOR_REWARD * 10) + (REPLICATOR_REWARD * 5) + ); + let message = Message::new_with_payer( vec![storage_instruction::claim_reward( + &owner_pubkey, &replicator_2_storage_id, &mining_pool_pubkey, )], @@ -417,37 +453,44 @@ fn test_validate_mining() { ); assert_matches!(bank_client.send_message(&[&mint_keypair], message), Ok(_)); - // TODO enable when rewards are working assert_eq!( - bank_client.get_balance(&replicator_1_storage_id).unwrap(), - 10 + (TOTAL_REPLICATOR_REWARDS * 5) + bank_client.get_balance(&owner_pubkey).unwrap(), + 1 + (VALIDATOR_REWARD * 10) + (REPLICATOR_REWARD * 10) ); } fn init_storage_accounts( + owner: &Pubkey, client: &BankClient, mint: &Keypair, validator_accounts_to_create: &[&Pubkey], replicator_accounts_to_create: &[&Pubkey], lamports: u64, ) { - let mut ixs: Vec<_> = validator_accounts_to_create - .into_iter() - .flat_map(|account| { - storage_instruction::create_validator_storage_account( - &mint.pubkey(), - &Pubkey::default(), - account, - lamports, - ) - }) - .collect(); + let mut ixs: Vec<_> = vec![system_instruction::create_user_account( + &mint.pubkey(), + owner, + 1, + )]; + ixs.append( + &mut validator_accounts_to_create + .into_iter() + .flat_map(|account| { + storage_instruction::create_validator_storage_account( + &mint.pubkey(), + owner, + account, + lamports, + ) + }) + .collect(), + ); replicator_accounts_to_create .into_iter() .for_each(|account| { ixs.append(&mut storage_instruction::create_replicator_storage_account( &mint.pubkey(), - &Pubkey::default(), + owner, account, lamports, )) @@ -481,7 +524,7 @@ fn submit_proof( storage_keypair: &Keypair, bank_client: &BankClient, segment_index: u64, -) -> CheckedProof { +) -> ProofStatus { let sha_state = Hash::new(Pubkey::new_rand().as_ref()); let message = Message::new_with_payer( vec![storage_instruction::mining_proof( @@ -489,6 +532,7 @@ fn submit_proof( sha_state, segment_index as usize, Signature::default(), + bank_client.get_recent_blockhash().unwrap().0, )], Some(&mint_keypair.pubkey()), ); @@ -497,14 +541,7 @@ fn submit_proof( bank_client.send_message(&[&mint_keypair, &storage_keypair], message), Ok(_) ); - CheckedProof { - proof: Proof { - signature: Signature::default(), - sha_state, - segment_index: segment_index as usize, - }, - status: ProofStatus::Valid, - } + ProofStatus::Valid } fn get_storage_blockhash(client: &C, account: &Pubkey) -> Hash { @@ -585,6 +622,7 @@ fn test_bank_storage() { Hash::default(), slot, Signature::default(), + bank_client.get_recent_blockhash().unwrap().0, )], Some(&mint_pubkey), ); diff --git a/replicator/src/main.rs b/replicator/src/main.rs index 39751322f..a4ef18923 100644 --- a/replicator/src/main.rs +++ b/replicator/src/main.rs @@ -2,8 +2,8 @@ use clap::{crate_description, crate_name, crate_version, App, Arg}; use solana::cluster_info::{Node, FULLNODE_PORT_RANGE}; use solana::contact_info::ContactInfo; use solana::replicator::Replicator; -use solana::socketaddr; use solana_sdk::signature::{read_keypair, Keypair, KeypairUtil}; +use std::net::SocketAddr; use std::process::exit; use std::sync::Arc; @@ -77,7 +77,8 @@ fn main() { .unwrap(); let gossip_addr = { - let mut addr = socketaddr!([127, 0, 0, 1], 8700); + let ip = solana_netutil::get_public_ip_addr(&entrypoint_addr).unwrap(); + let mut addr = SocketAddr::new(ip, 0); addr.set_ip(solana_netutil::get_public_ip_addr(&entrypoint_addr).unwrap()); addr }; diff --git a/wallet/src/wallet.rs b/wallet/src/wallet.rs index b10b32281..8c302c5c5 100644 --- a/wallet/src/wallet.rs +++ b/wallet/src/wallet.rs @@ -58,7 +58,7 @@ pub enum WalletCommand { CreateStorageMiningPoolAccount(Pubkey, u64), CreateReplicatorStorageAccount(Pubkey, Pubkey), CreateValidatorStorageAccount(Pubkey, Pubkey), - ClaimStorageReward(Pubkey, Pubkey), + ClaimStorageReward(Pubkey, Pubkey, Pubkey), ShowStorageAccount(Pubkey), Deploy(String), GetTransactionCount, @@ -299,10 +299,12 @@ pub fn parse_command( )) } ("claim-storage-reward", Some(matches)) => { + let node_account_pubkey = value_of(matches, "node_account_pubkey").unwrap(); let storage_mining_pool_account_pubkey = value_of(matches, "storage_mining_pool_account_pubkey").unwrap(); let storage_account_pubkey = value_of(matches, "storage_account_pubkey").unwrap(); Ok(WalletCommand::ClaimStorageReward( + node_account_pubkey, storage_mining_pool_account_pubkey, storage_account_pubkey, )) @@ -734,12 +736,14 @@ fn process_create_validator_storage_account( fn process_claim_storage_reward( rpc_client: &RpcClient, config: &WalletConfig, + node_account_pubkey: &Pubkey, storage_mining_pool_account_pubkey: &Pubkey, storage_account_pubkey: &Pubkey, ) -> ProcessResult { let (recent_blockhash, _fee_calculator) = rpc_client.get_recent_blockhash()?; let instruction = storage_instruction::claim_reward( + node_account_pubkey, storage_account_pubkey, storage_mining_pool_account_pubkey, ); @@ -763,7 +767,7 @@ fn process_show_storage_account( format!("Unable to deserialize storage account: {:?}", err).to_string(), ) })?; - println!("{:?}", storage_contract); + println!("{:#?}", storage_contract); println!("account lamports: {}", account.lamports); Ok("".to_string()) } @@ -1126,11 +1130,13 @@ pub fn process_command(config: &WalletConfig) -> ProcessResult { } WalletCommand::ClaimStorageReward( + node_account_pubkey, storage_mining_pool_account_pubkey, storage_account_pubkey, ) => process_claim_storage_reward( &rpc_client, config, + node_account_pubkey, &storage_mining_pool_account_pubkey, &storage_account_pubkey, ), @@ -1591,8 +1597,17 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' SubCommand::with_name("claim-storage-reward") .about("Redeem storage reward credits") .arg( - Arg::with_name("storage_mining_pool_account_pubkey") + Arg::with_name("node_account_pubkey") .index(1) + .value_name("NODE PUBKEY") + .takes_value(true) + .required(true) + .validator(is_pubkey) + .help("The node account to credit the rewards to"), + ) + .arg( + Arg::with_name("storage_mining_pool_account_pubkey") + .index(2) .value_name("MINING POOL PUBKEY") .takes_value(true) .required(true) @@ -1601,21 +1616,13 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' ) .arg( Arg::with_name("storage_account_pubkey") - .index(2) + .index(3) .value_name("STORAGE ACCOUNT PUBKEY") .takes_value(true) .required(true) .validator(is_pubkey) .help("Storage account address to redeem credits for"), - ) - .arg( - Arg::with_name("slot") - .index(3) - .value_name("SLOT") - .takes_value(true) - .required(true) - .help("The slot to claim rewards for"), - ),) + )) .subcommand( SubCommand::with_name("show-storage-account")