Substate interface #939

cabrador · 2024-01-12T04:53:09Z

Description

This PR creates TransactionData interface to unify the transaction like code.
Any transaction execution-like data should implement this interface in order to unify the code and avoid code redundancy.

Before merging this PR:

Nightly test of whole Aida passes (changes from substate are used)
Remove replace for substate in go.mod

TODOs in separate PR:
~~#942 - Remove setters from the interface~~ done
#943 - Remove hard substate dependency
#944 - Use new SubstateDB
#945 - Change AidaDb type to leveldb
#947 - Use execution data inside processor

Type of change

New feature (non-breaking change which adds functionality)
Refactoring (changes that do NOT affect functionality)

…ead of substate.

b-scholz

Please provide unit test cases for each adapter (old_substate_account/substate_account).
Also, there are some methods in the interface definition that require unit tests (equal and stringify methods). Finally, can you make sure that the new interface does not impose a performance penalty (e.g. measure the runtime before and after your change for checking substate execution). Great job!

executor/transaction/account.go

executor/transaction/alloc.go

executor/transaction/env.go

executor/transaction/result.go

executor/transaction/account.go

executor/transaction/alloc.go

executor/transaction/result.go

Env -> BlockEnvironment Result -> TransactionReceipt

wsodsong · 2024-01-17T05:59:59Z

We have mixed uses of ethereum/common and substate/common type. I suggest that we have an independent module for common and types. This will be used by Aida and substate. ethereum/common will only be used in state/geth.go. This is an action item for future work.

…face package under root.

cmd/aida-vm-sdb/run_tx_generator.go

executor/transaction/account.go

executor/transaction/receipt.go

executor/transaction/block_environment.go

executor/transaction/account.go

executor/transaction/substate_transaction/old_substate_account.go

wsodsong

Regarding file names, since the file is under substate package. I suggest rename them to
substate_alloc.go -> world_state.go or substate_world_state.go
substate_env -> block_environment.go or substate_block_env.go
substate_result -> reciept.go or substate_reciept.go

executor/transaction/validation_data.go

executor/transaction/transaction_data.go

executor/transaction/substate_transaction/substate_alloc.go

executor/transaction/substate_transaction/old_substate_alloc.go

executor/transaction/substate_transaction/old_substate_data.go

executor/transaction/substate_transaction/old_substate_env.go

executor/transaction/substate_transaction/old_substate_result.go

executor/transaction/substate_transaction/old_substate_account.go

Renaming according to gh comments.

wsodsong

Please check txcontact replace in comments. Some doesn't make sense.

cmd/util-db/db/lachesis-update.go

cmd/util-updateset/updateset/generate.go

executor/executor.go

txcontext/context.go

InputValidationData -> InputState OutputValidationData -> OutputState

executor/extension/profiler/operation_profiler_test.go

executor/extension/register/register_progress.go

wsodsong · 2024-01-18T10:40:52Z

executor/substate_processor.go

-	getHash := func(num uint64) common.Hash {
-		if inputEnv.BlockHashes == nil {
-			*hashError = fmt.Errorf("getHash(%d) invoked, no blockhashes provided", num)
-			return common.Hash{}
-		}
-		h, ok := inputEnv.BlockHashes[num]
-		if !ok {
-			*hashError = fmt.Errorf("getHash(%d) invoked, blockhash for that block not provided", num)
-		}
-		return h
-	}


Note that this getHash function returns common.Hash{} when Env.BlockHashes is nil. The function inputEnv.GetBlockHash doesn't seem to handle this.

I have brought back the hash error

executor/substate_provider.go

profile/blockprofile/context_test.go

txcontext/substate/newsubstate/substate.go

stochastic/replay.go

txcontext/substate/substate.go

utils/prime_ctx.go

wsodsong

Looks good! Let's do follow up items in later PRs.

b-scholz

Please can you double-check your comments? You may have replaced strings too eagerly (transaction to tx / txcontext/ etc.). Also please add some comments to your unit tests.
Great!

executor/extension/statedb/temporary_archive_prepper.go

executor/extension/validator/rpc_comparator_test.go

rpc/execute.go

txcontext/substate/account_test.go

cabrador added 7 commits January 12, 2024 11:49

Create TransactionData interface. Use it throughout the Executor inst…

c20c878

…ead of substate.

Use TransactionData in replay substate.

9f20288

Use TransactionData in stochastic record.

50ebc2d

Add functions to interface.

1431842

Add substate implementation of the interface

19ea8da

Finish using the interface thoughout the repository.

51f4c91

Finish using the interface thoughout the repository.

80bb737

b-scholz requested changes Jan 16, 2024

View reviewed changes

cabrador added 6 commits January 16, 2024 10:08

Alloc -> WorldState

45bb828

Env -> BlockEnvironment Result -> TransactionReceipt

Add account tests. Mark old substate as deprecated.

4310000

Fix test

0b2b40a

Add flag to builds.

512cc22

Update make file

4cc9774

Fix mock state.db to match the interface

7e1f9ff

cabrador force-pushed the petr/substate-interface branch from 930a63f to 7e1f9ff Compare January 16, 2024 04:21

cabrador added 7 commits January 16, 2024 20:42

Finish test fix

a03bde2

fix close substate db

fa56862

Fix more code

0a8ae75

Fix more code

0d63400

Revert makefile changes - this issue got fixed in Substate.

0bf9b99

Update substate version to newest develop

1920511

Return iter error

130d036

cabrador added 6 commits January 17, 2024 14:49

Remove new substate usage

a3ffeb8

Move substate_transaction into its own folder. Move Transaction inter…

b5cf5ac

…face package under root.

Revert changes to processor.

443ca82

Add tests to WorldState and receipt

0c5ace2

Remove local substate repo

35671c3

Remove local substate repo

794ef19

wsodsong reviewed Jan 18, 2024

View reviewed changes

cabrador added 2 commits January 18, 2024 12:31

Move transaction interface under root.

b37a2a4

Renaming according to gh comments.

Change type of extensions

2ea5b8c

wsodsong reviewed Jan 18, 2024

View reviewed changes

txcontext/context.go Outdated Show resolved Hide resolved

txcontext/context.go Outdated Show resolved Hide resolved

txcontext/context.go Outdated Show resolved Hide resolved

txcontext/context.go Outdated Show resolved Hide resolved

cabrador added 4 commits January 18, 2024 13:51

Revert replace mistakes.

caa41ab

Revert more replace mistakes.

28b532f

WithValidation -> TxContext

81a9f27

InputValidationData -> InputState OutputValidationData -> OutputState

Regenerate mocks

ad7807a

cabrador requested a review from b-scholz January 18, 2024 07:55

Fix tests

ff9dd19

cabrador force-pushed the petr/substate-interface branch from a27af1f to ff9dd19 Compare January 18, 2024 08:35

cabrador marked this pull request as ready for review January 18, 2024 09:40

wsodsong reviewed Jan 18, 2024

View reviewed changes

cabrador added 3 commits January 19, 2024 10:38

Bring back hash error

0b787a5

Remove WithValidation from factory functions. Fix more replace typos.

6e15cf3

Pass hash error in test

8dcf687

cabrador requested a review from wsodsong January 19, 2024 04:45

wsodsong approved these changes Jan 19, 2024

View reviewed changes

b-scholz approved these changes Jan 19, 2024

View reviewed changes

executor/extension/statedb/temporary_archive_prepper.go Outdated Show resolved Hide resolved

executor/extension/validator/rpc_comparator_test.go Outdated Show resolved Hide resolved

rpc/execute.go Outdated Show resolved Hide resolved

txcontext/substate/account_test.go Show resolved Hide resolved

Fix more replace typos. Add comments to tests

4e8b1b1

cabrador merged commit 387ce8e into develop Jan 22, 2024
1 check passed

wsodsong deleted the petr/substate-interface branch April 18, 2024 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Substate interface #939

Substate interface #939

cabrador commented Jan 12, 2024 •

edited

Loading

b-scholz left a comment •

edited

Loading

wsodsong commented Jan 17, 2024

wsodsong left a comment

wsodsong left a comment

wsodsong Jan 18, 2024

cabrador Jan 19, 2024 •

edited

Loading

wsodsong left a comment

b-scholz left a comment

Substate interface #939

Substate interface #939

Conversation

cabrador commented Jan 12, 2024 • edited Loading

Description

Type of change

b-scholz left a comment • edited Loading

Choose a reason for hiding this comment

wsodsong commented Jan 17, 2024

wsodsong left a comment

Choose a reason for hiding this comment

wsodsong left a comment

Choose a reason for hiding this comment

wsodsong Jan 18, 2024

Choose a reason for hiding this comment

cabrador Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

wsodsong left a comment

Choose a reason for hiding this comment

b-scholz left a comment

Choose a reason for hiding this comment

cabrador commented Jan 12, 2024 •

edited

Loading

b-scholz left a comment •

edited

Loading

cabrador Jan 19, 2024 •

edited

Loading