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

all: stateless witness builder and (self-)cross validator #29719

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented May 7, 2024

This PR does the witness construction during block execution, after which a custom self-stateless-cross-validation step is added (if the entire thing is enabled via a CLI). No external validator is defined yet, the purpose of the current integration is to run consensus tests with witnesses and run live benchmarkers with witnesses to sanity check any possible faults. Metrics and benchmarks will probably need a followup PR to focus on that.

@jwasinger jwasinger marked this pull request as ready for review May 8, 2024 22:40
@jwasinger jwasinger marked this pull request as draft May 9, 2024 05:01
@jwasinger jwasinger force-pushed the stateless-witness-builder branch 2 times, most recently from fce0ff4 to 3bef048 Compare May 13, 2024 19:50
@jwasinger jwasinger marked this pull request as ready for review May 14, 2024 06:50
@jwasinger
Copy link
Contributor Author

jwasinger commented May 14, 2024

This adds a new test suite TestStatelessBlockchain which adds a large amount of running time to CI. We may consider disabling it as a last step before merging this when it is approved.

@jwasinger jwasinger marked this pull request as draft May 14, 2024 18:05
@jwasinger
Copy link
Contributor Author

jwasinger commented May 14, 2024

Converted back to draft while I address remaining TODOs (parts of the code are confusing rn).

core/state/state_witness.go Outdated Show resolved Hide resolved
@jwasinger jwasinger force-pushed the stateless-witness-builder branch 3 times, most recently from 443f25b to be10f33 Compare June 10, 2024 02:37
@jwasinger jwasinger requested a review from s1na as a code owner June 10, 2024 03:36
@jwasinger jwasinger marked this pull request as draft June 10, 2024 03:46
@jwasinger jwasinger force-pushed the stateless-witness-builder branch 3 times, most recently from a8f4324 to 148d0a1 Compare June 12, 2024 21:47
@jwasinger
Copy link
Contributor Author

@karalabe this still fails 2 tests:

go test -run=StatelessBlockchain -timeout=20m
--- FAIL: TestStatelessBlockchain (0.34s)
    --- FAIL: TestStatelessBlockchain/InvalidBlocks/bcUncleHeaderValidity/gasLimitTooLowExactBound.json (0.01s)
        --- FAIL: TestStatelessBlockchain/InvalidBlocks/bcUncleHeaderValidity/gasLimitTooLowExactBound.json/gasLimitTooLowExactBound_Cancun (0.00s)
--- FAIL: TestStatelessBlockchain (0.51s)
    --- FAIL: TestStatelessBlockchain/InvalidBlocks/bcUncleHeaderValidity/gasLimitTooLowExactBound2.json (0.01s)
        --- FAIL: TestStatelessBlockchain/InvalidBlocks/bcUncleHeaderValidity/gasLimitTooLowExactBound2.json/gasLimitTooLowExactBound2_Cancun (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

I need to fix those and double-check and clean up the diff (parts are still a bit messy). Overall, I think it's getting close though.

trie/trie.go Outdated Show resolved Hide resolved
trie/secure_trie.go Outdated Show resolved Hide resolved
core/state/statedb.go Outdated Show resolved Hide resolved
@karalabe karalabe force-pushed the stateless-witness-builder branch 6 times, most recently from 3eb9519 to fb3450a Compare June 19, 2024 16:34
core/types/witness.go Outdated Show resolved Hide resolved
tests/block_test.go Outdated Show resolved Hide resolved
@karalabe karalabe marked this pull request as ready for review June 21, 2024 11:22
@karalabe
Copy link
Member

@jwasinger @rjl493456442 @fjl @holiman PTAL, the PR is ready for review/merge.

@karalabe
Copy link
Member

FWIW, I'd appreciate a merge commit vs. squash merge to retain ownership of my massive cleanup commit.

@karalabe
Copy link
Member

One thing I want to change. Replace the pre-root hash with the parent header. In the current shape of the code the pre-root is trusted, which allows the block being verified to be malicious (i.e. unverified pre-root == unverified starting state == arbitrary post state). By adding the parent header we are proving the link between the validated block and the pre-root state (via the parent header).

@karalabe karalabe changed the title Stateless witness builder all: stateless witness builder and (self-)cross validator Jun 21, 2024
core/stateless/encoding.go Outdated Show resolved Hide resolved
core/stateless/encoding.go Outdated Show resolved Hide resolved
core/block_validator.go Outdated Show resolved Hide resolved
core/state/database.go Show resolved Hide resolved
core/blockchain.go Show resolved Hide resolved
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

SGTM, just a few nits

@karalabe karalabe added this to the 1.14.6 milestone Jun 25, 2024
@karalabe
Copy link
Member

There are some wonkyness left in, but I have a followup PR which does the live integration and that does further cleanups. Better to have the witness stuff in here and then have the mushing into our live codebase separately. Also a lot of stuff gets deleted that will not be needed any more with a live hook.

@karalabe karalabe merged commit ed8fd0a into ethereum:master Jun 25, 2024
2 of 3 checks passed
@immense055
Copy link

d86b650

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.

6 participants