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

Restructured fee calculator API #3145

Merged
merged 15 commits into from
Jul 15, 2024

Conversation

abi87
Copy link
Contributor

@abi87 abi87 commented Jun 24, 2024

Why this should be merged

platformvm/txs/fee package will be extended to include a dynamic fee calculator. This PR restructure the fee calculator package and APIs to prepare for the changes

How this works

  • Introduce a fee.CalculatorAPI struct, to be used by clients and a fee.backend interface to be implemented by calculators (currently staticCalculator, soon dynamicCalculator as well)
  • Modify CalculateFee attribute to pick only unsignedTx and credentials as parameters. Currently chainTime is a parameters as well, but that won't be needed for the dynamic calculator, hence it has been moved to the constructor

How this was tested

CI

@abi87 abi87 marked this pull request as ready for review June 24, 2024 08:30
@abi87 abi87 requested review from marun and joshua-kim June 24, 2024 08:30
@abi87 abi87 self-assigned this Jun 24, 2024
Copy link
Contributor

@marun marun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a pretty straightforward refactor. Hard to judge its utility in isolation though.

vms/platformvm/txs/fee/calculator_api.go Outdated Show resolved Hide resolved
vms/platformvm/service_test.go Show resolved Hide resolved
vms/platformvm/txs/executor/staker_tx_verification.go Outdated Show resolved Hide resolved
vms/platformvm/txs/executor/standard_tx_executor.go Outdated Show resolved Hide resolved
vms/platformvm/txs/executor/standard_tx_executor.go Outdated Show resolved Hide resolved
vms/platformvm/txs/fee/calculator_api.go Outdated Show resolved Hide resolved
vms/platformvm/txs/fee/calculator_api.go Outdated Show resolved Hide resolved
vms/platformvm/txs/fee/calculator_api.go Outdated Show resolved Hide resolved
vms/platformvm/txs/fee/calculator_api.go Outdated Show resolved Hide resolved
@abi87 abi87 force-pushed the p-chain_improved_fee_calculator_structure branch from 7350e0a to 7f5952e Compare July 3, 2024 09:13
@abi87 abi87 force-pushed the p-chain_improved_fee_calculator_structure branch from 7f5952e to fc95471 Compare July 3, 2024 09:21
@abi87 abi87 requested review from marun and joshua-kim July 3, 2024 11:59
@StephenButtolph StephenButtolph added this to the v1.11.10 milestone Jul 15, 2024
@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 15, 2024
Merged via the queue into master with commit 730f72e Jul 15, 2024
20 checks passed
@StephenButtolph StephenButtolph deleted the p-chain_improved_fee_calculator_structure branch July 15, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants