Use 'wide merkle paths' to optimize Sparse_ledger.of_ledger_subset_exn

[mrmr1993]

This PR builds upon #14587, using the already-computed hashes for accounts and merkle paths instead of recomputing them when creating Sparse_ledger.ts.

Checklist:

• Dependency versions are unchanged
  • Notify Velocity team if dependencies must change in CI
• Modified the current draft of release notes with details on what is completed or incomplete within this project
• Document code purpose, how to use it
  • Mention expected invariants, implicit constraints
• Tests were added for the new behavior
  • Document test purpose, significance of failures
  • Test names should reflect their purpose
• All tests pass (CI will check this if you didn't)
• Serialized types are in stable-versioned modules
• Does this close issues? List them

[mrmr1993]

!ci-build-me

[mrmr1993]

!ci-nightly-me

[mrmr1993]

!ci-build-me

[georgeee]

Here the bug was fixed, version before refactoring had a different order in tuple.

Arguably, refactoring made the concern easier to identify.

[ghost-not-in-the-shell]

I would propose that we stick with the original control flow because it's easier to understand plus the original implementation has some assertion to check the correctness of the merkle path. If you want to skip the assertions, you could also easily do that:

  let add_wide_path depth0 tree0 path0 account =
    let rec build_tree h height p =
      match p with
      | `Left (h_l, h_r) :: path ->
        let l = build_tree h_l (height - 1) path in 
        Tree.Node (h, l, Hash h_r)
      | `Right (h_l, h_r) :: path ->
        let r = build_tree h_r (height - 1) path in 
        Node (h, Hash h_l, r)
      | [] ->
        assert (height = -1) ;
        Account account 
      in
      let rec union height tree path =
        match (tree, path) with 
        | Tree.Hash h, path ->
          let t = build_tree h height path in
          [%test_result: Hash.t]
            ~message:
              "Hashes in union are not equal, something is wrong with your \
               ledger"
            ~expect:h (hash t) ;
          t
        | Node (h, l, r), `Left _ :: path ->
          let l = union (height - 1) l path in 
          Node (h, l, r)
        | Node (h, l, r), `Right _ :: path ->
          let r = union (height - 1) r path in 
          Node (h, l, r) 
        | Node _, [] ->
          failwith "Path too short" 
        | Account _, _ :: _ ->
          failwith "Path too long" 
        | Account a, [] ->
          assert (Account.equal a account) ;
          tree 
        in 
        union (depth0 - 1) tree0 (List.rev path0)  

[nholland94]

LGTM overall, but had a couple of nits and a question.

[nholland94]

The logic for splitting hashes during the fold is shared across both specializations of the compute_path function. It would be better to lift this part of the logic up into the path_batch_impl function.

[ghost-not-in-the-shell]

I would oppose to do that. The splitting is actually different for merkle_path and wide_merkle_path. And the splitting of hashes goes together with how we compute the path. For merkle_path we split once and then combine the result with directions using List.map2_exn. For wide_merkle_path we split twice and then combine the result with directions using List.map3_exn. If we separate those 2 processes, then we would essentially do some unnecessary packing + unpacking for tuples of hashes and directions.

[nholland94]

This surprised me a bit. I would think this would work if the path was in reverse order only, but I think the path is traversing from the root down to the leaf, in which case this seems wrong to me at first glance.

[nholland94]

Why do we have a separate impl helper for this function? It seems we only call it in this one location.

[ghost-not-in-the-shell]

!ci-build-me

[ghost-not-in-the-shell]

!ci-nightly-me

[georgeee]

refactor add_wide_path

@ghost-not-in-the-shell this commit reverses version that computes hashes in the add_wide_path_unsafe, one of the purposes of this PR is to avoid it. So I want it to be reworked

[ghost-not-in-the-shell]

!ci-build-me

[ghost-not-in-the-shell]

!ci-nightly-me

[ghost-not-in-the-shell]

!ci-build-me

[ghost-not-in-the-shell]

!ci-nightly-me