-
Notifications
You must be signed in to change notification settings - Fork 87
chore: add missing compressible extension placeholder #2140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add missing compressible extension placeholder #2140
Conversation
WalkthroughIntroduces a new CompressibleExtension wrapper that nests CompressionInfo under an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
8c485c0 to
0eae076
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
0eae076 to
402fde6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
program-libs/ctoken-interface/tests/ctoken/spl_compat.rs (1)
577-602: Consider adding test coverage forcompression_onlyfield.The
PartialEqtests verify mismatches forcompress_to_pubkeyandaccount_version, but the newcompression_onlyfield is always set tofalse. Consider adding a test case that verifies the comparison behavior whencompression_onlydiffers between instances:// Test compression_only mismatch let ctoken_diff_compression_only = CToken { extensions: Some(vec![ExtensionStruct::Compressible(CompressibleExtension { compression_only: true, // Different from buffer which has false info: compression_info, })]), ..ctoken.clone() }; assert_ne!(zctoken, ctoken_diff_compression_only);
♻️ Duplicate comments (2)
program-libs/ctoken-interface/src/state/extensions/extension_struct.rs (2)
165-168: Stale comment: still referencesCompressionInfoinstead ofCompressibleExtension.The inline comment on line 166 says "CompressionInfo size" but the code correctly uses
CompressibleExtension::byte_len. This was flagged in a prior review and should be updated for consistency.ExtensionStructConfig::Compressible(config) => { - // 1 byte for discriminant + CompressionInfo size + // 1 byte for discriminant + CompressibleExtension size 1 + CompressibleExtension::byte_len(config)? }
257-257: DefineCompressibleExtensionConfigstruct inextension_struct.rs.The
ExtensionStructConfig::Compressiblevariant referencesCompressibleExtensionConfig, but this type is not defined anywhere in the crate. The struct is imported inzero_copy.rsand used in tests with a fieldinfo: CompressionInfoConfig, following the same pattern asTokenMetadataConfig. Add the missing struct definition inextension_struct.rsand ensure it's exported fromstate/extensions/mod.rs.
Changes:
CompressibleExtensionSummary by CodeRabbit
Refactor
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.