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

params: update 4844 parameters #28026

Merged
merged 6 commits into from
Sep 5, 2023
Merged

Conversation

lightclient
Copy link
Member

Back in ACD 163 we agreed to bump the target and max blob values from 2/4 to 3/6 for future devnets until we could decide on final mainnet number. I'm updating it here so that we can get master passing all the hive tests. The final decision is still to be made.

@lightclient
Copy link
Member Author

lightclient commented Aug 29, 2023

Oh we also updated the name of BlobTxTargetBlobGasPerBlock to TargetBlobGasPerBlock.

@holiman holiman added the cancun label Aug 30, 2023
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM
You had me at "makes the word blob not appear twice in a variable name"

@holiman
Copy link
Contributor

holiman commented Aug 30, 2023

Update the test-expects too

--- FAIL: TestCalcExcessBlobGas (0.00s)
    eip4844_test.go:55: excess blob gas mismatch: have 262144, want 131072
    eip4844_test.go:55: excess blob gas mismatch: have 131072, want 0
--- FAIL: TestCalcBlobFee (0.00s)
    eip4844_test.go:73: test 2: blobfee mismatch: have 1 want 2
    eip4844_test.go:73: test 3: blobfee mismatch: have 23 want 111

BlobTxPointEvaluationPrecompileGas = 50000 // Gas price for the point evaluation precompile.

MaxBlobGasPerBlock = 6 * BlobTxBlobGasPerBlob // Maximum consumable blob gas for data blobs per block
TargetBlobGasPerBlock = 3 * BlobTxBlobGasPerBlob // Target consumable blob gas for data blobs per block (for 1559-like pricing)
Copy link
Member

Choose a reason for hiding this comment

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

The params is already a huge collection of completely random names stuff that you have no idea where they come from. I.e. RefundQuotient, which EIP does it belong to? Nobody knows unless you really start digging deep. The BlobTx prefix was meant to follow the Bla12381 prefix idea to group the constants into more meaningful groups, even if a bit too verbose. I'd highly recommend keeping the prefix.

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2023-08-30 at 09 57 28

Copy link
Member Author

Choose a reason for hiding this comment

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

I can open a separate PR to adjust the naming. The change has been reverted in this PR.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman added this to the 1.13.0 milestone Sep 4, 2023
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman merged commit 25733a4 into ethereum:master Sep 5, 2023
1 check passed
jsvisa added a commit to jsvisa/go-ethereum that referenced this pull request Sep 5, 2023
This reverts commit 25733a4.

Signed-off-by: jsvisa <delweng@gmail.com>
jsvisa added a commit to jsvisa/go-ethereum that referenced this pull request Sep 5, 2023
This reverts commit a456438.

Signed-off-by: jsvisa <delweng@gmail.com>
DaniPopes added a commit to DaniPopes/revm that referenced this pull request Sep 6, 2023
rakita added a commit to bluealloy/revm that referenced this pull request Sep 15, 2023
* feat: point evaluation precompile

* feat: BLOBHASH opcode

* refactor: revme runner

* renames

* export global kzg settings

* feat: include kzg settings bytes with `include_bytes!`

* build.rs: remove second option, update docs

* revme: remove unused files and dead code

* feat: implement remaining block and tx env fields

* Add tests for helper functions, update constants

* Add EIP-4844 EF tests to CI, skip outdated ones

* chore: make skip_test more readable

* Fix tests

* Fix fmt

* Fix lints, review

* fix: validate new tx, block fields; add to balance check

* Restore `load_access_list`

* chore: drop c-kzg fork

* test: update tests from Geth

See: <ethereum/go-ethereum#28026>

* chore: revert `is_create` change

* chore: fmt toml

* chore: unnecessary import

* remove unsafe from fake_exponential

* Remove kzg global settings, and move it to CfgEnv

* enable kzg only in std. main README updated

* fmt and clippy

* Update README.md

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* nits and docs

* disable exception eip4844 tests, small refactor

* revert back last commit refactor

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Waylon Jepsen <57912727+0xJepsen@users.noreply.github.com>
tonyke-bot pushed a commit to fuzzland/revm that referenced this pull request Sep 19, 2023
* feat: point evaluation precompile

* feat: BLOBHASH opcode

* refactor: revme runner

* renames

* export global kzg settings

* feat: include kzg settings bytes with `include_bytes!`

* build.rs: remove second option, update docs

* revme: remove unused files and dead code

* feat: implement remaining block and tx env fields

* Add tests for helper functions, update constants

* Add EIP-4844 EF tests to CI, skip outdated ones

* chore: make skip_test more readable

* Fix tests

* Fix fmt

* Fix lints, review

* fix: validate new tx, block fields; add to balance check

* Restore `load_access_list`

* chore: drop c-kzg fork

* test: update tests from Geth

See: <ethereum/go-ethereum#28026>

* chore: revert `is_create` change

* chore: fmt toml

* chore: unnecessary import

* remove unsafe from fake_exponential

* Remove kzg global settings, and move it to CfgEnv

* enable kzg only in std. main README updated

* fmt and clippy

* Update README.md

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* nits and docs

* disable exception eip4844 tests, small refactor

* revert back last commit refactor

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Waylon Jepsen <57912727+0xJepsen@users.noreply.github.com>
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
On ACD 163, it was agreed to bump the target and max blob values from `2/4` to `3/6` for future devnets until we could decide on final mainnet number. This change contains said update, making master pass all the hive tests. The final decision for mainnet cancun is still to be made.
---------

Co-authored-by: Felix Lange <fjl@twurst.com>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
rising9719 pushed a commit to rising9719/revm-v-rust that referenced this pull request Oct 25, 2024
* feat: point evaluation precompile

* feat: BLOBHASH opcode

* refactor: revme runner

* renames

* export global kzg settings

* feat: include kzg settings bytes with `include_bytes!`

* build.rs: remove second option, update docs

* revme: remove unused files and dead code

* feat: implement remaining block and tx env fields

* Add tests for helper functions, update constants

* Add EIP-4844 EF tests to CI, skip outdated ones

* chore: make skip_test more readable

* Fix tests

* Fix fmt

* Fix lints, review

* fix: validate new tx, block fields; add to balance check

* Restore `load_access_list`

* chore: drop c-kzg fork

* test: update tests from Geth

See: <ethereum/go-ethereum#28026>

* chore: revert `is_create` change

* chore: fmt toml

* chore: unnecessary import

* remove unsafe from fake_exponential

* Remove kzg global settings, and move it to CfgEnv

* enable kzg only in std. main README updated

* fmt and clippy

* Update README.md

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* nits and docs

* disable exception eip4844 tests, small refactor

* revert back last commit refactor

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Waylon Jepsen <57912727+0xJepsen@users.noreply.github.com>
migramirez2 pushed a commit to migramirez2/Revm that referenced this pull request Nov 5, 2024
* feat: point evaluation precompile

* feat: BLOBHASH opcode

* refactor: revme runner

* renames

* export global kzg settings

* feat: include kzg settings bytes with `include_bytes!`

* build.rs: remove second option, update docs

* revme: remove unused files and dead code

* feat: implement remaining block and tx env fields

* Add tests for helper functions, update constants

* Add EIP-4844 EF tests to CI, skip outdated ones

* chore: make skip_test more readable

* Fix tests

* Fix fmt

* Fix lints, review

* fix: validate new tx, block fields; add to balance check

* Restore `load_access_list`

* chore: drop c-kzg fork

* test: update tests from Geth

See: <ethereum/go-ethereum#28026>

* chore: revert `is_create` change

* chore: fmt toml

* chore: unnecessary import

* remove unsafe from fake_exponential

* Remove kzg global settings, and move it to CfgEnv

* enable kzg only in std. main README updated

* fmt and clippy

* Update README.md

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* nits and docs

* disable exception eip4844 tests, small refactor

* revert back last commit refactor

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Waylon Jepsen <57912727+0xJepsen@users.noreply.github.com>
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.

6 participants