get() Root should not return a sub Tree

[dapplion]

When getting a property from a tree backed ssz type this if decides to return the "raw" value or tree backed.

ssz/src/backings/tree/container.ts

Line 138 in d6efe10

if (!isCompositeType(fieldType)) {

In my opinion I would assume that a Root will not return a sub Tree, but just the raw value. However this is not the case

const stateTree = config.types.phase0.BeaconState.tree.deserialize(stateBytes);
console.log({
      epoch: stateTree.finalizedCheckpoint.epoch,
      root: stateTree.finalizedCheckpoint.root,
});

// stdout of above code
{
  epoch: 0,
  root: Tree { _node: LeafNode { _root: [Uint8Array] }, hook: [Function] }
}

Due to the Tree hook anyone that holds any root part of a state will prevent to garbage collect that state, causing this memory issues ChainSafe/lodestar#2181 (comment)

[dapplion]

I propose to add constructor options to composite types so you can choose not to return a Tree (using shouldNotReturnSubTree for clarity, but we should use a better name)

export abstract class CompositeType<T extends object> extends Type<T> {
  shouldNotReturnSubTree: boolean;
  constructor(opts) {
    this.shouldNotReturnSubTree = opts.shouldNotReturnSubTree ?? false
  }

Then tree backing can use that info to not return a tree for those types

// ssz/src/backings/tree/container.ts:138
if (!isCompositeType(fieldType) || fieldType.shouldNotReturnSubTree) {
   // return raw value
} else {
  // return tree
}

This behavior can be customized in lodestar-types for roots, and won't affect other users.

export const Root = new ByteVectorType({length: 32, shouldNotReturnSubTree: true});

[wemeetagain]

right now, we use valueOf to return the primitive value. We use this a lot to get an actual Uint8Array from a Root.

{
  root: stateTree.finalizedCheckpoint.root.valueOf(),
}

I agree that a Root should always return a Uint8Array, since roots are treated as basic types and never partially updated. IMO we can just update RootType.tree_getProperty to return a Uint8Array.

[philknows]

SSZ has been refactored since this time, no longer relevant.