-
Notifications
You must be signed in to change notification settings - Fork 598
feat: split public inputs from proof #12816
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
Changes from all commits
adc4c74
f79e79b
4eecccf
240d1a9
6f9666b
5327e57
fe4e9f0
aba0481
b9edfe4
1cc406b
bc0ffe9
f1b2945
92f1d50
df407ba
25e097b
4edea76
0556524
5ab1dea
3448f64
1bf4645
36a629f
08c1dac
3e04edf
f78066c
69de125
90438e4
dbd9ca8
fb775fe
31f0d17
0245191
52dd528
175915b
fc47c55
09f2800
635971c
68d1081
786889f
15cdc65
ea6e789
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,12 +38,10 @@ function run_proof_generation { | |
| dump_fail "$prove_cmd" | ||
|
|
||
| local vk_fields=$(cat "$outdir/vk_fields.json") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. simplification of this logic to generate recursive proof inputs |
||
| local public_inputs_fields=$(cat "$outdir/public_inputs_fields.json") | ||
| local proof_fields=$(cat "$outdir/proof_fields.json") | ||
| local num_inner_public_inputs=$(( 16#$(echo "$vk_fields" | jq -r '.[1] | ltrimstr("0x")') - adjustment )) | ||
|
|
||
| echo "num_inner_public_inputs for $program = $num_inner_public_inputs" | ||
|
|
||
| generate_toml "$program" "$vk_fields" "$proof_fields" "$num_inner_public_inputs" | ||
| generate_toml "$program" "$vk_fields" "$proof_fields" "$public_inputs_fields" | ||
| } | ||
|
|
||
| function generate_toml { | ||
|
|
@@ -56,15 +54,15 @@ function generate_toml { | |
|
|
||
| jq -nr \ | ||
| --arg key_hash "$key_hash" \ | ||
| --argjson vkf "$vk_fields" \ | ||
| --argjson prooff "$proof_fields" \ | ||
| --argjson num_inner_public_inputs "$num_inner_public_inputs" \ | ||
| --argjson vk_f "$vk_fields" \ | ||
| --argjson public_inputs_f "$public_inputs_fields" \ | ||
| --argjson proof_f "$proof_fields" \ | ||
| '[ | ||
| "key_hash = \($key_hash)", | ||
| "proof = [\($prooff | .[$num_inner_public_inputs:] | map("\"" + . + "\"") | join(", "))]", | ||
| "public_inputs = [\($prooff | .[:$num_inner_public_inputs] | map("\"" + . + "\"") | join(", "))]", | ||
| "verification_key = [\($vkf | map("\"" + . + "\"") | join(", "))]" | ||
| '"$( [[ $program == *"double"* ]] && echo ',"proof_b = [\($prooff | .[$num_inner_public_inputs:] | map("\"" + . + "\"") | join(", "))]"' )"' | ||
| "proof = [\($proof_f | map("\"" + . + "\"") | join(", "))]", | ||
| "public_inputs = [\($public_inputs_f | map("\"" + . + "\"") | join(", "))]", | ||
| "verification_key = [\($vk_f | map("\"" + . + "\"") | join(", "))]" | ||
| '"$( [[ $program == *"double"* ]] && echo ',"proof_b = [\($proof_f | map("\"" + . + "\"") | join(", "))]"' )"' | ||
| ] | join("\n")' > "$output_file" | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ $BIN write_vk \ | |
| PROOF_FIELDS_LENGTH=$(jq 'length' $output_dir/proof_fields.json) | ||
| UH_PROOF_FIELDS_LENGTH=440 | ||
| NUM_PUBLIC_INPUTS=$((PROOF_FIELDS_LENGTH - UH_PROOF_FIELDS_LENGTH)) | ||
| jq ".[:$NUM_PUBLIC_INPUTS]" $output_dir/proof_fields.json > $output_dir/public-inputs | ||
| jq ".[:$NUM_PUBLIC_INPUTS]" $output_dir/proof_fields.json > $output_dir/public_inputs_fields.json | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change file name to be more apt |
||
|
|
||
| # Remove public inputs from the proof (first NUM_PUBLIC_INPUTS*32 bytes) | ||
| # Also remove the first 4 bytes, which is the proof length in fields | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ output_dir=$artifact_dir/bb-bbjs-tmp | |
| mkdir -p $output_dir | ||
|
|
||
| # Cleanup on exit | ||
| trap "rm -rf $output_dir" EXIT | ||
| # trap "rm -rf $output_dir" EXIT | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should undo |
||
|
|
||
| # Writes the proof, public inputs ./target; this also writes the VK | ||
| node ../../bbjs-test prove \ | ||
|
|
@@ -24,25 +24,34 @@ node ../../bbjs-test prove \ | |
| # Join the proof and public inputs to a single file | ||
| # this will not be needed after #11024 | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed some logic here to split up the public inputs and proof to be in separate files |
||
| NUM_PUBLIC_INPUTS=$(cat $output_dir/public-inputs | jq 'length') | ||
| NUM_PUBLIC_INPUTS=$(cat $output_dir/public_inputs_fields.json | jq 'length') | ||
| UH_PROOF_FIELDS_LENGTH=440 | ||
| PROOF_AND_PI_LENGTH_IN_FIELDS=$((NUM_PUBLIC_INPUTS + UH_PROOF_FIELDS_LENGTH)) | ||
| PROOF_LENGTH_IN_FIELDS=$((UH_PROOF_FIELDS_LENGTH)) | ||
| PI_LENGTH_IN_FIELDS=$((NUM_PUBLIC_INPUTS)) | ||
| # First 4 bytes is PROOF_AND_PI_LENGTH_IN_FIELDS | ||
| proof_header=$(printf "%08x" $PROOF_AND_PI_LENGTH_IN_FIELDS) | ||
| proof_header=$(printf "%08x" $PROOF_LENGTH_IN_FIELDS) | ||
| pi_header=$(printf "%08x" $PI_LENGTH_IN_FIELDS) | ||
|
|
||
| proof_bytes=$(cat $output_dir/proof | xxd -p) | ||
| public_inputs=$(cat $output_dir/public-inputs | jq -r '.[]') | ||
| public_inputs=$(cat $output_dir/public_inputs_fields.json | jq -r '.[]') | ||
|
|
||
| public_inputs_bytes="" | ||
| for input in $public_inputs; do | ||
| public_inputs_bytes+=$input | ||
| done | ||
|
|
||
| # Combine proof header, public inputs, and the proof to a single file | ||
| echo -n $proof_header$public_inputs_bytes$proof_bytes | xxd -r -p > $output_dir/proof | ||
| # Combine proof header and the proof to a single file | ||
| echo -n $proof_header$proof_bytes | xxd -r -p > $output_dir/proof | ||
| echo -n $pi_header$public_inputs_bytes | xxd -r -p > $output_dir/public_inputs | ||
| echo "$BIN verify \ | ||
| --scheme ultra_honk \ | ||
| -k $output_dir/vk \ | ||
| -p $output_dir/proof \ | ||
| -i $output_dir/public_inputs" | ||
|
|
||
| # Verify the proof with bb cli | ||
| $BIN verify \ | ||
| --scheme ultra_honk \ | ||
| -k $output_dir/vk \ | ||
| -p $output_dir/proof | ||
| -p $output_dir/proof \ | ||
| -i $output_dir/public_inputs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,9 +45,14 @@ case ${SYS:-} in | |
| FLAGS+=" --scheme $SYS --oracle_hash ${HASH:-poseidon2}" | ||
| [ "${ROLLUP:-false}" = "true" ] && FLAGS+=" --ipa_accumulation" | ||
| [ "${RECURSIVE}" = "true" ] && FLAGS+=" --init_kzg_accumulator" | ||
|
|
||
| OUTDIR=$(mktemp -d) | ||
| trap "rm -rf $OUTDIR" EXIT | ||
| $BIN prove $FLAGS $BFLAG -o $OUTDIR | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prove now has two output files so can't just do what we did before |
||
| $BIN verify $FLAGS \ | ||
| -k <($BIN write_vk $FLAGS $BFLAG -o - ) \ | ||
| -p <($BIN prove $FLAGS $BFLAG -o - ) | ||
| -p $OUTDIR/proof \ | ||
| -i $OUTDIR/public_inputs | ||
| ;; | ||
| "ultra_honk_deprecated") | ||
| # deprecated flow is necessary until we finish C++ api refactor and then align ts api | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -216,45 +216,54 @@ const killAnvil = () => { | |
| console.log(testName, " complete"); | ||
| }; | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the issue with this code is that we're trying to use it for multiple different scenarios, for plonk, honk bb, and honk bbjs... As a result we have an insane mess of if statements (and try/catch clauses that act as if statements), just trying to handle each scenario. We need to unify these interfaces or be more explicit in separating how we handle each context.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tagging @saleel since you recently edited this file and bbjs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add an issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also I agree, hopefully at some point we stop handling the plonk scenario - also I'm surprised this has anything to do with bbjs, I regarded this file as only useful to unit test that we can deploy the verifier contract (after we ensured that all forge tests pass so the contract is presumably correct) |
||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1316): Clean this code up. We are trying to use this logic for three different flows: bb plonk, bb honk, and bbjs honk, and all three have different setups. | ||
| try { | ||
| const proofPath = getEnvVar("PROOF"); | ||
| let publicInputsPath; | ||
|
|
||
| let proofStr = ""; | ||
|
|
||
| const proof = readFileSync(proofPath); | ||
| proofStr = proof.toString("hex"); | ||
|
|
||
| let publicInputsAsFieldsPath; // PUBLIC_INPUTS_AS_FIELDS is not defined for bb plonk, but is for bb honk and bbjs honk. | ||
| try { | ||
| publicInputsPath = getEnvVar("PUBLIC_INPUTS"); | ||
| publicInputsAsFieldsPath = getEnvVar("PUBLIC_INPUTS_AS_FIELDS"); | ||
| } catch (e) { | ||
| // noop | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we have some explicit error message in case we hit these catch-es?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not really an error in this case. It's expected for some of the flows |
||
| } | ||
|
|
||
| let proofStr = ''; | ||
| let publicInputs = []; | ||
|
|
||
| // If "path to public inputs" is provided, it means that the proof and public inputs are saved as separate files | ||
| // A bit hacky, but this can go away once BB CLI saves them as separate files - #11024 | ||
| if (publicInputsPath) { | ||
| const proof = readFileSync(proofPath); | ||
| proofStr = proof.toString("hex"); | ||
| publicInputs = JSON.parse(readFileSync(publicInputsPath).toString()); // assumes JSON array of PI hex strings | ||
| } else { | ||
| // Proof and public inputs are saved in a single file; we need to extract the PI from the proof | ||
| const proof = readFileSync(proofPath); | ||
| proofStr = proof.toString("hex"); | ||
|
|
||
| const proofAsFieldsPath = getEnvVar("PROOF_AS_FIELDS"); | ||
| var publicInputs; | ||
| let proofAsFieldsPath; // PROOF_AS_FIELDS is not defined for bbjs, but is for bb plonk and bb honk. | ||
| try { | ||
| proofAsFieldsPath = getEnvVar("PROOF_AS_FIELDS"); | ||
| } catch (e) { | ||
| // noop | ||
| } | ||
| let numExtraPublicInputs = 0; | ||
| let extraPublicInputs = []; | ||
| if (proofAsFieldsPath) { | ||
| const proofAsFields = readFileSync(proofAsFieldsPath); | ||
|
|
||
| let numPublicInputs; | ||
| [numPublicInputs, publicInputs] = readPublicInputs( | ||
| // We need to extract the public inputs from the proof. This might be empty, or just the pairing point object, or be the entire public inputs... | ||
| [numExtraPublicInputs, extraPublicInputs] = readPublicInputs( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we still have this logic to handle the aggregation object that is contained in the proof. It may or may not exist though
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it may sometimes be nothing, or it may be just the aggregation object, or it might be all of the inner public inputs AND the aggregation object... |
||
| JSON.parse(proofAsFields.toString()) | ||
| ); | ||
|
|
||
| proofStr = proofStr.substring(32 * 2 * numPublicInputs); // Remove the publicInput bytes from the proof | ||
|
|
||
| // Honk proof from the CLI have field length as the first 4 bytes. This should go away in the future | ||
| if (testingHonk) { | ||
| // Honk proof from the CLI have field length as the first 4 bytes. This should go away in the future | ||
| proofStr = proofStr.substring(8); | ||
| } | ||
| } | ||
| // We need to do this because plonk doesn't define this path | ||
| if (publicInputsAsFieldsPath) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the public inputs aren't stored separately, which is the case for plonk, then just only use the extraPublicInputs |
||
| const innerPublicInputs = JSON.parse( | ||
| readFileSync(publicInputsAsFieldsPath).toString() | ||
| ); // assumes JSON array of PI hex strings | ||
|
|
||
| publicInputs = innerPublicInputs.concat(extraPublicInputs); | ||
| } else { | ||
| // for plonk, the extraPublicInputs are all of the public inputs | ||
| publicInputs = extraPublicInputs; | ||
| } | ||
|
|
||
| proofStr = proofStr.substring(64 * numExtraPublicInputs); | ||
| proofStr = "0x" + proofStr; | ||
|
|
||
| const key = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,7 +123,9 @@ void write_standalone_vk(const std::string& output_data_type, | |
| auto proving_key = std::make_shared<DeciderProvingKey>(builder, trace_settings); | ||
| Prover prover{ proving_key }; | ||
| init_bn254_crs(prover.proving_key->proving_key.circuit_size); | ||
| ProofAndKey<VerificationKey> to_write{ {}, std::make_shared<VerificationKey>(prover.proving_key->proving_key) }; | ||
| PubInputsProofAndKey<VerificationKey> to_write{ | ||
| PublicInputsVector{}, HonkProof{}, std::make_shared<VerificationKey>(prover.proving_key->proving_key) | ||
| }; | ||
|
|
||
| write(to_write, output_data_type, "vk", output_path); | ||
| } | ||
|
|
@@ -283,6 +285,7 @@ void ClientIVCAPI::prove(const Flags& flags, | |
| } | ||
|
|
||
| bool ClientIVCAPI::verify([[maybe_unused]] const Flags& flags, | ||
| [[maybe_unused]] const std::filesystem::path& public_inputs_path, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another hack to unify the api. We don't use this for client IVC There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will you add TODOs (+issues) in all places that have hacks to not lose track of them? |
||
| const std::filesystem::path& proof_path, | ||
| const std::filesystem::path& vk_path) | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was the only change in this file: publicInputsPath -> publicInputsAsFieldsPath.