wip: use libevm (params, core/vm) #1335

darioush · 2024-09-05T22:08:44Z

initial attempt to use libevm params extensions

simplify nit: spelling

…vm-params

… be like Merge)

darioush · 2024-09-13T22:53:39Z

core/evm.go

+
+	// Durango enables the Shanghai upgrade of eth, which takes place after the
+	// Merge upgrade.
+	isDurango := params.GetExtra(chain.Config()).IsDurango(header.Time)


upstream avoids this dependency here (on chain config/rules) by checking if Difficulty is 0,
but we cannot rely on that because we already made blocks with difficulty set to 1.

…vm-params

ARR4N · 2024-09-17T20:41:41Z

params/hooks_libevm.go

 	chainConfig  *gethparams.ChainConfig
 	blockContext *BlockContext
 }

 func (a accessableState) GetStateDB() contract.StateDB {
 	// XXX: Whoa, this is a hack
-	return a.StateDB.(contract.StateDB)
+	return a.StateReader.(contract.StateDB)


Is this just to make it compliant with an existing interface?

To guarantee that it's safe, without using the , ok idiom you could:

var ( sr libevm.StateReader = a.StateReader _ contract.StateDB = (libevm.StateReader)(nil) // prove that the next line won't panic ) return sr.(contract.StateDB)

It's gross, but at least it's temporary!

…vm-params

ceyonur · 2024-10-03T10:27:29Z

accounts/abi/bind/precompilebind/precompile_bind_test.go

@@ -695,6 +695,11 @@ func TestPrecompileBind(t *testing.T) {
 	if out, err := replacer.CombinedOutput(); err != nil {
 		t.Fatalf("failed to replace binding test dependency to current source tree: %v\n%s", err, out)
 	}
+	replacer = exec.Command(gocmd, "mod", "edit", "-x", "-require", "github.com/ethereum/[email protected]", "-replace", "github.com/ethereum/go-ethereum=github.com/ava-labs/[email protected]")


any reason why we not directly use ava-labs/go-ethereum as directy dependency?

The module renaming is not ready yet, we will use it directly once it is ready.

ceyonur · 2024-10-03T11:14:58Z

cmd/evm/internal/t8ntool/transition.go

@@ -205,7 +205,7 @@ func Transition(ctx *cli.Context) error {

 func applyLondonChecks(env *stEnv, chainConfig *params.ChainConfig) error {
 	// NOTE: IsLondon replaced with IsSubnetEVM here
-	if !chainConfig.IsSubnetEVM(env.Timestamp) {
+	if !params.GetExtra(chainConfig).IsSubnetEVM(env.Timestamp) {


I wonder if we should just use London here

ceyonur · 2024-10-03T11:17:45Z

core/rawdb/accessors_metadata.go

@@ -80,7 +80,7 @@ func ReadChainConfig(db ethdb.KeyValueReader, hash common.Hash) *params.ChainCon
 	if len(data) == 0 {
 		return &config // return early if no upgrade config is found
 	}
-	if err := json.Unmarshal(data, &config.UpgradeConfig); err != nil {
+	if err := json.Unmarshal(data, &params.GetExtra(&config).UpgradeConfig); err != nil {


nit: should we instead treat UpgradeConfig as a seperate struct and then set to configExtra? I assume config.UpgradeConfig initially should be empty.

do you mean handling it entirely separately? I tried something like this here: #1351

ceyonur · 2024-10-03T11:41:35Z

core/txpool/validation.go

@@ -75,17 +75,17 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types
 		return fmt.Errorf("%w: transaction size %v, limit %v", ErrOversizedData, tx.Size(), opts.MaxSize)
 	}
 	// Ensure only transactions that have been enabled are accepted
-	if !opts.Config.IsSubnetEVM(head.Time) && tx.Type() != types.LegacyTxType {
+	if !params.GetExtra(opts.Config).IsSubnetEVM(head.Time) && tx.Type() != types.LegacyTxType {


should we rather use equivalent geth forks like london here?

ceyonur · 2024-10-03T11:45:08Z

core/types/tx_access_list.go

-	}
-	return sum
-}
+type AccessTuple = gethtypes.AccessTuple

 // AccessListTx is the data of EIP-2930 access list transactions.
 type AccessListTx struct {


why do we keep this as a seperate struct here?

ceyonur · 2024-10-04T15:27:56Z

params/config_extra.go

 }

-func (r *Rules) PredicatersExist() bool {
+func (r *RulesExtra) PredicatersExist() bool {
+	// Methods on *RulesExtra handle nil receiver so params.Rules is an initialized struct.


in what case we call function on a nil pointer?

I don't think it is needed, removed it for now

ceyonur · 2024-10-04T15:35:03Z

params/config_extra.go

+func WithExtra(c *ChainConfig, extra *ChainConfigExtra) *ChainConfig {
+	// XXX: Hack to initialize the ChainConfigExtra pointer in the ChainConfig.
+	jsonBytes, err := json.Marshal(c)
+	if err != nil {
+		panic(err)
+	}
+	var newCfg ChainConfig
+	if err := json.Unmarshal(jsonBytes, &newCfg); err != nil {
+		panic(err)
+	}
+
+	*GetExtra(&newCfg) = *extra


I'm a bit lost here. Why do we need to do marshal+unmarshaling? Also that setting double pointer with GetExtra seems too hacky. can we rather have a setter in libevm if we cannot initialize chainconfig without this? Maybe the whole WithExtra thing can be moved to libevm?

The marshal / unmarshal is no longer needed and we can use the SetOnChainConfig method here.

ceyonur · 2024-10-04T15:56:47Z

params/hooks_libevm.go

+			blockContext: &BlockContext{
+				number:           env.BlockNumber(),
+				time:             env.BlockTime(),
+				predicateResults: predicateResults,


seems we have removed predicateResults from vm.BlockContext (in core), should we rather pass them with header.extra and let the consumer parse it?

ceyonur · 2024-10-04T15:57:31Z

params/hooks_libevm.go

+var PredicateParser = func(extra []byte) (PredicateResults, error) {
+	return nil, nil


it's a bit hard to follow this and weird to expect some other pkg to set this to the actual implementation.

ceyonur · 2024-10-04T18:16:32Z

plugin/evm/block_verification.go

@@ -27,7 +27,7 @@ func NewBlockValidator() BlockValidator {
 	return &blockValidator{}
 }

-func (v blockValidator) SyntacticVerify(b *Block, rules params.Rules) error {
+func (v blockValidator) SyntacticVerify(b *Block, rules_ params.Rules) error {


nit: can we rename rules_ to rules and then below rules to rulesExtra?

ceyonur

can we also use this or create another as a base branch for libevm work? IMHO we should test these as a whole before merging to master. We can even constantly run a node with the libevm base branch.

darioush force-pushed the use-libevm-params branch 2 times, most recently from 95ce588 to 8574ab3 Compare September 10, 2024 16:16

darioush added 14 commits September 10, 2024 13:22

wip: use libevm (params)

6d60741

base change

d999563

test unmarshal capability

1c7e25b

separate avalanche stuff to its own type

a119516

separate RulesExtra

25bb446

separate calls to extra

c371a3a

use darioush/access-list-for-hook

9b27ebe

config_extra: move methods off Rules/ChainConfig receivers

784e2e6

un-embed RulesExtra

a8a5c80

params: use gethparams.Rules

d74cea6

simplify nit: spelling

unused

ed1165b

update chainConfig.Rules callsites to pass params.IsMergeTODO

e1088b1

params/config: type alias ConfigCompatError

846f52a

bring in gethparams.ChainConfig (wip)

932967d

darioush force-pushed the use-libevm-params branch from cc7d5e6 to 932967d Compare September 10, 2024 20:22

darioush added 13 commits September 10, 2024 15:42

params: add LondonBlock

053af95

params: add ShanghaiTime

44db6c7

params: add BerlinBlock

cd5adc9

params: unused code

70659d3

reduce diff

5d5fb59

update to darioush/json-root branch

07203b0

params: add IsLondon checks to Cancun (similar to upstream)

99c1b70

race free impl for WithExtra, still hack

e8febb8

Merge branch 'master' of github.com:ava-labs/subnet-evm into use-libe…

04338c7

…vm-params

use RulesExtra (instead of ptr)

a5a43cc

bump libevm

4a41e81

bump libevm

5049989

chain hooks for create

2a46ead

darioush added 7 commits September 11, 2024 15:54

use geth forks in core/vm/evm.go

672f54e

core/types, core/state: import gethparams

7b8ec21

core/vm: use gethparams and hook in precompiles

f30f450

core/types: use upstream Log type

dc4b361

core/types: use upstream for AccessList

5038b15

core/vm: use libevm

dad843a

linting

0fe10ff

darioush changed the title ~~wip: use libevm (params)~~ wip: use libevm (params, core/vm) Sep 12, 2024

darioush added 3 commits September 12, 2024 14:11

fix mockgen

5b073ad

adjustment for libevm chainconfig changes

34a17a6

populate vm.BlockContext.Random as Difficulty starting at Durango (to…

73b5559

… be like Merge)

darioush commented Sep 13, 2024

View reviewed changes

darioush added 4 commits September 13, 2024 16:10

nit

61b1694

Merge branch 'master' of github.com:ava-labs/subnet-evm into use-libe…

8d2493f

…vm-params

update libevm

12cd83e

update libevm

20854a1

ARR4N reviewed Sep 17, 2024

View reviewed changes

darioush added 8 commits September 24, 2024 10:48

rebase to latest libevm (+ fix commit on darioush/json-root)

e9ee414

add support for custom upstream tag

61eb933

remove eth/tracers subdirs

28caecc

update geth-allowed-packages.txt

86cdfbe

Merge branch 'master' of github.com:ava-labs/subnet-evm into use-libe…

f1d77d2

…vm-params

lint

7bfbe08

more lint

27a8bfb

more lint

d72ff25

ceyonur reviewed Oct 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wip: use libevm (params, core/vm) #1335

wip: use libevm (params, core/vm) #1335

darioush commented Sep 5, 2024 •

edited

Loading

darioush Sep 13, 2024

ARR4N Sep 17, 2024

ceyonur Oct 3, 2024

darioush Oct 11, 2024

ceyonur Oct 3, 2024

ceyonur Oct 3, 2024

darioush Oct 14, 2024

ceyonur Oct 3, 2024

ceyonur Oct 3, 2024

ceyonur Oct 4, 2024

darioush Oct 14, 2024

ceyonur Oct 4, 2024

darioush Oct 11, 2024

ceyonur Oct 4, 2024

ceyonur Oct 4, 2024

ceyonur Oct 4, 2024

ceyonur left a comment

		var PredicateParser = func(extra []byte) (PredicateResults, error) {
		return nil, nil

wip: use libevm (params, core/vm) #1335

Are you sure you want to change the base?

wip: use libevm (params, core/vm) #1335

Conversation

darioush commented Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceyonur left a comment

Choose a reason for hiding this comment

darioush commented Sep 5, 2024 •

edited

Loading