-
Notifications
You must be signed in to change notification settings - Fork 96
Add block_max_transactions and all gas price metadata to ConsensusParameters (creation of V3)
#905
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
base: master
Are you sure you want to change the base?
Conversation
|
Yes, we also can add this one to consensus parameters=) |
block_max_transactions to ConsensusParametersblock_max_transactions and gas_price_metadata to ConsensusParameters (creation of V3)
block_max_transactions and gas_price_metadata to ConsensusParameters (creation of V3)block_max_transactions and all gas price metadata to ConsensusParameters (creation of V3)
| pub gas_price_factor: NonZeroU64, | ||
| pub min_da_gas_price: u64, | ||
| pub max_da_gas_price: u64, | ||
| pub max_da_gas_price_change_percent: u16, |
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.
We only need the field max_gas_price_increase_percent, all other fields shouldn't be part of the consensus
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.
i want to disagree with this. while keeping just this one field is nice to ensure compatibility with all other operators, we have to make sure that all parameters with which the block producer determines the gas price (p, d, gas price factor etc) is included in the consensus parameters for transparency sake
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.
We don't know what algorithm we want to use for the gas price on consensus level. Right now, only one entity - the block producer, decides how the gas price is calculated. On the consensus level, we just agree that gas price shouldn't increase more than X %.
It is done for flexibility and to avoid setting the algorithm in stone.
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.
if that is the case then perhaps we should version this field in the consensus parameters. what do you think?
fuel-tx/src/transaction/consensus_parameters/gas_price_metadata.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
| pub gas_price_factor: NonZeroU64, | ||
| pub min_da_gas_price: u64, | ||
| pub max_da_gas_price: u64, | ||
| pub max_da_gas_price_change_percent: u16, |
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.
We don't know what algorithm we want to use for the gas price on consensus level. Right now, only one entity - the block producer, decides how the gas price is calculated. On the consensus level, we just agree that gas price shouldn't increase more than X %.
It is done for flexibility and to avoid setting the algorithm in stone.
Request from : #899 (review) and #888
Checklist
Before requesting review
After merging, notify other teams