Multi circuit proof integration

[vicsn]

Motivation

This adds support for batch proving executions and their inclusions.

This is a very rough first draft. I suggest we do the review in two parts: first only important architecture comments on e.g. the callchain, function signatures and objects. There is a lot of repetition in the code, and existing TODOs of mine, so please save yourself time by mentioning your ideas only once.

Some notes on this PR:

• I removed the generic from verify_execution<const VERIFY_INCLUSION: bool> which was never actually used, because verify_execution would still check whether the execution had an inclusion proof.
• I separate preparation from execution inside of vm.execute() and vm.execute_fee() to avoid us having to cast_ref! parameters up and down the call chain.
• Ray asked: "How are we storing the batched function and fee proofs in the Transaction in a way that clearly references which assignments were used and which inputs are required for verification?" I now store a proof in the Execution and Fee objects. I think ordering is straightforward: just adhere to the fixed ordering of the Vector/Array of the Execution/Fee's Assignments/Inputs.
• I replaced the Execute mode of the Callstack by a Prepare mode - running execute_function(...) in Prepare mode doesn't create a proof so instead the caller can afterwards extract the filled registers.
• inherent associated types are unstable if we try sth like: type ExecutionAssignments<'a, N> = Vec<&Assignment<N::Field>>;

Test Plan

• You'll have to resample parameters.
• It seems the original code never verified inclusion proofs. The likely reason for this is because this requires two input records, which was slightly more effort to program. Instead we had e.g. two test cases:
  • sample_execution_transaction_without_fee calls transfer() but fails as expected before verifying the inclusion
  • sample_execution_transaction_with_fee calls mint() which doesn't require an input record and corresponding
    inclusion proof. I adjusted the test now to call transfer(), there are still other tests which call mint().

Related PRs and design document

https://github.com/AleoHQ/snarkVM/pull/1382
https://docs.google.com/document/d/10f_VfrcL9POyBPdFWVov5vVieVL-YNW6gBCxidEoNq8/edit#heading=h.9e2ahww0jjqr

[ljedrz]

IIRC an existence of Arc clones here would indicate a leak/logic error

[vicsn]

Yes in a next iteration I'm following the same approach we use for e.g. execution/inclusion, just try_unwrap

[ljedrz]

is there a plan to support any 16bit architectures? if not, casting u32 to usize should never fail

[vicsn]

I think with that assumption in hand this PR would also be a lot simpler: https://github.com/AleoHQ/snarkVM/pull/1499
But we spent so much time on it already that I think not assuming that is fine for now.

[ljedrz]

this should be identical to an automatic #[derive(PartialOrd, Ord)]: the program_id is compared first, and if it's the same, the function_name is compared (source).

[vicsn]

Due to the way the derived Ord/PartialOrd works, the compiler also demands the networktype (Testnet3) to have those derivations, and to adjust a large amount of trait bounds to be updated to N: Network+Ord+PartialOrd. I think keep the code as is would be better.

For reference, a non-compiling partial implementation:

diff --git a/console/network/src/testnet3.rs b/console/network/src/testnet3.rs
index b29dcf99e..2ebd3435d 100644
--- a/console/network/src/testnet3.rs
+++ b/console/network/src/testnet3.rs
@@ -75,7 +75,7 @@ lazy_static! {
     };
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Ord, PartialOrd)]
 pub struct Testnet3;
 
 impl Testnet3 {
diff --git a/synthesizer/src/block/transition/mod.rs b/synthesizer/src/block/transition/mod.rs
index 1ac051851..863409eb9 100644
--- a/synthesizer/src/block/transition/mod.rs
+++ b/synthesizer/src/block/transition/mod.rs
@@ -431,24 +431,8 @@ impl<N: Network> Transition<N> {
     }
 }
 
-#[derive(Clone, Copy, PartialEq, Eq)]
-pub struct ProvingKeyId<N: Network> {
+#[derive(Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
+pub struct ProvingKeyId<N: Network + Ord> {
     pub program_id: ProgramID<N>,
     pub function_name: Identifier<N>,
-}
-
-impl<N: Network> Ord for ProvingKeyId<N> {
-    /// Ordering is determined by the program_id first, then the function_name second.
-    fn cmp(&self, other: &Self) -> Ordering {
-        match self.program_id == other.program_id {
-            true => self.function_name.cmp(&other.function_name),
-            false => self.program_id.cmp(&other.program_id),
-        }
-    }
-}
-
-impl<N: Network> PartialOrd for ProvingKeyId<N> {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
+}
\ No newline at end of file
diff --git a/synthesizer/src/process/execute.rs b/synthesizer/src/process/execute.rs
index eebdd5249..d761acc06 100644
--- a/synthesizer/src/process/execute.rs
+++ b/synthesizer/src/process/execute.rs
@@ -18,7 +18,7 @@ use super::*;
 
 use crate::{Key, KeyBatch, KeyMode, ProvingKeyId};
 
-impl<N: Network> Process<N> {
+impl<N: Network + Ord + PartialOrd> Process<N> {

[ljedrz]

should we mark the corresponding DataID as deprecated? if we don't intend to use it anymore, we can remove it before the next network reset

[vicsn]

Hmm good question, implemented it here: 50ab2d3
@raychu86 do we have a process in place for updating database objects? Should I document it in CONTRIBUTING?

[ljedrz]

related: https://github.com/AleoHQ/snarkVM/pull/1560; also, only the DataID variant needs to remain, so as to not perturb the order (and values) of the others (unless we explicitly assign values to the DataID variants)

[vicsn]

On advice from Howard, we're merging this PR into staging, allowing us to just remove the DataIDs

[ljedrz]

Left a few more comments.