Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: zero copy vec #1469

Merged
merged 14 commits into from
Jan 12, 2025
Merged

refactor: zero copy vec #1469

merged 14 commits into from
Jan 12, 2025

Conversation

ananas-block
Copy link
Contributor

@ananas-block ananas-block commented Jan 9, 2025

Changes:

  • light-zero-copy
    • removed unsafe code, WrappedMutPointer, WrappedPointer
    • use the zerocopy crate
    • combine traits into ZeroCopyTraits
    • enabled optional padding with const PAD: bool in ZeroCopyVec, ZeroCopyMutSlice and ZeroCopyCyclicVec
    • optional padding allows to work with unaligned bytes by deriving zerocopy::Unaligned for L & T
  • cleanup
  • batched-merkle-tree
    • replaced ZeroCopyVecUsize with ZeroCopyVecU64 (usize is no longer supported)
    • BatchMetadata in MerkleTreeAccount & QueueAccount refactor WrappedMutPointer -> zerocopy::Ref<&mut [u8], T>
      • implemented conversion From<u64> for BatchState
      • derive zerocopy traits for Batch (KnownLayout, Immutable, IntoBytes, FromBytes)
      • Batch replaced BatchState with u64 (to satisfy zerocopy traits)
      • Batch replaced bloom_filter_is_wiped bool with u8 and added getters and setters which return bool (zerocopy traits don't support bool)
  • merkle-tree-metadata
    • derive zerocopy traits
    • use light_utils::pubkey::Pubkey instead of solana_program::pubkey:.Pubkey for zero copy compatibility
  • light_utils
    • implement Pubkey with derived zerocopy traits, solana_program::pubkey::Pubkey and anchor_lang::prelude::Pubkey conversions
  • account-compression program
    • trait GroupAccess return Pubkey instead of &Pubkey necessary so that we can convert light_utils::pubkey::Pubkey to anchor::prelude::Pubkey (conversion is necessary because we need to derive zerocopy traits)
    • moved association check into light-batched-merkle-trees for batch_append, insert-into-queues,
  • light-system-program
    • check_program_owner_state_merkle_tree added generic condition to distinguish between append and nullify as an additional safeguard in case that invalid accounts are passed
    • refactored invoke/processor.rs
      • reading an account or address succeeds if the address does not exist or the account does exist prior to the transaction
        (prior reading an account that is modified in the same tx would fail, same for reading an address which is created in the same tx)
      • fixed and simplified condition to enter proof verification conditional (prior read-only accounts proven by index required a zkp in the same instruction even if reading the account didn't require an inclusion zkp)
      • introduce check_vec_capacity to check vector capacity (it's basically free in terms of CU)

@ananas-block ananas-block force-pushed the jorrit/experiment-zerocopy-vec branch 3 times, most recently from 7c38f05 to 5e0c6de Compare January 9, 2025 23:46

/// `ZeroCopyVec` is a custom vector implementation which forbids
/// post-initialization reallocations. The size is not known during compile
/// time (that makes it different from arrays), but can be defined only once
/// (that makes it different from [`Vec`](std::vec::Vec)).
pub struct ZeroCopyVec<LEN, T>
pub struct ZeroCopyVec<'a, L, T, const PAD: bool = true>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub struct ZeroCopyVec<'a, L, T, const PAD: bool = true>
#[repr(C)]
pub struct ZeroCopyVec<'a, L, T, const PAD: bool = true>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed all #[repr(C)] in this crate since the structs ZeroCopySliceMut, ZeroCopyVec, and ZeroCopyCyclicVec are not saved in bytes but just contain references in runtime.
zerocopy::Ref doesn't derive #[repr(C)] either see docs.

@ananas-block ananas-block marked this pull request as draft January 10, 2025 18:27
@ananas-block ananas-block force-pushed the jorrit/experiment-zerocopy-vec branch 2 times, most recently from f2ce3e8 to 3f216d8 Compare January 10, 2025 19:07
@ananas-block ananas-block marked this pull request as ready for review January 10, 2025 22:30
@ananas-block ananas-block force-pushed the jorrit/experiment-zerocopy-vec branch from 3f216d8 to c3f5680 Compare January 10, 2025 22:32
@ananas-block ananas-block force-pushed the jorrit/experiment-zerocopy-vec branch from b950843 to 63e0a85 Compare January 10, 2025 23:31
@ananas-block ananas-block force-pushed the jorrit/experiment-zerocopy-vec branch from 63e0a85 to 8dcceea Compare January 11, 2025 17:28
Comment on lines +525 to +542
// // 15. functional - proof by index for account which is invalidated in the same tx
// {
// perform_create_pda_with_event(
// &mut e2e_env.indexer,
// &mut e2e_env.rpc,
// &env,
// &payer,
// seed,
// &data,
// &ID,
// None,
// Some(vec![account_in_value_array.clone()]),
// CreatePdaMode::ReadOnlyProofOfInsertedAccount,
// )
// .await
// .unwrap();
// }
println!("post 15");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?

Comment on lines +624 to +689
// Doesn't yield the expected result due to unclean test setup
// // 18. functional - 1 read only account by zkp 3 regular inputs && failing - zkp for invalidated account
// {
// let seed = [254u8; 32];
// let data = [5u8; 31];
// let mut input_accounts = Vec::new();
// let compressed_accounts = e2e_env.indexer.get_compressed_accounts_by_owner(&ID);
// for i in 0..100 {
// let input_account_in_mt = compressed_accounts.iter().find(|x| {
// x.merkle_context.leaf_index == i
// && x.merkle_context.merkle_tree_pubkey == env.batched_state_merkle_tree
// && x.merkle_context.leaf_index
// != account_not_in_value_array_and_in_mt
// .merkle_context
// .leaf_index
// });
// if let Some(input_account_in_mt) = input_account_in_mt.clone() {
// input_accounts.push(input_account_in_mt.clone());
// }
// if input_accounts.len() == 3 {
// break;
// }
// }
// let result = perform_create_pda_with_event(
// &mut e2e_env.indexer,
// &mut e2e_env.rpc,
// &env,
// &payer,
// seed,
// &data,
// &ID,
// Some(input_accounts),
// Some(vec![account_not_in_value_array_and_in_mt.clone()]),
// CreatePdaMode::ReadOnlyZkpOfInsertedAccount,
// )
// .await;
// assert_rpc_error(
// result,
// 1,
// SystemProgramError::ReadOnlyAccountDoesNotExist.into(),
// )
// .unwrap();
// }
// // 12. failing - zkp for invalidated account
// {
// let result = perform_create_pda_with_event(
// &mut e2e_env.indexer,
// &mut e2e_env.rpc,
// &env,
// &payer,
// seed,
// &data,
// &ID,
// Some(vec![account_not_in_value_array_and_in_mt.clone()]),
// Some(vec![account_not_in_value_array_and_in_mt.clone()]),
// CreatePdaMode::ReadOnlyZkpOfInsertedAccount,
// )
// .await;
// assert_rpc_error(
// result,
// 1,
// SystemProgramError::ReadOnlyAccountDoesNotExist.into(),
// )
// .unwrap();
// }
// println!("post 12");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?

Comment on lines +794 to +813
// // The transaction inserts the address first, then checks read only addresses.
// let result = perform_create_pda_with_event(
// &mut test_indexer,
// &mut rpc,
// &env,
// &payer,
// seed,
// &data,
// &ID,
// None,
// None,
// CreatePdaMode::ReadOnlyProofOfInsertedAddress,
// )
// .await;
// assert_rpc_error(
// result,
// 0,
// SystemProgramError::ReadOnlyAddressAlreadyExists.into(),
// )
// .unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?

@sergeytimoshin sergeytimoshin self-requested a review January 12, 2025 16:34
@sergeytimoshin sergeytimoshin merged commit a92f42d into main Jan 12, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants