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

crypto/kzg4844: use the new trusted setup file and format #28383

Merged
merged 7 commits into from
Oct 22, 2023

Conversation

kevaundray
Copy link
Contributor

Changes made:

Consensus specs

As this commit the trusted setup file was changed to use the official version that will be used on mainnet.

This trusted setup file also has a different format to the one that was previously being used.

Cryptography libraries

As of this commit and this commit go-kzg and c-kzg are now conformant with the new file format and is running tests against the new consensus spec test vectors.

@kevaundray kevaundray changed the title chore!: use the new trusted setup file and format chore!(deneb): use the new trusted setup file and format Oct 19, 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,
the sha256sum does not check out, because the trusted_setup.json has two additional spaces on each line compared to the original here: https://github.com/ethereum/consensus-specs/blob/c5785899f7a8aba5e0872f8c444f58de56ef09c3/presets/mainnet/trusted_setups/trusted_setup_4096.json
If you remove the superfluous spaces, the shasum checks out.
I would prefer if you committed the original version, just so its easier to verify that this is the correct ceremony

@holiman holiman changed the title chore!(deneb): use the new trusted setup file and format crypto/kzg4844: use the new trusted setup file and format Oct 20, 2023
@holiman
Copy link
Contributor

holiman commented Oct 20, 2023

Please submit a trusted_setup.json which matches the expected signature

go.sum Outdated Show resolved Hide resolved
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

LGTM

for those who wanna verify at home:

$ wget https://raw.githubusercontent.com/ethereum/go-ethereum/4e16f39e151963f225e70d55712c9ec337effbbf/crypto/kzg4844/trusted_setup.json
$ shasum -a 256 trusted_setup.json
0229b43f4fac9b17374809520eb621b5ee1a7f74547e7d36918e7d4b122e178d

Compare with the official PR: ethereum/consensus-specs#3521

@MariusVanDerWijden
Copy link
Member

CI is red, because the execution-spec tests still use the old format

@MariusVanDerWijden
Copy link
Member

diff --git a/build/checksums.txt b/build/checksums.txt
index c752fb8ccd..dd8a9cdbf0 100644
--- a/build/checksums.txt
+++ b/build/checksums.txt
@@ -1,9 +1,9 @@
 # This file contains sha256 checksums of optional build dependencies.
 
-# version:spec-tests 1.0.5
+# version:spec-tests 1.0.6
 # https://github.com/ethereum/execution-spec-tests/releases
-# https://github.com/ethereum/execution-spec-tests/releases/download/v1.0.5/
-d4fd06a0e5f94beb970f3c68374b38ef9de82d4be77517d326bcf739c3cbf3a2  fixtures_develop.tar.gz
+# https://github.com/ethereum/execution-spec-tests/releases/download/v1.0.6/
+485af7b66cf41eb3a8c1bd46632913b8eb95995df867cf665617bbc9b4beedd1  fixtures_develop.tar.gz
 
 # version:golang 1.21.3
 # https://go.dev/dl/

This will update the execution-spec-tests to the newest release v1.0.6 which adds the correct 4844 tests (see https://github.com/ethereum/execution-spec-tests/releases/tag/v1.0.6)

There is also a static test case failing (core/vm/testdata/precompiles/pointEvaluation.json) I'll try to find a fix for that

@MariusVanDerWijden
Copy link
Member

diff --git a/core/vm/testdata/precompiles/pointEvaluation.json b/core/vm/testdata/precompiles/pointEvaluation.json
index 93fc66d836..f2a4fd7a72 100644
--- a/core/vm/testdata/precompiles/pointEvaluation.json
+++ b/core/vm/testdata/precompiles/pointEvaluation.json
@@ -1,9 +1,10 @@
 [
   {
-    "Input": "01d18459b334ffe8e2226eef1db874fda6db2bdd9357268b39220af2d59464fb564c0a11a0f704f4fc3e8acfe0f8245f0ad1347b378fbf96e206da11a5d3630624d25032e67a7e6a4910df58
34b8fe70e6bcfeeac0352434196bdf4b2485d5a1978a0d595c823c05947b1156175e72634a377808384256e9921ebf72181890be2d6b58d4a73a880541d1656875654806942307f266e636553e94006d11423f2
688945ff3bdf515859eba1005c1a7708d620a94d91a1c0c285f9584e75ec2f82a",
+    "Input": "01e798154708fe7789429634053cbf9f99b619f9f084048927333fce637f549b564c0a11a0f704f4fc3e8acfe0f8245f0ad1347b378fbf96e206da11a5d3630624d25032e67a7e6a4910df58
34b8fe70e6bcfeeac0352434196bdf4b2485d5a18f59a8d2a1a625a17f3fea0fe5eb8c896db3764f3185481bc22f91b4aaffcca25f26936857bc3a7c2539ea8ec3a952b7873033e038326e87ed3e1276fd14025
3fa08e9fc25fb2d9a98527fc22a2c9612fbeafdad446cbc7bcdbdcd780af2c16a",
     "Expected": "000000000000000000000000000000000000000000000000000000000000100073eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001",
     "Name": "pointEvaluation1",
     "Gas": 50000,
     "NoBenchmark": false
   }
 ]

Heres the fix to the static test case

@MariusVanDerWijden
Copy link
Member

Again I can't push it to your fork so you will need to apply the diff yourself @kevaundray

@kevaundray
Copy link
Contributor Author

Strange you cannot edit -- I have the correct settings ticked

Screenshot 2023-10-21 at 13 46 34

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
@kevaundray kevaundray force-pushed the kw/new-trusted-setup branch from d1218b5 to 2247f84 Compare October 21, 2023 13:14
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, thanks @kevaundray !

@holiman holiman added this to the 1.13.5 milestone Oct 22, 2023
@MariusVanDerWijden MariusVanDerWijden merged commit a6a0ae4 into ethereum:master Oct 22, 2023
1 check passed
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
…8383)

Changes the trusted_setup to the one created during the kzg-ceremony. The trusted setup file can be found in the consensus specs: https://github.com/ethereum/consensus-specs/blob/dev/presets/mainnet/trusted_setups/trusted_setup_4096.json
---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
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
Dergarcon pushed a commit to specialmechanisms/mev-geth-0x2mev that referenced this pull request Jan 31, 2024
…8383)

Changes the trusted_setup to the one created during the kzg-ceremony. The trusted setup file can be found in the consensus specs: https://github.com/ethereum/consensus-specs/blob/dev/presets/mainnet/trusted_setups/trusted_setup_4096.json
---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Jan 31, 2024
…8383)

Changes the trusted_setup to the one created during the kzg-ceremony. The trusted setup file can be found in the consensus specs: https://github.com/ethereum/consensus-specs/blob/dev/presets/mainnet/trusted_setups/trusted_setup_4096.json
---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 29, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 2, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 2, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 4, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 9, 2024
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.

5 participants