Refactor codebase

[wemeetagain]

• Move all struct and tree logic to the Type
  • Instead of having separate 'handler' instances with circular references back to the type, attach all methods to the type
• Rename all underlying struct and tree logic with struct_* and tree_*
  • High-level wrapper functions and backing-agnostic methods remain un-prefixed (eg: getMaxSerializedLength)
  • struct_* methods operate on structs, using type information + get/set to perform ssz operations
  • tree_* methods operate on Trees, using type information to perform ssz operations
  • tree_getProperty and tree_setProperty are high level methods that may either return a primitive or a Tree, depending on the property type
    • This is the primary distinction between BasicArrayType (which always gets/sets primitives), CompositeArrayType (which always gets/sets trees), and ContainerType (which gets/sets primitives or trees depending on the property type)
• Simplify tree-backed proxy logic
  • Instead of the tree logic instance doubling as a proxy handler, separate out proxy logic as a single object
  • TreeValue and its subclasses create a convenience wrapper around low level Type.tree_* functions, are wrapped in a proxy for convenient get/set
  • use createTreeBacked(type: CompositeType<T>, tree: Tree): TreeBacked<T> function to create TreeBacked objects

Benefits/tradeoffs of this refactor:

• Circular references in composite types were confusing, much of the previous logic looked like this._type.structural.foo()
  • Code split into structural / tree / byte array "handlers" is now unified on the Type. This leads to simpler code, but more code per file.
• Proxy logic now in a single place w/ simpler proxy logic. Previous implementation combined proxy handler with tree logic.
  • ITreeBacked methods were previously implemented as curried functions, always bound with a tree. This was very confusing and sometimes led to methods defined with a tree parameter that was never used.
• very few breaking changes
  • anywhere we used type.tree previously
  • readOnlyMap replaced with readonlyValues iterator

Example code:

import {phase0} from "@chainsafe/lodestar-types";
import {createTreeBacked} from "@chainsafe/ssz";

// tree_* methods operate on Trees
const tree = phase0.BeaconState.tree_defaultValue();
phase0.BeaconState.tree_serialize(tree);
const genesisTime = phase0.BeaconState.tree_getProperty(tree, "genesisTime"); // returns a number
const validators = phase0.BeaconState.tree_getProperty(tree, "validators"); // returns a Tree
phase0.BeaconState.tree_setProperty(tree, "genesisTime", 5);

// but usually you'd create a tree-backed beacon state
const state = phase0.BeaconState.createTreeBacked(tree);

state.genesisTime; // -> number
state.validators; // -> TreeBacked<List<Validator>>
state.genesisTime = 5;

// get the backing tree
state.tree
// get the backing type
state.type
// any ITreeBacked method
state.hashTreeRoot();
state.createProof([["validators", 0, "effectiveBalance"]]);
state.serialize();
state.entries();

// struct_* methods operate on structs
const struct = phase0.BeaconState.struct_defaultValue();
phase0.BeaconState.struct_hashTreeRoot(struct);
// but most of the struct_* methods are aliased without this prefix (and will also try to do the 'smart' thing with tree-backed values)
phase0.BeaconState.defaultValue();
phase0.BeaconState.hashTreeRoot(struct);

// Read-only iteration now performed via `readonlyValues` iterator
for (const validators of readonlyValues(state.validators)) {
  ...
}

[mpetrunic]

It would be supper awesome if we could make path typesafe. But it would require some recursive keyof and wouldn't work with arrays right?

[dapplion]

Yes it does! This compiles

function gett<
  T,
  K1 extends keyof T,
  K2 extends keyof T[K1],
  K3 extends keyof T[K1][K2],
  K4 extends keyof T[K1][K2][K3]
>(val: T, k1: K1, k2: K2, k3: K3, k4: K4) {}

type State = {
  a: {
    b: {
      c: {
        d: string;
      };
    }[];
  };
};

gett({} as State, "a", "b", 1, "c");

[mpetrunic]

why no camelcase?

[mpetrunic]

I've used https://www.npmjs.com/package/madge to generate:

Ugh, this is a bit annoying.

abstract <-> proxy is easy to resolve. Just create ITreeValue interface in interface.ts with properties required by proxy.

Circular references between proxy and implementations I'm not sure. It might work if abstract doesn't depend on proxy :/

[dapplion]

Could we prioritize merging the PR and then finding a way to split tree in multiple files? Would be really nice to have this merged

[wemeetagain]

I'm going to fully integrate with lodestar and run spec tests before merging.

[wemeetagain]

Integrated with Lodestar, passing all tests - unit, e2e, sim, spec, etc.