-
Notifications
You must be signed in to change notification settings - Fork 113
Issues/2282 fee args #2321
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
Issues/2282 fee args #2321
Conversation
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.
Pull request overview
This PR renames the --fee argument to --inclusion-fee to clarify its purpose as the transaction inclusion fee (separate from resource fees). It deprecates --fee while maintaining backward compatibility. A new stellar fees command is introduced with two subcommands: stats (fetches fee statistics) and use (sets default inclusion fee from CLI). The refactoring also reorganizes fee-related code by renaming fee::Args to soroban_data::Args to better represent its purpose of managing Soroban-specific resource data rather than general transaction fees.
Key changes:
- Introduces
--inclusion-feeargument with proper precedence handling (arg > env var > config > default 100) - Adds
stellar fees usecommand to configure default inclusion fees via CLI - Refactors fee handling by separating inclusion fees from resource fees (
resource_feeparameter)
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/soroban-cli/src/soroban_data.rs | Renamed from fee.rs; removed inclusion fee handling, focused on Soroban resource configuration |
| cmd/soroban-cli/src/config/mod.rs | Added inclusion_fee field to config with precedence logic (fee → inclusion_fee → default) |
| cmd/soroban-cli/src/commands/fees/default.rs | New command to set default inclusion fee from amount or fee stats |
| cmd/soroban-cli/src/commands/fees/stats.rs | New command to fetch and display fee statistics |
| cmd/soroban-cli/src/assembled.rs | Added resource_fee parameter to override simulated resource fees |
| cmd/soroban-cli/src/commands/contract/invoke.rs | Updated to use new fee structure; hardcoded fee in simulate method |
| cmd/soroban-cli/src/commands/contract/upload.rs | Updated to use soroban_data and get_inclusion_fee() |
| cmd/soroban-cli/src/commands/tx/args.rs | Moved build_only from fee args; uses get_inclusion_fee() |
| cmd/crates/soroban-test/tests/it/integration/fee_args.rs | New comprehensive tests for fee argument precedence |
| FULL_HELP_DOCS.md | Extensive documentation updates reflecting new fee structure |
Co-authored-by: Nando Vieira <me@fnando.com>
|
The relationship between |
Agreed, tracking here: #2344 |
Co-authored-by: Nando Vieira <me@fnando.com>
… into issues/2282-fee-args
What
Rename the
--feeargument to--inclusion-fee, and deprecate the fee argument.Introduces a new command
stellar feesto replacestellar feestats. This has two subcommands:-> w/ stroops
-> w/ fee stats
-> to remove
$ stellar fees unset ℹ️ The default inclusion fee has been clearedThis change also includes some minor reorganization of arguments now that
feeandinclusion-feehas been removed fromfee::Args.fee::Argswas renamed toresourcesto better represent the purpose of these args (edit transaction resources), and all fee / build / send related args were moved underOptionsinstead ofOptions (RPC).Why
This solves #2282 by allowing users to specify default inclusion fee args easily directly from the CLI. Also, it clears up ambiguity around the meaning of
--feeand other arguments that modifysoroban_data.Known limitations
There is a potentially weird edge case caused by how
--inclusion-feeis given precendence over--fee. The behavior is detailed in a test here: https://github.com/stellar/stellar-cli/blob/issues/2282-fee-args/cmd/crates/soroban-test/tests/it/integration/fee_args.rs#L82-L94TL;DR - is a user has some default config for
--inclusion-feebut provides a--feearg directly, the arg will be ignored in favor of the--inclusion-fee. This is mainly caused by us not knowing if clap sourced the--inclusion-feevalue from an env variable or directly via an argument.