Skip to content

Commit

Permalink
fix: proof by index check in system program
Browse files Browse the repository at this point in the history
  • Loading branch information
ananas-block committed Dec 9, 2024
1 parent 157aaee commit 9ba56ac
Show file tree
Hide file tree
Showing 15 changed files with 230 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@ pub fn process_insert_into_queues<'a, 'b, 'c: 'info, 'info, MerkleTreeAccount: O
// and pay rollover fees only once.
let mut current_index = 0;
for (index, element) in elements.iter().enumerate() {
msg!("Processing element {}", index);
msg!("remaining_accounts {:?}", ctx.remaining_accounts);
msg!("elements {:?}", elements);
msg!("tx_hash {:?}", tx_hash);
msg!("current_index {:?}", current_index);

let current_account_discriminator = ctx
.remaining_accounts
.get(current_index)
Expand Down
3 changes: 2 additions & 1 deletion programs/account-compression/src/state/batched_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ impl ZeroCopyBatchedQueueAccount {

pub fn could_exist_in_batches(&mut self, leaf_index: u64) -> Result<()> {
for batch in self.batches.iter() {
if batch.value_is_inserted_in_batch(leaf_index)? {
let res = batch.value_is_inserted_in_batch(leaf_index)?;
if res {
return Ok(());
}
}
Expand Down
18 changes: 1 addition & 17 deletions programs/system/src/invoke/emit_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,11 @@ use crate::{
pub fn emit_state_transition_event<'a, 'b, 'c: 'info, 'info, A: InvokeAccounts<'info> + Bumps>(
inputs: InstructionDataInvoke,
ctx: &'a Context<'a, 'b, 'c, 'info, A>,
mut input_compressed_account_hashes: Vec<[u8; 32]>,
input_compressed_account_hashes: Vec<[u8; 32]>,
output_compressed_account_hashes: Vec<[u8; 32]>,
output_leaf_indices: Vec<u32>,
sequence_numbers: Vec<MerkleTreeSequenceNumber>,
) -> Result<()> {
msg!(
"input_compressed_account_hashes: {:?}",
input_compressed_account_hashes
);
let mut num_removed_values = 0;
// Do not include read-only accounts in the event.
for (i, account) in inputs
.input_compressed_accounts_with_merkle_context
.iter()
.enumerate()
{
if account.read_only {
input_compressed_account_hashes.remove(i - num_removed_values);
num_removed_values += 1;
}
}
// Note: message is unimplemented
// (if we compute the tx hash in indexer we don't need to modify the event.)
let event = PublicTransactionEvent {
Expand Down
5 changes: 0 additions & 5 deletions programs/system/src/invoke/nullify_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ pub fn insert_nullifiers<
// used.
let mut network_fee_bundle = None;
for account in input_compressed_accounts_with_merkle_context.iter() {
msg!("nullify state: account: {:?}", account);
// Don't nullify read-only accounts.
if account.read_only {
continue;
}
leaf_indices.push(account.merkle_context.leaf_index);

let account_info =
Expand Down
19 changes: 11 additions & 8 deletions programs/system/src/invoke/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ pub fn process<
bench_sbf_end!("cpda_hash_input_compressed_accounts");
// Insert addresses into address merkle tree queue ---------------------------------------------------
let address_network_fee_bundle = if num_new_addresses != 0 {
msg!("inputs.new_address_params {:?}", inputs.new_address_params);
derive_new_addresses(
&invoking_program,
&inputs.new_address_params,
Expand Down Expand Up @@ -211,7 +210,6 @@ pub fn process<
// Access the current slot
let current_slot = Clock::get()?.slot;
let tx_hash = create_tx_hash(
&inputs.input_compressed_accounts_with_merkle_context,
&input_compressed_account_hashes,
&output_compressed_account_hashes,
current_slot,
Expand All @@ -238,6 +236,14 @@ pub fn process<
output_network_fee_bundle,
)?;

// Verify that all instances of queue_index.is_some() are plausible.
if num_prove_by_index_input_accounts != 0 {
verify_input_accounts_proof_by_index(
ctx.remaining_accounts,
&inputs.input_compressed_accounts_with_merkle_context,
)?;
}

// Proof inputs order:
// 1. input compressed accounts
// 2. read only compressed accounts
Expand All @@ -255,10 +261,7 @@ pub fn process<
b: proof.b,
c: proof.c,
};
verify_input_accounts_proof_by_index(
&ctx.remaining_accounts,
&inputs.input_compressed_accounts_with_merkle_context,
)?;

let mut input_compressed_account_roots =
Vec::with_capacity(num_input_compressed_accounts);
fetch_input_compressed_account_roots(
Expand All @@ -273,7 +276,7 @@ pub fn process<
&ctx,
&mut read_only_accounts_roots,
)?;
verify_read_only_account_inclusion(&ctx.remaining_accounts, &read_only_accounts)?;
verify_read_only_account_inclusion(ctx.remaining_accounts, &read_only_accounts)?;

fetch_roots_address_merkle_tree(
&inputs.new_address_params,
Expand All @@ -282,7 +285,7 @@ pub fn process<
&mut new_address_roots,
)?;
verify_read_only_address_queue_non_inclusion(
&ctx.remaining_accounts,
ctx.remaining_accounts,
&read_only_addresses,
)?;

Expand Down
9 changes: 4 additions & 5 deletions programs/system/src/invoke/sum_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@ pub fn sum_check(
{
num_prove_by_index_accounts += 1;
}
// Readonly accounts are not included in the sum check, since these are
// not invalidated in this transaction.
// Readonly accounts are only supported as separate inputs.
if compressed_account_with_context.read_only {
unimplemented!("read_only accounts are not supported. Set read_only to false.");
// num_read_only += 1;
// continue;
unimplemented!(
"Read accounts are only supported as separate inputs in the invoke_cpi_with_read_only instruction. Set read_only to false."
);
}
sum = sum
.checked_add(compressed_account_with_context.compressed_account.lamports)
Expand Down
108 changes: 13 additions & 95 deletions programs/system/src/invoke/verify_state_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ pub fn fetch_roots_address_merkle_tree<
ctx: &'a Context<'a, 'b, 'c, 'info, A>,
roots: &'a mut Vec<[u8; 32]>,
) -> Result<()> {
msg!("fetch_roots_address_merkle_tree");
for new_address_param in new_address_params.iter() {
fetch_address_root::<false, A>(
ctx,
Expand Down Expand Up @@ -146,9 +145,7 @@ pub fn verify_input_accounts_proof_by_index<'a>(
output_queue_account_info,
)
.map_err(ProgramError::from)?;
output_queue
.could_exist_in_batches(account.merkle_context.leaf_index as u64)
.map_err(|_| SystemProgramError::ReadOnlyAccountDoesNotExist)?;
output_queue.could_exist_in_batches(account.merkle_context.leaf_index as u64)?;
}
}
Ok(())
Expand Down Expand Up @@ -238,33 +235,6 @@ pub fn verify_read_only_address_queue_non_inclusion<'a>(
Ok(())
}

// #[inline(always)]
// pub fn verify_read_only_account<'a>(
// remaining_accounts: &'a [AccountInfo<'_>],
// accounts: &'a [PackedCompressedAccountWithMerkleContext],
// input_compressed_account_hashes: &'a [[u8; 32]],
// ) -> Result<()> {
// for (i, account) in accounts.iter().enumerate() {
// if account.read_only && account.merkle_context.queue_index.is_some() {
// msg!("verify_read_only_account");
// msg!("merkle_context: {:?}", account.merkle_context);
// let output_queue_account_info =
// &remaining_accounts[account.merkle_context.nullifier_queue_pubkey_index as usize];

// let output_queue =
// &mut ZeroCopyBatchedQueueAccount::output_queue_from_account_info_mut(
// output_queue_account_info,
// )
// .map_err(ProgramError::from)?;
// output_queue.prove_inclusion_by_index(
// account.merkle_context.leaf_index as u64,
// &input_compressed_account_hashes[i],
// )?;
// }
// }
// Ok(())
// }

fn fetch_address_root<
'a,
'b,
Expand All @@ -278,7 +248,6 @@ fn fetch_address_root<
address_merkle_tree_root_index: u16,
roots: &'a mut Vec<[u8; 32]>,
) -> Result<()> {
msg!("fetch_address_root");
let merkle_tree_account_info =
&ctx.remaining_accounts[address_merkle_tree_account_index as usize];
let mut discriminator_bytes = [0u8; 8];
Expand All @@ -301,7 +270,7 @@ fn fetch_address_root<
}
BatchedMerkleTreeAccount::DISCRIMINATOR => {
let merkle_tree = ZeroCopyBatchedMerkleTreeAccount::address_tree_from_account_info_mut(
&merkle_tree_account_info,
merkle_tree_account_info,
)
.map_err(ProgramError::from)?;
(*roots).push(merkle_tree.root_history[address_merkle_tree_root_index as usize]);
Expand Down Expand Up @@ -343,19 +312,15 @@ pub fn hash_input_compressed_accounts<'a, 'b, 'c: 'info, 'info>(
.iter()
.enumerate()
{
// Skip read-only accounts. Read-only accounts are just included in
// proof verification, but since these accounts are not invalidated the
// address and lamports must not be used in sum and address checks.
if !input_compressed_account_with_context.read_only {
// For heap neutrality we cannot allocate new heap memory in this function.
match &input_compressed_account_with_context
.compressed_account
.address
{
Some(address) => addresses[j] = Some(*address),
None => {}
};
}
// For heap neutrality we cannot allocate new heap memory in this function.
match &input_compressed_account_with_context
.compressed_account
.address
{
Some(address) => addresses[j] = Some(*address),
None => {}
};

#[allow(clippy::comparison_chain)]
if current_mt_index
!= input_compressed_account_with_context
Expand Down Expand Up @@ -430,6 +395,7 @@ pub fn hash_input_compressed_accounts<'a, 'b, 'c: 'info, 'info>(
Ok(())
}

#[allow(clippy::too_many_arguments)]
#[heap_neutral]
pub fn verify_state_proof(
input_compressed_accounts_with_merkle_context: &[PackedCompressedAccountWithMerkleContext],
Expand All @@ -453,18 +419,14 @@ pub fn verify_state_proof(
})
.map(|x| *x.1)
.collect::<Vec<[u8; 32]>>();
msg!("proof_input_leaves: {:?}", proof_input_leaves);

read_only_accounts.iter().for_each(|x| {
if x.merkle_context.queue_index.is_none() {
proof_input_leaves.extend_from_slice(&[x.account_hash]);
}
});
msg!("roots: {:?}", roots);

roots.extend_from_slice(read_only_roots);
msg!("proof_input_leaves: {:?}", proof_input_leaves);
msg!("roots: {:?}", roots);

if !addresses.is_empty() && !proof_input_leaves.is_empty() {
verify_create_addresses_and_merkle_proof_zkp(
&roots,
Expand All @@ -485,55 +447,11 @@ pub fn verify_state_proof(
}

pub fn create_tx_hash(
input_compressed_accounts_with_merkle_context: &[PackedCompressedAccountWithMerkleContext],
input_compressed_account_hashes: &[[u8; 32]],
output_compressed_account_hashes: &[[u8; 32]],
current_slot: u64,
) -> [u8; 32] {
use light_hasher::Hasher;
// Do not include read-only accounts in the event.
let index = find_first_non_read_only_account(input_compressed_accounts_with_merkle_context);
// TODO: extend with message hash (first 32 bytes of the message)
let mut tx_hash = input_compressed_account_hashes[index];
for (i, hash) in input_compressed_account_hashes
.iter()
.skip(index + 1)
.enumerate()
{
if input_compressed_accounts_with_merkle_context[i].read_only {
continue;
}
tx_hash = Poseidon::hashv(&[&tx_hash, hash]).unwrap();
}
tx_hash = Poseidon::hashv(&[&tx_hash, &current_slot.to_be_bytes()]).unwrap();
for hash in output_compressed_account_hashes.iter() {
tx_hash = Poseidon::hashv(&[&tx_hash, hash]).unwrap();
}
tx_hash
}

fn find_first_non_read_only_account(
input_compressed_accounts_with_merkle_context: &[PackedCompressedAccountWithMerkleContext],
) -> usize {
for (i, account) in input_compressed_accounts_with_merkle_context
.iter()
.enumerate()
{
if !account.read_only {
return i;
}
}
0
}

pub fn create_tx_hash_offchain(
input_compressed_account_hashes: &[[u8; 32]],
output_compressed_account_hashes: &[[u8; 32]],
current_slot: u64,
) -> [u8; 32] {
use light_hasher::Hasher;
// Do not include read-only accounts in the event.
// TODO: extend with message hash (first 32 bytes of the message)
let mut tx_hash = input_compressed_account_hashes[0];
for hash in input_compressed_account_hashes.iter().skip(1) {
tx_hash = Poseidon::hashv(&[&tx_hash, hash]).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions programs/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub mod light_system_program {
process_invoke_cpi(ctx, inputs, None, None)
}

pub fn invoke_cpi_with_read_only_address<'a, 'b, 'c: 'info, 'info>(
pub fn invoke_cpi_with_read_only<'a, 'b, 'c: 'info, 'info>(
ctx: Context<'a, 'b, 'c, 'info, InvokeCpiInstruction<'info>>,
inputs: Vec<u8>,
) -> Result<()> {
Expand All @@ -91,7 +91,7 @@ pub mod light_system_program {
// disable set cpi context because cpi context account uses InvokeCpiInstruction
if let Some(cpi_context) = inputs.invoke_cpi.cpi_context {
if cpi_context.set_context {
msg!("Cannot set cpi context in invoke_cpi_with_read_only_address.");
msg!("Cannot set cpi context in invoke_cpi_with_read_only.");
msg!("Please use invoke_cpi instead.");
return Err(SystemProgramError::InstructionNotCallable.into());
}
Expand Down
6 changes: 1 addition & 5 deletions programs/system/src/sdk/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ pub fn create_invoke_instruction(
is_compress: bool,
decompression_recipient: Option<Pubkey>,
sort: bool,
is_read_only: &[bool],
) -> Instruction {
let (remaining_accounts, mut inputs_struct) =
create_invoke_instruction_data_and_remaining_accounts(
Expand All @@ -50,7 +49,6 @@ pub fn create_invoke_instruction(
proof,
compress_or_decompress_lamports,
is_compress,
is_read_only,
);
if sort {
inputs_struct
Expand Down Expand Up @@ -95,7 +93,6 @@ pub fn create_invoke_instruction_data_and_remaining_accounts(
proof: Option<CompressedProof>,
compress_or_decompress_lamports: Option<u64>,
is_compress: bool,
is_read_only: &[bool],
) -> (Vec<AccountMeta>, InstructionDataInvoke) {
let mut remaining_accounts = HashMap::<Pubkey, usize>::new();
let mut _input_compressed_accounts: Vec<PackedCompressedAccountWithMerkleContext> =
Expand Down Expand Up @@ -138,7 +135,7 @@ pub fn create_invoke_instruction_data_and_remaining_accounts(
leaf_index: context.leaf_index,
queue_index,
},
read_only: is_read_only[i],
read_only: false,
root_index: root_index.unwrap_or_default(),
});
}
Expand Down Expand Up @@ -309,7 +306,6 @@ mod test {
true,
None,
true,
&vec![false; input_compressed_accounts.len()],
);
assert_eq!(instruction.program_id, crate::ID);

Expand Down
Loading

0 comments on commit 9ba56ac

Please sign in to comment.