This repository was archived by the owner on Jan 22, 2025. It is now read-only.
Add SystemInstruction::CreateAccountWithSeed #7390
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,55 @@ | ||
| use log::*; | ||
|
|
||
| use solana_sdk::{ | ||
| account::KeyedAccount, | ||
| instruction::InstructionError, | ||
| instruction_processor_utils::{limited_deserialize, next_keyed_account}, | ||
| pubkey::Pubkey, | ||
| system_instruction::{ | ||
| create_address_with_seed, SystemError, SystemInstruction, MAX_PERMITTED_DATA_LENGTH, | ||
| }, | ||
| system_program, sysvar, | ||
| }; | ||
|
|
||
| fn create_account_with_seed( | ||
| from: &mut KeyedAccount, | ||
| to: &mut KeyedAccount, | ||
| seed: &str, | ||
| lamports: u64, | ||
| data_length: u64, | ||
| program_id: &Pubkey, | ||
| ) -> Result<(), InstructionError> { | ||
| // `from` is the source of the derived address, the caller must have | ||
| // signed, even if no lamports will be transferred | ||
| if from.signer_key().is_none() { | ||
| debug!("CreateAccountWithSeed: from must sign"); | ||
| return Err(InstructionError::MissingRequiredSignature); | ||
| } | ||
|
|
||
| // re-derive the address, must match `to` | ||
| let address = create_address_with_seed(from.unsigned_key(), seed, program_id)?; | ||
|
|
||
| if to.unsigned_key() != &address { | ||
| debug!( | ||
| "CreateAccountWithSeed: invalid argument; generated {} does not match to {}", | ||
| address, | ||
| to.unsigned_key() | ||
| ); | ||
| return Err(SystemError::AddressWithSeedMismatch.into()); | ||
| } | ||
|
|
||
| // all of finish_create_account's rules apply | ||
| finish_create_account(from, to, lamports, data_length, program_id) | ||
| } | ||
|
|
||
| fn create_account( | ||
| from: &mut KeyedAccount, | ||
| to: &mut KeyedAccount, | ||
| lamports: u64, | ||
| data_length: u64, | ||
| program_id: &Pubkey, | ||
| ) -> Result<(), InstructionError> { | ||
| // if lamports == 0, the `from` account isn't touched | ||
| if lamports != 0 && from.signer_key().is_none() { | ||
| debug!("CreateAccount: from must sign"); | ||
| return Err(InstructionError::MissingRequiredSignature); | ||
|
|
@@ -28,7 +60,17 @@ fn create_system_account( | |
| return Err(InstructionError::MissingRequiredSignature); | ||
| } | ||
|
|
||
| finish_create_account(from, to, lamports, data_length, program_id) | ||
| } | ||
|
|
||
| fn finish_create_account( | ||
| from: &mut KeyedAccount, | ||
| to: &mut KeyedAccount, | ||
| lamports: u64, | ||
| data_length: u64, | ||
| program_id: &Pubkey, | ||
| ) -> Result<(), InstructionError> { | ||
| // if it looks like the `to` account is already in use, bail | ||
| if to.account.lamports != 0 | ||
| || !to.account.data.is_empty() | ||
| || !system_program::check_id(&to.account.owner) | ||
|
|
@@ -61,11 +103,18 @@ fn create_system_account( | |
| return Err(SystemError::InvalidAccountDataLength.into()); | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Not calling
Contributor
Author
There was a problem hiding this comment. that check is already present in this change was intentional note that no tests needed to change, we're still requiring |
||
| // guard against sysvars being assigned | ||
| if sysvar::check_id(&program_id) { | ||
| debug!("Assign: program id {} invalid", program_id); | ||
| return Err(SystemError::InvalidProgramId.into()); | ||
| } | ||
| to.account.owner = *program_id; | ||
|
|
||
| from.account.lamports -= lamports; | ||
| to.account.lamports += lamports; | ||
| to.account.data = vec![0; data_length as usize]; | ||
| to.account.executable = false; | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
|
|
@@ -134,7 +183,17 @@ pub fn process_instruction( | |
| } => { | ||
| let from = next_keyed_account(keyed_accounts_iter)?; | ||
| let to = next_keyed_account(keyed_accounts_iter)?; | ||
| create_account(from, to, lamports, space, &program_id) | ||
| } | ||
| SystemInstruction::CreateAccountWithSeed { | ||
| seed, | ||
| lamports, | ||
| space, | ||
| program_id, | ||
| } => { | ||
| let from = next_keyed_account(keyed_accounts_iter)?; | ||
| let to = next_keyed_account(keyed_accounts_iter)?; | ||
| create_account_with_seed(from, to, &seed, lamports, space, &program_id) | ||
| } | ||
| SystemInstruction::Assign { program_id } => { | ||
| let account = next_keyed_account(keyed_accounts_iter)?; | ||
|
|
@@ -163,38 +222,120 @@ mod tests { | |
| use solana_sdk::transaction::TransactionError; | ||
|
|
||
| #[test] | ||
| fn test_create_account() { | ||
| let new_program_owner = Pubkey::new(&[9; 32]); | ||
| let from = Pubkey::new_rand(); | ||
| let to = Pubkey::new_rand(); | ||
| let mut from_account = Account::new(100, 0, &system_program::id()); | ||
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
|
|
||
| assert_eq!( | ||
| process_instruction( | ||
| &Pubkey::default(), | ||
| &mut [ | ||
| KeyedAccount::new(&from, true, &mut from_account), | ||
| KeyedAccount::new(&to, true, &mut to_account) | ||
| ], | ||
| &bincode::serialize(&SystemInstruction::CreateAccount { | ||
| lamports: 50, | ||
| space: 2, | ||
| program_id: new_program_owner | ||
| }) | ||
| .unwrap() | ||
| ), | ||
| Ok(()) | ||
| ); | ||
| assert_eq!(from_account.lamports, 50); | ||
| assert_eq!(to_account.lamports, 50); | ||
| assert_eq!(to_account.owner, new_program_owner); | ||
| assert_eq!(to_account.data, [0, 0]); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_create_account_with_seed() { | ||
| let new_program_owner = Pubkey::new(&[9; 32]); | ||
| let from = Pubkey::new_rand(); | ||
| let seed = "shiny pepper"; | ||
| let to = create_address_with_seed(&from, seed, &new_program_owner).unwrap(); | ||
|
|
||
| let mut from_account = Account::new(100, 0, &system_program::id()); | ||
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
|
|
||
| assert_eq!( | ||
| process_instruction( | ||
| &Pubkey::default(), | ||
| &mut [ | ||
| KeyedAccount::new(&from, true, &mut from_account), | ||
| KeyedAccount::new(&to, false, &mut to_account) | ||
| ], | ||
| &bincode::serialize(&SystemInstruction::CreateAccountWithSeed { | ||
| seed: seed.to_string(), | ||
| lamports: 50, | ||
| space: 2, | ||
| program_id: new_program_owner | ||
| }) | ||
| .unwrap() | ||
| ), | ||
| Ok(()) | ||
| ); | ||
| assert_eq!(from_account.lamports, 50); | ||
| assert_eq!(to_account.lamports, 50); | ||
| assert_eq!(to_account.owner, new_program_owner); | ||
| assert_eq!(to_account.data, [0, 0]); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_create_account_with_seed_mismatch() { | ||
| let new_program_owner = Pubkey::new(&[9; 32]); | ||
| let from = Pubkey::new_rand(); | ||
| let seed = "dull boy"; | ||
| let to = Pubkey::new_rand(); | ||
|
|
||
| let mut from_account = Account::new(100, 0, &system_program::id()); | ||
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
|
|
||
| assert_eq!( | ||
| create_account_with_seed( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to, false, &mut to_account), | ||
| seed, | ||
| 50, | ||
| 2, | ||
| &new_program_owner, | ||
| ), | ||
| Err(SystemError::AddressWithSeedMismatch.into()) | ||
| ); | ||
| assert_eq!(from_account.lamports, 100); | ||
| assert_eq!(to_account, Account::default()); | ||
| } | ||
| #[test] | ||
| fn test_create_account_with_seed_missing_sig() { | ||
| let new_program_owner = Pubkey::new(&[9; 32]); | ||
| let from = Pubkey::new_rand(); | ||
| let seed = "dull boy"; | ||
| let to = create_address_with_seed(&from, seed, &new_program_owner).unwrap(); | ||
|
|
||
| let mut from_account = Account::new(100, 0, &system_program::id()); | ||
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
|
|
||
| assert_eq!( | ||
| create_account_with_seed( | ||
| &mut KeyedAccount::new(&from, false, &mut from_account), | ||
| &mut KeyedAccount::new(&to, false, &mut to_account), | ||
| seed, | ||
| 50, | ||
| 2, | ||
| &new_program_owner, | ||
| ), | ||
| Err(InstructionError::MissingRequiredSignature) | ||
| ); | ||
| assert_eq!(from_account.lamports, 100); | ||
| assert_eq!(to_account, Account::default()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_create_with_zero_lamports() { | ||
| // create account with zero lamports tranferred | ||
| let new_program_owner = Pubkey::new(&[9; 32]); | ||
| let from = Pubkey::new_rand(); | ||
| let mut from_account = Account::new(100, 0, &Pubkey::new_rand()); // not from system account | ||
|
|
@@ -203,7 +344,7 @@ mod tests { | |
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
|
|
||
| assert_eq!( | ||
| create_account( | ||
| &mut KeyedAccount::new(&from, false, &mut from_account), // no signer | ||
| &mut KeyedAccount::new(&to, true, &mut to_account), | ||
| 0, | ||
|
|
@@ -234,7 +375,7 @@ mod tests { | |
| let mut to_account = Account::new(0, 0, &Pubkey::default()); | ||
| let unchanged_account = to_account.clone(); | ||
|
|
||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to, true, &mut to_account), | ||
| 150, | ||
|
|
@@ -255,7 +396,7 @@ mod tests { | |
| let to_account_key = Pubkey::new_rand(); | ||
|
|
||
| // Trying to request more data length than permitted will result in failure | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from_account_key, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to_account_key, true, &mut to_account), | ||
| 50, | ||
|
|
@@ -269,7 +410,7 @@ mod tests { | |
| ); | ||
|
|
||
| // Trying to request equal or less data length than permitted will be successful | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from_account_key, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to_account_key, true, &mut to_account), | ||
| 50, | ||
|
|
@@ -293,7 +434,7 @@ mod tests { | |
| let mut owned_account = Account::new(0, 0, &original_program_owner); | ||
| let unchanged_account = owned_account.clone(); | ||
|
|
||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&owned_key, true, &mut owned_account), | ||
| 50, | ||
|
|
@@ -308,7 +449,7 @@ mod tests { | |
|
|
||
| let mut owned_account = Account::new(10, 0, &Pubkey::default()); | ||
| let unchanged_account = owned_account.clone(); | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&owned_key, true, &mut owned_account), | ||
| 50, | ||
|
|
@@ -333,7 +474,7 @@ mod tests { | |
| let unchanged_account = owned_account.clone(); | ||
|
|
||
| // Haven't signed from account | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, false, &mut from_account), | ||
| &mut KeyedAccount::new(&owned_key, true, &mut owned_account), | ||
| 50, | ||
|
|
@@ -345,7 +486,7 @@ mod tests { | |
| assert_eq!(owned_account, unchanged_account); | ||
|
|
||
| // Haven't signed to account | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&owned_key, false, &mut owned_account), | ||
| 50, | ||
|
|
@@ -357,7 +498,7 @@ mod tests { | |
| assert_eq!(owned_account, unchanged_account); | ||
|
|
||
| // support creation/assignment with zero lamports (ephemeral account) | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, false, &mut from_account), | ||
| &mut KeyedAccount::new(&owned_key, true, &mut owned_account), | ||
| 0, | ||
|
|
@@ -379,7 +520,7 @@ mod tests { | |
| let mut to_account = Account::default(); | ||
|
|
||
| // fail to create a sysvar::id() owned account | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to, true, &mut to_account), | ||
| 50, | ||
|
|
@@ -392,7 +533,7 @@ mod tests { | |
| let mut to_account = Account::default(); | ||
|
|
||
| // fail to create an account with a sysvar id | ||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&to, true, &mut to_account), | ||
| 50, | ||
|
|
@@ -419,7 +560,7 @@ mod tests { | |
| }; | ||
| let unchanged_account = populated_account.clone(); | ||
|
|
||
| let result = create_account( | ||
| &mut KeyedAccount::new(&from, true, &mut from_account), | ||
| &mut KeyedAccount::new(&populated_key, true, &mut populated_account), | ||
| 50, | ||
|
|
@@ -447,9 +588,13 @@ mod tests { | |
| ); | ||
|
|
||
| assert_eq!( | ||
| process_instruction( | ||
| &Pubkey::default(), | ||
| &mut [KeyedAccount::new(&from, true, &mut from_account)], | ||
| &bincode::serialize(&SystemInstruction::Assign { | ||
| program_id: new_program_owner | ||
| }) | ||
| .unwrap() | ||
| ), | ||
| Ok(()) | ||
| ); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for the restriction to 32B ASCII string?
[u8; 32]could support some internationalization and hash digestsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah good point, we don't care that the seed is valid utf-8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to keep the space smaller than 256 bytes of seed to deter grinding attacks. 32 ascii chars takes away 32 bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rob-solana grinding on what though?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trying to DoS a guy you think wants his authorized staker+"shrill conveyance 23/50"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rob-solana you can always grind with pubkey generation. I think we should either have a Hash/[u8;32], or u64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requiring
from's signature effectively makes griefing the address space a second-preimage attack on SHA256. We should probably be OK with that. Though fewer bits do minimize opportunities for shenanigans. Would it make sense to restrict the seed to something as small as au16?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
were it as small as a u16, mnemonics get harder. ascii is nice for mnemonics (in English). opening to utf-8 would make it look less amateur-hour.
The grinding attack is picking another source
from(obs having the keypair), and griefing some other dude'sfrom. The goal is to make that griefing look hard or futile.I think we can do this with:
We can also say "who cares? grind away!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm planning to remove the ascii stuff, appreciate input here @t-nelson @aeyakovenko @garious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the
u16suggestion would discard the mnemonic for an index (fromgetsIaccounts underprogram_id). Which gives cheap introspection, but no pseudonymity.Yep. A SHA256 second preimage search is pretty futile IMO. Restricting free bits per
fromis about the most effective deterrent we can enact. Grindingfroms is always possible and slower than the hashing step. You've already covered the other speedup. That is, requiring fixed bits at the end of the input data, preventing working from a midstate.I think (valid) utf-8 would actually be more restricted. For non-ASCII code points, the top two bits of every byte are fixed and the first byte also consumes up to three more bits to encode the length. Not all code pages are in use or full either, further restricting valid bitstrings
Probably fine. See, https://lbc.cryptoguru.org/about