Remove congestion multiplier from calculate fee #34865
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
as Draft until rebaseed after #34821 is merged |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #34865 +/- ##
=========================================
- Coverage 81.7% 81.7% -0.1%
=========================================
Files 826 826
Lines 223413 223411 -2
=========================================
- Hits 182614 182573 -41
- Misses 40799 40838 +39 |
tao-stones
force-pushed
the
remove-congestion-multiplier-from-calculate-fee
branch
from
4920552
to
49adf4f
Compare
remove congestion_multiplier from calculacte_fee(), leave parameters unused for now.
tao-stones
force-pushed
the
remove-congestion-multiplier-from-calculate-fee
branch
from
49adf4f
to
d8fff7f
Compare
tao-stones
commented
Jan 23, 2024
| // new bank's fee_structure.lamports_per_signature should be inline with | ||
| // what's configured in GenesisConfig | ||
| self.fee_structure.lamports_per_signature = | ||
| derived_fee_rate_governor.lamports_per_signature; |
There was a problem hiding this comment.
currently behavior:
- genesis_config either have fee_rate_gonervor set to 0 (or any other value) lamports_per_signature for test or 5_000 in production;
- fee_structure is always default to 5_000 lamports_per_signature regardless.
This change is to initialize fee_structure to have same lamports_per_signature as in fee_rate_governor.
tao-stones
commented
Jan 23, 2024
| @@ -80,17 +80,10 @@ impl FeeStructure { | |||
| pub fn calculate_fee( | |||
| &self, | |||
| message: &SanitizedMessage, | |||
| lamports_per_signature: u64, | |||
| _lamports_per_signature: u64, | |||
There was a problem hiding this comment.
will remove this parameter in separate PR involving removing fetching nonce lamports_per_signature.
tao-stones
deleted the
remove-congestion-multiplier-from-calculate-fee
branch
This was referenced Jan 29, 2024
Closed
Closed
tao-stones
added a commit
to tao-stones/solana
that referenced
this pull request
Jan 30, 2024
tao-stones
added a commit
that referenced
this pull request
Jan 30, 2024
This reverts commit 73d3973.
tao-stones
added a commit
that referenced
this pull request
Jan 31, 2024
* Revert "refactor unused parameter (#34970)" This reverts commit 0838909. (cherry picked from commit 1542392) # Conflicts: # sdk/src/fee.rs * Revert "separate priority fee and transaction fee from fee calculation (#34757)" This reverts commit 5ecc47e. (cherry picked from commit df2ee12) # Conflicts: # sdk/src/fee.rs * Revert "Remove congestion multiplier from calculate fee (#34865)" This reverts commit 73d3973. (cherry picked from commit 0dcac3f) # Conflicts: # sdk/src/fee.rs * fix merge conflicts --------- Co-authored-by: Tao Zhu <tao@solana.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
congestion_multipliervariable insdk::fee_structure::calculate_fee(...)does not have to do with congestion, it actually zeros out transaction fee for tests that don't handle transaction fees - those tests sets fee rate to zero in genesis config.It is better to sync up fee_structure with fee_rate_governor during bank initialization to support those tests.
Summary of Changes
congestion_multiplierin calculation_fee()Fixes #