Skip to content
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

Implement fee schedule in cost calculation #20314

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

tao-stones
Copy link
Contributor

Problem

fee schedule should be implemented in cost calculation.

Summary of Changes

Fixes #

@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #20314 (80fa77d) into master (258c3bc) will decrease coverage by 0.1%.
The diff coverage is 89.3%.

@@            Coverage Diff            @@
##           master   #20314     +/-   ##
=========================================
- Coverage    82.3%    82.2%   -0.2%     
=========================================
  Files         497      492      -5     
  Lines      136959   137286    +327     
=========================================
+ Hits       112854   112952     +98     
- Misses      24105    24334    +229     

@jackcmay
Copy link
Contributor

jackcmay commented Oct 7, 2021

Great stuff! We want to switch the fee calculation from just sigs to using these calculated tx costs.

What do you think about passing a (tx, cost) pair into the bank batches? The bank can then use a fee governor to calculate the final tx fee for each transaction?

We also need to make these fee calculations available for transaction simulation. Maybe we give banks a ref to the cost tracker?

We should also be incorporating the cost of the account data sizes. We could feed that information back to the cost tracker like we currently do for execution costs. If an account size is not in the cache we could keep a list of account size averages per program.

@tao-stones
Copy link
Contributor Author

tao-stones commented Oct 7, 2021

We also need to make these fee calculations available for transaction simulation. Maybe we give banks a ref to the cost tracker?

Yea, was thinking to actually move cost tracker into bank, working PR #20527

@tao-stones
Copy link
Contributor Author

What do you think about passing a (tx, cost) pair into the bank batches? The bank can then use a fee governor to calculate the final tx fee for each transaction?

Yea, perhaps it can fit into bankless leader process: banking stage get tx cost from cost model, before it loads accounts to check balance, it can use fee governor to convert cost into fee; Is this what you are thinking?

@tao-stones
Copy link
Contributor Author

tao-stones commented Oct 7, 2021

We should also be incorporating the cost of the account data sizes. We could feed that information back to the cost tracker like we currently do for execution costs. If an account size is not in the cache we could keep a list of account size averages per program.

That would be great! want to create a ticket for it?

@jackcmay
Copy link
Contributor

jackcmay commented Oct 7, 2021

What do you think about passing a (tx, cost) pair into the bank batches? The bank can then use a fee governor to calculate the final tx fee for each transaction?

Yea, perhaps it can fit into bankless leader process: banking stage get tx cost from cost model, before it loads accounts to check balance, it can use fee governor to convert cost into fee; Is this what you are thinking?

banking stage is already calculating the cost of each tx in order to schedule, rather than re-calc again later just pass this info along

@tao-stones
Copy link
Contributor Author

We should also be incorporating the cost of the account data sizes. We could feed that information back to the cost tracker like we currently do for execution costs. If an account size is not in the cache we could keep a list of account size averages per program.

That would be great! want to create a ticket for it?

#20511 , once the info is fed to cost model, I can update the calculation to incorporate it.

- update cost calculation to closely proposed fee schedule solana-labs#16984
@tao-stones tao-stones force-pushed the fee_structure_to_cost branch from 46a987a to 80fa77d Compare October 7, 2021 16:42
@tao-stones tao-stones merged commit 675fa69 into solana-labs:master Oct 8, 2021
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants