Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

feat: eip 1159 like pricing for deposits charged on L1#438

Closed
tynes wants to merge 8 commits intomainfrom
feat/gasmetering
Closed

feat: eip 1159 like pricing for deposits charged on L1#438
tynes wants to merge 8 commits intomainfrom
feat/gasmetering

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented May 13, 2022

Description
Meter deposits on L1, WIP

Implements #401

Replaces:

Metadata
Fixes: ENG-2187

@codecov-commenter
Copy link

codecov-commenter commented May 13, 2022

Codecov Report

Merging #438 (d45baa7) into main (5b692b5) will decrease coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #438      +/-   ##
==========================================
- Coverage   51.86%   51.68%   -0.19%     
==========================================
  Files          63      133      +70     
  Lines        6696    14278    +7582     
==========================================
+ Hits         3473     7379    +3906     
- Misses       2784     5937    +3153     
- Partials      439      962     +523     
Flag Coverage Δ
unittests 51.51% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
op-node/l1/request_sema.go 100.00% <0.00%> (ø)
op-node/node/api.go 59.25% <0.00%> (ø)
op-node/node/bundle_builder.go 100.00% <0.00%> (ø)
op-proposer/mock/l1client.go 0.00% <0.00%> (ø)
op-node/rollup/derive/batch.go 52.05% <0.00%> (ø)
op-node/node/config.go 33.33% <0.00%> (ø)
op-node/l1/source.go 58.64% <0.00%> (ø)
op-node/backoff/operation.go 88.88% <0.00%> (ø)
op-node/rollup/driver/driver.go 100.00% <0.00%> (ø)
op-node/p2p/config.go 7.83% <0.00%> (ø)
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b692b5...d45baa7. Read the comment docs.


storage1 = SlotPacking128x64x64.set(prevBaseFee, prevNum, prevBoughtGas);

storage2 = SlotPacking128x128.set(gasTargetLimit, gasSanityLimit);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gasTargetLimit and gasSanityLimit were technically defined as uint64s in the spec, I made them into uint128s out of convenience. If the must be uint64s then I will update this code

slot := or(slot, shl(64, b))
slot := or(slot, c)
}
return slot;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can delete this return

}
}

// TODO: rename variables + update spec with renamed variables
Copy link
Contributor Author

@tynes tynes May 13, 2022

Choose a reason for hiding this comment

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

Clamp the basefee to 0 if subtraction would underflow, it needs to not underflow and cause a revert. Use saturating math

if (baseFeePerGasDelta == 0) {
baseFeePerGasDelta = 1;
}
nowBaseFee = prevBaseFee - baseFeePerGasDelta;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be a + and not a -

uint128 baseFeePerGasDelta = (prevBaseFee * gasUsedDelta) /
gasTarget /
BASE_FEE_MAX_CHANGE_DENOMINATOR;
if (baseFeePerGasDelta == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: office hours on whether or not we want this in the spec

uint128 nowBaseFee = prevBaseFee;
uint64 nowBoughtGas = prevBoughtGas + requestedGas;
uint64 nowNum = uint64(block.number);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: add comments explaining that dividing by 0 is impossible

@tynes
Copy link
Contributor Author

tynes commented May 13, 2022

Simple way: if msg.value > 0; payable path; else burnable path

@trianglesphere
Copy link
Contributor

Closing in favor of ethereum-optimism/optimism#2575

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants