Programs were not spawned by SystemProgram #1533
Conversation
src/system_program.rs
Outdated
| SystemProgram::Spawn => { | ||
| if !accounts[0].executable || accounts[0].loader_program_id != Pubkey::default() | ||
| { | ||
| Err(Error::AssignOfUnownedAccount)?; |
There was a problem hiding this comment.
AssignOfUnownedAccount: emitting an Assign error from Spawn seems is a little confusing to me
src/bank.rs
Outdated
| // return Err(BankError::ModifiedContractId(instruction_index as u8)); | ||
|
|
||
| // Make sure that program_id is still the same or this was just assigned by the system call contract | ||
| if !(*pre_program_id == account.program_id || SystemProgram::check_id(&tx_program_id)) { |
There was a problem hiding this comment.
Because this logic enforces that the only way for an account's program_id to be changed is if it starts the transaction owned by the SystemProgram. In the case of programs, the Accounts come into the Spawn assigned to the loader.
There was a problem hiding this comment.
The negated logic got me, I find this much more readable myself:
| if !(*pre_program_id == account.program_id || SystemProgram::check_id(&tx_program_id)) { | |
| if *pre_program_id != account.program_id && !SystemProgram::check_id(&tx_program_id) { |
There was a problem hiding this comment.
Sounds good, was keeping the rest of the original logic.
src/native_loader.rs
Outdated
| } | ||
| } else if let Ok(instruction) = deserialize(tx_data) { | ||
| match instruction { | ||
| LoaderInstruction::Write { offset, bytes } => { | ||
| trace!("NativeLoader::Write offset {} bytes {:?}", offset, bytes); | ||
| if keyed_accounts[0].account.executable { |
There was a problem hiding this comment.
This check can be removed right? The bank should enforce that the account is read-only
There was a problem hiding this comment.
How do you propose the loader determine if this is a dispatch or a write/finalize? (This is where call came in handy)
There was a problem hiding this comment.
Why does the loader care either way here? If the bank enforces that an account marked executable is read-only then Write doesn’t need to check for executable (or rather if it doesn’t check then no big deal) as the bank would reject the transaction during it’s final verification phase.
There was a problem hiding this comment.
Sounds good, is that enforcement already in the bank?
…solana-labs#1533) * replace `source`, `destination`, and `auditor` variable names in sigma proofs * replace `source`, `destination`, and `auditor` variable names in elgamal program
No description provided.