[NIT-2479] - pathdb support for full nodes #2324

diegoximenes · 2024-05-21T14:35:11Z

pathdb support for full nodes.

There are two steps for running tests default tests and race tests, one to run with HashScheme, and other with PathScheme.

Nodes gracefully shutdown if the provided scheme is not compatible with stored scheme:

Usage of /usr/local/bin/nitro:
ERROR[05-20|22:53:16.690] error initializing database err="incompatible state scheme, stored: hash, provided: path"

execution/gethexec/blockchain.go

magicxyyz

great work! I left some comments, mainly considering tests. We still need to discuss StateHistory default value.

cmd/nitro/nitro.go

util/testhelpers/stackconfig.go

arbos/arbosState/arbosstate.go

arbos/storage/storage.go

arbos/arbosState/initialization_test.go

execution/gethexec/blockchain.go

Tristan-Wilson

LGTM pending resolving @magicxyyz's comments.

I didn't know this area of the code well before so it was interesting to read through it.

tsahee

still a lot left to review.. but look in my comments about defaults for testing

arbos/arbosState/arbosstate.go

arbos/arbosState/initialization_test.go

arbos/arbosState/arbosstate.go

cmd/nitro/init_test.go

magicxyyz

I am still for inlining the unique errors instead of defining new variables, but we might need a third opinion to resolve that.

Otherwise looks good :)

arbos/arbosState/initialize.go

cmd/nitro/nitro.go

execution/gethexec/blockchain.go

.github/workflows/ci.yml

tsahee

Good job.
Small comments + Mostly need to solve conflicts vs the builder.

arbos/arbosState/initialize.go

execution/gethexec/blockchain.go

tsahee · 2024-07-18T02:43:17Z

util/env/env.go

+
+// There are two CI steps, one to run tests using the path state scheme, and one to run tests using the hash state scheme.
+// An environment variable controls that behavior.
+func GetTestStateScheme() string {


this function is only used for testing, so fits better in a "testhelpers" package (could be testhelpers/env.. but probably not worth it)

I moved to "github.com/offchainlabs/nitro/util/testhelpers/env" package.

It will be not straightforward to move this file to be part of the "github.com/offchainlabs/nitro/util/testhelpers" package.
replay depends indirectly on testhelpers, and GetTestStateScheme indirectly depends on "github.com/gofrs/flock".
wasm doens't support some syscalls required by flock:

~/Documents/offchain/git/nitro pathdb_fullnode⇣⇡* ❯ make test-go 52s mkdir -p `dirname [target/machines/latest/replay.wasm](http://target/machines/latest/replay.wasm)` GOOS=wasip1 GOARCH=wasm go build -o [target/machines/latest/replay.wasm](http://target/machines/latest/replay.wasm) ./cmd/replay/... # [github.com/gofrs/flock](http://github.com/gofrs/flock) /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:28:30: undefined: syscall.LOCK_EX /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:39:30: undefined: syscall.LOCK_SH /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:57:20: undefined: syscall.Flock /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:67:20: undefined: syscall.Flock /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:97:20: undefined: syscall.Flock /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:97:50: undefined: syscall.LOCK_UN /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:119:29: undefined: syscall.LOCK_EX /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:131:29: undefined: syscall.LOCK_SH /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:151:17: undefined: syscall.Flock /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:151:52: undefined: syscall.LOCK_NB /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/gofrs/[email protected]/flock_unix.go:151:52: too many errors # [github.com/syndtr/goleveldb/leveldb/storage](http://github.com/syndtr/goleveldb/leveldb/storage) /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:107:16: undefined: newFileLock /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:192:3: undefined: rename /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:267:12: undefined: rename /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:272:12: undefined: syncDir /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:555:9: undefined: rename /home/diego/.gvm/pkgsets/go1.21/global/pkg/mod/github.com/syndtr/[email protected]/leveldb/storage/file_storage.go:591:13: undefined: syncDir make: *** [Makefile:273: [target/machines/latest/replay.wasm](http://target/machines/latest/replay.wasm)] Error 1

system_tests/recreatestate_rpc_test.go

…h scheme.

tsahee

LGTM

tsahee

LGTM

required changes done

magicxyyz

LGTM

cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label May 21, 2024

diegoximenes commented May 21, 2024

View reviewed changes

execution/gethexec/blockchain.go Outdated Show resolved Hide resolved

diegoximenes marked this pull request as ready for review May 21, 2024 19:06

diegoximenes marked this pull request as draft May 23, 2024 00:14

diegoximenes force-pushed the pathdb_fullnode branch 3 times, most recently from f06ff8e to 3a21f0f Compare May 24, 2024 23:49

diegoximenes marked this pull request as ready for review May 27, 2024 19:40

diegoximenes force-pushed the pathdb_fullnode branch from f4d49dd to c3c8e88 Compare May 31, 2024 17:17

diegoximenes requested review from magicxyyz, ganeshvanahalli and Tristan-Wilson May 31, 2024 20:08

magicxyyz reviewed Jun 7, 2024

View reviewed changes

Tristan-Wilson previously approved these changes Jun 14, 2024

View reviewed changes

tsahee reviewed Jun 14, 2024

View reviewed changes

arbos/arbosState/arbosstate.go Outdated Show resolved Hide resolved

arbos/arbosState/initialization_test.go Outdated Show resolved Hide resolved

arbos/arbosState/arbosstate.go Outdated Show resolved Hide resolved

cmd/nitro/init_test.go Show resolved Hide resolved

diegoximenes mentioned this pull request Jun 17, 2024

Exposes CacheConfig.triedbConfig OffchainLabs/go-ethereum#330

Merged

diegoximenes dismissed Tristan-Wilson’s stale review via 6610db5 June 17, 2024 15:45

diegoximenes force-pushed the pathdb_fullnode branch from c3c8e88 to 6610db5 Compare June 17, 2024 15:45

diegoximenes requested review from magicxyyz, tsahee and Tristan-Wilson June 17, 2024 16:45

magicxyyz previously requested changes Jun 25, 2024

View reviewed changes

arbos/arbosState/initialize.go Outdated Show resolved Hide resolved

cmd/nitro/nitro.go Outdated Show resolved Hide resolved

execution/gethexec/blockchain.go Outdated Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

diegoximenes force-pushed the pathdb_fullnode branch from 6610db5 to 7e031ee Compare July 8, 2024 15:38

diegoximenes requested a review from magicxyyz July 8, 2024 17:45

diegoximenes force-pushed the pathdb_fullnode branch from 7e031ee to eb4df87 Compare July 10, 2024 17:41

tsahee reviewed Jul 18, 2024

View reviewed changes

tsahee added the design-approved label Jul 18, 2024

diegoximenes force-pushed the pathdb_fullnode branch from 6273c13 to 69c6593 Compare July 20, 2024 01:04

diegoximenes marked this pull request as draft July 20, 2024 02:12

add pathdb related notes

ffe3a59

diegoximenes added 11 commits July 22, 2024 09:22

Do not create new error variables

6cfde26

Runs race tests with hash and path schemes

ffda7a7

Removes unncessary path logic when initializing arbos in database

875d815

By now -> For now

e3a0460

Moves GetTestStateScheme to util/testhelpers/env

82c5ec7

Do not WriteOrTestGenBlock when blockchain is not empty and using pat…

6256b5b

…h scheme.

Fixes TrieDB().Close() error handling in InitializeArbosInDatabase

3379ab6

Fixex github defaults test for hash scheme

18575a7

Moves test configs from gethexec to system_tests

2425295

Fixes triedb import from geth

5993bc8

Fixes ci, runs gotestsum package by package

5736c56

diegoximenes force-pushed the pathdb_fullnode branch from 69c6593 to 5736c56 Compare July 22, 2024 12:35

diegoximenes added 2 commits July 22, 2024 13:35

Updates go-ethereum

fc900cc

Uses TriedbConfig from geth

6f04d12

diegoximenes marked this pull request as ready for review July 22, 2024 19:28

diegoximenes requested a review from tsahee July 22, 2024 19:28

tsahee previously approved these changes Jul 23, 2024

View reviewed changes

Merge remote-tracking branch 'origin/master' into pathdb_fullnode

97b5bda

tsahee dismissed their stale review via 97b5bda July 23, 2024 02:09

tsahee approved these changes Jul 23, 2024

View reviewed changes

tsahee enabled auto-merge July 23, 2024 02:10

vanpt1114 mentioned this pull request Jul 23, 2024

Feature: Implement PBSS #2494

Closed

magicxyyz approved these changes Jul 23, 2024

View reviewed changes

diegoximenes added 3 commits July 23, 2024 11:33

Merge branch 'master' into pathdb_fullnode

44a15ea

Merge branch 'master' into pathdb_fullnode

af590c3

Merge branch 'master' into pathdb_fullnode

3af4343

tsahee merged commit 3778d98 into master Jul 23, 2024
13 checks passed

tsahee deleted the pathdb_fullnode branch July 23, 2024 17:02

Zorato mentioned this pull request Aug 8, 2024

Question: state schema path & pebble db #2323

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NIT-2479] - pathdb support for full nodes #2324

[NIT-2479] - pathdb support for full nodes #2324

diegoximenes commented May 21, 2024 •

edited

Loading

magicxyyz left a comment

Tristan-Wilson left a comment

tsahee left a comment

magicxyyz left a comment

tsahee left a comment

tsahee Jul 18, 2024

diegoximenes Jul 22, 2024

tsahee left a comment

tsahee left a comment

magicxyyz left a comment

[NIT-2479] - pathdb support for full nodes #2324

[NIT-2479] - pathdb support for full nodes #2324

Conversation

diegoximenes commented May 21, 2024 • edited Loading

magicxyyz left a comment

Choose a reason for hiding this comment

Tristan-Wilson left a comment

Choose a reason for hiding this comment

tsahee left a comment

Choose a reason for hiding this comment

magicxyyz left a comment

Choose a reason for hiding this comment

tsahee left a comment

Choose a reason for hiding this comment

tsahee Jul 18, 2024

Choose a reason for hiding this comment

diegoximenes Jul 22, 2024

Choose a reason for hiding this comment

tsahee left a comment

Choose a reason for hiding this comment

tsahee left a comment

Choose a reason for hiding this comment

magicxyyz left a comment

Choose a reason for hiding this comment

diegoximenes commented May 21, 2024 •

edited

Loading