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

Reject transactions for which there’s no chance for the chain to reach the required gas price #4820

Closed

Conversation

rafal-ch
Copy link
Contributor

@rafal-ch rafal-ch commented Aug 1, 2024

This PR adds another check to the transaction acceptor.

It makes the acceptor reject transactions with gas price tolerance set to a value that is not achievable by the chain within the given TTL.

Calculation is based on the assumptions that chain gas price can only go up and down by 1 every each era.

Additionally, this PR also fixes the problem with the gas price being reset after the network upgrade. With the fix, the gas price is preserved across the upgrades, which is covered by a new NCTL upgrade test in the corresponding PR.

@rafal-ch
Copy link
Contributor Author

rafal-ch commented Aug 1, 2024

bors try

casperlabs-bors-ng bot added a commit that referenced this pull request Aug 1, 2024
Copy link
Contributor

try

Build succeeded:

@rafal-ch
Copy link
Contributor Author

bors try

Copy link
Contributor

try

Build succeeded:

@rafal-ch rafal-ch changed the title Reject transactions for which there's no chance to the chain to reach the required gas price Reject transactions for which there’s no chance for the chain to reach the required gas price Aug 30, 2024
/// Returns the gas price for the upcoming era (if this is a switch block).
pub fn next_era_gas_price(&self) -> Option<u8> {
match self {
BlockHeader::V1(_) => None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could just be 1

@devendran-m devendran-m added rc-5 Release Candidate 5 and removed rc-5 Release Candidate 5 labels Sep 9, 2024
@devendran-m
Copy link
Contributor

Moving it back to backlog

@EdHastingsCasperAssociation
Copy link
Collaborator

EdHastingsCasperAssociation commented Oct 15, 2024

After some back and forth deliberation, we decided to not include this in 2.0 rel

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.

3 participants