-
Notifications
You must be signed in to change notification settings - Fork 14
Revert "feat: Cleaner gasless implementation" #405
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
Conversation
This reverts commit 84fd7b8.
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.
27 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
let config = toml::from_str::<EphemeralConfig>(toml).unwrap(); | ||
assert!(config.accounts.payer.try_init_lamports().is_err()); |
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.
style: Add assertion for specific error type instead of just checking is_err()
assert_eq!(post.len(), 1); | ||
assert_eq!(post[0], [LAMPORTS_PER_SOL - 2 * expected_fee, 9103680, 1, 1141440]); | ||
assert_eq!(post[0], [LAMPORTS_PER_SOL - 2 * LAMPORTS_PER_SIGNATURE , 9103680, 1, 1141440]); |
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.
syntax: Extra space before the closing bracket in array access
assert_eq!(post.len(), 1); | |
assert_eq!(post[0], [LAMPORTS_PER_SOL - 2 * expected_fee, 9103680, 1, 1141440]); | |
assert_eq!(post[0], [LAMPORTS_PER_SOL - 2 * LAMPORTS_PER_SIGNATURE , 9103680, 1, 1141440]); | |
assert_eq!(post.len(), 1); | |
assert_eq!(post[0], [LAMPORTS_PER_SOL - 2 * LAMPORTS_PER_SIGNATURE, 9103680, 1, 1141440]); |
/// The payer init balance in lamports. | ||
/// Read it via [Self::try_init_lamports]. | ||
pub init_lamports: Option<u64>, |
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.
logic: The public visibility of init_lamports combined with the comment directing to use try_init_lamports() creates a potential source of bugs. Consider making init_lamports private.
} | ||
Ok(match self.init_lamports { | ||
Some(lamports) => Some(lamports), | ||
None => self.init_sol.map(|sol| sol * LAMPORTS_PER_SOL), |
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.
logic: Integer overflow possible when multiplying sol by LAMPORTS_PER_SOL. Add checked_mul() to handle large values safely.
None => self.init_sol.map(|sol| sol * LAMPORTS_PER_SOL), | |
None => self.init_sol.and_then(|sol| sol.checked_mul(LAMPORTS_PER_SOL)), |
* master: release: v0.1.7 (#409) Feat: simpler gasless implementation (#406) perf: run ensure accounts concurrently feat: account hydration on background task (#407) Revert "feat: Cleaner gasless implementation" (#405) feat: Cleaner gasless implementation (#360) fix: cleanup slots on non-fresh ledger (#404) release: v0.1.5 (#403) fix: properly init geyser plugin for rpc and use valid dummy Library (#402) release: v0.1.4 (#401) fix: remove grpc plugin altogether from geyser manager (#400) Patch tests from failing on subscription due to rate limits (#386)
Reverts #360
Greptile Summary
This PR reverts a gasless transaction implementation (#360), restoring traditional fee handling across the codebase.
LAMPORTS_PER_SIGNATURE
from 0 back to 5000 inmagicblock-bank/src/consts.rs
, re-enabling standard transaction feesconfigs/ephem-*.toml
files, requiring SOL for transaction fees againmagicblock-config/src/errors.rs
to prevent ambiguous account initialization (lamports vs SOL)Cargo.toml
magicblock-bank/src/genesis_utils.rs
to restore fee-based transaction processing