ChainSafe / ssz

Typescript implementation of Simple Serialize (SSZ)
https://simpleserialize.com/
Other
53 stars 20 forks source link

ViewDU container rebinds node with read-only operations #379

Open twoeths opened 4 months ago

twoeths commented 4 months ago

Describe the bug

Consider this unit test here in #378

  it("should not recompute hashTreeRoot() when no fields is changed", () => {
    const root = state.hashTreeRoot();
    // this causes viewsChanged inside BeaconState container
    state.validators.length;
    state.balances.length;
    // but we should not recompute root, should get from cache instead
    const root2 = state.hashTreeRoot();
    expect(root2).to.equal(root, "should not recompute hashTreeRoot() when no fields are changed");
  });

the BeaconState container tracked validators and length as "viewsChanged" and it rebinds the underlying tree to compute new root with same nodes, which results in the same value but different tree/nodes, and we need to rehash the different nodes

note that #378 implemented a cache that returns the same root for multiple ViewDU.hashTreeRoot() calls

Expected behavior

Steps to Reproduce

philknows commented 1 month ago

@twoeths Can this be closed via #380?

twoeths commented 1 month ago

@twoeths Can this be closed via #380?

@philknows not yet since I merged it to the te/batch_hash_tree_root branch. Will close it once we merge to master