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

[coprocessor] add inputs support for coprocessor #9

Merged
merged 2 commits into from
Aug 25, 2024
Merged

Conversation

david-zk
Copy link
Collaborator

Adds inputs support for coprocessor

No signature verification yet.

Inputs endpoint:

  1. accepts multiple user input blobs with multiple signatures
  2. expands those blobs
  3. generates the handles
  4. returns output handles to the user

@dartdart26 @immortal-tofu

@david-zk david-zk changed the title Add inputs support for coprocessor [coprocessor] add inputs support for coprocessor Aug 23, 2024
Copy link
Collaborator

@dartdart26 dartdart26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I added some minor comments that I think should be fixed, if I am not missing something.

@@ -3,5 +3,6 @@ resolver = "2"
members = ["coprocessor", "executor", "fhevm-engine-common"]

[workspace.dependencies]
tfhe = { version = "0.8.0-alpha.2", features = ["boolean", "shortint", "integer", "aarch64-unix"] }
tfhe = { version = "0.8.0-alpha.2", features = ["boolean", "shortint", "integer", "aarch64-unix", "zk-pok"] }
tfhe-zk-pok = "0.2.1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need this dependency as the alpha version of tfhe has a dependency on 0.3...-alpha for tfhe-zk-pok. Or maybe you need to use the 0.3..-alpha one.

@@ -25,6 +25,7 @@ bigdecimal = "0.4"
fhevm-engine-common = { path = "../fhevm-engine-common" }
strum = { version = "0.26", features = ["derive"] }
bincode = "1.3.3"
keccak-asm = "0.1.3"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc for this crate says:

[!WARNING] Code is somewhat [tested](https://github.com/danipopes/keccak-asm/blob/HEAD/tests/test.rs) and [benchmarked](https://github.com/DaniPopes/bench-keccak256). Use at your own risk.

It doesn't have too much downloads. Maybe we can use a more popular one? Maybe sha3?


for (ct_idx, the_ct) in corresponding_unpacked.iter().enumerate() {
let mut handle: [u8; 32] = [0; 32];
handle.copy_from_slice(&blob_hash);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have to include the index in the hash computation, say hash(input_payload || index)? And then add the index as byte [29] too.

Copy link
Collaborator

@dartdart26 dartdart26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dartdart26 dartdart26 merged commit 71984ed into main Aug 25, 2024
2 checks passed
@dartdart26 dartdart26 deleted the davidk/inputs branch August 25, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants