minaprotocol / mina Goto Github PK

Right now as_prover code throws sometimes, it would be nice if error'ing were more explicit. So, we should add Or_error to the As_prover monad stack

All things sent remotely should be Stable.V1s

I suggest changing the type of Gossip_net.broadcast_all to

t -> Message.t -> (unit -> [`Done | `Continue] Deferred.t) Staged.t

To do so:
[ ] Move the c-bindings to a separate project with libsnark as a submodule
[ ] Inline those c-bindings into our monorepo, make libsnark a submodule (recursively)

Move base proof generation into gen

We need to make these types stable for rpcs.

I previously thought adding a duplicate would overwrite but that should be set. adding a dupe keeps the first value

We should have inline-tests

Have a variant of libsnark's is_satisfied which doesn't print all the failed constraints

Miner loop doesn't yield to the scheduler. As a work around we've stuck a sleep in the loop

Move to a non-uniform representation of booleans eventually, e.g.

type var =
  | True
  | False
  | Var of Field.var

This will make an efficient "if_" with boolean/bitstring result automatic.

For some reason deserialization of keys in libsnark is very slow. This should be fixed

We should use something like $XDG_CONFIG_HOME/coda on Linux, maybe Library/Preferences/coda on macOS, and maybe FOLDERID_RoamingAppData on Windows.

Right now the genesis proof is generated afresh each time. It should be stored on disk and loaded in.

I suggest getting rid of write_or_drop as its use can be a bit error prone since there is not necessarily any relationship between the writer and reader provided.

Right now in snarky there are a bunch of different interpreters, which all essentially do the same recursion over the AST. I think these should be unified to reduce bug surface area

Or decide we do want to expose that?

NB: Libsnark multicore doesn't work in processes that are using async, so we'd have to start a process that didn't use async

Right now every process that does proving has a separate copy of the proving keys. The result is memory usage is several times worse than it should be. This is bad, we should change it.

Conservation of wealth quickcheck test takes ~10seconds per trial inside Nanobit_base.ledger.ml

Keeping code here so we can put it back when we functor out the slow things.

let gen_perturbations ledger keys : t Quickcheck.Generator.t =
  let open Quickcheck.Generator in
  let open Quickcheck.Generator.Let_syntax in
  let ledger = copy ledger in
  let%bind changes = Int.gen_incl 1 100 in
  let%map transactions =
    list_with_length changes
      (Transaction.With_valid_signature.gen ~keys ~max_amount:100 ~max_fee:0)
  in
  List.iter transactions ~f:(fun txn ->
    apply_transaction ledger txn |> Or_error.ok_exn
  );
  ledger

let gen : (t * Signature_keypair.t array) Quickcheck.Generator.t =
  let ledger = create () in
  let open Quickcheck.Generator in
  let open Quickcheck.Generator.Let_syntax in
  let%bind num_accounts = Int.gen_incl 1 1000 in
  let keys = Array.init num_accounts ~f:(fun _ -> Signature_keypair.create ()) in
  (* Setup ledger *)
  printf "Begin setup\n%!";
  Array.iter keys ~f:(fun k ->
    let public_key = Public_key.compress k.public_key in
    update ledger public_key
      { public_key; balance = Currency.Balance.of_int 10_000 });
  printf "end setup\n%!";
  let%map ledger' = gen_perturbations ledger keys in
  printf "end perturbation\n%!";
  (ledger', keys)

let%test_unit "conservation_of_wealth" =
  let open Quickcheck.Generator in
  let open Quickcheck.Generator.Let_syntax in
  let gen =
    let%bind (ledger, keys) = gen in
    let%map ledger' = gen_perturbations ledger keys in
    (ledger, ledger')
  in
  let x = ref 0 in
  Quickcheck.test ~trials:100 gen ~f:(fun (ledger, ledger') ->
    let wealth ledger : Int64.t =
      List.fold (to_list ledger) ~init:Int64.zero ~f:(fun acc {balance} ->
        Int64.(+) acc (balance |> Currency.Balance.to_string |> Int64.of_string) )
    in
    let old_wealth = wealth ledger in
    let new_wealth = wealth ledger' in
    if new_wealth <> old_wealth then begin
      failwithf !"New_wealth %{sexp: Int64.t} is different from old_wealth %{sexp: Int64.t}" new_wealth old_wealth ();
    end;
    x := !x + 1;
    printf "Done with one trial %d\n%!" !x
  )

There is an inexhaustive pattern match on line 179 of nanobit main.ml

Merkle ledger should have a delete operation. Specifically, we should have (at least)

val remove_at_index : t -> index -> unit

and keep track of which indices are empty, and then update can put new accounts into some arbitrary empty index.

Correctly implement public key compression

Right now it's super dumb and just writes the whole state every time a new ledger is received.

Right now witness generation is kinda slow. I suspect a lot of time is eaten up in evaluating Cvar.ts, which are just ASTs of + and * with indices into an array at the leaves. There is sharing between Cvar.ts so this should be reflected in sharing intermediate results of computation.

ensure resistance against attacks such as the ethereum kademlia exploit:

• https://arstechnica.com/information-technology/2018/03/ethereum-fixes-serious-eclipse-flaw-that-could-be-exploited-by-any-kid/#p3
• https://www.cs.bu.edu/~goldbe/projects/eclipseEth.pdf
• https://news.ycombinator.com/item?id=16509522

We need to:

1. Not make keys deterministic based on the address
2. Not try peers in a deterministic order
3. Change some of the config (ex. to make the ping time 1hour, ala Cardano)

See comment in Main.hs of Kademlia-Haskell

Use a different hash function on each client for comparing transactions to prevent grinding for priority.

Instead of hard coding the verification key size, compute it dynamically from the input spec

Figure out a consistent system for debian, ocaml packages in the docker images

We should use openssl or something to have a constant time signature scheme implementation to prevent timing attacks

definitions like

module Boolean : sig
  type var = private Cvar.t
end

are ultimately I think bad, because the coercion to Cvar.t leaves implicit the mapping between booleans and field elements. It would be better to have a function Boolean.to_zero_one : Boolean.var -> Cvar.t

Eviction policy right now is "evict no one and crash if the buffer gets too big"

Alternatives:

1. Queue up every new partial state as they enter the ledger fetcher and then evict all but the strongest state's ledger-hash
2. Implement #177 and just cancel a few whenever we get too far behind, but keep downloading all of them.

More thought is required to actually decide which of the alternatives is best

We have write_or_drop (which is prefer earlier thing)
We have write_or_exn (which means die if data gets backed up)
We don't have the "prefer later things" notion. What we'd want to do here is make some way to cancel downstream tasks that haven't finished yet to unblock the pipes.

Specifically we possibly want to use this when we're being too slow at getting ledgers but fast at generating ledger-hashes for the ledger-fetcher since this will prevent the miner from getting the most up-to-date information fast enough.

Logs are hard to trace when every number is 300 (soon to be 800) bits long. When tracing logs to understand behavior, we should have a switch on logproc to hide the details here.

Transition hashes should probably just go in the state so that states have computationally unique histories. As is, there is "malleability" in terms of what the nonce is as it's not included in the state.

Defined appropriately, the pedersen hash satisfies a composition property H(a || b) = H(a) H(b). This is used at various points for efficiency but in an ad hoc manner. There should be some abstraction that captures this nicely.

We currently spawn long-running processes to help us do work (see Kademlia).

Unfortunately, Core's Process module doesn't kill these spawned processes for us.

We should change Process to automatically cleanup after itself. ie: When this OCaml process dies, the process we spawned is also killed.

• replace all printfs with logs
• add logs to show program state

Have a system like "number" which unifies packed and unpacked variables. It's annoying having to manually track around things that get packed and unpacked. Would be better to have "lazy" evaluation for unpacking so that any repeated unpacking is free.