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

Make sure one can easily use the library from the README #527

Closed
mimoo opened this issue May 2, 2022 · 4 comments
Closed

Make sure one can easily use the library from the README #527

mimoo opened this issue May 2, 2022 · 4 comments

Comments

@mimoo
Copy link
Contributor

mimoo commented May 2, 2022

There's a few things that we should do:

  • make sure the code example in README is run as test in cargo (so it doesn't break)
  • give some guidelines on how to use this library

right now you have to import things like this:

[dependencies]
kimchi = { git = "https://github.com/o1-labs/proof-systems" }
mina-curves = { git = "https://github.com/o1-labs/proof-systems" }
oracle = { git = "https://github.com/o1-labs/proof-systems" }
commitment_dlog = { git = "https://github.com/o1-labs/proof-systems" }

which imo is not clean. Kimchi should re-export these crates so you don't have to import them yourself

@mimoo
Copy link
Contributor Author

mimoo commented May 2, 2022

I think the README should include a simple circuit example (like that we can compile it). For example something that takes a hash output and makes sure that you know the hash input.

Something like this:

use itertools::iterate;

/// create a simple circuit to hash an input
fn create_circuit() -> Vec<CircuitGate<Fp>> {
    let gates = vec![];
    let mut gates_row = iterate(0, |&i| i + 1);
    let row = || gates_row.next().unwrap();

    // the output to the hash function is a public input
    gates.push(CircuitGate::create_generic_gadget(
        Wire::new(row()),
        GenericGateSpec::Pub,
        None,
    ));
    gates.push(CircuitGate::create_generic_gadget(
        Wire::new(row()),
        GenericGateSpec::Pub,
        None,
    ));
    gates.push(CircuitGate::create_generic_gadget(
        Wire::new(row()),
        GenericGateSpec::Pub,
        None,
    ));

    // hash a private input
    let poseidon_params = oracle::pasta::fp_kimchi::params();
    let round_constants = &poseidon_params.round_constants;
    let row = row();
    let (g, final_row) = CircuitGate::<Fp>::create_poseidon_gadget(
        row,
        [Wire::new(row), Wire::new(row + 11)], // TODO: this argument should be deleted from the fn
        round_constants,
    );
    gates.extend(g);

    // wire the output to the public input
    // TODO: it'd be nice if we had functions that would do this for us, and panic if a permutation is already in place
    let last_row = gates.iter_mut().last().unwrap();
    last_row.wires[0] = Wire { row: 0, col: 0 };
    last_row.wires[1] = Wire { row: 0, col: 1 };
    last_row.wires[2] = Wire { row: 0, col: 2 };

    gates[0].wires[0] = Wire {
        row: final_row,
        col: 0,
    };
    gates[0].wires[1] = Wire {
        row: final_row,
        col: 1,
    };
    gates[0].wires[2] = Wire {
        row: final_row,
        col: 2,
    };

    gates
}

@querolita querolita self-assigned this May 2, 2022
@querolita
Copy link
Member

querolita commented May 2, 2022

Regarding having to write dependencies manually, I found this proposal that was rejected that was trying to solve the handling of multiple crates inside the same repository: https://internals.rust-lang.org/t/multiple-libraries-in-a-cargo-project/8259/11 and rust-lang/rfcs#2224

Since this seems to not be addressed any time soon by Rust, would you suggest to think of a way to implement a "transitive" dependencies list deriving from kimchi?

@mimoo
Copy link
Contributor Author

mimoo commented May 2, 2022

@querolita usually the way you do this is to just use pub use.

For example, rand relies on a number of stuff in rand_core, which might be useful to users of rand. So instead of making them import rand_core they re-export these in rand's lib.rs: https://docs.rs/rand/0.8.5/src/rand/lib.rs.html#94

@mimoo
Copy link
Contributor Author

mimoo commented May 11, 2022

Probably not a big deal to include a circuit example, since we'll have the circuit writer soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants