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

Dynamic Gas Implementation #639

Merged
merged 20 commits into from
Aug 21, 2020
Merged

Dynamic Gas Implementation #639

merged 20 commits into from
Aug 21, 2020

Conversation

ec2
Copy link
Member

@ec2 ec2 commented Aug 19, 2020

Summary of changes
Changes introduced in this pull request:

  • Implemented the math for calculated base fees
  • Added new Gas api calls
  • Added base fee verification of the block header
  • Updated messages fields

TODO: Fix broken existing tests including the message serializations

Reference issue to close (if applicable)

Closes

Other information and links

Copy link
Contributor

@StaticallyTypedAnxiety StaticallyTypedAnxiety left a comment

Choose a reason for hiding this comment

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

Looking really good so far, will continue later just had some comments!

{
let mut total_limit = 0;
for b in ts.blocks() {
let (msg1, msg2) = crate::block_messages(db, &b)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to have a function like block_messages which returns ChainMessage because this is one of the cases where we don't care if it is a signed or unsigned message that is returned. We are just worrying about the gas_limit on that particular message.

Edit : This is not necessary to change btw. Was just a comment in passing

blockchain/chain/src/store/base_fee.rs Show resolved Hide resolved
node/rpc/src/gas_api.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

Will review more in detail later

blockchain/state_manager/src/lib.rs Outdated Show resolved Hide resolved
node/rpc/src/gas_api.rs Outdated Show resolved Hide resolved
node/rpc/src/gas_api.rs Outdated Show resolved Hide resolved
blockchain/chain/src/store/base_fee.rs Outdated Show resolved Hide resolved
blockchain/chain/src/store/base_fee.rs Outdated Show resolved Hide resolved
vm/interpreter/src/vm.rs Outdated Show resolved Hide resolved
vm/interpreter/src/vm.rs Show resolved Hide resolved
vm/interpreter/src/vm.rs Outdated Show resolved Hide resolved
@ec2 ec2 requested a review from austinabell August 21, 2020 15:43
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

just small stuff and then good

blockchain/chain/src/store/base_fee.rs Outdated Show resolved Hide resolved
blockchain/chain/src/store/base_fee.rs Outdated Show resolved Hide resolved
blockchain/state_manager/src/lib.rs Show resolved Hide resolved
vm/interpreter/src/vm.rs Outdated Show resolved Hide resolved
vm/interpreter/src/vm.rs Outdated Show resolved Hide resolved
@ec2 ec2 requested a review from austinabell August 21, 2020 18:18
@ec2 ec2 merged commit bbd20cc into main Aug 21, 2020
@ec2 ec2 deleted the ec2/1559 branch August 21, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants