Common: Remove Genesis State & Functionality

[holgerd77]

As seen in #1673 (also dedicatedly reported in #1695) the inclusion of genesis states as well as associated functionality in the Common library (see here) puts a large burden on bundle sizes of upstream libraries.

I just had a closer look when and how this is used upstream and I am coming to the conclusion that this is generally misplaced and we should consider to move this functionality out of Common.

As some summary: the only occasion where genesis is actually read is in the generateCanonicalStateManager() function of the StateManager. So one could actually say on first thought: "Ah, a VM dependency the most".

Half true, half wrong. The VM only uses this in VM.runBlockchain(). One can actually very well argue if this function belongs in the VM context at all. This is in fact just a mini-script with no meaningful input and output and we could - a bit of a separate discussion - discuss if we would want to remove this from the VM package to be able to remove the burden with the Blockchain dependency there.

Structurally we use the genesis state in a first round in our Client package as some input, and there we actually are also not using the VM.runBlockchain() function and have somewhat decided to reimplement the VM.runBlockchain() functionality, because it was not really serving our needs. I am relatively sure that the VM.runBlockchain() functionality is generally a seldom-used function in the VM context, and the very most people run runCode() or runTx() or runBlock().

Here is a conservative suggestion on what I think we should minimally do based on this framing and analysis:

1. We remove the genesis state files and functionality from Common
2. We simplify the generateCanonicalGenesis() and generateGenesis() functions in the StateManager to one function which always takes the genesis state as a parameter. Alternatively we can also discuss to remove this function from the StateManager at all - I think Sina already asked this question some years ago if this functionality belongs there at all and I would say this question is justified.
3. Same for VM.runBlockchain(), so we can take the state as an input parameter there. Alternatively we can also discuss to remove the function completely.
4. We move the state files over to the client package and store in some genesisStates folder or so there and use this ourselves as well as an input to the functions above in the VM. We can link to this folder in READMEs of other packages (VM, eventually still Common) for users who want to take in the respective genesis states.

Open for discussion here but I would say - also judging from some feedback on and experience with genesis related functionality in the past - that this would structurally make more sense.

Note that it is currently possible to pass in some custom genesis state to the Common library and we would find to find some coherent solution for this which aligns well with the pre-defined chains (overall reduction of functionality, possibility to also pass in the genesis state in Common for the pre-defined chains, just to open up the possible design space here).

[holgerd77]

Alternative proposal (I might like this even more after writing this long thread, lol): we might want to modify the API of Common a bit and generalize with a top level option (so: still in the options dict but not tied to a specific chain) genesisStates where the user just has the responsibility to provide all the genesisStates needed, and the genesisState() function will just return if available or otherwise throw.

Eventually this can also be combined with some ideas from above to get to a consistent but reduced API.

[holgerd77]

I guess this needs some further reflection to bring this all together (role of the StateManager plays also a part here).

[cbrzn]

I have some thoughts regarding this issue:

Based on my noob knowledge so far I think that we should not remove the genesis state but rather improve the way we are handling it to make it easier to use.

In Common constructor we accept chain and customChains arguments, which I think the main objective of those is to help to load the configuration and genesis state of the chain properly - The first does it through a pre-stored file (.json) that lives in the package; the latter does it through a received object through the initialization that gets parsed and stored in the state in a proper way (if any).

Based on your last point:

Note that it is currently possible to pass in some custom genesis state to the Common library and we would find to find some coherent solution for this which aligns well with the pre-defined chains (overall reduction of functionality, possibility to also pass in the genesis state in Common for the pre-defined chains, just to open up the possible design space here).

I think this is the path we should take - I imagine this as Common having the attribute chain that accepts:
Or [string | number | IChain, GenesisState][] for predefined or custom chain with given genesis state
Or string | number for predefined chain
Or IChain for custom chain without genesis state

Instead of chain and customChain attributes. The functionality would consist in:

1.- Get the configuration of the chain and set it up with the expected parameters: Currently, we have the configuration .json files in the chains folder. If the user selects a pre-defined chain (with a string -> chainName or a number -> chainId), it will load the JSON, otherwise, configuration object (as currently expected in customChain attribute) is going to be expected
2.- Load genesis state: This is the main point to solve, we don't want these files anymore. So basically we make the developer pass genesis state if he wants one - We can add documentation on where the genesis states we currently have will live, and that developers can copy those and use them if they want it (Probably we can keep these files in the common package but ignore them on build? This way they wont be in the bundle).

Something like:

const customChain: IChain = {
    ...
}
const common = new Common({ chains: ["mainnet", [customChain, customState] ] })
const common = new Common({ chains: customChain })

Here I am just brainstorming and speaking my mind to see if I am seeing the problem from the correct point of view. I am aware that this is a huge change and should be discussed, so just trying to give my two cents here 😄

I haven't mentioned anything about the VM.runBlockchain or the stateManager because I do not have concrete ideas/knowledge about it - But I would say that we indeed should remove the generateCanonicalGenesis and just use the generateGenesis based on the state given in the parameters

So, in conclusion, some possible next steps can be:

• I think we should remove the genesis state files (or probably just ignore them from the bundle)
• Not remove the genesisState function but instead return the one that has been given in the parameters. This will mean that we will need to be able to somehow pass the genesis state in the chain parameter? For now, we can make the chain attribute to be something like:

type chainType = string | number | Chain | BN | object
type chainWithState = [chainType, genesisState][]
chain: chainType | chainWithState

• Add documentation of new genesis state and where u can the one from pre-defined networks

Hope this makes sense 😅

[holgerd77]

Thanks for the additional proposal, I think we need some time here to decide, maybe let's wait (at least) until the separation of the StateManager and the decisions taken there. Also still not sure if the identified "problem" (bundle size) justifies a rather heavy refactor.

[holgerd77]

Closed by #1916