fix(programs): interpret max_top_up as units of 1,000 lamports (L-07)#2265
fix(programs): interpret max_top_up as units of 1,000 lamports (L-07)#2265ananas-block merged 2 commits intomainfrom
Conversation
max_top_up is u16 (max 65,535). As raw lamports this only allows ~0.065 SOL which is insufficient for many use cases. Interpreting as units of 1,000 lamports gives a max of ~65.5M lamports (~0.065 SOL becomes ~65 SOL), covering realistic rent top-up scenarios.
📝 WalkthroughWalkthroughThis PR changes interpretation of Changes
Sequence Diagram(s)(omitted — changes are arithmetic/documentation and do not introduce new multi-component control flow) Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
programs/compressed-token/program/src/shared/compressible_top_up.rs (1)
25-25:⚠️ Potential issue | 🟡 MinorDoc comment should reflect the new unit semantics.
The
max_top_upparameter doc says "Maximum lamports for top-ups combined" but it's no longer raw lamports — it's in units of 1,000 lamports. This will confuse callers who read the doc without looking at the implementation.📝 Suggested doc update
-/// * `max_top_up` - Maximum lamports for top-ups combined (0 = no limit) +/// * `max_top_up` - Maximum top-up budget in units of 1,000 lamports (0 = no limit)programs/compressed-token/program/src/compressed_token/transfer2/compression/mod.rs (1)
40-40:⚠️ Potential issue | 🟡 MinorSame doc staleness as the other file — update the units.
📝 Suggested doc update
-/// * `max_top_up` - Maximum lamports for rent and top-up combined. Transaction fails if exceeded. (0 = no limit) +/// * `max_top_up` - Maximum top-up budget in units of 1,000 lamports. Transaction fails if exceeded. (0 = no limit)
🤖 Fix all issues with AI agents
In
`@programs/compressed-token/program/src/compressed_token/mint_action/actions/process_actions.rs`:
- Around line 52-56: The comment above the lamports_budget calculation is stale
and misleading: update or remove the line that says "+1 allows exact match
(total == max_top_up)" so the comment matches the actual math. Locate the code
around parsed_instruction_data.max_top_up and the variable lamports_budget (the
computation using (max_top_up as u64).saturating_mul(1000).saturating_add(1))
and either remove the redundant/incorrect sentence or reword it to reflect that
max_top_up is in units of 1,000 lamports and the +1 allows an exact match
against max_top_up * 1000.
| // Initialize budget: +1 allows exact match (total == max_top_up) | ||
| let max_top_up: u16 = parsed_instruction_data.max_top_up.get(); | ||
| let mut lamports_budget = (max_top_up as u64).saturating_add(1); | ||
| // max_top_up is in units of 1,000 lamports (max ~65.5M lamports). | ||
| // +1 allows exact match (total == max_top_up * 1000). | ||
| let mut lamports_budget = (max_top_up as u64).saturating_mul(1000).saturating_add(1); |
There was a problem hiding this comment.
Stale comment on line 52 — it still references the old formula.
Line 52 says +1 allows exact match (total == max_top_up) which is the pre-change wording. Lines 54-55 already carry the correct updated explanation. Line 52 is now both redundant and misleading.
📝 Remove or update the stale comment
// Array to accumulate transfer amounts by account index
let mut transfer_map = [0u64; MAX_PACKED_ACCOUNTS];
- // Initialize budget: +1 allows exact match (total == max_top_up)
let max_top_up: u16 = parsed_instruction_data.max_top_up.get();
// max_top_up is in units of 1,000 lamports (max ~65.5M lamports).
// +1 allows exact match (total == max_top_up * 1000).
let mut lamports_budget = (max_top_up as u64).saturating_mul(1000).saturating_add(1);📝 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.
| // Initialize budget: +1 allows exact match (total == max_top_up) | |
| let max_top_up: u16 = parsed_instruction_data.max_top_up.get(); | |
| let mut lamports_budget = (max_top_up as u64).saturating_add(1); | |
| // max_top_up is in units of 1,000 lamports (max ~65.5M lamports). | |
| // +1 allows exact match (total == max_top_up * 1000). | |
| let mut lamports_budget = (max_top_up as u64).saturating_mul(1000).saturating_add(1); | |
| let max_top_up: u16 = parsed_instruction_data.max_top_up.get(); | |
| // max_top_up is in units of 1,000 lamports (max ~65.5M lamports). | |
| // +1 allows exact match (total == max_top_up * 1000). | |
| let mut lamports_budget = (max_top_up as u64).saturating_mul(1000).saturating_add(1); |
🤖 Prompt for AI Agents
In
`@programs/compressed-token/program/src/compressed_token/mint_action/actions/process_actions.rs`
around lines 52 - 56, The comment above the lamports_budget calculation is stale
and misleading: update or remove the line that says "+1 allows exact match
(total == max_top_up)" so the comment matches the actual math. Locate the code
around parsed_instruction_data.max_top_up and the variable lamports_budget (the
computation using (max_top_up as u64).saturating_mul(1000).saturating_add(1))
and either remove the redundant/incorrect sentence or reword it to reflect that
max_top_up is in units of 1,000 lamports and the +1 allows an exact match
against max_top_up * 1000.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
programs/compressed-token/program/docs/ctoken/MINT_TO.md (1)
92-96:⚠️ Potential issue | 🟡 MinorLine 93 doesn't reflect the
*1000scaling — update to match the implementation and earlier documentation.Line 20 correctly states that
max_top_upis in units of 1,000 lamports, but line 93 describes budget initialization asmax_top_up.saturating_add(1)without mentioning the scaling. The actual implementation (inprocess_actions.rs) uses(max_top_up as u64).saturating_mul(1000).saturating_add(1). This inconsistency is confusing since a reader learns the unit specification at line 20, then sees line 93 with no scaling factor applied.Fix
- - Initialize budget to max_top_up.saturating_add(1) (allowing exact match) + - Initialize budget to (max_top_up as u64) * 1000 + 1 (units of 1,000 lamports; +1 allows exact match)
🤖 Fix all issues with AI agents
In `@programs/compressed-token/program/docs/ctoken/APPROVE.md`:
- Line 23: Add a brief audit/context note explaining why `max_top_up` is
interpreted in units of 1,000 lamports (e.g., to extend range while preserving
compatibility and reduce overflow risk) immediately after the "Bytes 8-9
(optional): `max_top_up`" line in APPROVE.md and replicate that same explanatory
sentence near the corresponding occurrences around the other documented uses of
`max_top_up` (the sections currently at the equivalents of lines 116-118 and
137-139) so readers understand the rationale for the scaling change.
|
|
||
| - Bytes 0-7: `amount` (u64, little-endian) - Number of tokens to delegate | ||
| - Bytes 8-9 (optional): `max_top_up` (u16, little-endian) - Maximum lamports for top-up (0 = no limit, default for legacy format) | ||
| - Bytes 8-9 (optional): `max_top_up` (u16, little-endian) - Maximum lamports for top-up in units of 1,000 lamports (e.g., max_top_up=1 means 1,000 lamports, max_top_up=65535 means ~65.5M lamports). 0 = no limit, default for legacy format. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Documentation accurately reflects the unit scaling change.
The documentation correctly explains that max_top_up is now interpreted as units of 1,000 lamports, with accurate examples and consistent code snippets. The math checks out: max_top_up=65535 → 65,535,000 lamports, which addresses the insufficient range issue mentioned in the PR objectives.
The use of saturating_mul(1000) is appropriate for defensive programming, and the "0 = no limit" semantics are clearly documented.
Optional enhancement: Add audit context
Consider adding a brief note explaining the rationale for this unit interpretation. For example, after line 23:
- Bytes 8-9 (optional): `max_top_up` (u16, little-endian) - Maximum lamports for top-up in units of 1,000 lamports (e.g., max_top_up=1 means 1,000 lamports, max_top_up=65535 means ~65.5M lamports). 0 = no limit, default for legacy format.
+
+ **Note**: The 1,000-lamport unit interpretation extends the effective range to ~65.5M lamports, addressing the limitation where raw u16 values (max 65,535 lamports) were insufficient for typical rent requirements (e.g., ~2.5M lamports for a 270-byte account).This would help readers understand why the scaling was introduced.
Also applies to: 116-118, 137-139
🤖 Prompt for AI Agents
In `@programs/compressed-token/program/docs/ctoken/APPROVE.md` at line 23, Add a
brief audit/context note explaining why `max_top_up` is interpreted in units of
1,000 lamports (e.g., to extend range while preserving compatibility and reduce
overflow risk) immediately after the "Bytes 8-9 (optional): `max_top_up`" line
in APPROVE.md and replicate that same explanatory sentence near the
corresponding occurrences around the other documented uses of `max_top_up` (the
sections currently at the equivalents of lines 116-118 and 137-139) so readers
understand the rationale for the scaling change.
2 participants
Summary
max_top_upis a u16, so its max value (65,535) only allows ~65K lamports of rent budget. Typical rent for a 270-byte account is ~2.5M lamports, making the cap ineffective.max_top_upas units of 1,000 lamports, extending the effective range to ~65.5M lamports.Changes
compressible_top_up.rs,compression/mod.rs,process_actions.rs: Changed budget calculation from(max_top_up as u64) + 1to(max_top_up as u64) * 1000 + 1.Summary by CodeRabbit
Bug Fixes
Documentation