diff --git a/.circleci/config.yml b/.circleci/config.yml index b72cf547ca..fc8884df89 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1290,6 +1290,7 @@ workflows: or pipeline.git.branch == "testnet" or pipeline.git.branch == "mainnet" or pipeline.git.branch == "ensure_finalize_scopes_match" + or pipeline.git.branch == "scalar_conversion" jobs: - check-unused-dependencies # This can be cleaned up before releases - check-cargo-semver-checks # This can be cleaned up before releases diff --git a/circuit/environment/src/canary_circuit.rs b/circuit/environment/src/canary_circuit.rs index f989777017..38c7f7b9be 100644 --- a/circuit/environment/src/canary_circuit.rs +++ b/circuit/environment/src/canary_circuit.rs @@ -286,6 +286,9 @@ impl Environment for CanaryCircuit { /// Returns the R1CS circuit, resetting the circuit. fn eject_r1cs_and_reset() -> R1CS { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + CANARY_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -308,6 +311,9 @@ impl Environment for CanaryCircuit { /// Returns the R1CS assignment of the circuit, resetting the circuit. fn eject_assignment_and_reset() -> Assignment<::Field> { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + CANARY_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -329,6 +335,9 @@ impl Environment for CanaryCircuit { /// Clears the circuit and initializes an empty environment. fn reset() { + // Clear the unconverted-values tracking set. + Self::clear_unconverted_values(); + CANARY_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -343,6 +352,8 @@ impl Environment for CanaryCircuit { assert_eq!(0, circuit.borrow().num_private()); assert_eq!(1, circuit.borrow().num_variables()); assert_eq!(0, circuit.borrow().num_constraints()); + // Ensure the unconverted-value tracking set is empty. + assert!(Self::check_unconverted_values().is_ok()); }); } } diff --git a/circuit/environment/src/circuit.rs b/circuit/environment/src/circuit.rs index da89fcc674..af8134a773 100644 --- a/circuit/environment/src/circuit.rs +++ b/circuit/environment/src/circuit.rs @@ -310,6 +310,9 @@ impl Environment for Circuit { /// Returns the R1CS circuit, resetting the circuit. fn eject_r1cs_and_reset() -> R1CS { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -332,6 +335,9 @@ impl Environment for Circuit { /// Returns the R1CS assignment of the circuit, resetting the circuit. fn eject_assignment_and_reset() -> Assignment<::Field> { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -353,6 +359,9 @@ impl Environment for Circuit { /// Clears the circuit and initializes an empty environment. fn reset() { + // Clear the unconverted-values tracking set. + Self::clear_unconverted_values(); + CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -367,6 +376,8 @@ impl Environment for Circuit { assert_eq!(0, circuit.borrow().num_private()); assert_eq!(1, circuit.borrow().num_variables()); assert_eq!(0, circuit.borrow().num_constraints()); + // Ensure the unconverted-value tracking set is empty. + assert!(Self::check_unconverted_values().is_ok()); }); } } diff --git a/circuit/environment/src/environment.rs b/circuit/environment/src/environment.rs index dba668f1fd..efc972ea00 100644 --- a/circuit/environment/src/environment.rs +++ b/circuit/environment/src/environment.rs @@ -18,6 +18,29 @@ use snarkvm_curves::AffineCurve; use snarkvm_fields::traits::*; use core::{fmt, hash}; +use std::{cell::RefCell, collections::BTreeMap}; + +use anyhow::Result; + +thread_local! { + /// Stores certain injected values which have not been converted to bits yet, alongside a + /// closure that converts each of them to bits. + // This gives higher-level crates the ability to track any values which must be converted to + // bits before the end of synthesis (e.g. Scalars, which are otherwise never range-checked) and + // convert them if they have not been. + static UNCONVERTED_VALUES: RefCell>> = RefCell::new(BTreeMap::new()); +} + +/// Returns a tracking key for a non-constant variable, or `None` for constants. An index shift of +/// 2^63 is used to distinguish private variables from public ones, as they share the same index space. +fn value_tracking_key(variable: &Variable) -> Option { + const PRIVATE_FLAG: u64 = 1 << 63; + match variable.mode() { + Mode::Constant => None, + Mode::Public => Some(variable.index()), + Mode::Private => Some(variable.index() | PRIVATE_FLAG), + } +} /// Attention: Do not use `Send + Sync` on this trait, as it is not thread-safe. pub trait Environment: 'static + Copy + Clone + fmt::Debug + fmt::Display + Eq + PartialEq + hash::Hash { @@ -174,6 +197,62 @@ pub trait Environment: 'static + Copy + Clone + fmt::Debug + fmt::Display + Eq + /// Sets the constraint limit for the circuit. fn set_constraint_limit(limit: Option); + /// Records an injected value which has not been converted to bits yet, alongside a closure + /// that converts it to bits. + // The main purpose of this function is to track (non-constant) Aleo Scalars in higher-level + // crates, whose underlying base-field element is not checked to be in the canonical scalar range + // until to_bits is called. + fn track_unconverted_value(value: &LinearCombination, convert: Box) { + if let Some(key) = value.to_terms().iter().find_map(|(variable, _)| value_tracking_key(variable)) { + UNCONVERTED_VALUES.with(|set| { + set.borrow_mut().insert(key, convert); + }); + } + } + + /// Removes a value from the set of unconverted values. + fn untrack_unconverted_value(value: &LinearCombination) { + UNCONVERTED_VALUES.with(|set| { + let mut set = set.borrow_mut(); + for (variable, _) in value.to_terms() { + if let Some(key) = value_tracking_key(variable) { + set.remove(&key); + } + } + }); + } + + /// Returns `Ok` if no unconverted values remain in the tracking set and `Err` otherwise. + fn check_unconverted_values() -> Result<(), String> { + UNCONVERTED_VALUES.with(|set| { + let set = set.borrow(); + if set.is_empty() { + Ok(()) + } else { + Err(format!("{} unconverted value(s) remain(s) in the tracking set: {:#?}", set.len(), set.keys())) + } + }) + } + + /// Converts every unconverted value to bits by calling the stored closure, adding the + /// relevant constraints to the current circuit. + fn convert_unconverted_values() { + // Take ownership of the closures, releasing the borrow before invoking them: each closure is + // a `FnOnce` (so must be consumed by value) and converts a value to bits, which re-enters + // `untrack_unconverted_value` and would otherwise conflict with the outstanding borrow. + let pending: Vec> = + UNCONVERTED_VALUES.with(|set| core::mem::take(&mut *set.borrow_mut()).into_values().collect()); + for convert in pending { + convert(); + } + } + + /// Clears the set of unconverted values, dropping the stored conversion closures without + /// invoking them. Used when resetting the circuit between syntheses. + fn clear_unconverted_values() { + UNCONVERTED_VALUES.with(|set| set.borrow_mut().clear()); + } + /// Halts the program from further synthesis, evaluation, and execution in the current environment. fn halt, T>(message: S) -> T { ::halt(message) diff --git a/circuit/environment/src/helpers/assignment.rs b/circuit/environment/src/helpers/assignment.rs index 04d7bcbd74..e53b55f025 100644 --- a/circuit/environment/src/helpers/assignment.rs +++ b/circuit/environment/src/helpers/assignment.rs @@ -303,9 +303,11 @@ pub fn compare_constraints(assignment_1: &Assignment, assignme #[cfg(test)] mod tests { + use console::TestRng; use snarkvm_algorithms::{AlgebraicSponge, SNARK, r1cs::ConstraintSynthesizer, snark::varuna::VarunaVersion}; use snarkvm_circuit::prelude::*; use snarkvm_curves::bls12_377::Fr; + use snarkvm_utilities::Uniform; /// Compute 2^EXPONENT - 1, in a purposefully constraint-inefficient manner for testing. fn create_example_circuit() -> Field { @@ -396,4 +398,67 @@ mod tests { .unwrap() ); } + + fn sample_circuit_for_scalars( + // Whether to inject a scalar which is never converted or not + inject_unconverted_scalar: bool, + // True if the assignment should be ejected, false if the R1CS should be ejected + eject_assignment: bool, + rng: &mut TestRng, + ) { + // Inject a few innocuous values and add a constraint. + let field_1 = Field::::new(Mode::Private, Uniform::rand(rng)); + let field_2 = Field::::new(Mode::Public, Uniform::rand(rng)); + let _product = &field_1 * &field_2; + + // Inject a Scalar and convert it to bits: this is not problematic. + let scalar = Scalar::::new(Mode::Private, Uniform::rand(rng)); + let _scalar_bits = scalar.to_bits_le(); + + if inject_unconverted_scalar { + // Inject a private scalar which is never converted to bits: ejection should fail + let _unconverted_scalar = Scalar::::new(Mode::Private, Uniform::rand(rng)); + } + + // Hash the injected fields to add a few more constraints. + let _fields_hash = AleoV0::hash_psd8(&[field_1, field_2]); + + match (eject_assignment, inject_unconverted_scalar) { + // Ejection must fail when an injected scalar was never converted to bits. + (true, true) => { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(AleoV0::eject_assignment_and_reset)); + assert!(result.is_err()); + AleoV0::reset(); + } + (false, true) => { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(AleoV0::eject_r1cs_and_reset)); + assert!(result.is_err()); + AleoV0::reset(); + } + // Ejection succeeds when every injected scalar was converted to bits. + (true, false) => { + let _assignment = AleoV0::eject_assignment_and_reset(); + } + (false, false) => { + let _r1cs = AleoV0::eject_r1cs_and_reset(); + } + } + + AleoV0::reset(); + } + + #[test] + // Checks that a (sample) circuit fails to eject the assignment and the R1CS if a Scalar is + // injected and never converted to bits. + fn test_unconverted_scalar_rejection() { + let rng = &mut TestRng::default(); + + // Cases with unconverted scalars at the end: panic expected (caught by assert_panics) + sample_circuit_for_scalars(true, true, rng); + sample_circuit_for_scalars(true, false, rng); + + // Sanity check: when the (unconverted) scalar is not injected, ejection works + sample_circuit_for_scalars(false, true, rng); + sample_circuit_for_scalars(false, false, rng); + } } diff --git a/circuit/environment/src/helpers/linear_combination.rs b/circuit/environment/src/helpers/linear_combination.rs index a931610138..528c561b66 100644 --- a/circuit/environment/src/helpers/linear_combination.rs +++ b/circuit/environment/src/helpers/linear_combination.rs @@ -132,7 +132,7 @@ impl LinearCombination { } /// Returns the terms (excluding the constant value) in the linear combination. - pub(super) fn to_terms(&self) -> &[(Variable, F)] { + pub(crate) fn to_terms(&self) -> &[(Variable, F)] { &self.terms } diff --git a/circuit/environment/src/testnet_circuit.rs b/circuit/environment/src/testnet_circuit.rs index 07e9421361..23b857af7a 100644 --- a/circuit/environment/src/testnet_circuit.rs +++ b/circuit/environment/src/testnet_circuit.rs @@ -286,6 +286,9 @@ impl Environment for TestnetCircuit { /// Returns the R1CS circuit, resetting the circuit. fn eject_r1cs_and_reset() -> R1CS { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + TESTNET_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -308,6 +311,9 @@ impl Environment for TestnetCircuit { /// Returns the R1CS assignment of the circuit, resetting the circuit. fn eject_assignment_and_reset() -> Assignment<::Field> { + // Ensure all Scalars have been converted to bits. + Self::check_unconverted_values().unwrap(); + TESTNET_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -329,6 +335,9 @@ impl Environment for TestnetCircuit { /// Clears the circuit and initializes an empty environment. fn reset() { + // Clear the unconverted-values tracking set. + Self::clear_unconverted_values(); + TESTNET_CIRCUIT.with(|circuit| { // Reset the witness mode. IN_WITNESS.with(|in_witness| in_witness.replace(false)); @@ -343,6 +352,8 @@ impl Environment for TestnetCircuit { assert_eq!(0, circuit.borrow().num_private()); assert_eq!(1, circuit.borrow().num_variables()); assert_eq!(0, circuit.borrow().num_constraints()); + // Ensure the unconverted-value tracking set is empty. + assert!(Self::check_unconverted_values().is_ok()); }); } } diff --git a/circuit/types/scalar/src/helpers/to_bits.rs b/circuit/types/scalar/src/helpers/to_bits.rs index 12dfeae275..2585224a60 100644 --- a/circuit/types/scalar/src/helpers/to_bits.rs +++ b/circuit/types/scalar/src/helpers/to_bits.rs @@ -50,6 +50,10 @@ impl ToBits for &Scalar { Boolean::assert_less_than_or_equal_constant(&bits_le, &modulus_minus_one.to_bits_le()); } + // Now that underlying Field has been checked to represent a correct Scalar, we remove + // it from the unconverted-value tracking set. + E::untrack_unconverted_value(&LinearCombination::from(&self.field)); + bits_le }); // Extend the vector with the bits of the scalar. diff --git a/circuit/types/scalar/src/lib.rs b/circuit/types/scalar/src/lib.rs index d16e4ac6cc..86eca523b7 100644 --- a/circuit/types/scalar/src/lib.rs +++ b/circuit/types/scalar/src/lib.rs @@ -57,7 +57,21 @@ impl Inject for Scalar { // Initialize the scalar as a field element. match console::ToField::to_field(&scalar) { - Ok(field) => Self { field: Field::new(mode, field), bits_le: OnceCell::new() }, + Ok(field) => { + let scalar = Self { field: Field::new(mode, field), bits_le: OnceCell::new() }; + // Track the underlying variable so we can detect, at the end of synthesis, any + // scalar that is never range-checked. The closure converts this scalar to bits + // (range-checking it) if it is never otherwise converted during synthesis. + let lc = LinearCombination::from(&scalar.field); + let pending = scalar.clone(); + E::track_unconverted_value( + &lc, + Box::new(move || { + let _ = pending.to_bits_le(); + }), + ); + scalar + } Err(error) => E::halt(format!("Unable to initialize a scalar circuit as a field element: {error}")), } } diff --git a/ledger/puzzle/epoch/src/synthesis/program/to_r1cs.rs b/ledger/puzzle/epoch/src/synthesis/program/to_r1cs.rs index 425bd19749..abe0f22ace 100644 --- a/ledger/puzzle/epoch/src/synthesis/program/to_r1cs.rs +++ b/ledger/puzzle/epoch/src/synthesis/program/to_r1cs.rs @@ -94,6 +94,9 @@ impl EpochProgram { ); lap!(timer, "Ensure the circuit is satisfied"); + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + // Eject the R1CS and reset the circuit. let r1cs = A::eject_r1cs_and_reset(); finish!(timer, "Eject the circuit assignment and reset the circuit"); diff --git a/synthesizer/benches/kary_merkle_tree.rs b/synthesizer/benches/kary_merkle_tree.rs index af50d80449..6ca6e01d50 100644 --- a/synthesizer/benches/kary_merkle_tree.rs +++ b/synthesizer/benches/kary_merkle_tree.rs @@ -112,6 +112,9 @@ fn batch_prove(c: &mut Criterion) { let candidate = path.verify(&circuit_leaf_hasher, &circuit_path_hasher, &root, &leaf); assert!(candidate.eject_value()); + // Convert all tracked, still-unconverted values to bits. + CurrentAleo::convert_unconverted_values(); + // Eject the assignment. CurrentAleo::eject_assignment_and_reset() }; diff --git a/synthesizer/process/src/stack/call/dynamic.rs b/synthesizer/process/src/stack/call/dynamic.rs index 6be0994d03..ab2063705d 100644 --- a/synthesizer/process/src/stack/call/dynamic.rs +++ b/synthesizer/process/src/stack/call/dynamic.rs @@ -254,6 +254,9 @@ impl CallTrait for CallDynamic { // Indicate that external calls are never a root request. let is_root = false; + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + // Eject the existing circuit. let r1cs = A::eject_r1cs_and_reset(); let (request, caller_response_outputs) = { diff --git a/synthesizer/process/src/stack/call/standard.rs b/synthesizer/process/src/stack/call/standard.rs index 54aadf913a..6d6a61b739 100644 --- a/synthesizer/process/src/stack/call/standard.rs +++ b/synthesizer/process/src/stack/call/standard.rs @@ -255,6 +255,9 @@ impl CallTrait for Call { // Indicate that external calls are never a root request. let is_root = false; + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + use circuit::Eject; // Eject the existing circuit. let r1cs = A::eject_r1cs_and_reset(); diff --git a/synthesizer/process/src/stack/execute.rs b/synthesizer/process/src/stack/execute.rs index bb461bdfaa..489eaec26c 100644 --- a/synthesizer/process/src/stack/execute.rs +++ b/synthesizer/process/src/stack/execute.rs @@ -567,6 +567,9 @@ impl Stack { } } + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + // Eject the circuit assignment and reset the circuit. let assignment = A::eject_assignment_and_reset(); diff --git a/synthesizer/process/src/trace/inclusion/assignment.rs b/synthesizer/process/src/trace/inclusion/assignment.rs index 15a866b8cc..4e9a551dcc 100644 --- a/synthesizer/process/src/trace/inclusion/assignment.rs +++ b/synthesizer/process/src/trace/inclusion/assignment.rs @@ -127,6 +127,9 @@ impl InclusionAssignment { "InclusionAssignment".to_string(), ); + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + // Eject the assignment and reset the circuit environment. Ok(A::eject_assignment_and_reset()) } diff --git a/synthesizer/process/src/trace/inclusion/assignment_v0.rs b/synthesizer/process/src/trace/inclusion/assignment_v0.rs index 6cce0467ee..1773ce7898 100644 --- a/synthesizer/process/src/trace/inclusion/assignment_v0.rs +++ b/synthesizer/process/src/trace/inclusion/assignment_v0.rs @@ -88,6 +88,9 @@ impl InclusionV0Assignment { "InclusionV0Assignment".to_string(), ); + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + // Eject the assignment and reset the circuit environment. Ok(A::eject_assignment_and_reset()) } diff --git a/synthesizer/process/src/trace/translation/assignment.rs b/synthesizer/process/src/trace/translation/assignment.rs index d94a7f3f98..359513ace3 100644 --- a/synthesizer/process/src/trace/translation/assignment.rs +++ b/synthesizer/process/src/trace/translation/assignment.rs @@ -282,6 +282,9 @@ impl TranslationAssignment { format_args!("Translation circuit for dynamic record with nonce {}", self.record_static.nonce()), "TranslationAssignment", ); + // Convert all tracked, still-unconverted values to bits. + A::convert_unconverted_values(); + Ok(A::eject_assignment_and_reset()) } } diff --git a/synthesizer/src/vm/tests/test_v14/mod.rs b/synthesizer/src/vm/tests/test_v14/mod.rs index ba412b1e32..e178037572 100644 --- a/synthesizer/src/vm/tests/test_v14/mod.rs +++ b/synthesizer/src/vm/tests/test_v14/mod.rs @@ -213,3 +213,91 @@ pub(crate) fn add_and_test_with_costs( } } } + +// Deploys three programs: one whose function adds two private scalar inputs and outputs the private +// sum, one whose function hashes two private inputs and outputs the private hash, and one defining a +// record with a `scalar` entry. +#[test] +fn test_scalar_behavior() { + // Initialize an RNG. + let rng = &mut TestRng::default(); + + // Initialize a new caller. + let caller_private_key = sample_genesis_private_key(rng); + + // Initialize the VM at the V14 height. + let v14_height = CurrentNetwork::CONSENSUS_HEIGHT(ConsensusVersion::V14).unwrap(); + let vm = sample_vm_at_height(v14_height, rng); + + // A program whose function adds two private scalar inputs and outputs the private sum. + let scalar_add_program = Program::::from_str( + r" +program scalar_add_test.aleo; + +function add_scalars: + input r0 as scalar.private; + input r1 as scalar.private; + add r0 r1 into r2; + +constructor: + assert.eq true true; +", + ) + .unwrap(); + + // A program whose function hashes two private inputs and outputs the private hash. + let hash_program = Program::::from_str( + r" +program hash_inputs_test.aleo; + +function hash_inputs: + input r0 as field.private; + input r1 as field.private; + hash.bhp256 r0 into r2 as field; + hash.bhp256 r1 into r3 as field; + output r3 as field.private; + +constructor: + assert.eq true true; +", + ) + .unwrap(); + + // A program defining a record with a `scalar` entry, minted by a function. + let scalar_record_program = Program::::from_str( + r" +program scalar_record_test.aleo; + +record holder: + owner as address.private; + val as scalar.private; + +record holder2: + owner as address.private; + val as scalar.public; + +function mint: + input r0 as address.private; + input r1 as scalar.private; + cast r0 r1 into r2 as holder.record; + output r2 as holder.record; + +constructor: + assert.eq true true; +", + ) + .unwrap(); + + // Deploy each program (in its own block) and ensure the deployment is accepted. + for program in [&scalar_add_program, &hash_program, &scalar_record_program] { + println!("Deploying program: {}", program.id()); + let deployment = vm.deploy(&caller_private_key, program, None, 0, None, rng).unwrap(); + assert!(vm.check_transaction(&deployment, None, rng).is_ok()); + + let block = sample_next_block(&vm, &caller_private_key, &[deployment], rng).unwrap(); + assert_eq!(block.transactions().num_accepted(), 1); + assert_eq!(block.transactions().num_rejected(), 0); + assert_eq!(block.aborted_transaction_ids().len(), 0); + vm.add_next_block(&block).unwrap(); + } +}