Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert breaking change to versioned type #10018

Merged
merged 1 commit into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/lib/mina_base/pending_coinbase.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1018,8 +1018,7 @@ module T = struct
let root_hash = hash_at_level depth in
{ Poly.tree =
make_tree
(Merkle_tree.of_hash ~depth ~next_available_token:()
~next_available_index:None root_hash)
(Merkle_tree.of_hash ~depth ~next_available_token:() root_hash)
Stack_id.zero
; pos_list = []
; new_pos = Stack_id.zero
Expand Down
54 changes: 22 additions & 32 deletions src/lib/mina_base/sparse_ledger.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,37 +129,23 @@ M.
, data
, iteri
, depth
, next_available_token
, next_available_index )]
, next_available_token )]

(* TODO: share this cache globally across modules *)
let empty_hash =
Empty_hashes.extensible_cache
(module Ledger_hash)
~init_hash:(Ledger_hash.of_digest Account.empty_digest)

let of_root ~depth ~next_available_token ~next_available_index
(h : Ledger_hash.t) =
of_hash ~depth ~next_available_token ~next_available_index
let of_root ~depth ~next_available_token (h : Ledger_hash.t) =
of_hash ~depth ~next_available_token
(* TODO: Is this of_digest casting really necessary? What's it doing? *)
(Ledger_hash.of_digest (h :> Random_oracle.Digest.t))

let of_any_ledger_root ledger =
let next_available_index =
match Ledger.Any_ledger.M.last_filled ledger with
| Some (Ledger.Location.Account addr) ->
Option.map (Ledger.Addr.next addr) ~f:Ledger.Addr.to_int
| Some _ ->
failwith
"unable to get next available index from ledger: last filled \
location is invalid"
| None ->
Some 0
in
of_root
~depth:(Ledger.Any_ledger.M.depth ledger)
~next_available_token:(Ledger.Any_ledger.M.next_available_token ledger)
~next_available_index
(Ledger.Any_ledger.M.merkle_root ledger)

let of_any_ledger (ledger : Ledger.Any_ledger.witness) =
Expand Down Expand Up @@ -318,7 +304,7 @@ let%test_unit "adding paths generated via derive_next_arm_exn does not \
let ledger =
of_root ~depth:3
~next_available_token:(Token_id.of_uint64 Unsigned_extended.UInt64.one)
~next_available_index:(Some 1) root
root
in
let ledger = add_path ledger path (Account.identifier account) account in
let ledger, _ =
Expand All @@ -334,7 +320,7 @@ let%test_unit "adding paths generated via derive_next_arm_exn does not \
in
[%test_eq: Ledger_hash.t] (merkle_root ledger) root

let of_sparse_ledger_subset_exn base_ledger account_ids =
let of_sparse_ledger_subset_exn base_ledger ~next_idx account_ids =
let account_ids = dedup_list account_ids ~comparator:(module Account_id) in
let add_existing_accounts l =
List.fold_right account_ids ~init:(l, [])
Expand All @@ -348,15 +334,11 @@ let of_sparse_ledger_subset_exn base_ledger account_ids =
(l, id :: missing_accounts))
in
let add_missing_accounts l missing_account_ids =
let next_idx =
Option.value_exn (next_available_index l)
~message:"not enough space in ledger"
in
let last_arm =
if next_idx > 0 then arm_exn base_ledger (next_idx - 1)
else left_empty_arm (depth l)
in
let result, _, _ =
let result, _, next_idx =
(* TODO: we could just check the remaining available slots in the ledger upfront instead of folding over the index (which is otherwise unused here) *)
List.fold_left missing_account_ids ~init:(l, last_arm, Some next_idx)
~f:(fun (l, prev_arm, idx_opt) id ->
Expand All @@ -368,24 +350,23 @@ let of_sparse_ledger_subset_exn base_ledger account_ids =
, next_arm
, next_index ~depth:(depth l) idx ))
in
result
(result, next_idx)
in
let result_ledger =
let result_ledger, next_idx =
let ledger =
of_root ~depth:(depth base_ledger)
~next_available_token:(next_available_token base_ledger)
~next_available_index:(next_available_index base_ledger)
(merkle_root base_ledger)
in
let ledger, missing_account_ids = add_existing_accounts ledger in
if List.is_empty missing_account_ids then ledger
if List.is_empty missing_account_ids then (ledger, Some next_idx)
else add_missing_accounts ledger missing_account_ids
in
Debug_assert.debug_assert (fun () ->
[%test_eq: Ledger_hash.t]
(merkle_root result_ledger)
(merkle_root base_ledger)) ;
result_ledger
(result_ledger, next_idx)

(* IMPORTANT TODO: rightmost path in parent sparse ledger must exist in order for derivation on new accounts to work *)
(* ^^^ should probably setup a test to ensure the error message is decent *)
Expand Down Expand Up @@ -454,7 +435,10 @@ let%test_module "of_sparse_ledger_subset_exn" =
let sl = of_ledger_subset_exn ledger account_ids in
[%test_result: Ledger_hash.t]
~expect:(Ledger.merkle_root ledger)
(merkle_root @@ of_sparse_ledger_subset_exn sl account_ids))
( merkle_root @@ fst
@@ of_sparse_ledger_subset_exn
~next_idx:(Ledger.num_accounts ledger)
sl account_ids ))

let%test_unit "with new accounts" =
let depth = 4 in
Expand All @@ -477,7 +461,10 @@ let%test_module "of_sparse_ledger_subset_exn" =
in
[%test_result: Ledger_hash.t]
~expect:(Ledger.merkle_root ledger)
(merkle_root @@ of_sparse_ledger_subset_exn sl account_ids))
( merkle_root @@ fst
@@ of_sparse_ledger_subset_exn
~next_idx:(Ledger.num_accounts ledger)
sl account_ids ))

let%test_unit "on an empty ledger" =
let depth = 4 in
Expand All @@ -491,7 +478,10 @@ let%test_module "of_sparse_ledger_subset_exn" =
let sl = of_ledger_subset_exn ledger [] in
[%test_result: Ledger_hash.t]
~expect:(Ledger.merkle_root ledger)
(merkle_root @@ of_sparse_ledger_subset_exn sl account_ids))
( merkle_root @@ fst
@@ of_sparse_ledger_subset_exn
~next_idx:(Ledger.num_accounts ledger)
sl account_ids ))
end )

let get_or_initialize_exn account_id t idx =
Expand Down
10 changes: 3 additions & 7 deletions src/lib/mina_base/sparse_ledger.mli
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ val path_exn :

val find_index_exn : t -> Account_id.t -> int

val of_root :
depth:int
-> next_available_token:Token_id.t
-> next_available_index:int option
-> Ledger_hash.t
-> t
val of_root : depth:int -> next_available_token:Token_id.t -> Ledger_hash.t -> t

val has_locked_tokens_exn :
global_slot:Mina_numbers.Global_slot.t -> account_id:Account_id.t -> t -> bool
Expand Down Expand Up @@ -68,7 +63,8 @@ val of_ledger_subset_exn : Ledger.t -> Account_id.t list -> t

val of_ledger_index_subset_exn : Ledger.Any_ledger.witness -> int list -> t

val of_sparse_ledger_subset_exn : t -> Account_id.t list -> t
val of_sparse_ledger_subset_exn :
t -> next_idx:int -> Account_id.t list -> t * int option

(* TODO: erase Account_id.t from here (doesn't make sense to have it) *)
val data : t -> (int * Account.t) list
Expand Down
36 changes: 6 additions & 30 deletions src/lib/sparse_ledger_lib/sparse_ledger.ml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ module T = struct
{ indexes : ('key * int) list
; depth : int
; tree : ('hash, 'account) Tree.Stable.V1.t
; next_available_index : int option
; next_available_token : 'token_id
}
[@@deriving sexp, yojson]
Expand All @@ -43,7 +42,6 @@ module T = struct
{ indexes : ('key * int) list
; depth : int
; tree : ('hash, 'account) Tree.t
; next_available_index : int option
; next_available_token : 'token_id
}
[@@deriving sexp, yojson]
Expand All @@ -60,12 +58,7 @@ module type S = sig

type t = (hash, account_id, account, token_id) T.t [@@deriving sexp, yojson]

val of_hash :
depth:int
-> next_available_token:token_id
-> next_available_index:int option
-> hash
-> t
val of_hash : depth:int -> next_available_token:token_id -> hash -> t

val get_exn : t -> int -> account

Expand All @@ -90,8 +83,6 @@ module type S = sig

val depth : t -> int

val next_available_index : t -> int option

val next_available_token : t -> token_id
end

Expand All @@ -105,15 +96,8 @@ let max_index depth =
in
set_bits 0 depth

let of_hash ~depth ~next_available_index ~next_available_token h =
Option.iter next_available_index ~f:(fun idx ->
assert (idx <= max_index depth)) ;
{ T.indexes = []
; depth
; tree = Hash h
; next_available_index
; next_available_token
}
let of_hash ~depth ~next_available_token h =
{ T.indexes = []; depth; tree = Hash h; next_available_token }

module Make (Hash : sig
type t [@@deriving equal, sexp, yojson, compare]
Expand Down Expand Up @@ -146,9 +130,8 @@ end = struct
type t = (Hash.t, Account_id.t, Account.t, Token_id.t) T.t
[@@deriving sexp, yojson]

let of_hash ~depth ~next_available_token ~next_available_index (hash : Hash.t)
=
of_hash ~depth ~next_available_token ~next_available_index hash
let of_hash ~depth ~next_available_token (hash : Hash.t) =
of_hash ~depth ~next_available_token hash

let hash : (Hash.t, Account.t) Tree.t -> Hash.t = function
| Account a ->
Expand All @@ -164,8 +147,6 @@ end = struct

let merkle_root { T.tree; _ } = hash tree

let next_available_index { T.next_available_index; _ } = next_available_index

let next_available_token { T.next_available_token; _ } = next_available_token

let add_path depth0 tree0 path0 account =
Expand Down Expand Up @@ -440,12 +421,7 @@ let%test_module "sparse-ledger-test" =
in
let%bind depth = Int.gen_incl 0 16 in
let%map tree = gen depth >>| prune_hash_branches in
{ T.tree
; depth
; indexes = indexes depth tree
; next_available_token = ()
; next_available_index = None
}
{ T.tree; depth; indexes = indexes depth tree; next_available_token = () }

let%test_unit "iteri consistent indices with t.indexes" =
Quickcheck.test gen ~f:(fun t ->
Expand Down
Loading