feat: zero copy macro #1851
feat: zero copy macro #1851
Conversation
WalkthroughThis update introduces a new procedural macro crate, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RustStruct
participant MacroDerive
participant ZeroCopyCrate
User->>RustStruct: Define struct with #[derive(ZeroCopy, ZeroCopyMut, ZeroCopyEq)]
RustStruct->>MacroDerive: Macro expansion during compilation
MacroDerive->>RustStruct: Generate zero-copy wrapper(s) and trait impls
User->>ZeroCopyCrate: Serialize struct with Borsh
User->>ZeroCopyCrate: Deserialize bytes as zero-copy reference/mutable
ZeroCopyCrate->>RustStruct: Provide zero-copy access to fields
User->>RustStruct: (Optionally) Convert zero-copy view back to owned struct
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
♻️ Duplicate comments (1)
program-libs/zero-copy/src/borsh.rs (1)
125-126: Security: Add validation to prevent memory exhaustionThis TODO identifies the same DoS vector as in borsh_mut.rs - malicious input could cause excessive memory allocation.
🧹 Nitpick comments (17)
.gitignore (1)
88-91: Consider making the ignore pattern recursive
expand.rswill be ignored only at the repository root.
If the macro expansion ever produces the same file name inside a nested crate (e.g.target/expand.rsorprogram-libs/zero-copy-derive/expand.rs), it will still be tracked.- expand.rs + **/expand.rsA glob keeps the intent while covering future placements at no extra cost.
.github/workflows/rust.yml (1)
56-58: Use--all-featuresor drop the spaces after commas for cleaner CLICargo accepts both comma- and space-separated feature lists, but the current mix (
"std, mut, derive") yields tokens with leading whitespace that must be trimmed internally. Two cleaner options:- cargo test -p light-zero-copy --features "std, mut, derive" + cargo test -p light-zero-copy --all-featuresor
- cargo test -p light-zero-copy --features "std, mut, derive" + cargo test -p light-zero-copy --features "std,mut,derive"Either improves readability and sidesteps any future string-processing surprises.
program-libs/zero-copy/Cargo.toml (1)
14-16: Feature namemutmay invite tooling frictionA bare feature called
mutis perfectly legal in Cargo, yet:
- It collides with the Rust keyword in logs and search queries.
- Shell auto-completion and some editors colour-highlight it oddly.
- Quoting is required whenever it is referenced from bash‐style scripts (
cargo build --features "mut").Renaming to something explicit such as
mut-zcorzero-copy-mutavoids these annoyances while remaining semver-compatible via thealiasesmechanism if necessary.No blocker—just a polish suggestion.
program-libs/zero-copy-derive/Cargo.toml (1)
27-27: Remove trailing empty line-zerocopy = { workspace = true, features = ["derive"] } - +zerocopy = { workspace = true, features = ["derive"] }program-libs/zero-copy-derive/src/shared/meta_struct.rs (1)
21-34: Simplify attribute handling logicThe current logic creates an unnecessary empty vector when
hasheris false. Consider simplifying this.let meta_fields_with_converted_types = meta_fields.iter().map(|field| { let field_name = &field.ident; - let attributes = if hasher { - field - .attrs - .iter() - .map(|attr| { - quote! { #attr } - }) - .collect::<Vec<_>>() - } else { - vec![quote! {}] - }; + let attributes = if hasher { + field.attrs.iter().map(|attr| quote! { #attr }).collect::<Vec<_>>() + } else { + vec![] + }; let field_type = convert_to_zerocopy_type(&field.ty); quote! { #(#attributes)* pub #field_name: #field_type } });program-libs/zero-copy-derive/tests/from_test.rs (1)
2-2: Remove unnecessary Vec import.
Vecis part of Rust's prelude and doesn't need to be explicitly imported.-use std::vec::Vec; -program-libs/zero-copy-derive/src/zero_copy_eq.rs (1)
109-120: Address the TODO for proper Option field comparison.The TODO indicates that different field types require different dereference patterns for comparison. This incomplete implementation could cause compilation errors.
This TODO suggests that the Option comparison logic needs type-specific handling. Would you like me to implement a proper solution that handles different dereference patterns based on the field type?
program-libs/zero-copy-derive/src/shared/z_struct.rs (1)
295-300: Remove commented-out code.This commented-out Bool field type handling appears to be obsolete since bool is now handled as a Primitive type. Consider removing it to keep the code clean.
- // FieldType::Bool(field_name) => { - // quote! { - // #(#attributes)* - // pub #field_name: <bool as #trait_name<'a>>::Output - // } - // }program-libs/zero-copy-derive/src/zero_copy.rs (1)
197-564: Remove or restore commented test codeThis large block of commented test code should either be removed if obsolete or uncommented if still needed.
Consider moving these tests to a separate test file if they're still valuable, or remove them entirely to improve code maintainability.
program-libs/zero-copy/src/borsh_mut.rs (1)
18-19: Consider the TODO: Rename Output typeThe TODO suggests renaming
OutputtoZeroCopyfor better clarity. This would be a breaking change but might improve API consistency.Would you like me to help create a migration plan for this rename across the codebase?
program-libs/zero-copy/src/borsh.rs (2)
350-352: Remove incomplete commented codeThis appears to be an incomplete function stub that should be removed or completed.
Either implement the hash function or remove these lines to keep the code clean.
311-867: Consider moving test module to separate fileThis 550+ line test module makes the main source file quite large.
Consider moving these tests to
tests/borsh_manual.rsor similar to improve maintainability and reduce compilation time for the main module.program-libs/zero-copy-derive/src/shared/utils.rs (1)
28-29: Document the cache population side effect.The comment mentions populating the cache, but it's not clear why this is done here rather than when actually needed. Consider clarifying the purpose.
- // Populate the cache by checking if this struct implements Copy + // Populate the cache early to ensure consistent results when this struct is referenced by other structs let _ = struct_implements_copy(input);program-libs/zero-copy/tests/borsh_2.rs (1)
40-42: Remove commented-out code.The commented function stub doesn't provide value and should be removed.
-// pub fn data_hash_struct_1(a: u8, b: u16) -> [u8; 32] { - -// } -program-libs/zero-copy-derive/tests/random.rs (2)
41-174: Consider extracting common population logic.The
populate_invoke_cpi_zero_copyandpopulate_invoke_zero_copyfunctions share significant duplicate code. Consider extracting the common logic into helper functions.Example of extracting common proof population:
fn populate_proof(src: &Option<CompressedProof>, dst: &mut Option<ZCompressedProofMut>) { if let (Some(src_proof), Some(dst_proof)) = (src, dst) { dst_proof.a.copy_from_slice(&src_proof.a); dst_proof.b.copy_from_slice(&src_proof.b); dst_proof.c.copy_from_slice(&src_proof.c); } }
514-514: Consider consistent iteration counts or document the difference.The first test uses 1,000 iterations while the second uses 10,000. If the difference is intentional (e.g., due to complexity), consider adding a comment explaining why.
- let num_iters = 1000; + let num_iters = 1000; // Lower iterations due to selective input/output generationAlso applies to: 605-605
program-libs/zero-copy-derive/tests/instruction_data.rs (1)
27-32: Thread safety concern in new_unique().Using
thread_rng()creates a new RNG instance each time. For better performance in multi-threaded contexts, consider using a thread-local RNG.pub fn new_unique() -> Self { use rand::{thread_rng, Rng}; let mut rng = thread_rng(); let bytes = rng.gen::<[u8; 32]>(); Pubkey(bytes) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.github/workflows/rust.yml(1 hunks).gitignore(1 hunks)Cargo.toml(2 hunks)program-libs/zero-copy-derive/Cargo.toml(1 hunks)program-libs/zero-copy-derive/README.md(1 hunks)program-libs/zero-copy-derive/src/lib.rs(1 hunks)program-libs/zero-copy-derive/src/shared/from_impl.rs(1 hunks)program-libs/zero-copy-derive/src/shared/meta_struct.rs(1 hunks)program-libs/zero-copy-derive/src/shared/mod.rs(1 hunks)program-libs/zero-copy-derive/src/shared/utils.rs(1 hunks)program-libs/zero-copy-derive/src/shared/z_struct.rs(1 hunks)program-libs/zero-copy-derive/src/shared/zero_copy_new.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy_eq.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy_mut.rs(1 hunks)program-libs/zero-copy-derive/test_debug.rs(1 hunks)program-libs/zero-copy-derive/tests/config_test.rs(1 hunks)program-libs/zero-copy-derive/tests/cross_crate_copy.rs(1 hunks)program-libs/zero-copy-derive/tests/from_test.rs(1 hunks)program-libs/zero-copy-derive/tests/instruction_data.rs(1 hunks)program-libs/zero-copy-derive/tests/random.rs(1 hunks)program-libs/zero-copy/Cargo.toml(1 hunks)program-libs/zero-copy/README.md(0 hunks)program-libs/zero-copy/src/borsh.rs(4 hunks)program-libs/zero-copy/src/borsh_mut.rs(1 hunks)program-libs/zero-copy/src/init_mut.rs(1 hunks)program-libs/zero-copy/src/lib.rs(1 hunks)program-libs/zero-copy/src/slice_mut.rs(1 hunks)program-libs/zero-copy/tests/borsh.rs(1 hunks)program-libs/zero-copy/tests/borsh_2.rs(1 hunks)
💤 Files with no reviewable changes (1)
- program-libs/zero-copy/README.md
🧰 Additional context used
🧬 Code Graph Analysis (8)
program-libs/zero-copy-derive/src/shared/mod.rs (3)
program-libs/zero-copy-derive/src/zero_copy_eq.rs (1)
from_impl(259-259)program-libs/zero-copy-derive/src/zero_copy.rs (1)
meta_struct(602-602)program-libs/zero-copy-derive/src/zero_copy_mut.rs (2)
meta_struct(26-26)z_struct(31-31)
program-libs/zero-copy-derive/src/shared/meta_struct.rs (2)
program-libs/zero-copy-derive/src/shared/utils.rs (1)
convert_to_zerocopy_type(170-211)program-libs/zero-copy-derive/src/shared/z_struct.rs (1)
field(180-186)
program-libs/zero-copy-derive/tests/from_test.rs (1)
program-libs/zero-copy/src/borsh.rs (15)
zero_copy_at(20-20)zero_copy_at(27-30)zero_copy_at(36-49)zero_copy_at(56-62)zero_copy_at(69-75)zero_copy_at(113-116)zero_copy_at(122-134)zero_copy_at(162-172)zero_copy_at(377-380)zero_copy_at(435-439)zero_copy_at(492-497)zero_copy_at(574-589)zero_copy_at(630-633)zero_copy_at(667-670)zero_copy_at(745-749)
program-libs/zero-copy/src/init_mut.rs (5)
program-libs/zero-copy/src/lib.rs (1)
size_of(36-36)program-libs/zero-copy/src/borsh.rs (6)
size_of(37-37)size_of(57-57)size_of(60-60)size_of(70-70)size_of(73-73)new(140-142)program-libs/zero-copy/src/borsh_mut.rs (17)
size_of(44-44)zero_copy_at_mut(20-21)zero_copy_at_mut(29-34)zero_copy_at_mut(41-56)zero_copy_at_mut(63-67)zero_copy_at_mut(74-78)zero_copy_at_mut(86-91)zero_copy_at_mut(97-111)zero_copy_at_mut(180-192)zero_copy_at_mut(399-404)zero_copy_at_mut(462-470)zero_copy_at_mut(523-530)zero_copy_at_mut(561-566)zero_copy_at_mut(619-638)zero_copy_at_mut(708-715)zero_copy_at_mut(749-754)new(158-160)program-libs/zero-copy/src/slice_mut.rs (4)
size_of(97-97)size_of(107-107)len(116-119)new(35-37)program-libs/zero-copy-derive/tests/instruction_data.rs (3)
byte_len(72-74)new_zero_copy(76-81)zero_copy_at_mut(52-56)
program-libs/zero-copy-derive/tests/config_test.rs (2)
program-libs/zero-copy/src/init_mut.rs (25)
config(238-241)byte_len(23-23)byte_len(42-51)byte_len(77-79)byte_len(94-96)byte_len(111-113)byte_len(128-130)byte_len(146-148)byte_len(169-171)byte_len(187-189)byte_len(203-205)byte_len(219-221)byte_len(236-242)new_zero_copy(28-31)new_zero_copy(53-69)new_zero_copy(81-87)new_zero_copy(98-104)new_zero_copy(115-121)new_zero_copy(132-138)new_zero_copy(150-156)new_zero_copy(173-179)new_zero_copy(191-196)new_zero_copy(207-212)new_zero_copy(223-228)new_zero_copy(244-264)program-libs/zero-copy/src/borsh_mut.rs (15)
zero_copy_at_mut(20-21)zero_copy_at_mut(29-34)zero_copy_at_mut(41-56)zero_copy_at_mut(63-67)zero_copy_at_mut(74-78)zero_copy_at_mut(86-91)zero_copy_at_mut(97-111)zero_copy_at_mut(180-192)zero_copy_at_mut(399-404)zero_copy_at_mut(462-470)zero_copy_at_mut(523-530)zero_copy_at_mut(561-566)zero_copy_at_mut(619-638)zero_copy_at_mut(708-715)zero_copy_at_mut(749-754)
program-libs/zero-copy/src/borsh_mut.rs (3)
program-libs/zero-copy/src/borsh.rs (17)
new(140-142)deref(147-149)deref(369-371)deref(427-429)deref(484-486)deref(566-568)deref(737-739)deref_mut(153-155)test_vecu8(225-231)test_vecu8_deref(293-301)test_struct_1(384-390)eq(415-421)eq(725-731)eq(812-814)test_struct_2(443-456)test_struct_3(501-516)test_struct_7(753-779)program-libs/zero-copy/src/slice_mut.rs (6)
new(35-37)eq(263-265)len(116-119)from_bytes(56-58)to_vec(178-180)from_bytes_at(61-85)program-libs/zero-copy-derive/src/zero_copy_mut.rs (2)
zero_copy(39-39)zero_copy(44-44)
program-libs/zero-copy-derive/tests/random.rs (4)
js/stateless.js/src/state/types.ts (6)
CompressedAccountData(199-212)CompressedCpiContext(353-368)CompressedProof(373-386)InstructionDataInvoke(277-306)InstructionDataInvokeCpi(311-345)OutputCompressedAccountWithPackedContext(170-173)program-libs/zero-copy-derive/tests/instruction_data.rs (6)
rng(30-30)new_unique(27-32)default(342-348)byte_len(72-74)new_zero_copy(76-81)zero_copy_at(43-45)program-libs/zero-copy/src/init_mut.rs (25)
config(238-241)byte_len(23-23)byte_len(42-51)byte_len(77-79)byte_len(94-96)byte_len(111-113)byte_len(128-130)byte_len(146-148)byte_len(169-171)byte_len(187-189)byte_len(203-205)byte_len(219-221)byte_len(236-242)new_zero_copy(28-31)new_zero_copy(53-69)new_zero_copy(81-87)new_zero_copy(98-104)new_zero_copy(115-121)new_zero_copy(132-138)new_zero_copy(150-156)new_zero_copy(173-179)new_zero_copy(191-196)new_zero_copy(207-212)new_zero_copy(223-228)new_zero_copy(244-264)program-libs/zero-copy/src/borsh.rs (15)
zero_copy_at(20-20)zero_copy_at(27-30)zero_copy_at(36-49)zero_copy_at(56-62)zero_copy_at(69-75)zero_copy_at(113-116)zero_copy_at(122-134)zero_copy_at(162-172)zero_copy_at(377-380)zero_copy_at(435-439)zero_copy_at(492-497)zero_copy_at(574-589)zero_copy_at(630-633)zero_copy_at(667-670)zero_copy_at(745-749)
program-libs/zero-copy/tests/borsh_2.rs (3)
program-libs/zero-copy/src/borsh.rs (3)
eq(415-421)eq(725-731)eq(812-814)program-libs/zero-copy/src/borsh_mut.rs (3)
eq(442-448)eq(809-815)eq(901-903)program-libs/zero-copy/src/slice_mut.rs (4)
eq(263-265)to_vec(178-180)from_bytes_at(61-85)iter(246-248)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: lint
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v1
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test sdk-libs
- GitHub Check: Test program-libs-slow
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
🔇 Additional comments (22)
Cargo.toml (2)
16-17: Workspace member addition looks good
program-libs/zero-copy-deriveis correctly registered as a new workspace member.
No further action required.
171-172: Dependency wire-up verified
light-zero-copy-deriveis added as a workspace dependency; path and version align with the member entry above.
Looks correct.program-libs/zero-copy-derive/src/shared/mod.rs (1)
1-6: Module skeleton is straightforward and correctThe shared hub cleanly exposes the sub-modules and gates
zero_copy_newbehind themutfeature. Compiles fine as-is.program-libs/zero-copy/Cargo.toml (2)
22-23: Optional dependency correctly marked
light-zero-copy-deriveis set asoptional = true, ensuring consumers don’t pull in the proc-macro crate unless they opt intoderive/mut.
Looks good.
27-28: Dev-dependency addition is harmlessAdding
borshfor test coverage matches the new derive tests. No issues spotted.program-libs/zero-copy/src/slice_mut.rs (1)
280-291: LGTM!The
DeserializeMuttrait implementation is correctly feature-gated and properly delegates to the existingfrom_bytes_atmethod. The trait bounds ensure type safety for mutable zero-copy deserialization.program-libs/zero-copy-derive/tests/from_test.rs (1)
35-38: Clarify the byte_len availability issue.The comments state "byte_len not available for non-mut derivations", but this file is compiled with
cfg(feature = "mut"). Either:
- The
byte_lenmethod should be available and these assertions should work, or- There's another condition preventing
byte_lenfrom being available that should be documentedPlease either uncomment and fix these assertions or remove the commented code entirely.
Also applies to: 59-62
program-libs/zero-copy/src/lib.rs (1)
13-30: LGTM! Clean module organization and proper feature gating.The new modules and re-exports are well-organized and correctly feature-gated. The separation between
std-only features and derive-specific features is clear and follows Rust best practices.program-libs/zero-copy-derive/tests/cross_crate_copy.rs (1)
1-51: Well-structured test cases for Copy trait identification.The test structs effectively cover different field ordering scenarios and type combinations to validate the zero-copy derive macro's field classification logic.
program-libs/zero-copy/src/init_mut.rs (2)
6-32: Well-designed trait for configuration-based initialization.The
ZeroCopyNewtrait provides a clean and intuitive interface for initializing zero-copy structures with configuration parameters. The separation ofbyte_lencalculation andnew_zero_copyinitialization is a good design choice.
231-265: Correct Vec implementation with proper length encoding.The implementation properly handles Vec serialization with a 4-byte length prefix followed by elements. The use of individual configs per element provides fine-grained control over initialization.
program-libs/zero-copy-derive/src/shared/z_struct.rs (2)
7-151: Comprehensive field classification system.The
FieldTypeenum and classification functions provide excellent coverage of different field types with appropriate special handling for common patterns likeVec<u8>and integer options. The recursive type analysis is well-structured.
513-628: Excellent fuzz testing coverage.The fuzz test with 10,000 iterations provides strong confidence in the code generation logic. The assertions validate key aspects of the generated structs including proper derives, Deref implementations, and field presence.
program-libs/zero-copy-derive/src/shared/zero_copy_new.rs (1)
102-103: Stale TODO comment – no action neededI couldn’t find a
TODOat lines 102–103 (or elsewhere) inprogram-libs/zero-copy-derive/src/shared/zero_copy_new.rs. It looks like that comment has already been removed. No changes are required to address it.Likely an incorrect or invalid review comment.
program-libs/zero-copy/tests/borsh.rs (1)
1-336: Well-structured test coverageThe test suite provides comprehensive coverage for zero-copy deserialization functionality, including:
- Simple and complex struct scenarios
- Mutable operations verification
- Nested structures and vectors
- Array handling
- Edge cases with options
program-libs/zero-copy/src/borsh.rs (1)
65-76: Verify bool deserialization returns u8The bool type deserializes to u8 rather than bool, which might be unexpected for users.
This design choice should be documented, as it differs from typical bool deserialization expectations. Is this intentional for zero-copy performance?
program-libs/zero-copy-derive/src/shared/utils.rs (2)
300-304: Good handling of array types for Copy trait detection.The recursive check for array element types correctly implements Rust's rule that arrays are Copy if their element type is Copy.
323-325: Good test isolation with cache clearing.Clearing the cache at the start of tests ensures proper test isolation and prevents interference between tests.
program-libs/zero-copy/tests/borsh_2.rs (2)
104-112: Correct endianness handling in PartialEq implementation.The comparison correctly converts the little-endian
U16value to native endianness before comparing with the u16 field.
73-80: Well-structured zero-copy deserialization tests.The tests consistently verify:
- Successful deserialization
- Correct field values including endianness conversion
- All bytes are consumed (remaining slice is empty)
This provides good coverage of the zero-copy deserialization functionality.
Also applies to: 132-146, 190-206
program-libs/zero-copy-derive/tests/random.rs (1)
505-593: Good use of seed-based randomization for reproducible tests.The test prints the seed value which allows reproducing failures. This is a best practice for randomized testing.
program-libs/zero-copy-derive/tests/instruction_data.rs (1)
1149-1160: Correct Option comparison pattern using match expressions.This PartialEq implementation correctly uses match expressions to handle all Option cases, avoiding the unwrap issues seen in other implementations.
Also applies to: 1211-1219
| use borsh::{BorshDeserialize, BorshSerialize}; | ||
| use light_zero_copy_derive::ZeroCopy; | ||
|
|
||
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)] | ||
| pub struct SimpleTest { | ||
| pub id: u64, | ||
| pub active: bool, | ||
| pub values: Vec<u32>, | ||
| } | ||
|
|
||
| fn main() {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Convert to proper test module with assertions
This appears to be a test file based on its path, but it only contains a struct definition without any actual tests. Consider adding test functions to verify the behavior of the ZeroCopy derive macro.
use borsh::{BorshDeserialize, BorshSerialize};
use light_zero_copy_derive::ZeroCopy;
#[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)]
pub struct SimpleTest {
pub id: u64,
pub active: bool,
pub values: Vec<u32>,
}
-fn main() {}
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_zero_copy_derive() {
+ // Add test assertions here to verify ZeroCopy behavior
+ // Example: test serialization/deserialization round-trip
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use borsh::{BorshDeserialize, BorshSerialize}; | |
| use light_zero_copy_derive::ZeroCopy; | |
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)] | |
| pub struct SimpleTest { | |
| pub id: u64, | |
| pub active: bool, | |
| pub values: Vec<u32>, | |
| } | |
| fn main() {} | |
| use borsh::{BorshDeserialize, BorshSerialize}; | |
| use light_zero_copy_derive::ZeroCopy; | |
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)] | |
| pub struct SimpleTest { | |
| pub id: u64, | |
| pub active: bool, | |
| pub values: Vec<u32>, | |
| } | |
| #[cfg(test)] | |
| mod tests { | |
| use super::*; | |
| #[test] | |
| fn test_zero_copy_derive() { | |
| // Add test assertions here to verify ZeroCopy behavior | |
| // Example: test serialization/deserialization round-trip | |
| } | |
| } |
🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/test_debug.rs lines 1 to 11, the file currently
only defines a struct without any test functions. Convert this into a proper
test module by adding a #[cfg(test)] mod with test functions that create
instances of SimpleTest and include assertions to verify the behavior of the
ZeroCopy derive macro, such as serialization and deserialization correctness.
| pub fn derive_zero_copy_mut_impl(fn_input: TokenStream) -> syn::Result<proc_macro2::TokenStream> { | ||
| // Parse the input DeriveInput | ||
| let input: DeriveInput = syn::parse(fn_input.clone())?; | ||
|
|
||
| let hasher = false; | ||
|
|
||
| // Process the input to extract struct information | ||
| let (name, z_struct_name, z_struct_meta_name, fields) = utils::process_input(&input)?; | ||
|
|
||
| // Process the fields to separate meta fields and struct fields | ||
| let (meta_fields, struct_fields) = utils::process_fields(fields); | ||
|
|
||
| let meta_struct_def_mut = if !meta_fields.is_empty() { | ||
| meta_struct::generate_meta_struct::<true>(&z_struct_meta_name, &meta_fields, hasher)? | ||
| } else { | ||
| quote! {} | ||
| }; | ||
|
|
||
| let z_struct_def_mut = z_struct::generate_z_struct::<true>( | ||
| &z_struct_name, | ||
| &z_struct_meta_name, | ||
| &struct_fields, | ||
| &meta_fields, | ||
| hasher, | ||
| )?; | ||
|
|
||
| let zero_copy_struct_inner_impl_mut = zero_copy::generate_zero_copy_struct_inner::<true>( | ||
| name, | ||
| &format_ident!("{}Mut", z_struct_name), | ||
| )?; | ||
|
|
||
| let deserialize_impl_mut = zero_copy::generate_deserialize_impl::<true>( | ||
| name, | ||
| &z_struct_name, | ||
| &z_struct_meta_name, | ||
| &struct_fields, | ||
| meta_fields.is_empty(), | ||
| quote! {}, | ||
| )?; | ||
|
|
||
| // Parse the input DeriveInput | ||
| let input: DeriveInput = syn::parse(fn_input)?; | ||
|
|
||
| // Process the input to extract struct information | ||
| let (name, _z_struct_name, _z_struct_meta_name, fields) = utils::process_input(&input)?; | ||
|
|
||
| // Use the same field processing logic as other derive macros for consistency | ||
| let (meta_fields, struct_fields) = utils::process_fields(fields); | ||
|
|
||
| // Process ALL fields uniformly by type (no position dependency for config generation) | ||
| let all_fields: Vec<&syn::Field> = meta_fields | ||
| .iter() | ||
| .chain(struct_fields.iter()) | ||
| .cloned() | ||
| .collect(); | ||
| let all_field_types = analyze_struct_fields(&all_fields)?; | ||
|
|
||
| // Generate configuration struct based on all fields that need config (type-based) | ||
| let config_struct = generate_config_struct(name, &all_field_types)?; | ||
|
|
||
| // Generate ZeroCopyNew implementation using the existing field separation | ||
| let init_mut_impl = generate_init_mut_impl(name, &meta_fields, &struct_fields)?; |
There was a problem hiding this comment.
Eliminate duplicate parsing and processing
The input is parsed twice and fields are processed twice with identical logic. This is inefficient and makes the code harder to maintain.
pub fn derive_zero_copy_mut_impl(fn_input: TokenStream) -> syn::Result<proc_macro2::TokenStream> {
// Parse the input DeriveInput
let input: DeriveInput = syn::parse(fn_input.clone())?;
let hasher = false;
// Process the input to extract struct information
let (name, z_struct_name, z_struct_meta_name, fields) = utils::process_input(&input)?;
// Process the fields to separate meta fields and struct fields
let (meta_fields, struct_fields) = utils::process_fields(fields);
let meta_struct_def_mut = if !meta_fields.is_empty() {
meta_struct::generate_meta_struct::<true>(&z_struct_meta_name, &meta_fields, hasher)?
} else {
quote! {}
};
let z_struct_def_mut = z_struct::generate_z_struct::<true>(
&z_struct_name,
&z_struct_meta_name,
&struct_fields,
&meta_fields,
hasher,
)?;
let zero_copy_struct_inner_impl_mut = zero_copy::generate_zero_copy_struct_inner::<true>(
name,
&format_ident!("{}Mut", z_struct_name),
)?;
let deserialize_impl_mut = zero_copy::generate_deserialize_impl::<true>(
name,
&z_struct_name,
&z_struct_meta_name,
&struct_fields,
meta_fields.is_empty(),
quote! {},
)?;
- // Parse the input DeriveInput
- let input: DeriveInput = syn::parse(fn_input)?;
-
- // Process the input to extract struct information
- let (name, _z_struct_name, _z_struct_meta_name, fields) = utils::process_input(&input)?;
-
- // Use the same field processing logic as other derive macros for consistency
- let (meta_fields, struct_fields) = utils::process_fields(fields);
-
// Process ALL fields uniformly by type (no position dependency for config generation)
let all_fields: Vec<&syn::Field> = meta_fields
.iter()
.chain(struct_fields.iter())
.cloned()
.collect();
let all_field_types = analyze_struct_fields(&all_fields)?;
// Generate configuration struct based on all fields that need config (type-based)
let config_struct = generate_config_struct(name, &all_field_types)?;
// Generate ZeroCopyNew implementation using the existing field separation
let init_mut_impl = generate_init_mut_impl(name, &meta_fields, &struct_fields)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn derive_zero_copy_mut_impl(fn_input: TokenStream) -> syn::Result<proc_macro2::TokenStream> { | |
| // Parse the input DeriveInput | |
| let input: DeriveInput = syn::parse(fn_input.clone())?; | |
| let hasher = false; | |
| // Process the input to extract struct information | |
| let (name, z_struct_name, z_struct_meta_name, fields) = utils::process_input(&input)?; | |
| // Process the fields to separate meta fields and struct fields | |
| let (meta_fields, struct_fields) = utils::process_fields(fields); | |
| let meta_struct_def_mut = if !meta_fields.is_empty() { | |
| meta_struct::generate_meta_struct::<true>(&z_struct_meta_name, &meta_fields, hasher)? | |
| } else { | |
| quote! {} | |
| }; | |
| let z_struct_def_mut = z_struct::generate_z_struct::<true>( | |
| &z_struct_name, | |
| &z_struct_meta_name, | |
| &struct_fields, | |
| &meta_fields, | |
| hasher, | |
| )?; | |
| let zero_copy_struct_inner_impl_mut = zero_copy::generate_zero_copy_struct_inner::<true>( | |
| name, | |
| &format_ident!("{}Mut", z_struct_name), | |
| )?; | |
| let deserialize_impl_mut = zero_copy::generate_deserialize_impl::<true>( | |
| name, | |
| &z_struct_name, | |
| &z_struct_meta_name, | |
| &struct_fields, | |
| meta_fields.is_empty(), | |
| quote! {}, | |
| )?; | |
| // Parse the input DeriveInput | |
| let input: DeriveInput = syn::parse(fn_input)?; | |
| // Process the input to extract struct information | |
| let (name, _z_struct_name, _z_struct_meta_name, fields) = utils::process_input(&input)?; | |
| // Use the same field processing logic as other derive macros for consistency | |
| let (meta_fields, struct_fields) = utils::process_fields(fields); | |
| // Process ALL fields uniformly by type (no position dependency for config generation) | |
| let all_fields: Vec<&syn::Field> = meta_fields | |
| .iter() | |
| .chain(struct_fields.iter()) | |
| .cloned() | |
| .collect(); | |
| let all_field_types = analyze_struct_fields(&all_fields)?; | |
| // Generate configuration struct based on all fields that need config (type-based) | |
| let config_struct = generate_config_struct(name, &all_field_types)?; | |
| // Generate ZeroCopyNew implementation using the existing field separation | |
| let init_mut_impl = generate_init_mut_impl(name, &meta_fields, &struct_fields)?; | |
| pub fn derive_zero_copy_mut_impl(fn_input: TokenStream) -> syn::Result<proc_macro2::TokenStream> { | |
| // Parse the input DeriveInput | |
| let input: DeriveInput = syn::parse(fn_input.clone())?; | |
| let hasher = false; | |
| // Process the input to extract struct information | |
| let (name, z_struct_name, z_struct_meta_name, fields) = utils::process_input(&input)?; | |
| // Process the fields to separate meta fields and struct fields | |
| let (meta_fields, struct_fields) = utils::process_fields(fields); | |
| let meta_struct_def_mut = if !meta_fields.is_empty() { | |
| meta_struct::generate_meta_struct::<true>(&z_struct_meta_name, &meta_fields, hasher)? | |
| } else { | |
| quote! {} | |
| }; | |
| let z_struct_def_mut = z_struct::generate_z_struct::<true>( | |
| &z_struct_name, | |
| &z_struct_meta_name, | |
| &struct_fields, | |
| &meta_fields, | |
| hasher, | |
| )?; | |
| let zero_copy_struct_inner_impl_mut = zero_copy::generate_zero_copy_struct_inner::<true>( | |
| name, | |
| &format_ident!("{}Mut", z_struct_name), | |
| )?; | |
| let deserialize_impl_mut = zero_copy::generate_deserialize_impl::<true>( | |
| name, | |
| &z_struct_name, | |
| &z_struct_meta_name, | |
| &struct_fields, | |
| meta_fields.is_empty(), | |
| quote! {}, | |
| )?; | |
| // Process ALL fields uniformly by type (no position dependency for config generation) | |
| let all_fields: Vec<&syn::Field> = meta_fields | |
| .iter() | |
| .chain(struct_fields.iter()) | |
| .cloned() | |
| .collect(); | |
| let all_field_types = analyze_struct_fields(&all_fields)?; | |
| // Generate configuration struct based on all fields that need config (type-based) | |
| let config_struct = generate_config_struct(name, &all_field_types)?; | |
| // Generate ZeroCopyNew implementation using the existing field separation | |
| let init_mut_impl = generate_init_mut_impl(name, &meta_fields, &struct_fields)?; | |
| // …rest of the function… | |
| } |
🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/src/zero_copy_mut.rs between lines 13 and 74,
the input TokenStream is parsed twice and the fields are processed twice with
the same logic, causing redundancy and inefficiency. Refactor the function to
parse the input and process the fields only once, then reuse the parsed data and
processed fields for all subsequent operations to eliminate duplication and
improve maintainability.
| 2. `#[derive(ZeroCopyEq)]` - Adds equality comparison support: | ||
| - Compare zero-copy instances with regular struct instances | ||
| - Can be used alongside `ZeroCopy` for complete functionality | ||
| - Derivation for Options<struct> is not robust and may not compile. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the Option type limitation with specific examples.
The documentation mentions that "Derivation for Options is not robust and may not compile" but doesn't explain what patterns fail or why. Please provide specific examples of what works and what doesn't, along with any workarounds.
Consider expanding this line:
- - Derivation for Options<struct> is not robust and may not compile.
+ - Derivation for Options<struct> has limitations:
+ - Option<CustomStruct> may fail to compile due to dereference pattern issues
+ - Option<T> where T implements Copy typically works correctly
+ - Workaround: Consider using a different representation or manual implementation📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Derivation for Options<struct> is not robust and may not compile. | |
| - Derivation for Options<struct> has limitations: | |
| - Option<CustomStruct> may fail to compile due to dereference pattern issues | |
| - Option<T> where T implements Copy typically works correctly | |
| - Workaround: Consider using a different representation or manual implementation |
🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/README.md at line 18, the note about the
limitation of deriving for Options<struct> lacks clarity and examples. Expand
this section by adding specific examples showing which Option patterns compile
successfully and which fail, explaining the reasons behind these failures. Also,
include any known workarounds or alternative approaches to handle these cases to
improve user understanding.
| ### With Equality Comparison | ||
|
|
||
| ```rust | ||
| use borsh::{BorshDeserialize, BorshSerialize}; | ||
| use light_zero_copy_derive::ZeroCopy; | ||
|
|
||
| #[repr(C)] | ||
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)] | ||
| pub struct MyStruct { | ||
| pub a: u8, | ||
| pub b: u16, | ||
| pub vec: Vec<u8>, | ||
| pub c: u64, | ||
| } | ||
| let my_struct = MyStruct { | ||
| a: 1, | ||
| b: 2, | ||
| vec: vec![1u8; 32], | ||
| c: 3, | ||
| }; | ||
| // Use the struct with zero-copy deserialization | ||
| let mut bytes = my_struct.try_to_vec().unwrap(); | ||
| let (zero_copy, _remaining) = MyStruct::zero_copy_at(&bytes).unwrap(); | ||
| assert_eq!(zero_copy, my_struct); | ||
| ``` |
There was a problem hiding this comment.
Fix the equality comparison example to actually demonstrate ZeroCopyEq.
The example titled "With Equality Comparison" doesn't actually demonstrate the ZeroCopyEq derive macro. It's missing the import and derive attribute for ZeroCopyEq.
Apply this diff to fix the example:
use borsh::{BorshDeserialize, BorshSerialize};
-use light_zero_copy_derive::ZeroCopy;
+use light_zero_copy_derive::{ZeroCopy, ZeroCopyEq};
#[repr(C)]
-#[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)]
+#[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy, ZeroCopyEq)]
pub struct MyStruct {
pub a: u8,
pub b: u16,
pub vec: Vec<u8>,
pub c: u64,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### With Equality Comparison | |
| ```rust | |
| use borsh::{BorshDeserialize, BorshSerialize}; | |
| use light_zero_copy_derive::ZeroCopy; | |
| #[repr(C)] | |
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy)] | |
| pub struct MyStruct { | |
| pub a: u8, | |
| pub b: u16, | |
| pub vec: Vec<u8>, | |
| pub c: u64, | |
| } | |
| let my_struct = MyStruct { | |
| a: 1, | |
| b: 2, | |
| vec: vec![1u8; 32], | |
| c: 3, | |
| }; | |
| // Use the struct with zero-copy deserialization | |
| let mut bytes = my_struct.try_to_vec().unwrap(); | |
| let (zero_copy, _remaining) = MyStruct::zero_copy_at(&bytes).unwrap(); | |
| assert_eq!(zero_copy, my_struct); | |
| ``` | |
| use borsh::{BorshDeserialize, BorshSerialize}; | |
| use light_zero_copy_derive::{ZeroCopy, ZeroCopyEq}; | |
| #[repr(C)] | |
| #[derive(Debug, PartialEq, BorshSerialize, BorshDeserialize, ZeroCopy, ZeroCopyEq)] | |
| pub struct MyStruct { | |
| pub a: u8, | |
| pub b: u16, | |
| pub vec: Vec<u8>, | |
| pub c: u64, | |
| } | |
| let my_struct = MyStruct { | |
| a: 1, | |
| b: 2, | |
| vec: vec![1u8; 32], | |
| c: 3, | |
| }; | |
| // Use the struct with zero-copy deserialization | |
| let mut bytes = my_struct.try_to_vec().unwrap(); | |
| let (zero_copy, _remaining) = MyStruct::zero_copy_at(&bytes).unwrap(); | |
| assert_eq!(zero_copy, my_struct); |
🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/README.md between lines 79 and 103, the example
titled "With Equality Comparison" does not demonstrate the ZeroCopyEq derive
macro properly. To fix this, add the import for ZeroCopyEq from
light_zero_copy_derive and include ZeroCopyEq in the derive attribute list for
MyStruct. This will correctly show how to use the ZeroCopyEq macro for equality
comparison in the example.
ananas-block
force-pushed
the
jorrit/refactor-zero-copy-borsh
branch
from
f1e4c46 to
f5870e8
Compare
fix: light-zero-copy tests comment derive mut commented byte len fix: derive macro for non mut pre bytelen refactor: detach bytelen trait stash adding config simple config derive works stash stash new at stash new at Compressed Account stash man InstructionDataInvoke new_zero_copy works stash simple config zero_copy_new tests stash refactor fixed lifetime issue stash instruction data tests work move byte_len to init_mut added randomized tests stash got failing random tests fixed u8 and bool remove bytelen renamed trait fix lint fix tests apply feedback meta_struct use syn to parse options instead of strings primitive types Replace string-based type comparisons with proper syn AST matching replace parse_str with parse_quote replace empty quote with unreachable! add byte len check borsh_vec_u8_as_slice_mut converted unimplemented to panic cleanup redundant as u64 etc fix docs cleanup cleanup commtend code cleanup mut conditionals remove bytelen derive cleanup refactor: replace duplicate code with generate_deserialize_call refactor detecting copy moved to internal refactor: add error handling cleanup cleanup file structure stash wip transform all primitive types to zero copy types simplify analyze_struct_fields fix empty meta struct generation stash zero copy changes unified some with Deserialize::Output unified integer field type enum renam VecNonStaticZeroCopy -> VecDynamicZeroCopy Simplify Option inner type extraction using syn utilities. Add bounds check before writing discriminant byte. improve generate_field_initialization remove debug test Incorrect type conversion from u8 to u32, add note options in arrays are not supported Error context lost in conversion format and add heap allocation check Check the last path segment for accurate type detection fix: test fix: test improve cache robustness
ananas-block
force-pushed
the
jorrit/refactor-zero-copy-borsh
branch
from
f5870e8 to
bdf3cf6
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
program-libs/zero-copy-derive/tests/config_test.rs (1)
22-30: Inconsistency between tests and macro implementation.The tests use
SimpleVecStructConfigand other config structs, but looking atlib.rs, theZeroCopyNewmacro is commented out (lines 130-166). This creates an inconsistency where tests expect functionality that appears to be disabled in the main macro implementation.Either:
- Enable the
ZeroCopyNewmacro inlib.rsif it's implemented- Manually define the config structs these tests expect
- Add feature gates to disable these tests when the macro is unavailable
#!/bin/bash # Verify if ZeroCopyNew is implemented elsewhere or config structs exist ast-grep --pattern $'pub struct $_ Config { $$$ }' # Check for ZeroCopyNew implementations rg -A 5 "impl.*ZeroCopyNew" # Check if ZeroCopyNew is re-exported from other modules rg "ZeroCopyNew" --exclude-dir=testsprogram-libs/zero-copy-derive/src/lib.rs (1)
130-166: Commented-out ZeroCopyNew macro conflicts with test expectations.The
ZeroCopyNewderive macro is commented out, butconfig_test.rstests actively use the generated config structs andZeroCopyNewtrait implementations. This creates a build inconsistency.To resolve this issue:
- Implement the macro by uncommenting and ensuring it works correctly with the existing test suite
- Remove dependent tests until the macro is ready
- Add feature gates to conditionally enable/disable the functionality
#!/bin/bash # Check if ZeroCopyNew functionality is implemented in the zero_copy_mut module rg -A 10 -B 5 "ZeroCopyNew" program-libs/zero-copy-derive/src/ # Verify test dependencies on commented-out functionality rg "ZeroCopyNew\|Config.*{" program-libs/zero-copy-derive/tests/config_test.rsprogram-libs/zero-copy-derive/src/shared/zero_copy_new.rs (3)
233-235: Critical: Integer overflow risk in split operationsConverting
config.#field_nametousizewithout bounds checking could cause issues if the config value exceedsusize::MAXon 32-bit systems.Consider using checked conversion:
- let (#field_name, bytes) = bytes.split_at_mut(config.#field_name as usize); + let len = usize::try_from(config.#field_name) + .map_err(|_| light_zero_copy::errors::ZeroCopyError::InvalidCapacity)?; + let (#field_name, bytes) = bytes.split_at_mut(len);
241-244: Critical: Unsafe integer conversion in Vec initializationThe
.into()conversion from u32 to the slice's length type happens without bounds checking.Replace with explicit conversion that handles potential overflow:
- let (#field_name, bytes) = light_zero_copy::slice_mut::ZeroCopySliceMutBorsh::<#inner_type>::new_at( - config.#field_name.into(), - bytes - )?; + let len = light_zero_copy::slice::Length::try_from(config.#field_name) + .map_err(|_| light_zero_copy::errors::ZeroCopyError::InvalidCapacity)?; + let (#field_name, bytes) = light_zero_copy::slice_mut::ZeroCopySliceMutBorsh::<#inner_type>::new_at( + len, + bytes + )?;
316-317: Critical: Potential arithmetic overflow in byte length calculationThe arithmetic operations could overflow if config values are large.
Use checked arithmetic:
- (4 + config.#field_name as usize) // 4 bytes for length + actual data + 4usize.checked_add(config.#field_name as usize) + .ok_or(light_zero_copy::errors::ZeroCopyError::InvalidCapacity)?program-libs/zero-copy-derive/src/shared/utils.rs (2)
83-99: Type detection correctly handles qualified paths.The functions properly use
path.segments.last()to check the final segment, correctly identifyingVecandOptiontypes regardless of qualification (e.g.,std::vec::Vec).
167-174: Hardcoded "Pubkey" detection remains inflexible.The hardcoded check for "Pubkey" could match unintended types or miss qualified paths. Consider using a configurable list of known Copy types or documenting this assumption clearly.
+// Known Copy types that should be treated specially +const KNOWN_COPY_TYPES: &[&str] = &["Pubkey"]; + pub fn is_pubkey_type(ty: &Type) -> bool { if let Type::Path(TypePath { path, .. }) = ty { if let Some(segment) = path.segments.last() { - return segment.ident == "Pubkey"; + return KNOWN_COPY_TYPES.contains(&segment.ident.to_string().as_str()); } } false }
🧹 Nitpick comments (12)
program-libs/zero-copy-derive/src/zero_copy.rs (2)
197-564: Consider removing commented test codeLarge blocks of commented test code reduce readability and may become outdated. If these tests are valuable, consider either:
- Moving them to a separate test file if they're still relevant
- Removing them entirely if they're no longer needed
56-60: Potential issue: Complex trait path generationThe trait path generation for
VecCopyfields is quite complex. Consider extracting this into a helper function for better maintainability.Example refactor:
+fn generate_copy_slice_trait_path<const MUT: bool>(inner_type: &syn::Type) -> TokenStream { + let inner_type = utils::convert_to_zerocopy_type(inner_type); + if MUT { + quote!(light_zero_copy::slice_mut::ZeroCopySliceMutBorsh::<'a, <#inner_type as light_zero_copy::borsh_mut::ZeroCopyStructInnerMut>::ZeroCopyInnerMut>) + } else { + quote!(light_zero_copy::slice::ZeroCopySliceBorsh::<'a, <#inner_type as light_zero_copy::borsh::ZeroCopyStructInner>::ZeroCopyInner>) + } +} let trait_path = if MUT { - quote!(light_zero_copy::slice_mut::ZeroCopySliceMutBorsh::<'a, <#inner_type as light_zero_copy::borsh_mut::ZeroCopyStructInnerMut>::ZeroCopyInnerMut>) + generate_copy_slice_trait_path::<true>(&inner_type) } else { - quote!(light_zero_copy::slice::ZeroCopySliceBorsh::<'a, <#inner_type as light_zero_copy::borsh::ZeroCopyStructInner>::ZeroCopyInner>) + generate_copy_slice_trait_path::<false>(&inner_type) };program-libs/zero-copy/src/borsh_mut.rs (1)
665-666: Consider more robust error handling innew_with_data.The method uses
unwrap()which will panic if deserialization fails. While acceptable for test code, consider returning aResultfor more robust error handling.- let (mut zero_copy, bytes) = - <Struct4 as DeserializeMut>::zero_copy_at_mut(bytes).unwrap(); + let (mut zero_copy, bytes) = + <Struct4 as DeserializeMut>::zero_copy_at_mut(bytes)?;program-libs/zero-copy/tests/borsh_2.rs (3)
40-43: Remove commented-out code.The commented-out function
data_hash_struct_1should be removed to keep the codebase clean.-// pub fn data_hash_struct_1(a: u8, b: u16) -> [u8; 32] { - -// }
320-327: Complex nested deserialization pattern.The
Vec::<ZeroCopySliceBorsh<...>>::zero_copy_atpattern is quite complex. Consider adding a comment explaining this nested zero-copy deserialization.fn zero_copy_at(bytes: &'a [u8]) -> Result<(Self::Output, &'a [u8]), ZeroCopyError> { + // Deserialize a vector of zero-copy slices for nested Vec<Vec<u8>> structure let (a, bytes) = Vec::<ZeroCopySliceBorsh<<u8 as ZeroCopyStructInner>::ZeroCopyInner>>::zero_copy_at( bytes, )?;
544-557: Reference comparison pattern repeated.The same reference-based comparison pattern is used here. Consider creating a helper function if this pattern is common.
Consider extracting a helper function for these reference-based vector comparisons to improve readability and reduce duplication.
program-libs/zero-copy-derive/tests/random.rs (6)
96-98: Inefficient byte-by-byte copying.The inner loop copies bytes one by one, which is inefficient. Use
copy_from_slicefor better performance.- for (src_byte, dst_byte) in src_data.data.iter().zip(dst_data.data.iter_mut()) { - *dst_byte = *src_byte; - } + let copy_len = src_data.data.len().min(dst_data.data.len()); + dst_data.data[..copy_len].copy_from_slice(&src_data.data[..copy_len]);
304-324: Hardcoded array size in random data generation.The hardcoded size of 64 for the
bfield should be defined as a constant to improve maintainability.+const COMPRESSED_PROOF_B_SIZE: usize = 64; + fn get_rnd_instruction_data_invoke_cpi(rng: &mut StdRng) -> InstructionDataInvokeCpi { InstructionDataInvokeCpi { proof: Some(CompressedProof { a: rng.gen(), - b: (0..64) + b: (0..COMPRESSED_PROOF_B_SIZE) .map(|_| rng.gen()) .collect::<Vec<u8>>() .try_into() .unwrap(),
337-337: Magic number for data size.The hardcoded value of 100 for data length should be configurable or defined as a constant.
+const TEST_DATA_SIZE: usize = 100; + fn get_rnd_test_account_data(rng: &mut StdRng) -> CompressedAccountData { CompressedAccountData { discriminator: rng.gen(), - data: (0..100).map(|_| rng.gen()).collect::<Vec<u8>>(), + data: (0..TEST_DATA_SIZE).map(|_| rng.gen()).collect::<Vec<u8>>(), data_hash: rng.gen(), } }
514-515: Large iteration counts may cause slow tests.The tests run 1,000 and 10,000 iterations respectively, which could make the test suite slow. Consider making these configurable via environment variables.
let num_iters = std::env::var("TEST_ITERATIONS") .map(|s| s.parse().unwrap_or(100)) .unwrap_or(100);Also applies to: 605-605
577-584: Missing error context in assertions.The assertion messages could be more descriptive to help with debugging when tests fail.
- assert_eq!(invoke_ref, borsh_ref); + assert_eq!( + invoke_ref, borsh_ref, + "Round-trip serialization/deserialization failed for iteration {}", i + );
588-591: Verbose test output.The test produces output for every successful iteration, which could spam test logs. Consider reducing verbosity or making it conditional.
- println!("✓ Successfully tested InstructionDataInvoke with {} inputs, {} outputs, {} new_addresses", - invoke_ref.input_compressed_accounts_with_merkle_context.len(), - invoke_ref.output_compressed_accounts.len(), - invoke_ref.new_address_params.len()); + if i % 100 == 0 { + println!("✓ Completed {} iterations", i + 1); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/workflows/rust.yml(1 hunks).gitignore(1 hunks)Cargo.toml(2 hunks)program-libs/compressed-account/src/instruction_data/with_account_info.rs(1 hunks)program-libs/compressed-account/src/instruction_data/with_readonly.rs(1 hunks)program-libs/zero-copy-derive/Cargo.toml(1 hunks)program-libs/zero-copy-derive/README.md(1 hunks)program-libs/zero-copy-derive/src/lib.rs(1 hunks)program-libs/zero-copy-derive/src/shared/from_impl.rs(1 hunks)program-libs/zero-copy-derive/src/shared/meta_struct.rs(1 hunks)program-libs/zero-copy-derive/src/shared/mod.rs(1 hunks)program-libs/zero-copy-derive/src/shared/utils.rs(1 hunks)program-libs/zero-copy-derive/src/shared/z_struct.rs(1 hunks)program-libs/zero-copy-derive/src/shared/zero_copy_new.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy_eq.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy_mut.rs(1 hunks)program-libs/zero-copy-derive/tests/config_test.rs(1 hunks)program-libs/zero-copy-derive/tests/cross_crate_copy.rs(1 hunks)program-libs/zero-copy-derive/tests/from_test.rs(1 hunks)program-libs/zero-copy-derive/tests/instruction_data.rs(1 hunks)program-libs/zero-copy-derive/tests/random.rs(1 hunks)program-libs/zero-copy/Cargo.toml(1 hunks)program-libs/zero-copy/README.md(0 hunks)program-libs/zero-copy/src/borsh.rs(4 hunks)program-libs/zero-copy/src/borsh_mut.rs(1 hunks)program-libs/zero-copy/src/init_mut.rs(1 hunks)program-libs/zero-copy/src/lib.rs(1 hunks)program-libs/zero-copy/src/slice_mut.rs(1 hunks)program-libs/zero-copy/tests/borsh.rs(1 hunks)program-libs/zero-copy/tests/borsh_2.rs(1 hunks)
💤 Files with no reviewable changes (1)
- program-libs/zero-copy/README.md
✅ Files skipped from review due to trivial changes (2)
- program-libs/zero-copy-derive/src/shared/mod.rs
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (15)
- .gitignore
- .github/workflows/rust.yml
- program-libs/zero-copy-derive/src/zero_copy_mut.rs
- program-libs/zero-copy-derive/src/shared/meta_struct.rs
- program-libs/zero-copy-derive/Cargo.toml
- program-libs/zero-copy-derive/README.md
- program-libs/zero-copy/src/slice_mut.rs
- program-libs/zero-copy/Cargo.toml
- program-libs/zero-copy-derive/src/shared/from_impl.rs
- program-libs/zero-copy-derive/tests/from_test.rs
- program-libs/zero-copy-derive/src/zero_copy_eq.rs
- program-libs/zero-copy/src/lib.rs
- program-libs/zero-copy/src/init_mut.rs
- program-libs/zero-copy/tests/borsh.rs
- program-libs/zero-copy-derive/tests/instruction_data.rs
🧰 Additional context used
🧬 Code Graph Analysis (3)
program-libs/zero-copy-derive/tests/config_test.rs (3)
program-libs/zero-copy/src/init_mut.rs (25)
config(241-244)byte_len(23-23)byte_len(42-51)byte_len(81-83)byte_len(98-100)byte_len(115-117)byte_len(132-134)byte_len(149-151)byte_len(172-174)byte_len(190-192)byte_len(206-208)byte_len(222-224)byte_len(239-245)new_zero_copy(28-31)new_zero_copy(53-73)new_zero_copy(85-91)new_zero_copy(102-108)new_zero_copy(119-125)new_zero_copy(136-142)new_zero_copy(153-159)new_zero_copy(176-182)new_zero_copy(194-199)new_zero_copy(210-215)new_zero_copy(226-231)new_zero_copy(247-267)program-libs/zero-copy/src/slice_mut.rs (1)
len(116-119)program-libs/zero-copy/src/borsh_mut.rs (15)
zero_copy_at_mut(20-21)zero_copy_at_mut(29-34)zero_copy_at_mut(41-56)zero_copy_at_mut(63-67)zero_copy_at_mut(74-78)zero_copy_at_mut(86-91)zero_copy_at_mut(97-117)zero_copy_at_mut(186-197)zero_copy_at_mut(404-409)zero_copy_at_mut(467-478)zero_copy_at_mut(531-538)zero_copy_at_mut(569-574)zero_copy_at_mut(627-646)zero_copy_at_mut(716-723)zero_copy_at_mut(757-762)
program-libs/zero-copy/src/borsh_mut.rs (3)
program-libs/zero-copy/src/borsh.rs (16)
new(146-148)deref(153-155)deref(375-377)deref(433-435)deref(490-492)deref(572-574)deref(743-745)deref_mut(159-161)test_vecu8(231-237)test_vecu8_deref(299-307)eq(421-427)eq(731-737)eq(818-820)test_struct_4(599-619)test_struct_7(759-785)test_struct_8(839-872)program-libs/zero-copy/src/slice_mut.rs (7)
new(35-37)iter(246-248)eq(263-265)len(116-119)from_bytes(56-58)to_vec(178-180)from_bytes_at(61-85)program-libs/zero-copy-derive/src/zero_copy_mut.rs (2)
zero_copy(40-40)zero_copy(45-45)
program-libs/zero-copy/tests/borsh_2.rs (2)
program-libs/zero-copy/src/borsh_mut.rs (10)
deref(171-173)deref(390-392)deref(459-461)deref(523-525)deref(619-621)deref(829-831)test_struct_1(413-422)eq(447-453)eq(817-823)eq(909-911)program-libs/zero-copy/src/borsh.rs (21)
deref(153-155)deref(375-377)deref(433-435)deref(490-492)deref(572-574)zero_copy_at(20-20)zero_copy_at(27-30)zero_copy_at(36-49)zero_copy_at(56-62)zero_copy_at(69-75)zero_copy_at(113-116)zero_copy_at(122-140)zero_copy_at(168-178)zero_copy_at(383-386)zero_copy_at(441-445)zero_copy_at(498-503)zero_copy_at(580-595)test_struct_1(390-396)eq(421-427)eq(731-737)eq(818-820)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: cli-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test sdk-libs
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: cli-v1
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-v2
🔇 Additional comments (31)
program-libs/compressed-account/src/instruction_data/with_readonly.rs (1)
350-357: Excellent security improvement to prevent heap exhaustion attacks.This validation correctly prevents potential DoS attacks by ensuring that
num_slicesdoesn't exceed the available data length before allocating a vector. The check is minimal yet effective - since each serialized element requires at least 1 byte, this prevents unrealistic allocation requests.program-libs/compressed-account/src/instruction_data/with_account_info.rs (1)
403-408: Consistent application of heap exhaustion prevention.This implements the same security validation as in
with_readonly.rs, ensuring consistent protection across both instruction data deserialization paths. The approach correctly prevents malicious inputs from causing excessive memory allocation.program-libs/zero-copy-derive/tests/cross_crate_copy.rs (1)
1-296: Comprehensive test coverage for Copy trait detection.This test module provides excellent coverage for validating the zero-copy derive macro's Copy trait detection logic. The tests effectively cover:
- Primitive Copy types in meta fields vs struct fields
- Array handling with different element types
- Vec field splitting behavior based on field ordering
- All derive macros (ZeroCopy, ZeroCopyEq, ZeroCopyMut)
- Cross-crate boundary Copy detection
The field ordering tests are particularly valuable for ensuring the macro correctly implements the "split at first Vec" rule described in the main macro documentation.
program-libs/zero-copy-derive/tests/config_test.rs (1)
32-89: Test logic demonstrates proper zero-copy mutable initialization.Despite the macro availability concern, the test logic correctly validates:
- Exact buffer size calculation via
byte_len()- Zero-copy mutable initialization via
new_zero_copy()- Field access patterns for meta fields vs struct fields
- Round-trip serialization/deserialization consistency
The test patterns are well-designed for validating zero-copy mutable struct functionality.
program-libs/zero-copy-derive/src/lib.rs (2)
57-64: Robust error handling in procedural macro implementation.The error handling correctly converts
syn::Errorresults into compile-time errors usingto_compile_error(), ensuring that macro parsing failures provide clear diagnostic messages to users.
86-128: Comprehensive documentation for ZeroCopyMut macro.The documentation clearly explains the macro's purpose, generated code, and usage patterns. The examples demonstrate both standalone usage and combination with other derive macros.
program-libs/zero-copy-derive/src/shared/z_struct.rs (4)
10-24: LGTM: Well-designed FieldType enum for zero-copy field classificationThe enum provides comprehensive coverage of different field types needed for zero-copy deserialization, with clear documentation about array limitations.
67-83: Good pattern matching for option type classificationThe classification logic correctly handles special cases for primitive integer options while falling back to generic option handling.
170-318: Complex but well-structured field generation logicThe generic const parameter approach for handling mutability is elegant, and the field type matching covers all necessary cases comprehensively.
514-629: Excellent fuzz testing implementationThe fuzz test with 10,000 iterations and comprehensive validation provides strong confidence in the code generation correctness. The use of seeded RNG for reproducibility is a good practice.
program-libs/zero-copy-derive/src/zero_copy.rs (1)
589-636: LGTM: Well-structured main derive implementationThe
derive_zero_copy_implfunction properly orchestrates all the code generation steps and combines them into a cohesive implementation.program-libs/zero-copy/src/borsh_mut.rs (3)
14-22: Well-designed trait for mutable zero-copy deserialization.The
DeserializeMuttrait provides a clean interface for zero-copy mutable deserialization with proper lifetime management and error handling.
102-109: Good bounds checking to prevent heap exhaustion.The implementation correctly validates that the remaining bytes are sufficient for the declared number of elements, providing protection against malicious input that could trigger excessive memory allocation.
336-366: Excellent comprehensive test coverage with clear patterns.The test module provides extensive coverage of zero-copy mutable deserialization scenarios and demonstrates best practices for implementing zero-copy structs manually. The documented rules (lines 349-357) are particularly valuable for understanding the design patterns.
program-libs/zero-copy/src/borsh.rs (5)
8-8: Good addition of signed integer support.The signed integer types are properly imported and implemented using the same macro pattern as unsigned integers, providing consistent zero-copy deserialization capabilities.
Also applies to: 97-106
108-117: Clean implementation for fixed-size array deserialization.The implementation correctly uses
Ref::from_prefixto provide zero-copy deserialization for fixed-size arrays, following the established patterns in the codebase.
125-132: Appropriate bounds checking for heap exhaustion protection.The validation ensures sufficient bytes are available before allocation, providing reasonable protection against malicious input attempting to trigger excessive memory allocation.
181-219: Well-designed trait for zero-copy type associations.The
ZeroCopyStructInnertrait provides a clean mapping between native types and their zero-copy representations, with comprehensive implementations for primitives, containers, and arrays.
317-347: Excellent comprehensive test framework.The test module provides extensive coverage with clear documentation of the rules and patterns for implementing zero-copy structs. This serves as both testing and valuable documentation for users.
program-libs/zero-copy-derive/src/shared/utils.rs (4)
15-19: Good solution for cache key uniqueness.Using span information in
create_unique_type_keyaddresses the namespace collision concern from previous reviews, providing better uniqueness than struct name alone.
247-265: Well-implemented Copy trait detection with proper caching.The implementation correctly detects
#[derive(Copy)]attributes and uses caching for performance. The documentation clearly explains the approach and limitations.
176-217: Solid type conversion for zero-copy compatibility.The function correctly maps primitive types to their little-endian zero-copy equivalents and handles container types recursively, providing the necessary type transformations for zero-copy deserialization.
317-437: Comprehensive test coverage for Copy trait detection.The test suite thoroughly covers various scenarios including edge cases, providing confidence in the correctness of the Copy trait detection logic. The tests are well-documented and clearly structured.
program-libs/zero-copy/tests/borsh_2.rs (7)
1-12: LGTM! Clean imports and feature gating.The feature gates and imports are well-organized and appropriate for the zero-copy functionality being tested.
14-30: Excellent documentation of zero-copy design rules.The comments clearly document the structural patterns and requirements for zero-copy deserialization, which will be valuable for understanding the derive macro implementation.
104-112: Good implementation of cross-type equality comparison.The
PartialEq<Struct2>implementation correctly handles the conversion fromU16tou16and compares vector slices appropriately.
381-391: Reference comparison in assertion.The test uses references in the expected vector (
&Struct2 { ... }). This seems unusual - verify this is the intended comparison pattern.The assertion compares
zero_copy.a.iter().collect::<Vec<_>>()(which creates a vector of references) with a vector of references toStruct2instances. This pattern should be documented or simplified.
423-423: Simple option comparison.Good that the option comparison works directly between zero-copy and owned types.
298-301: Verify U16 conversion invec_2comparison
- Location:
program-libs/zero-copy/tests/borsh_2.rs, lines 298–301- The test does:
assert_eq!( zero_copy.vec_2.to_vec(), vec![ZStruct4Nested { a: 1, b: 2.into() }; 32] );- Please confirm that
zerocopy::little_endian::U16implementsFrom<u16>(and thusInto<U16>) so that2.into()produces the correctU16.- If it does not, replace the conversion with an explicit call, e.g.
U16::new(2)orFrom::from(2).
77-78: ConfirmPartialEq<u16>forU16
The test inprogram-libs/zero-copy/tests/borsh_2.rs(lines 77–78) compares alittle_endian::U16value directly to au16. SinceU16is re-exported from the externalzerocopycrate, please verify that it implementsPartialEq<u16>(and that its little-endian conversion returns the expectedu16). If it doesn’t, consider one of the following adjustments:
- Change the assertion to compare the raw value, e.g.
assert_eq!(struct1.b.get(), 2u16);- Wrap the literal in the same type, e.g.
assert_eq!(struct1.b, U16::new(2u16));program-libs/zero-copy-derive/tests/random.rs (1)
1-11: Clean feature gating and imports.The feature guard and imports are appropriate for the mutable zero-copy functionality being tested.
| FieldType::Copy(field_name, field_type) => { | ||
| quote! { | ||
| let (#field_name, bytes) = <#field_type as light_zero_copy::init_mut::ZeroCopyNew>::new_zero_copy(bytes)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Issue: Incomplete field initialization for Copy types
The FieldType::Copy case is missing the required config parameter in the new_zero_copy call.
Fix the function call signature:
- let (#field_name, bytes) = <#field_type as light_zero_copy::init_mut::ZeroCopyNew>::new_zero_copy(bytes)?;
+ let (#field_name, bytes) = <#field_type as light_zero_copy::init_mut::ZeroCopyNew>::new_zero_copy(bytes, ())?;🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/src/shared/zero_copy_new.rs around lines 301 to
305, the FieldType::Copy match arm calls new_zero_copy without the required
config parameter. Modify the call to include the config argument as the second
parameter to new_zero_copy, ensuring the function signature matches the expected
usage.
| fn zero_copy_at_mut( | ||
| bytes: &'a mut [u8], | ||
| ) -> Result<(Self::Output, &'a mut [u8]), ZeroCopyError> { | ||
| let (num_slices, mut bytes) = Ref::<&mut [u8], u8>::from_prefix(bytes)?; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the intentional use of u8 length prefix for VecU8.
The VecU8 implementation uses a u8 length prefix, limiting it to 255 elements, while the regular Vec<T> implementation uses U32. If this size limitation is intentional for VecU8, consider documenting it clearly.
🏁 Script executed:
#!/bin/bash
# Check if VecU8's u8 length prefix limitation is documented or tested elsewhere
rg -A 5 -B 5 "VecU8.*255|255.*VecU8" program-libs/zero-copy/Length of output: 62
🏁 Script executed:
#!/bin/bash
# Locate the VecU8 definition and surrounding context
rg -n 'struct VecU8' -n program-libs/zero-copy/src/borsh_mut.rs
# Show the implementation and any doc comments around VecU8 (lines 160–240)
sed -n '160,240p' program-libs/zero-copy/src/borsh_mut.rsLength of output: 2145
Document VecU8’s 1-byte length prefix
The VecU8 zero-copy implementation always reads its element count from a single u8 (max 255), whereas the standard Vec uses a U32 prefix. If this 255-element cap is intentional, please add a doc comment on VecU8 to surface this constraint. Otherwise, consider switching to a 32-bit length prefix or renaming VecU8 to make the byte-length behavior clear.
Locations to update:
- program-libs/zero-copy/src/borsh_mut.rs: struct VecU8 (around line 162) – add documentation of the u8 length prefix and its 255-element cap.
- program-libs/zero-copy/src/borsh_mut.rs: impl DeserializeMut for VecU8 (around the zero_copy_at_mut method) – verify tests cover the boundary (255 vs. 256) and update docs or code accordingly.
🤖 Prompt for AI Agents
In program-libs/zero-copy/src/borsh_mut.rs around line 162, add a doc comment to
the VecU8<T> struct clearly stating that it uses a 1-byte u8 length prefix
limiting the element count to 255. Then, in the impl DeserializeMut for VecU8<T>
near the zero_copy_at_mut method, verify that tests cover the boundary condition
between 255 and 256 elements and update the documentation or code to reflect
this limitation or consider changing the length prefix to u32 if the 255-element
cap is not intended.
| let (vec, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?; | ||
| let (c, bytes) = | ||
| Ref::<&[u8], <u64 as ZeroCopyStructInner>::ZeroCopyInner>::from_prefix(bytes)?; | ||
| let (vec_2, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?; |
There was a problem hiding this comment.
Inconsistent use of zero_copy_at vs from_bytes_at.
Line 266 uses ZeroCopySliceBorsh::from_bytes_at while line 184 uses ZeroCopySliceBorsh::zero_copy_at. This inconsistency could indicate a bug or should be standardized.
- let (vec, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?;
+ let (vec, bytes) = ZeroCopySliceBorsh::zero_copy_at(bytes)?;- let (vec_2, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?;
+ let (vec_2, bytes) = ZeroCopySliceBorsh::zero_copy_at(bytes)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let (vec, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?; | |
| let (c, bytes) = | |
| Ref::<&[u8], <u64 as ZeroCopyStructInner>::ZeroCopyInner>::from_prefix(bytes)?; | |
| let (vec_2, bytes) = ZeroCopySliceBorsh::from_bytes_at(bytes)?; | |
| let (vec, bytes) = ZeroCopySliceBorsh::zero_copy_at(bytes)?; | |
| let (c, bytes) = | |
| Ref::<&[u8], <u64 as ZeroCopyStructInner>::ZeroCopyInner>::from_prefix(bytes)?; | |
| let (vec_2, bytes) = ZeroCopySliceBorsh::zero_copy_at(bytes)?; |
🤖 Prompt for AI Agents
In program-libs/zero-copy/tests/borsh_2.rs around lines 266 to 269, the method
ZeroCopySliceBorsh::from_bytes_at is used inconsistently compared to
ZeroCopySliceBorsh::zero_copy_at used earlier at line 184. Review the intended
behavior and replace from_bytes_at with zero_copy_at on line 266 to maintain
consistency, or vice versa, ensuring the same method is used throughout for
similar operations.
| .owner | ||
| .copy_from_slice(&src_input.compressed_account.owner); | ||
| dst_input.compressed_account.lamports = src_input.compressed_account.lamports.into(); | ||
|
|
||
| // Copy address if present | ||
| if let (Some(src_addr), Some(dst_addr)) = ( | ||
| &src_input.compressed_account.address, | ||
| &mut dst_input.compressed_account.address, | ||
| ) { | ||
| dst_addr.copy_from_slice(src_addr); | ||
| } | ||
|
|
||
| // Copy data if present | ||
| if let (Some(src_data), Some(dst_data)) = ( | ||
| &src_input.compressed_account.data, | ||
| &mut dst_input.compressed_account.data, | ||
| ) { | ||
| dst_data | ||
| .discriminator | ||
| .copy_from_slice(&src_data.discriminator); | ||
| dst_data.data_hash.copy_from_slice(&src_data.data_hash); | ||
| for (src_byte, dst_byte) in src_data.data.iter().zip(dst_data.data.iter_mut()) { | ||
| *dst_byte = *src_byte; | ||
| } | ||
| } | ||
|
|
||
| // Copy merkle context | ||
| dst_input.merkle_context.merkle_tree_pubkey_index = | ||
| src_input.merkle_context.merkle_tree_pubkey_index; | ||
| dst_input.merkle_context.nullifier_queue_pubkey_index = | ||
| src_input.merkle_context.nullifier_queue_pubkey_index; | ||
| dst_input.merkle_context.leaf_index = src_input.merkle_context.leaf_index.into(); | ||
| dst_input.merkle_context.prove_by_index = if src_input.merkle_context.prove_by_index { | ||
| 1 | ||
| } else { | ||
| 0 | ||
| }; | ||
|
|
||
| *dst_input.root_index = src_input.root_index.into(); | ||
| *dst_input.read_only = if src_input.read_only { 1 } else { 0 }; | ||
| } | ||
|
|
||
| // Copy output_compressed_accounts | ||
| for (src_output, dst_output) in src | ||
| .output_compressed_accounts | ||
| .iter() | ||
| .zip(dst.output_compressed_accounts.iter_mut()) | ||
| { | ||
| // Copy compressed account | ||
| dst_output | ||
| .compressed_account | ||
| .owner | ||
| .copy_from_slice(&src_output.compressed_account.owner); | ||
| dst_output.compressed_account.lamports = src_output.compressed_account.lamports.into(); | ||
|
|
||
| // Copy address if present | ||
| if let (Some(src_addr), Some(dst_addr)) = ( | ||
| &src_output.compressed_account.address, | ||
| &mut dst_output.compressed_account.address, | ||
| ) { | ||
| dst_addr.copy_from_slice(src_addr); | ||
| } | ||
|
|
||
| // Copy data if present | ||
| if let (Some(src_data), Some(dst_data)) = ( | ||
| &src_output.compressed_account.data, | ||
| &mut dst_output.compressed_account.data, | ||
| ) { | ||
| dst_data | ||
| .discriminator | ||
| .copy_from_slice(&src_data.discriminator); | ||
| dst_data.data_hash.copy_from_slice(&src_data.data_hash); | ||
| for (src_byte, dst_byte) in src_data.data.iter().zip(dst_data.data.iter_mut()) { | ||
| *dst_byte = *src_byte; | ||
| } | ||
| } | ||
|
|
||
| *dst_output.merkle_tree_index = src_output.merkle_tree_index; | ||
| } | ||
|
|
||
| // Copy relay_fee if present | ||
| if let (Some(src_fee), Some(dst_fee)) = (&src.relay_fee, &mut dst.relay_fee) { | ||
| **dst_fee = (*src_fee).into(); | ||
| } | ||
|
|
||
| // Copy compress_or_decompress_lamports if present | ||
| if let (Some(src_lamports), Some(dst_lamports)) = ( | ||
| &src.compress_or_decompress_lamports, | ||
| &mut dst.compress_or_decompress_lamports, | ||
| ) { | ||
| **dst_lamports = (*src_lamports).into(); | ||
| } | ||
|
|
||
| // Copy cpi_context if present | ||
| if let (Some(src_ctx), Some(dst_ctx)) = (&src.cpi_context, &mut dst.cpi_context) { | ||
| dst_ctx.set_context = if src_ctx.set_context { 1 } else { 0 }; | ||
| dst_ctx.first_set_context = if src_ctx.first_set_context { 1 } else { 0 }; | ||
| dst_ctx.cpi_context_account_index = src_ctx.cpi_context_account_index; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Significant code duplication between populate functions.
The populate_invoke_cpi_zero_copy and populate_invoke_zero_copy functions are nearly identical with ~95% code duplication. This violates the DRY principle and makes maintenance difficult.
Consider extracting the common logic into a shared helper function or using generics to eliminate this duplication:
fn populate_common_fields<T, U>(
src_proof: &Option<CompressedProof>,
dst_proof: &mut Option<T>,
// ... other common parameters
) {
// Common population logic
}
fn populate_invoke_zero_copy(src: &InstructionDataInvoke, dst: &mut ZInstructionDataInvokeMut) {
populate_common_fields(&src.proof, &mut dst.proof, /* ... */);
// Handle invoke-specific fields
}
fn populate_invoke_cpi_zero_copy(src: &InstructionDataInvokeCpi, dst: &mut ZInstructionDataInvokeCpiMut) {
populate_common_fields(&src.proof, &mut dst.proof, /* ... */);
// Handle CPI-specific fields like cpi_context
}Also applies to: 176-300
🤖 Prompt for AI Agents
In program-libs/zero-copy-derive/tests/random.rs around lines 41 to 174, the
function populate_invoke_cpi_zero_copy contains nearly identical code to
populate_invoke_zero_copy, causing significant duplication. To fix this, extract
the shared logic for copying common fields such as proof, new_address_params,
compressed accounts, and other repeated structures into a generic helper
function that accepts source and destination references. Then, refactor both
populate_invoke_zero_copy and populate_invoke_cpi_zero_copy to call this helper
for the common parts and handle their specific fields separately, reducing
duplication and improving maintainability. Apply the same refactoring approach
to lines 176 to 300 where similar duplication exists.
TODO:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
.gitignoreto exclude generated files.