feat: light program 1 byte discriminator#2302
Merged
SwenSchaeferjohann merged 7 commits intomainfrom Feb 20, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds support for variable-length, custom discriminators for LightPinocchio accounts via a new Changes
Sequence Diagram(s)sequenceDiagram
participant DeriveMacro as Macro (derive)
participant Codegen as Compress Generator
participant Backend as CodegenBackend/Pinocchio
participant Runtime as SDK Types (compression/decompression)
DeriveMacro->>DeriveMacro: parse `#[light_pinocchio(discriminator=[...])]`
DeriveMacro-->>Codegen: emit LightDiscriminator + LIGHT_DISCRIMINATOR_SLICE
Codegen->>Backend: generate dispatch + generate_discriminator_collision_checks
Backend-->>Codegen: backend-specific tokens (Pinocchio checks)
Codegen-->>Runtime: emit compression/decompression code using LIGHT_DISCRIMINATOR_SLICE
Runtime->>Runtime: serialize/deserialize using dynamic discriminator prefix length
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
ananas-block
force-pushed
the
jorrit/test-light-program-1-byte-discriminator
branch
from
ddfd0de to
e1fde1a
Compare
ananas-block
changed the title
Contributor
Author
|
@CodeRabbit pls review |
Contributor
|
I'll examine the implementation details, verify the correctness of the 1-byte discriminator logic, and ensure it integrates properly with the existing discriminator infrastructure. 🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/sdk-types/src/interface/program/compression/pda.rs (1)
95-95:⚠️ Potential issue | 🟡 MinorStale comment — discriminator is no longer always 8 bytes.
Line 95 says "includes 8-byte discriminator" but the code on lines 100-103 now writes a variable-length discriminator via
LIGHT_DISCRIMINATOR_SLICE. The comment should reflect this.📝 Suggested fix
- // Serialize updated account data back (includes 8-byte discriminator) + // Serialize updated account data back (includes discriminator prefix)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/sdk-types/src/interface/program/compression/pda.rs` at line 95, Update the stale comment that says "includes 8-byte discriminator" to reflect the variable-length discriminator now used; mention that serialization includes the discriminator provided by LIGHT_DISCRIMINATOR_SLICE (not always 8 bytes). Locate the comment above the serialization step where updated account data is written and replace the fixed-size wording with wording like "includes discriminator from LIGHT_DISCRIMINATOR_SLICE (variable length)" so it accurately describes the behavior of the serialization logic that writes the discriminator.sdk-libs/macros/src/lib.rs (1)
366-398: 🧹 Nitpick | 🔵 TrivialDoc comment doesn't mention the new
light_pinocchioattribute for custom discriminators.The
LightPinocchioAccountderive now accepts#[light_pinocchio(discriminator = [...])]to override the default 8-byte SHA256 discriminator, but the doc block doesn't document this. Users will have to discover it elsewhere.📝 Suggested doc addition
/// ## Requirements /// /// - The `compression_info` field must be non-Option `CompressionInfo` type /// - The `compression_info` field must be first or last field in the struct /// - Struct should be `#[repr(C)]` for predictable memory layout /// - Use `[u8; 32]` instead of `Pubkey` for address fields +/// +/// ## Custom discriminator +/// +/// Use `#[light_pinocchio(discriminator = [1u8])]` to override the default +/// 8-byte SHA256 discriminator with a shorter custom discriminator (e.g. 1 byte). +/// Types with short discriminators must be declared last in `ProgramAccounts` enums +/// to avoid prefix-matching collisions during dispatch. #[proc_macro_derive(LightPinocchioAccount, attributes(compress_as, skip, light_pinocchio))]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/macros/src/lib.rs` around lines 366 - 398, Update the doc comment for the LightPinocchioAccount derive macro to document the new #[light_pinocchio(discriminator = [...])] attribute: explain that you can supply an 8-byte array to override the default 8-byte SHA256 discriminator, show the attribute syntax (e.g., #[light_pinocchio(discriminator = [0x01,0x02,...])]), and note it applies to the LightPinocchioAccount derive (proc_macro_derive(LightPinocchioAccount)) so users know how to provide custom discriminators when deriving the macro.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/macros/src/light_pdas/account/derive.rs`:
- Around line 833-910: Add a unit test that verifies discriminators longer than
8 bytes are rejected: create a test (e.g.
test_light_pinocchio_custom_discriminator_too_long_rejected) that calls
derive_light_pinocchio_account with an input annotated
#[light_pinocchio(discriminator = [1,2,3,4,5,6,7,8,9])] and assert that
result.is_err() (optionally checking the error mentions max length); this uses
the same pattern as test_light_pinocchio_custom_discriminator and
test_light_pinocchio_custom_discriminator_empty_rejected so you can reuse the
parse_quote structure and assertions to ensure derive_light_pinocchio_account
enforces the 8-byte maximum.
- Around line 195-197: The code silently truncates discriminator byte arrays
longer than 8 bytes causing a mismatch between LIGHT_DISCRIMINATOR (fixed
[u8;8]) and LIGHT_DISCRIMINATOR_SLICE; add an explicit upper-bound check in
parse_pinocchio_discriminator to validate that the provided discriminator length
is <= 8 and return an error (or panic) if it exceeds 8 bytes, ensuring callers
(and the attribute parser) reject values like [1,2,...,9] instead of truncating;
reference parse_pinocchio_discriminator, LIGHT_DISCRIMINATOR,
LIGHT_DISCRIMINATOR_SLICE and InAccountInfo.discriminator when adding the
validation.
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 344-354: The zero-copy dispatch slices pod_bytes without ensuring
the data buffer contains the full POD; update the check in the block that uses
<#name as `#account_crate`::LightDiscriminator>::LIGHT_DISCRIMINATOR_SLICE so that
you verify data.len() >= __disc_len + core::mem::size_of::<#name>() before
creating pod_bytes and calling bytemuck::from_bytes; if the buffer is too short,
return an appropriate error instead of proceeding (mirror the error-handling
pattern used around prepare_account_for_compression), and apply the same guard
pattern used in generate_dispatch_fn for the earlier 8-byte discriminator case.
- Around line 314-325: Add a runtime check in compress_dispatch that validates
the enum variant discriminators (the values referenced by
LIGHT_DISCRIMINATOR_SLICE) are ordered from longest to shortest so no shorter
discriminator is a prefix of a later longer one; if the check fails return a
clear error (e.g., InvalidDiscriminatorOrder) or panic with a message pointing
users to reorder variants. Locate the check near the dispatch
construction/iteration in compress_dispatch and iterate the discriminators to
ensure each subsequent discriminator length is <= the previous (and optionally
check prefix conflicts), failing fast on violation. Also update the README and
examples to explicitly document the requirement that non-standard (short)
discriminator variants must be declared last and show a short example of correct
ordering.
---
Outside diff comments:
In `@sdk-libs/macros/src/lib.rs`:
- Around line 366-398: Update the doc comment for the LightPinocchioAccount
derive macro to document the new #[light_pinocchio(discriminator = [...])]
attribute: explain that you can supply an 8-byte array to override the default
8-byte SHA256 discriminator, show the attribute syntax (e.g.,
#[light_pinocchio(discriminator = [0x01,0x02,...])]), and note it applies to the
LightPinocchioAccount derive (proc_macro_derive(LightPinocchioAccount)) so users
know how to provide custom discriminators when deriving the macro.
In `@sdk-libs/sdk-types/src/interface/program/compression/pda.rs`:
- Line 95: Update the stale comment that says "includes 8-byte discriminator" to
reflect the variable-length discriminator now used; mention that serialization
includes the discriminator provided by LIGHT_DISCRIMINATOR_SLICE (not always 8
bytes). Locate the comment above the serialization step where updated account
data is written and replace the fixed-size wording with wording like "includes
discriminator from LIGHT_DISCRIMINATOR_SLICE (variable length)" so it accurately
describes the behavior of the serialization logic that writes the discriminator.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/macros/src/light_pdas/account/derive.rs`:
- Around line 839-937: Add a new test to cover an intermediate-length
discriminator (e.g., 4 bytes) to verify padding logic: create a test function
(e.g., test_light_pinocchio_custom_discriminator_4_bytes) that calls
derive_light_pinocchio_account with #[light_pinocchio(discriminator =
[10,20,30,40])] and assert result.is_ok(); then inspect the generated token
stream string to assert LIGHT_DISCRIMINATOR is padded to 8 bytes
(10,20,30,40,0,0,0,0) and LIGHT_DISCRIMINATOR_SLICE is exactly the 4-byte slice
(&[10,20,30,40]), using the same output.contains(...) pattern as the existing
tests to locate these symbols in the generated output.
SwenSchaeferjohann
approved these changes
Feb 20, 2026
Contributor
SwenSchaeferjohann
left a comment
SwenSchaeferjohann
left a comment
There was a problem hiding this comment.
lgtm, added one question
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 314-325: The compile-time assertion that checks discriminator
prefix relationships is misleading: it rejects any prefix or duplicate pair so
reordering cannot fix collisions; update the user-facing error text emitted by
the const assertion to clearly state that discriminators must be unique and not
be prefixes of one another (mentioning exact duplicates), and revise the
documentation around LIGHT_DISCRIMINATOR_SLICE / the ProgramAccounts enum /
LightProgramPinocchio to remove the suggestion that declaring shorter
discriminators last will resolve collisions and instead state the actual
invariant (no prefix relationships allowed). Locate the const assertion and its
panic/message near the discriminator-checking code and replace the message with
a concise instruction to change one of the discriminator bytes, and edit the
nearby doc comment to reflect the stronger, enforced invariant.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/macros/src/light_pdas/program/compress.rs (1)
525-565: 🧹 Nitpick | 🔵 TrivialPinocchio size validation correctly uses dynamic discriminator length.
For completeness: the non-Pinocchio paths (Lines 545, 557) hardcode
8for the discriminator component. This is correct today because Anchor discriminators are always 8 bytes, but the enum dispatch path at Line 401 now readsLIGHT_DISCRIMINATOR_SLICE.len()dynamically. If Anchor ever supports variable-length discriminators, the size check would silently diverge from dispatch. Consider aligning the non-Pinocchio paths to also useLIGHT_DISCRIMINATOR_SLICE.len()for forward-compatibility, or adding a brief comment explaining the8assumption.♻️ Optional: align non-pinocchio size checks with dispatch for consistency
} else if info.is_zero_copy { // For Pod types, use core::mem::size_of for size calculation quote! { const _: () = { - const COMPRESSED_SIZE: usize = 8 + core::mem::size_of::<#qualified_type>(); + // Anchor discriminators are always 8 bytes. + const COMPRESSED_SIZE: usize = 8 + core::mem::size_of::<#qualified_type>(); if COMPRESSED_SIZE > 800 {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk-libs/macros/src/light_pdas/program/compress.rs` around lines 525 - 565, The size checks for non-Pinocchio branches use a hardcoded 8 for the discriminator, which diverges from the enum dispatch that uses <#qualified_type as `#account_crate`::LightDiscriminator>::LIGHT_DISCRIMINATOR_SLICE.len(); update the two quote! blocks in the info.is_zero_copy and the Borsh/compressed branch to compute COMPRESSED_SIZE using that same LIGHT_DISCRIMINATOR_SLICE.len() instead of 8 (i.e., const COMPRESSED_SIZE: usize = <#qualified_type as `#account_crate`::LightDiscriminator>::LIGHT_DISCRIMINATOR_SLICE.len() + ...), or alternatively add a short comment next to the `8` explaining the 8-byte discriminator assumption if you prefer not to change behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 525-565: The size checks for non-Pinocchio branches use a
hardcoded 8 for the discriminator, which diverges from the enum dispatch that
uses <#qualified_type as
`#account_crate`::LightDiscriminator>::LIGHT_DISCRIMINATOR_SLICE.len(); update the
two quote! blocks in the info.is_zero_copy and the Borsh/compressed branch to
compute COMPRESSED_SIZE using that same LIGHT_DISCRIMINATOR_SLICE.len() instead
of 8 (i.e., const COMPRESSED_SIZE: usize = <#qualified_type as
`#account_crate`::LightDiscriminator>::LIGHT_DISCRIMINATOR_SLICE.len() + ...), or
alternatively add a short comment next to the `8` explaining the 8-byte
discriminator assumption if you prefer not to change behavior.
---
Duplicate comments:
In `@sdk-libs/macros/src/light_pdas/program/compress.rs`:
- Around line 340-352: The bounds-checking logic around the discriminator and
POD bytes is correct so no code change is needed; leave the block in compress.rs
(the snippet using __disc_slice, __disc_len, __expected_len, pod_bytes, and
prepare_account_for_compression) as-is because it validates data.len() >=
__expected_len before slicing and returns early to avoid panics.
SwenSchaeferjohann
deleted the
jorrit/test-light-program-1-byte-discriminator
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes