Conversation
WalkthroughAdds a new no_std crate Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant AM as ArrayMap<K,V,N>
Note over Client,AM: high-level ArrayMap interactions
Client->>AM: new()
Client->>AM: insert(key, value, error_token)
alt capacity available
AM-->>Client: Ok(index)
else capacity exceeded
AM-->>Client: Err(ArrayMapError::CapacityExceeded.into())
end
Client->>AM: find(&key)
alt found
AM-->>Client: Some((index, &(K,V)))
Note right of AM: update last_updated_index
else not found
AM-->>Client: None
end
Client->>AM: get(index)
alt in-bounds
AM-->>Client: Some(&(K,V))
else out-of-bounds
AM-->>Client: None
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
.github/workflows/rust.yml (1)
48-65: Also build light-array-map in no_std to catch regressions earlyMirror light-zero-copy by adding a no-default-features build for light-array-map.
cargo test -p aligned-sized + cargo build -p light-array-map --no-default-features # Ensure no_std builds cargo test -p light-array-map --all-featuresprogram-libs/compressed-account/src/pubkey.rs (1)
78-80: array_ref() addition is useful; consider also implementing AsRef<[u8; 32]>This makes the API more idiomatic for consumers that want a typed 32-byte reference.
Outside this hunk, add:
impl AsRef<[u8; 32]> for Pubkey { fn as_ref(&self) -> &[u8; 32] { &self.0 } }program-libs/array-map/tests/array_map_tests.rs (2)
43-45: Avoid temporary String allocations in assertionsCompare by str to prevent needless allocs and borrowed temporaries.
- assert_eq!(map.get_by_key(&1), Some(&"one".to_string())); - assert_eq!(map.get_by_key(&2), Some(&"two".to_string())); + assert_eq!(map.get_by_key(&1).map(String::as_str), Some("one")); + assert_eq!(map.get_by_key(&2).map(String::as_str), Some("two"));
1-14: Add tests for pubkey-optimized accessorsCover get_by_pubkey/find_pubkey_index and pubkey_eq to ensure the fast-path stays correct.
I can draft a small test using ArrayMap<[u8; 32], u8, 4> with a few fixed keys if helpful.
program-libs/array-map/src/lib.rs (8)
10-12: Doc nit: clarify last_accessed_index semanticsCurrently it’s updated on insert and via set_last_accessed_index, not on reads.
-/// Maintains insertion order and tracks the last accessed entry index. +/// Maintains insertion order. +/// Tracks an optional `last_accessed_index`, updated on insert and via `set_last_accessed_index`.
12-19: Remove unnecessary Default bounds from ArrayMapK/V don’t need Default for current operations; keeping only K: PartialEq widens usability.
-pub struct ArrayMap<K, V, const N: usize> -where - K: PartialEq + Default, - V: Default, -{ +pub struct ArrayMap<K, V, const N: usize> +where + K: PartialEq, +{
21-25: Same here: drop Default bounds in impl blockNo methods rely on Default.
-impl<K, V, const N: usize> ArrayMap<K, V, N> -where - K: PartialEq + Default, - V: Default, -{ +impl<K, V, const N: usize> ArrayMap<K, V, N> +where + K: PartialEq, +{
110-118: Default impl doesn’t require K/V: DefaultTighten bounds to match actual needs.
-impl<K, V, const N: usize> Default for ArrayMap<K, V, N> -where - K: PartialEq + Default, - V: Default, -{ +impl<K, V, const N: usize> Default for ArrayMap<K, V, N> +where + K: PartialEq, +{
120-124: Specialization for [u8; 32]: V doesn’t need DefaultDrop the bound to keep API flexible.
-impl<V, const N: usize> ArrayMap<[u8; 32], V, N> -where - V: Default, -{ +impl<V, const N: usize> ArrayMap<[u8; 32], V, N> {
99-107: Consider using CapacityExceeded variant instead of passing an error tokenYou already define ArrayMapError::CapacityExceeded and require E: From; returning that (via .into()) simplifies the API and avoids threading an error value through all callsites.
If you want to keep both behaviors, we can add
try_insert(&mut self, key, value) -> Result<usize, ArrayMapError>and keepinsertas-is.
158-171: ArrayMapError::CapacityExceeded is currently unusedEither use it in insert (preferred) or remove to reduce API surface.
173-184: Avoid const on pubkey_eq to reduce MSRV pressure
read_unalignedin const contexts depends on newer compilers. Unless you rely on const-eval here, a plain inline fn is safer for MSRV.-#[inline(always)] -pub const fn pubkey_eq(p1: &[u8; 32], p2: &[u8; 32]) -> bool { +#[inline(always)] +pub fn pubkey_eq(p1: &[u8; 32], p2: &[u8; 32]) -> bool {If you prefer keeping it const, please confirm the workspace rust-version supports
ptr::read_unalignedin const fns (and set rust-version accordingly).Cargo.toml (1)
64-67: Add workspacerust-versionto Cargo.toml
Under[workspace.package], addrust-version = "1.90.0"(to match your rust-toolchain.toml channel) and lock the MSRV:[workspace.package] version = "0.1.0" edition = "2021" +rust-version = "1.90.0"
📜 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 (6)
.github/workflows/rust.yml(1 hunks)Cargo.toml(2 hunks)program-libs/array-map/Cargo.toml(1 hunks)program-libs/array-map/src/lib.rs(1 hunks)program-libs/array-map/tests/array_map_tests.rs(1 hunks)program-libs/compressed-account/src/pubkey.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T21:59:25.201Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.201Z
Learning: Applies to program-libs/account-checks/**/Cargo.toml : Define features solana, pinocchio, and test-only in Cargo.toml; default build should enable none
Applied to files:
.github/workflows/rust.yml
🧬 Code graph analysis (1)
program-libs/array-map/tests/array_map_tests.rs (1)
program-libs/array-map/src/lib.rs (9)
new(26-31)len(33-35)is_empty(37-39)last_accessed_index(41-43)get(45-47)get_by_key(61-63)find_index(83-85)get_u8(53-55)get_mut_u8(57-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: cli-v2
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: stateless-js-v1
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: lint
- GitHub Check: cli-v1
🔇 Additional comments (1)
program-libs/array-map/Cargo.toml (1)
1-14: Manifest looks goodClear features, workspace dep wiring, license and metadata are correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
program-libs/compressed-account/src/pubkey.rs (1)
78-80: Accessor looks good; consider adding inline and AsRef<[u8; 32]>
- Add #[inline(always)] to hint inlining.
- Provide impl AsRef<[u8; 32]> for Pubkey to integrate with generic APIs taking AsRef<[u8;32]>.
- Optional: alias name as as_array() for consistency with as_ref/as_bytes-style API.
Example:
pub fn array_ref(&self) -> &[u8; 32] { &self.0 } + + #[inline(always)] + pub fn as_array(&self) -> &[u8; 32] { + &self.0 + }And add:
impl AsRef<[u8]> for Pubkey { fn as_ref(&self) -> &[u8] { &self.0 } } + +impl AsRef<[u8; 32]> for Pubkey { + fn as_ref(&self) -> &[u8; 32] { + &self.0 + } +}program-libs/array-map/Cargo.toml (1)
9-14: Clarify/justify theallocfeature; it may be a no‑op hereThis crate currently uses ArrayVec only; enabling
tinyvec/allocaffects TinyVec, not ArrayVec. If the intent is merely to gate certain tests (heap-allocated value types), consider:
- Keeping the feature but document its purpose; or
- Gating tests with
cfg(feature = "alloc")via a dev‑feature, not forwarding tinyvec/alloc; or- Removing the feature until TinyVec is introduced.
Based on learnings
program-libs/array-map/tests/array_map_tests.rs (3)
43-45: Avoid allocs in assertsConstructing
Stringin assertions allocates. Compare as&strinstead:- assert_eq!(map.get_by_key(&1), Some(&"one".to_string())); - assert_eq!(map.get_by_key(&2), Some(&"two".to_string())); + assert_eq!(map.get_by_key(&1).map(|s| s.as_str()), Some("one")); + assert_eq!(map.get_by_key(&2).map(|s| s.as_str()), Some("two"));
58-59: Same here: compare as&str- assert_eq!(map.get_by_key(&1), Some(&"ONE".to_string())); + assert_eq!(map.get_by_key(&1).map(|s| s.as_str()), Some("ONE"));
141-186: Add coverage for pubkey-optimized methodsPlease add tests for
[u8; 32]key specialization:
- get_by_pubkey / get_mut_by_pubkey
- find_by_pubkey / find_mut_by_pubkey / find_pubkey_index
I can draft a test module if helpful.
program-libs/array-map/src/lib.rs (4)
10-12: Doc/behavior mismatch: “tracks last accessed index” but accessors don’t update itCurrently only insert() and set_last_accessed_index() update the field; get*/find* do not. Either:
- Update the doc to “tracks last inserted or explicitly set index”, or
- Update successful get*/find* paths to set last_accessed_index.
Minimal doc fix:
-/// Maintains insertion order and tracks the last accessed entry index. +/// Maintains insertion order and tracks the last inserted (or explicitly set) entry index.Or update, e.g., get():
pub fn get(&self, index: usize) -> Option<&(K, V)> { - self.entries.get(index) + let r = self.entries.get(index); + if r.is_some() { + // SAFETY: requires &mut; consider making `self` mutable or providing a separate accessor that mutates state. + } + r }If you choose to track access on reads, prefer dedicated mutating accessors (e.g., get_and_track) to avoid hidden interior mutability.
99-107: Provide a standardized capacity error path (non-alloc friendly)Insert returns a caller-provided error; meanwhile ArrayMapError::CapacityExceeded exists but isn’t used here. Consider adding a sibling API that uses it:
pub fn try_insert<E>(&mut self, key: K, value: V) -> Result<usize, E> where E: From<ArrayMapError>, { let new_idx = self.entries.len(); if self.entries.try_push((key, value)).is_some() { return Err(ArrayMapError::CapacityExceeded.into()); } self.last_accessed_index = Some(new_idx); Ok(new_idx) }This keeps
insertfor custom error types while offering a consistent default.
125-156: Generalize pubkey APIs to accept any AsRef<[u8; 32]>To reduce friction with types like Pubkey that can expose an array ref, consider:
-pub fn get_by_pubkey(&self, key: &[u8; 32]) -> Option<&V> { +pub fn get_by_pubkey<T: AsRef<[u8; 32]>>(&self, key: T) -> Option<&V> { - self.entries.iter().find(|(k, _)| pubkey_eq(k, key)).map(|(_, v)| v) + let key = key.as_ref(); + self.entries.iter().find(|(k, _)| pubkey_eq(k, key)).map(|(_, v)| v) }Apply similarly to get_mut_by_pubkey/find*/find*_index.
173-184: Simplify pubkey_eq to safe slice equalityAt program-libs/array-map/src/lib.rs:173–184, replace the
const fnand unsafe block with:#[inline(always)] pub fn pubkey_eq(p1: &[u8; 32], p2: &[u8; 32]) -> bool { p1 == p2 }LLVM will optimize this to a fast memcmp, removing unsafe and avoiding any const stability concerns.
.github/workflows/rust.yml (1)
45-47: Also build light-array-map in no_std modeAdd a no-default-features build to ensure the no_std crate compiles without std:
cargo test -p aligned-sized + cargo build -p light-array-map --no-default-features cargo test -p light-array-map --all-featuresAlso applies to: 50-50
📜 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 (6)
.github/workflows/rust.yml(1 hunks)Cargo.toml(2 hunks)program-libs/array-map/Cargo.toml(1 hunks)program-libs/array-map/src/lib.rs(1 hunks)program-libs/array-map/tests/array_map_tests.rs(1 hunks)program-libs/compressed-account/src/pubkey.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T21:59:25.201Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.201Z
Learning: Applies to program-libs/account-checks/**/Cargo.toml : Define features solana, pinocchio, and test-only in Cargo.toml; default build should enable none
Applied to files:
.github/workflows/rust.yml
🧬 Code graph analysis (1)
program-libs/array-map/tests/array_map_tests.rs (1)
program-libs/array-map/src/lib.rs (9)
new(26-31)len(33-35)is_empty(37-39)last_accessed_index(41-43)get(45-47)get_by_key(61-63)find_index(83-85)get_u8(53-55)get_mut_u8(57-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: cli-v2
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: stateless-js-v1
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: lint
- GitHub Check: cli-v1
ananas-block
force-pushed
the
jorrit/feat-array-map
branch
2 times, most recently
from
7a3fcf4 to
3d94c88
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
program-libs/array-map/src/lib.rs (3)
96-104: Ergonomics: add a non-allocating, canonicaltry_insertreturningArrayMapErrorCurrent
insertforces constructing an error value even on success and leavesCapacityExceededunused. Keepinsertfor custom errors, but add a canonicaltry_insertthat returnsArrayMapError.pub fn insert<E>(&mut self, key: K, value: V, error: E) -> Result<usize, E> { let new_idx = self.entries.len(); // tinyvec's try_push returns Some(item) on failure, None on success if self.entries.try_push((key, value)).is_some() { return Err(error); } self.last_accessed_index = Some(new_idx); Ok(new_idx) } + + /// Insert without requiring a pre-constructed error; returns a canonical error on overflow. + pub fn try_insert(&mut self, key: K, value: V) -> Result<usize, ArrayMapError> { + let new_idx = self.entries.len(); + if self + .entries + .try_push((key, value)) + .is_some() + { + return Err(ArrayMapError::CapacityExceeded); + } + self.last_accessed_index = Some(new_idx); + Ok(new_idx) + }
30-36: Addcapacity()helperExposing fixed capacity helps callers preflight inserts without attempting them.
pub fn is_empty(&self) -> bool { self.entries.is_empty() } + #[inline] + pub fn capacity(&self) -> usize { + N + }
170-181: Replacepubkey_eqwith safe slice equalityThe current
const fn+ unsafe impl isn’t needed for comparing fixed‐size arrays, adds risk, and may break older MSRVs—simplify to:#[inline(always)] -pub const fn pubkey_eq(p1: &[u8; 32], p2: &[u8; 32]) -> bool { - let p1_ptr = p1.as_ptr() as *const u64; - let p2_ptr = p2.as_ptr() as *const u64; - - unsafe { - read_unaligned(p1_ptr) == read_unaligned(p2_ptr) - && read_unaligned(p1_ptr.add(1)) == read_unaligned(p2_ptr.add(1)) - && read_unaligned(p1_ptr.add(2)) == read_unaligned(p2_ptr.add(2)) - && read_unaligned(p1_ptr.add(3)) == read_unaligned(p2_ptr.add(3)) - } -} +pub fn pubkey_eq(p1: &[u8; 32], p2: &[u8; 32]) -> bool { + p1 == p2 +}And remove the unused import:
-use core::ptr::read_unaligned;If you need a
const fnor observed a perf benefit, please provide benchmarks and confirm your MSRV supportsconstequality.program-libs/array-map/tests/array_map_tests.rs (2)
43-45: Avoid temporary String allocations in assertionsCompare by &str to remove needless allocations and temporaries.
- assert_eq!(map.get_by_key(&1), Some(&"one".to_string())); - assert_eq!(map.get_by_key(&2), Some(&"two".to_string())); + assert_eq!(map.get_by_key(&1).map(String::as_str), Some("one")); + assert_eq!(map.get_by_key(&2).map(String::as_str), Some("two"));
1-198: Add coverage for pubkey-optimized APIsNo tests cover get_by_pubkey/get_mut_by_pubkey/find_pubkey_index or pubkey_eq. Add a small test to exercise the [u8; 32] path.
#[test] fn test_pubkey_apis() { let mut m = ArrayMap::<[u8; 32], u32, 4>::new(); let a = [1u8; 32]; let b = [2u8; 32]; // try_insert variant if added; otherwise use insert with a custom error. m.try_insert(a, 10).unwrap(); m.try_insert(b, 20).unwrap(); assert!(light_array_map::pubkey_eq(&a, &a)); assert!(!light_array_map::pubkey_eq(&a, &b)); assert_eq!(m.get_by_pubkey(&a), Some(&10)); assert_eq!(m.find_pubkey_index(&b), Some(1)); if let Some(v) = m.get_mut_by_pubkey(&b) { *v += 1; } assert_eq!(m.get_by_pubkey(&b), Some(&21)); }
📜 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 (6)
.github/workflows/rust.yml(1 hunks)Cargo.toml(2 hunks)program-libs/array-map/Cargo.toml(1 hunks)program-libs/array-map/src/lib.rs(1 hunks)program-libs/array-map/tests/array_map_tests.rs(1 hunks)program-libs/compressed-account/src/pubkey.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- program-libs/array-map/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T21:59:25.201Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.201Z
Learning: Applies to program-libs/account-checks/**/Cargo.toml : Define features solana, pinocchio, and test-only in Cargo.toml; default build should enable none
Applied to files:
.github/workflows/rust.yml
🧬 Code graph analysis (1)
program-libs/array-map/tests/array_map_tests.rs (1)
program-libs/array-map/src/lib.rs (9)
new(23-28)len(30-32)is_empty(34-36)last_accessed_index(38-40)get(42-44)get_by_key(58-60)find_index(80-82)get_u8(50-52)get_mut_u8(54-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: lint
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: cli-v2
ananas-block
force-pushed
the
jorrit/feat-array-map
branch
from
3d94c88 to
e0b67df
Compare
ananas-block
force-pushed
the
jorrit/feat-array-map
branch
from
e0b67df to
e19435c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
program-libs/compressed-account/src/pubkey.rs (1)
78-80: LGTM! Useful addition for typed array access.The method provides efficient typed access to the internal byte array by reference, filling the gap between the slice reference from
AsRef<[u8]>and the owned copy fromto_bytes(). This is particularly useful for interfacing with code that expects&[u8; 32], such as the new array-map crate mentioned in the PR.Optional: Consider adding doc comments for clarity.
While the method name is self-explanatory, adding a brief doc comment could help clarify when to use this method versus the alternatives:
/// Returns a reference to the 32-byte array. /// /// Use this when you need a reference to the sized array type `&[u8; 32]`. /// For a slice reference `&[u8]`, use `AsRef::<[u8]>::as_ref()`. /// For an owned copy, use `to_bytes()`. pub fn array_ref(&self) -> &[u8; 32] { &self.0 }.github/workflows/rust.yml (1)
45-51: Add a no_std build check forlight-array-map.Thanks for wiring the new crate into CI. Because
light-array-mapis advertised asno_std, we should mirror what we do for other no_std crates (e.g.,light-zero-copy) and add a--no-default-featuresbuild to catch regressions early. Right now we only exercise--all-features, so a future change could silently break theno_stdconfiguration.Suggested patch:
cargo test -p light-macros cargo test -p aligned-sized + cargo build -p light-array-map --no-default-features cargo test -p light-array-map --all-features
📜 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 (6)
.github/workflows/rust.yml(1 hunks)Cargo.toml(2 hunks)program-libs/array-map/Cargo.toml(1 hunks)program-libs/array-map/src/lib.rs(1 hunks)program-libs/array-map/tests/array_map_tests.rs(1 hunks)program-libs/compressed-account/src/pubkey.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- program-libs/array-map/Cargo.toml
- program-libs/array-map/tests/array_map_tests.rs
- Cargo.toml
- program-libs/array-map/src/lib.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T21:59:25.201Z
Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.201Z
Learning: Applies to program-libs/account-checks/**/Cargo.toml : Define features solana, pinocchio, and test-only in Cargo.toml; default build should enable none
Applied to files:
.github/workflows/rust.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: system-programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-t...
- GitHub Check: system-programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test -- --skip te...
- GitHub Check: system-programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test ...
- GitHub Check: system-programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test",...
- GitHub Check: system-programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_addre...
- GitHub Check: system-programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test...
- GitHub Check: system-programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_c...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: stateless-js-v2
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v1
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client, cargo test...
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: lint
- GitHub Check: cli-v1
|
Instead of a hash map. for example to store hashes in the ctoken program. |
2 participants
Summary by CodeRabbit
New Features
Tests
Chores