ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.58k stars 751 forks source link

Common: Separate Chain configs for Tree Shaking #3448

Closed holgerd77 closed 1 month ago

holgerd77 commented 3 months ago

Part of #3446

Note: This issue is breaking, please do not start until breaking release work has officially started!

Picking up on the following from @roninjin10 :

Individual chains and hardforks are not tree shakeable

Problem: many uses of Ethereumjs common will use a single chain or a single hardfork. But all chain or hardfork specific data is not tree shakable Solution: Seperate all chain data into a single chain config

This is IMO the biggest opportunity the common package has. example, another example

There are many solutions as long as the chain data ends up seperate (never on same class, never on same object). One is an abstract class

export abstract class Common {
  abstract readonly chainId: number
  ...
}

// Then seperately

export class Optimism extends Common {
  chainId: 10
  ...
}

export class Mainnet extends Common {
  chainId: 1
 ...
}

Ok, I had some somewhat larger battle with this and went through several iterations of thinking if feasible or not 😋 . Here is my latest stance, happy to further discuss.

Hardforks

This is basically all config values from hardforks.ts. There is heavy one-way dependency here and values are taken throughout all the HF configurations, analyzing the changes. I do not see enough value here respectively too much effort respectively trade-offs to be done to justify to tear this apart, unless someone comes up with a very brilliant and non-invasive idea.

What we can and very much should do is simplify the structure overall from:

dup: {
  v: 3,
  d: 'Base fee of the DUP opcode',
}

To:

dup:  3, // Base fee of the DUP opcode
}

Im pretty sure no-one is actively using these comments programmatically (even had a quick look at https://www.evm.codes/ and their mouseovers and the like). Not sure about the tree shaking effect here (so these d values might (?) even be tree shaked out, but apart from that the above change will already cut the code size of the whole thing basically into half.

Chains

The main thing to consider here: Common is deeply into integrated into our code base respectively the most exposed auxiliary code part we have in the library suite and code like the following is all over the place:

const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Shanghai })
const stateManager = new DefaultStateManager()
const blockchain = await Blockchain.create()

const evm = await EVM.create({
  common,
  stateManager,
  blockchain,
})

When looking at chains.ts, weighting in on the code size saved compared to the effort of telling people to change their initialization code I was already thinking: this is not worth doing it for sure.

I then thought about if we could retain full compatibility and at the same time leverage some of the benefits:

So what we might work would be that we separate the chains dict from chains.ts into:


export const Mainnet: ChainConfig = {
  chainId: 1,
  networkId: 1, // Or rather kill this with fire along
  / ...
}

export const chains: ChainsDict = {
  mainnet: Mainnet,
  // ...
}

And then we could apply the following structure:

// baseCommon.ts,  Internal class where all code from Common is moved
class BaseCommon {

}

// commons.ts
import { chains (as CHAIN_SPECS) } from './chains.js' 
class Common extends BaseCommon {
}

// mainnet.ts
import { Mainnet } from './chains.js'
class MainnetCommon extends BaseCommon {
}

This would likely need to go along with some refactoring of the whole chain setting logic and code (which would be a good thing anyhow!), maybe also making setChain() private respectively remove along (runtime chain switching does not turned out to be a very viable use case, maybe Remix is doing this though), but then this might work and I would have a tendency to think that it might also be worth the effort.

Common Functions

Not sure, do not want to reserve too much time to further analyze right now. But generally to say that the Common code has bloated a lot over the last years and maybe we can safe some solid amount of code already by some descent refactor

Custom Chains (custom())

Not fully sure if it's worth the effort to tear this apart as well.

holgerd77 commented 1 month ago

Closed by #3545