-
Notifications
You must be signed in to change notification settings - Fork 86
chore: LightProgramInterface program rent sponsor #2214
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughConsolidates rent‑sponsor PDA to version‑1, threads rent_sponsor bump/seeds through config and decompression paths, and changes the initialize builder to emit a multi‑instruction flow that first funds the rent sponsor PDA then initializes LightConfig; tests and macros updated to match new APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test/Client
participant Builder as InitializeRentFreeConfig Builder
participant Instr as Instructions
participant TX as Transaction Sender
participant Funder as Fee Payer
participant RentSponsor as Rent Sponsor PDA
participant Config as Light Config PDA
Test->>Builder: new(..., rent_sponsor_pubkey, rent_sponsor_funding)
Builder->>Builder: derive rent_sponsor PDA & bump (v1)
Builder->>Instr: build() -> [transfer_ix, init_config_ix]
Instr-->>Test: return Vec<Instruction>, config_pda
Test->>TX: create_and_send_transaction([transfer_ix, init_config_ix])
TX->>Funder: debit lamports
Funder->>RentSponsor: transfer rent_sponsor_funding (transfer_ix)
TX->>Config: invoke initialize_compression_config (init_config_ix includes rent_sponsor_bump)
Config-->>TX: init success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk-libs/client/src/interface/initialize_config.rs (1)
46-73: Constructor changes look good — consider documenting zero-funding behavior.The new
rent_sponsor_fundingparameter is well-documented. However, ifrent_sponsor_fundingis0, the transfer instruction will still be generated (transferring 0 lamports). While this won't fail, it's an unnecessary instruction.You might want to conditionally include the transfer only when funding > 0:
♻️ Optional optimization
pub fn build(self) -> (Vec<Instruction>, Pubkey) { let authority = self.authority.unwrap_or(self.fee_payer); let (config_pda, _) = LightConfig::derive_pda(&self.program_id, self.config_bump); - // 1. Transfer to fund rent sponsor PDA - let transfer_ix = system_instruction::transfer( - &self.fee_payer, - &self.rent_sponsor, - self.rent_sponsor_funding, - ); + let mut instructions = Vec::new(); + + // 1. Transfer to fund rent sponsor PDA (only if funding > 0) + if self.rent_sponsor_funding > 0 { + instructions.push(system_instruction::transfer( + &self.fee_payer, + &self.rent_sponsor, + self.rent_sponsor_funding, + )); + } // ... rest of build - (vec![transfer_ix, init_config_ix], config_pda) + instructions.push(init_config_ix); + (instructions, config_pda) }
🤖 Fix all issues with AI agents
In `@sdk-libs/client/src/interface/initialize_config.rs`:
- Line 10: The crate fails to compile because the import use
solana_system_interface::instruction as system_instruction; is unresolved—open
this crate's Cargo.toml and add solana-system-interface = { workspace = true }
to the [dependencies] section so the solana_system_interface crate is available
for the initialize_config.rs import; ensure the dependency entry matches the
workspace style used by sibling crates (sdk, token-client) so the
system_instruction symbol resolves.
In `@sdk-libs/sdk-types/src/lib.rs`:
- Around line 37-42: Rename the instance method RentSponsors::default(&self) ->
&RentSponsor to avoid shadowing the Default trait; update the method name to a
clearer identifier such as v1 or primary (e.g., RentSponsors::v1 or
RentSponsors::primary) and update all call sites to use the new name, keeping
the implementation unchanged (returning &self.sponsors[0]) and preserving
#[inline] and visibility. Ensure any documentation comment is adjusted to
reflect the new name ("Returns the default rent sponsor (version 1)" → "Returns
the version 1 rent sponsor").
sdk-libs/sdk-types/src/lib.rs
Outdated
| impl RentSponsors { | ||
| /// Returns the default rent sponsor (version 1). | ||
| #[inline] | ||
| pub const fn default(&self) -> &RentSponsor { | ||
| &self.sponsors[0] | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consider renaming default() to avoid shadowing the Default trait.
The method default(&self) -> &RentSponsor shadows the conventional Default::default() pattern. While RentSponsors doesn't implement Default, this naming could cause confusion for developers expecting trait-based default behavior.
Consider renaming to v1() or primary() to be more explicit about returning the version 1 sponsor.
♻️ Suggested rename
- /// Returns the default rent sponsor (version 1).
- #[inline]
- pub const fn default(&self) -> &RentSponsor {
+ /// Returns the primary rent sponsor (version 1).
+ #[inline]
+ pub const fn v1(&self) -> &RentSponsor {
&self.sponsors[0]
}📝 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.
| impl RentSponsors { | |
| /// Returns the default rent sponsor (version 1). | |
| #[inline] | |
| pub const fn default(&self) -> &RentSponsor { | |
| &self.sponsors[0] | |
| } | |
| impl RentSponsors { | |
| /// Returns the primary rent sponsor (version 1). | |
| #[inline] | |
| pub const fn v1(&self) -> &RentSponsor { | |
| &self.sponsors[0] | |
| } |
🤖 Prompt for AI Agents
In `@sdk-libs/sdk-types/src/lib.rs` around lines 37 - 42, Rename the instance
method RentSponsors::default(&self) -> &RentSponsor to avoid shadowing the
Default trait; update the method name to a clearer identifier such as v1 or
primary (e.g., RentSponsors::v1 or RentSponsors::primary) and update all call
sites to use the new name, keeping the implementation unchanged (returning
&self.sponsors[0]) and preserving #[inline] and visibility. Ensure any
documentation comment is adjusted to reflect the new name ("Returns the default
rent sponsor (version 1)" → "Returns the version 1 rent sponsor").
- Remove RentSponsors (4 versions) -> single RentSponsor (version 1) - Rename derive_light_rent_sponsors! -> derive_light_rent_sponsor! - Add rent_sponsor_bump to LightConfig for signing during decompression - Add solana-system-interface dependency to light-client - Pass rent_sponsor_seeds to invoke_signed for PDA creation
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 (2)
sdk-libs/sdk/src/interface/config.rs (1)
196-288: Validate rent sponsor PDA + bump during initialization and updates.
rent_sponsorandrent_sponsor_bumpare accepted without verification; if they don't match a canonical PDA derivation, laterinvoke_signed()calls will fail and leave the config inconsistent. Derive the expected PDA and bump usingPubkey::find_program_address()with seeds[RENT_SPONSOR_SEED, version.to_le_bytes()](version = 1) and validate they match the provided values during initialization. Additionally, whenprocess_update_light_config()updatesrent_sponsor, ensurerent_sponsor_bumpis also re-derived and updated in sync, or reject updates that changerent_sponsorwithout a corresponding bump derivation.sdk-libs/macros/src/light_pdas/program/instructions.rs (1)
429-456: Validate rent_sponsor_bump before storing in config.The
rent_sponsor_bumpparameter is accepted but never validated. It should be re-derived usingPubkey::create_program_addresswith seeds[RENT_SPONSOR_SEED, &VERSION.to_le_bytes(), &[rent_sponsor_bump]]and verified to match the suppliedrent_sponsorPubkey before storing. Otherwise, a bad bump persisted in config will cause signing operations to fail later whenrent_sponsor_signer_seeds()attempts to use it.
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: 1
🤖 Fix all issues with AI agents
In `@sdk-libs/macros/src/light_pdas/program/instructions.rs`:
- Around line 551-564: The generated helper light_rent_sponsor() should return
the PDA and its bump so callers (like initialize_compression_config) don't need
to rederive seeds and risk mismatched bumps; change the function signature to
return (::solana_pubkey::Pubkey, u8), call
::solana_pubkey::Pubkey::find_program_address(...) as currently done but capture
the bump (second tuple element) and cast/return it as u8 alongside the pda, and
update callers to use the returned bump value for rent_sponsor_bump instead of
recomputing it.
| // Generate light_rent_sponsor() helper function at crate level (outside the module) | ||
| // This uses crate::ID which is set by declare_id!() | ||
| Ok(quote! { | ||
| /// Returns the program's rent sponsor PDA (version 1). | ||
| /// | ||
| /// Derives the rent sponsor PDA from this program's ID. | ||
| /// This is automatically generated by `#[light_program]`. | ||
| pub fn light_rent_sponsor() -> ::solana_pubkey::Pubkey { | ||
| let (pda, _) = ::solana_pubkey::Pubkey::find_program_address( | ||
| &[b"rent_sponsor", &1u16.to_le_bytes()], | ||
| &crate::ID, | ||
| ); | ||
| pda | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Expose the bump alongside the PDA to prevent caller mismatches.
initialize_compression_config now requires rent_sponsor_bump, but the generated helper only returns the PDA. Consider returning (Pubkey, u8) (or adding a second helper) so callers don’t have to re-derive seeds and accidentally pass a mismatched bump.
♻️ Proposed change
- /// Returns the program's rent sponsor PDA (version 1).
+ /// Returns the program's rent sponsor PDA (version 1) and bump.
///
/// Derives the rent sponsor PDA from this program's ID.
/// This is automatically generated by `#[light_program]`.
- pub fn light_rent_sponsor() -> ::solana_pubkey::Pubkey {
- let (pda, _) = ::solana_pubkey::Pubkey::find_program_address(
+ pub fn light_rent_sponsor_pda() -> (::solana_pubkey::Pubkey, u8) {
+ ::solana_pubkey::Pubkey::find_program_address(
&[b"rent_sponsor", &1u16.to_le_bytes()],
&crate::ID,
- );
- pda
+ )
}
+
+ pub fn light_rent_sponsor() -> ::solana_pubkey::Pubkey {
+ light_rent_sponsor_pda().0
+ }📝 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.
| // Generate light_rent_sponsor() helper function at crate level (outside the module) | |
| // This uses crate::ID which is set by declare_id!() | |
| Ok(quote! { | |
| /// Returns the program's rent sponsor PDA (version 1). | |
| /// | |
| /// Derives the rent sponsor PDA from this program's ID. | |
| /// This is automatically generated by `#[light_program]`. | |
| pub fn light_rent_sponsor() -> ::solana_pubkey::Pubkey { | |
| let (pda, _) = ::solana_pubkey::Pubkey::find_program_address( | |
| &[b"rent_sponsor", &1u16.to_le_bytes()], | |
| &crate::ID, | |
| ); | |
| pda | |
| } | |
| // Generate light_rent_sponsor() helper function at crate level (outside the module) | |
| // This uses crate::ID which is set by declare_id!() | |
| Ok(quote! { | |
| /// Returns the program's rent sponsor PDA (version 1) and bump. | |
| /// | |
| /// Derives the rent sponsor PDA from this program's ID. | |
| /// This is automatically generated by `#[light_program]`. | |
| pub fn light_rent_sponsor_pda() -> (::solana_pubkey::Pubkey, u8) { | |
| ::solana_pubkey::Pubkey::find_program_address( | |
| &[b"rent_sponsor", &1u16.to_le_bytes()], | |
| &crate::ID, | |
| ) | |
| } | |
| pub fn light_rent_sponsor() -> ::solana_pubkey::Pubkey { | |
| light_rent_sponsor_pda().0 | |
| } |
🤖 Prompt for AI Agents
In `@sdk-libs/macros/src/light_pdas/program/instructions.rs` around lines 551 -
564, The generated helper light_rent_sponsor() should return the PDA and its
bump so callers (like initialize_compression_config) don't need to rederive
seeds and risk mismatched bumps; change the function signature to return
(::solana_pubkey::Pubkey, u8), call
::solana_pubkey::Pubkey::find_program_address(...) as currently done but capture
the bump (second tuple element) and cast/return it as u8 alongside the pda, and
update callers to use the returned bump value for rent_sponsor_bump instead of
recomputing it.
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
🤖 Fix all issues with AI agents
In `@sdk-libs/client/src/interface/initialize_config.rs`:
- Around line 112-119: Replace the panic via assert_eq! in the builder's
validation (the Pubkey::find_program_address check that compares
derived_rent_sponsor to self.rent_sponsor) with an Err return so build() returns
a Result instead of unwinding; specifically, change the surrounding
build()/finalize method signature to return Result<(Vec<Instruction>, Pubkey),
YourErrorType>, compute (derived_rent_sponsor, rent_sponsor_bump) with
Pubkey::find_program_address, and if they differ return a descriptive Err (e.g.,
RentSponsorMismatch { expected: derived_rent_sponsor, found: self.rent_sponsor
}) rather than panicking—update call sites to propagate or handle the Result
accordingly.
- Around line 121-126: The transfer instruction is created even when
rent_sponsor_funding is zero, producing a no-op; modify the logic around
system_instruction::transfer that builds transfer_ix to either validate
rent_sponsor_funding > 0 at construction (where the config or struct is
initialized) or skip/omit creating/appending transfer_ix when
rent_sponsor_funding == 0. Ensure you reference and enforce the
rent_sponsor_funding field and change the code that constructs transfer_ix
(system_instruction::transfer(&self.fee_payer, &self.rent_sponsor,
self.rent_sponsor_funding)) to only run when the amount is positive or raise an
error in the initializer.
| // Derive rent sponsor bump (version 1, hardcoded) | ||
| let (derived_rent_sponsor, rent_sponsor_bump) = | ||
| Pubkey::find_program_address(&[b"rent_sponsor", &1u16.to_le_bytes()], &self.program_id); | ||
| assert_eq!( | ||
| derived_rent_sponsor, self.rent_sponsor, | ||
| "Rent sponsor PDA mismatch: derived {:?} != provided {:?}", | ||
| derived_rent_sponsor, self.rent_sponsor | ||
| ); |
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.
🧹 Nitpick | 🔵 Trivial
Consider returning Result instead of panicking on PDA mismatch.
Using assert_eq! in a builder's build() method will cause a panic at runtime if the caller provides an incorrect rent_sponsor. For library code, returning a Result<(Vec<Instruction>, Pubkey), Error> would be more idiomatic and allow callers to handle the error gracefully rather than unwinding the stack.
That said, the current implementation does catch misconfigurations early with a clear error message, so this is a design tradeoff rather than a bug.
♻️ Suggested approach
- pub fn build(self) -> (Vec<Instruction>, Pubkey) {
+ pub fn build(self) -> Result<(Vec<Instruction>, Pubkey), InitConfigError> {
let authority = self.authority.unwrap_or(self.fee_payer);
let (config_pda, _) = LightConfig::derive_pda(&self.program_id, self.config_bump);
// Derive rent sponsor bump (version 1, hardcoded)
let (derived_rent_sponsor, rent_sponsor_bump) =
Pubkey::find_program_address(&[b"rent_sponsor", &1u16.to_le_bytes()], &self.program_id);
- assert_eq!(
- derived_rent_sponsor, self.rent_sponsor,
- "Rent sponsor PDA mismatch: derived {:?} != provided {:?}",
- derived_rent_sponsor, self.rent_sponsor
- );
+ if derived_rent_sponsor != self.rent_sponsor {
+ return Err(InitConfigError::RentSponsorMismatch {
+ derived: derived_rent_sponsor,
+ provided: self.rent_sponsor,
+ });
+ }🤖 Prompt for AI Agents
In `@sdk-libs/client/src/interface/initialize_config.rs` around lines 112 - 119,
Replace the panic via assert_eq! in the builder's validation (the
Pubkey::find_program_address check that compares derived_rent_sponsor to
self.rent_sponsor) with an Err return so build() returns a Result instead of
unwinding; specifically, change the surrounding build()/finalize method
signature to return Result<(Vec<Instruction>, Pubkey), YourErrorType>, compute
(derived_rent_sponsor, rent_sponsor_bump) with Pubkey::find_program_address, and
if they differ return a descriptive Err (e.g., RentSponsorMismatch { expected:
derived_rent_sponsor, found: self.rent_sponsor }) rather than panicking—update
call sites to propagate or handle the Result accordingly.
| // 1. Transfer to fund rent sponsor PDA | ||
| let transfer_ix = system_instruction::transfer( | ||
| &self.fee_payer, | ||
| &self.rent_sponsor, | ||
| self.rent_sponsor_funding, | ||
| ); |
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.
🧩 Analysis chain
🏁 Script executed:
# Find the InitializeRentFreeConfig struct definition and its constructor
rg -n "struct InitializeRentFreeConfig\|impl.*InitializeRentFreeConfig" --type rust -A 15 | head -100Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Search for usages of InitializeRentFreeConfig::new
rg -n "InitializeRentFreeConfig\s*\{|InitializeRentFreeConfig::new" --type rust -B 2 -A 5 | head -150Repository: Lightprotocol/light-protocol
Length of output: 12460
🏁 Script executed:
# Find the initialize_config.rs file to examine the full context
fd "initialize_config.rs" --type fRepository: Lightprotocol/light-protocol
Length of output: 122
🏁 Script executed:
# Look for CLAUDE.md and rent-related documentation
fd "CLAUDE.md" --type f
fd -e md | grep -i "rent\|config" | head -20Repository: Lightprotocol/light-protocol
Length of output: 837
🏁 Script executed:
# Read the initialize_config.rs file to see the full implementation
cat -n sdk-libs/client/src/interface/initialize_config.rsRepository: Lightprotocol/light-protocol
Length of output: 7342
🏁 Script executed:
# Check the CLAUDE.md file at the root to understand rent sponsor semantics
cat -n CLAUDE.md | head -100Repository: Lightprotocol/light-protocol
Length of output: 6365
🏁 Script executed:
# Check the compressible CLAUDE.md for rent sponsor information
cat -n program-libs/compressible/CLAUDE.md | head -150Repository: Lightprotocol/light-protocol
Length of output: 2496
🏁 Script executed:
# Search for validation of rent_sponsor_funding across the codebase
rg "rent_sponsor_funding" --type rust -B 2 -A 2Repository: Lightprotocol/light-protocol
Length of output: 2179
Add validation to prevent zero-amount transfer instruction.
The code creates a transfer instruction without checking if rent_sponsor_funding is zero, which would generate a no-op instruction. Since the rent sponsor PDA is meant to fund rent payments for compressed accounts, zero funding contradicts the intent. Either validate that rent_sponsor_funding > 0 in the constructor, or conditionally skip the transfer instruction when funding is zero.
🤖 Prompt for AI Agents
In `@sdk-libs/client/src/interface/initialize_config.rs` around lines 121 - 126,
The transfer instruction is created even when rent_sponsor_funding is zero,
producing a no-op; modify the logic around system_instruction::transfer that
builds transfer_ix to either validate rent_sponsor_funding > 0 at construction
(where the config or struct is initialized) or skip/omit creating/appending
transfer_ix when rent_sponsor_funding == 0. Ensure you reference and enforce the
rent_sponsor_funding field and change the code that constructs transfer_ix
(system_instruction::transfer(&self.fee_payer, &self.rent_sponsor,
self.rent_sponsor_funding)) to only run when the amount is positive or raise an
error in the initializer.
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: 1
🤖 Fix all issues with AI agents
In `@forester/tests/test_compressible_pda.rs`:
- Around line 284-307: Replace duplicated numeric literals used for funding with
named constants to avoid drift: introduce constants like FUNDING_AIRDROP (e.g.,
20_000_000_000) and RENT_SPONSOR_FUND (e.g., 10_000_000_000) and use them
instead of the raw numbers in the calls to
rpc.airdrop_lamports(&authority.pubkey(), ...) and
InitializeRentFreeConfig::new(..., RENT_SPONSOR_FUND). Update all other tests
that repeat the same literals so they reference the new constants (also adjust
program_rent_sponsor() usage only if it needs the constant); keep references to
rpc.create_and_send_transaction and InitializeRentFreeConfig::new intact.
| // Fund the authority account (extra for tx fees + rent sponsor funding) | ||
| rpc.airdrop_lamports(&authority.pubkey(), 20_000_000_000) | ||
| .await | ||
| .expect("Failed to airdrop to authority"); | ||
|
|
||
| // Fund rent sponsor | ||
| rpc.airdrop_lamports(&RENT_SPONSOR, 10_000_000_000) | ||
| .await | ||
| .expect("Failed to fund rent sponsor"); | ||
|
|
||
| // Initialize compression config | ||
| let (init_config_ix, config_pda) = InitializeRentFreeConfig::new( | ||
| // Initialize compression config (includes rent sponsor funding) | ||
| let rent_sponsor = program_rent_sponsor(); | ||
| let (init_config_ixs, config_pda) = InitializeRentFreeConfig::new( | ||
| &program_id, | ||
| &authority.pubkey(), | ||
| &Pubkey::find_program_address( | ||
| &[program_id.as_ref()], | ||
| &solana_sdk::pubkey!("BPFLoaderUpgradeab1e11111111111111111111111"), | ||
| ) | ||
| .0, | ||
| RENT_SPONSOR, | ||
| rent_sponsor, | ||
| authority.pubkey(), | ||
| 10_000_000_000, | ||
| ) | ||
| .build(); | ||
|
|
||
| rpc.create_and_send_transaction(&[init_config_ix], &authority.pubkey(), &[&authority]) | ||
| rpc.create_and_send_transaction(&init_config_ixs, &authority.pubkey(), &[&authority]) | ||
| .await | ||
| .expect("Initialize config should succeed"); |
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.
🧹 Nitpick | 🔵 Trivial
Avoid magic numbers for funding (prevent drift across tests).
The airdrop amount and rent sponsor funding are duplicated across tests; a small constant set reduces maintenance risk if funding changes again.
✅ Suggested refactor
+const RENT_SPONSOR_FUNDING_LAMPORTS: u64 = 10_000_000_000;
+const AUTHORITY_AIRDROP_LAMPORTS: u64 = RENT_SPONSOR_FUNDING_LAMPORTS + 10_000_000_000; // fees + buffer
@@
-rpc.airdrop_lamports(&authority.pubkey(), 20_000_000_000)
+rpc.airdrop_lamports(&authority.pubkey(), AUTHORITY_AIRDROP_LAMPORTS)
@@
- 10_000_000_000,
+ RENT_SPONSOR_FUNDING_LAMPORTS,Apply the same constants to the other two tests as well.
🤖 Prompt for AI Agents
In `@forester/tests/test_compressible_pda.rs` around lines 284 - 307, Replace
duplicated numeric literals used for funding with named constants to avoid
drift: introduce constants like FUNDING_AIRDROP (e.g., 20_000_000_000) and
RENT_SPONSOR_FUND (e.g., 10_000_000_000) and use them instead of the raw numbers
in the calls to rpc.airdrop_lamports(&authority.pubkey(), ...) and
InitializeRentFreeConfig::new(..., RENT_SPONSOR_FUND). Update all other tests
that repeat the same literals so they reference the new constants (also adjust
program_rent_sponsor() usage only if it needs the constant); keep references to
rpc.create_and_send_transaction and InitializeRentFreeConfig::new intact.
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: 1
🤖 Fix all issues with AI agents
In `@forester/tests/test_compressible_pda.rs`:
- Around line 289-290: The three tests performing airdrops should use a
consistent rate-limit pause: add the same
tokio::time::sleep(Duration::from_secs(3)).await after the airdrop calls in
test_compressible_pda_compression and test_compressible_pda_subscription, or
refactor by creating a helper like airdrop_with_rate_limit and call it from
test_compressible_pda_bootstrap, test_compressible_pda_compression, and
test_compressible_pda_subscription so all airdrops include the 3‑second sleep to
avoid RPC throttling.
♻️ Duplicate comments (1)
forester/tests/test_compressible_pda.rs (1)
292-310: New multi-instruction initialization flow looks correct.The updated
InitializeRentFreeConfigAPI properly threads the rent sponsor PDA and funding amount through the builder. The tuple return(init_config_ixs, config_pda)correctly handles the multi-instruction flow that funds the rent sponsor then initializes the config.Note: The magic number duplication (500_000_000 for rent sponsor funding) was already flagged in a previous review.
| // for rate limit airdrop | ||
| tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; |
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.
🧹 Nitpick | 🔵 Trivial
Rate limit sleep inconsistently applied across tests.
This 3-second sleep for rate limiting is only present in test_compressible_pda_bootstrap, but the other two tests (test_compressible_pda_compression at line 475 and test_compressible_pda_subscription at line 705) also perform airdrops without the rate limit pause. If RPC throttling is a concern, apply the same pattern consistently.
♻️ Suggested fix
Add the same sleep after airdrops in the other tests, or extract a helper:
async fn airdrop_with_rate_limit<R: Rpc>(rpc: &R, pubkey: &Pubkey, lamports: u64) -> Result<(), Box<dyn std::error::Error>> {
rpc.airdrop_lamports(pubkey, lamports).await?;
tokio::time::sleep(tokio::time::Duration::from_secs(3)).await;
Ok(())
}🤖 Prompt for AI Agents
In `@forester/tests/test_compressible_pda.rs` around lines 289 - 290, The three
tests performing airdrops should use a consistent rate-limit pause: add the same
tokio::time::sleep(Duration::from_secs(3)).await after the airdrop calls in
test_compressible_pda_compression and test_compressible_pda_subscription, or
refactor by creating a helper like airdrop_with_rate_limit and call it from
test_compressible_pda_bootstrap, test_compressible_pda_compression, and
test_compressible_pda_subscription so all airdrops include the 3‑second sleep to
avoid RPC throttling.
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.