-
Notifications
You must be signed in to change notification settings - Fork 90
fix(programs): interpret max_top_up as units of 1,000 lamports (L-07) #2265
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,7 +51,9 @@ pub fn process_actions<'a>( | |||||||||||||||||||||
| 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(); | ||||||||||||||||||||||
| 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); | ||||||||||||||||||||||
|
Comment on lines
52
to
+56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale comment on line 52 — it still references the old formula. Line 52 says 📝 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Start metadata authority with same value as mint authority | ||||||||||||||||||||||
| for action in parsed_instruction_data.actions.iter() { | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
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
LGTM! Documentation accurately reflects the unit scaling change.
The documentation correctly explains that
max_top_upis 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:
This would help readers understand why the scaling was introduced.
Also applies to: 116-118, 137-139
🤖 Prompt for AI Agents