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

[HackerOne-2300725] Limit the number of allowed constraints for deployments #2271

Merged
merged 40 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
88364b0
Limit the number of allowed constraints for deployments
vicsn Dec 29, 2023
5e0e2f6
Increase allowed number of deployment constraints for existing test s…
vicsn Dec 29, 2023
f429701
Correct test expectations. The RNG depends on FinalizeState
vicsn Jan 2, 2024
696d332
Add test for too many constraints
vicsn Jan 2, 2024
d872a42
Actually limit at max_constraints
vicsn Jan 10, 2024
0be9950
Use Self::set_constraint_maximum
vicsn Jan 10, 2024
06ab667
Comment nit
vicsn Jan 10, 2024
6b6fc40
Convert type early
vicsn Jan 10, 2024
726a614
Check that the number of functions matches the number of verifying keys
vicsn Jan 10, 2024
4573348
Clean up deployment_cost function
vicsn Jan 10, 2024
5d3c2b7
Nit: fix comment
vicsn Jan 10, 2024
61edab1
Merge branch 'testnet3' into limit_deployment_num_constraints
howardwu Jan 11, 2024
d696985
Merge mainnet
raychu86 Jan 19, 2024
6b4b78b
Rewrite expectations
raychu86 Jan 19, 2024
54495e7
Merge branch 'mainnet' into limit_deployment_num_constraints
howardwu Jan 21, 2024
722b6d0
Fix terminology, fix vulnerability
howardwu Jan 21, 2024
2284e88
Merge branch 'limit_deployment_num_constraints' of https://github.com…
howardwu Jan 21, 2024
a22ab6d
Fix the deployment limit usage
howardwu Jan 21, 2024
dc8fb72
Write like an adult
howardwu Jan 21, 2024
e919f7b
nit: comments
howardwu Jan 21, 2024
d2e52b0
Update comments
howardwu Jan 21, 2024
1568ce0
Adds a getter for the constraint limit from the circuit
howardwu Jan 21, 2024
e39f9d2
Fix names, set limit to 1<<20
howardwu Jan 21, 2024
4383518
Missing period (.)
howardwu Jan 21, 2024
3841a3e
Include the synthesis cost in the return for the deployment cost
howardwu Jan 21, 2024
5d0c2da
Add enforcement that the number of synthesized constraints matches th…
howardwu Jan 21, 2024
01b95d2
WIP: scaffolding for testing vk manipulation
vicsn Jan 22, 2024
cc87b98
Remove redundant check
evan-schott Jan 31, 2024
1b02d5c
Revise tests
evan-schott Jan 31, 2024
1ae6626
Modify constraint limit to account for the constraint added after syn…
evan-schott Feb 1, 2024
32053f9
Add back synthesis check to catch overreports
evan-schott Feb 1, 2024
ff10297
tx id changes bc deploy fee increased w/ this PR
evan-schott Feb 1, 2024
32d3ca4
revise new tests
evan-schott Feb 1, 2024
7d443bc
clippy
evan-schott Feb 1, 2024
763d5fe
fixes
evan-schott Feb 2, 2024
6adc557
fixes
evan-schott Feb 2, 2024
b746a19
Correct underreport test program name
vicsn Feb 8, 2024
ed6cfa7
Merge remote-tracking branch 'origin/mainnet' into limit_deployment_n…
vicsn Feb 8, 2024
58e6c39
Correct test expectations
vicsn Feb 8, 2024
411218b
nit: use let-else syntax
howardwu Feb 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions circuit/environment/src/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use core::{
type Field = <console::Testnet3 as console::Environment>::Field;

thread_local! {
pub(super) static CONSTRAINT_LIMIT: Cell<Option<u64>> = Cell::new(None);
pub(super) static CIRCUIT: RefCell<R1CS<Field>> = RefCell::new(R1CS::new());
pub(super) static IN_WITNESS: Cell<bool> = Cell::new(false);
pub(super) static ZERO: LinearCombination<Field> = LinearCombination::zero();
Expand Down Expand Up @@ -146,6 +147,15 @@ impl Environment for Circuit {
// Ensure we are not in witness mode.
if !in_witness.get() {
CIRCUIT.with(|circuit| {
// Ensure that we do not surpass the constraint limit for the circuit.
CONSTRAINT_LIMIT.with(|constraint_limit| {
if let Some(limit) = constraint_limit.get() {
if circuit.borrow().num_constraints() >= limit {
Self::halt(format!("Surpassed the constraint limit ({limit})"))
}
}
});

let (a, b, c) = constraint();
let (a, b, c) = (a.into(), b.into(), c.into());

Expand Down Expand Up @@ -248,8 +258,16 @@ impl Environment for Circuit {
panic!("{}", &error)
}

/// TODO (howardwu): Abstraction - Refactor this into an appropriate design.
/// Circuits should not have easy access to this during synthesis.
/// Returns the constraint limit for the circuit, if one exists.
fn get_constraint_limit() -> Option<u64> {
CONSTRAINT_LIMIT.with(|current_limit| current_limit.get())
}

/// Sets the constraint limit for the circuit.
fn set_constraint_limit(limit: Option<u64>) {
CONSTRAINT_LIMIT.with(|current_limit| current_limit.replace(limit));
}

/// Returns the R1CS circuit, resetting the circuit.
fn inject_r1cs(r1cs: R1CS<Self::BaseField>) {
CIRCUIT.with(|circuit| {
Expand All @@ -268,13 +286,13 @@ impl Environment for Circuit {
})
}

/// TODO (howardwu): Abstraction - Refactor this into an appropriate design.
/// Circuits should not have easy access to this during synthesis.
/// Returns the R1CS circuit, resetting the circuit.
fn eject_r1cs_and_reset() -> R1CS<Self::BaseField> {
CIRCUIT.with(|circuit| {
// Reset the witness mode.
IN_WITNESS.with(|in_witness| in_witness.replace(false));
// Reset the constraint limit.
Self::set_constraint_limit(None);
// Eject the R1CS instance.
let r1cs = circuit.replace(R1CS::<<Self as Environment>::BaseField>::new());
// Ensure the circuit is now empty.
Expand All @@ -287,13 +305,13 @@ impl Environment for Circuit {
})
}

/// TODO (howardwu): Abstraction - Refactor this into an appropriate design.
/// Circuits should not have easy access to this during synthesis.
/// Returns the R1CS assignment of the circuit, resetting the circuit.
fn eject_assignment_and_reset() -> Assignment<<Self::Network as console::Environment>::Field> {
CIRCUIT.with(|circuit| {
// Reset the witness mode.
IN_WITNESS.with(|in_witness| in_witness.replace(false));
// Reset the constraint limit.
Self::set_constraint_limit(None);
// Eject the R1CS instance.
let r1cs = circuit.replace(R1CS::<<Self as Environment>::BaseField>::new());
assert_eq!(0, circuit.borrow().num_constants());
Expand All @@ -310,6 +328,9 @@ impl Environment for Circuit {
CIRCUIT.with(|circuit| {
// Reset the witness mode.
IN_WITNESS.with(|in_witness| in_witness.replace(false));
// Reset the constraint limit.
Self::set_constraint_limit(None);
// Reset the circuit.
*circuit.borrow_mut() = R1CS::<<Self as Environment>::BaseField>::new();
assert_eq!(0, circuit.borrow().num_constants());
assert_eq!(1, circuit.borrow().num_public());
Expand Down
6 changes: 6 additions & 0 deletions circuit/environment/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ pub trait Environment: 'static + Copy + Clone + fmt::Debug + fmt::Display + Eq +
<Self::Network as console::Environment>::halt(message)
}

/// Returns the constraint limit for the circuit, if one exists.
fn get_constraint_limit() -> Option<u64>;

/// Sets the constraint limit for the circuit.
fn set_constraint_limit(limit: Option<u64>);

/// Returns the R1CS circuit, resetting the circuit.
fn inject_r1cs(r1cs: R1CS<Self::BaseField>);

Expand Down
10 changes: 10 additions & 0 deletions circuit/network/src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,16 @@ impl Environment for AleoV0 {
E::halt(message)
}

/// Returns the constraint limit for the circuit, if one exists.
fn get_constraint_limit() -> Option<u64> {
E::get_constraint_limit()
}

/// Sets the constraint limit for the circuit.
fn set_constraint_limit(limit: Option<u64>) {
E::set_constraint_limit(limit)
}

/// Returns the R1CS circuit, resetting the circuit.
fn inject_r1cs(r1cs: R1CS<Self::BaseField>) {
E::inject_r1cs(r1cs)
Expand Down
4 changes: 4 additions & 0 deletions console/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ pub trait Network:
const STARTING_SUPPLY: u64 = 1_500_000_000_000_000; // 1.5B credits
/// The cost in microcredits per byte for the deployment transaction.
const DEPLOYMENT_FEE_MULTIPLIER: u64 = 1_000; // 1 millicredit per byte
/// The cost in microcredits per constraint for the deployment transaction.
const SYNTHESIS_FEE_MULTIPLIER: u64 = 25; // 25 microcredits per constraint
/// The maximum number of constraints in a deployment.
const MAX_DEPLOYMENT_LIMIT: u64 = 1 << 20; // 1,048,576 constraints
/// The maximum number of microcredits that can be spent as a fee.
const MAX_FEE: u64 = 1_000_000_000_000_000;

Expand Down
17 changes: 17 additions & 0 deletions ledger/block/src/transaction/deployment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ impl<N: Network> Deployment<N> {
&self.verifying_keys
}

/// Returns the sum of the constraint counts for all functions in this deployment.
pub fn num_combined_constraints(&self) -> Result<u64> {
// Initialize the accumulator.
let mut num_combined_constraints = 0u64;
// Iterate over the functions.
for (_, (vk, _)) in &self.verifying_keys {
// Add the number of constraints.
// Note: This method must be *checked* because the claimed constraint count
// is from the user, not the synthesizer.
num_combined_constraints = num_combined_constraints
.checked_add(vk.circuit_info.num_constraints as u64)
.ok_or_else(|| anyhow!("Overflow when counting constraints for '{}'", self.program_id()))?;
}
// Return the number of combined constraints.
Ok(num_combined_constraints)
}

/// Returns the deployment ID.
pub fn to_deployment_id(&self) -> Result<Field<N>> {
Ok(*Transaction::deployment_tree(self, None)?.root())
Expand Down
5 changes: 5 additions & 0 deletions synthesizer/process/src/stack/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@ impl<N: Network> CallTrait<N> for Call<N> {
// Inject the existing circuit.
A::inject_r1cs(r1cs);

// If in 'CheckDeployment' mode, set the expected constraint limit.
if let CallStack::CheckDeployment(_, _, _, constraint_limit) = &registers.call_stack() {
A::set_constraint_limit(*constraint_limit);
}

Comment on lines +385 to +389
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced this logic is necessary.

When triggering call, no logic above this resets the constraint limit AFAIK.

use circuit::Inject;

// Inject the network ID as `Mode::Constant`.
Expand Down
31 changes: 27 additions & 4 deletions synthesizer/process/src/stack/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,21 @@ impl<N: Network> Stack<N> {

let program_id = self.program.id();

// Check that the number of combined constraints does not exceed the deployment limit.
ensure!(deployment.num_combined_constraints()? <= N::MAX_DEPLOYMENT_LIMIT);

// Construct the call stacks and assignments used to verify the certificates.
let mut call_stacks = Vec::with_capacity(deployment.verifying_keys().len());

// Check that the number of functions matches the number of verifying keys.
ensure!(
deployment.program().functions().len() == deployment.verifying_keys().len(),
"The number of functions in the program does not match the number of verifying keys"
);
// Iterate through the program functions and construct the callstacks and corresponding assignments.
for function in deployment.program().functions().values() {
for (function, (_, (verifying_key, _))) in
deployment.program().functions().values().zip_eq(deployment.verifying_keys())
ljedrz marked this conversation as resolved.
Show resolved Hide resolved
{
// Initialize a burner private key.
let burner_private_key = PrivateKey::new(rng)?;
// Compute the burner address.
Expand Down Expand Up @@ -114,18 +124,31 @@ impl<N: Network> Stack<N> {
lap!(timer, "Compute the request for {}", function.name());
// Initialize the assignments.
let assignments = Assignments::<N>::default();
// Initialize the constraint limit. Account for the constraint added after synthesis that randomizes vars.
vicsn marked this conversation as resolved.
Show resolved Hide resolved
let constraint_limit = match verifying_key.circuit_info.num_constraints.checked_sub(1) {
// Since a deployment must always pay non-zero fee, it must always have at least one constraint.
None => {
bail!("The constraint limit of 0 for function '{}' is invalid", function.name());
}
Some(limit) => limit,
};
// Initialize the call stack.
let call_stack = CallStack::CheckDeployment(vec![request], burner_private_key, assignments.clone());
let call_stack = CallStack::CheckDeployment(
vec![request],
burner_private_key,
assignments.clone(),
Some(constraint_limit as u64),
);
// Append the function name, callstack, and assignments.
call_stacks.push((function.name(), call_stack, assignments));
}

// Verify the certificates.
let rngs = (0..call_stacks.len()).map(|_| StdRng::from_seed(rng.gen())).collect::<Vec<_>>();
cfg_iter!(call_stacks).zip_eq(deployment.verifying_keys()).zip_eq(rngs).try_for_each(
cfg_into_iter!(call_stacks).zip_eq(deployment.verifying_keys()).zip_eq(rngs).try_for_each(
|(((function_name, call_stack, assignments), (_, (verifying_key, certificate))), mut rng)| {
// Synthesize the circuit.
if let Err(err) = self.execute_function::<A, _>(call_stack.clone(), None, &mut rng) {
if let Err(err) = self.execute_function::<A, _>(call_stack, None, &mut rng) {
bail!("Failed to synthesize the circuit for '{function_name}': {err}")
}
// Check the certificate.
Expand Down
7 changes: 6 additions & 1 deletion synthesizer/process/src/stack/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ impl<N: Network> StackExecute<N> for Stack<N> {
// Ensure the circuit environment is clean.
A::reset();

// If in 'CheckDeployment' mode, set the constraint limit.
if let CallStack::CheckDeployment(_, _, _, constraint_limit) = &call_stack {
A::set_constraint_limit(*constraint_limit);
}

// Retrieve the next request.
let console_request = call_stack.pop()?;

Expand Down Expand Up @@ -416,7 +421,7 @@ impl<N: Network> StackExecute<N> for Stack<N> {
lap!(timer, "Save the transition");
}
// If the circuit is in `CheckDeployment` mode, then save the assignment.
else if let CallStack::CheckDeployment(_, _, ref assignments) = registers.call_stack() {
else if let CallStack::CheckDeployment(_, _, ref assignments, _) = registers.call_stack() {
// Construct the call metrics.
let metrics = CallMetrics {
program_id: *self.program_id(),
Expand Down
15 changes: 9 additions & 6 deletions synthesizer/process/src/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub type Assignments<N> = Arc<RwLock<Vec<(circuit::Assignment<<N as Environment>
pub enum CallStack<N: Network> {
Authorize(Vec<Request<N>>, PrivateKey<N>, Authorization<N>),
Synthesize(Vec<Request<N>>, PrivateKey<N>, Authorization<N>),
CheckDeployment(Vec<Request<N>>, PrivateKey<N>, Assignments<N>),
CheckDeployment(Vec<Request<N>>, PrivateKey<N>, Assignments<N>, Option<u64>),
Evaluate(Authorization<N>),
Execute(Authorization<N>, Arc<RwLock<Trace<N>>>),
PackageRun(Vec<Request<N>>, PrivateKey<N>, Assignments<N>),
Expand Down Expand Up @@ -109,11 +109,14 @@ impl<N: Network> CallStack<N> {
CallStack::Synthesize(requests, private_key, authorization) => {
CallStack::Synthesize(requests.clone(), *private_key, authorization.replicate())
}
CallStack::CheckDeployment(requests, private_key, assignments) => CallStack::CheckDeployment(
requests.clone(),
*private_key,
Arc::new(RwLock::new(assignments.read().clone())),
),
CallStack::CheckDeployment(requests, private_key, assignments, constraint_limit) => {
CallStack::CheckDeployment(
requests.clone(),
*private_key,
Arc::new(RwLock::new(assignments.read().clone())),
*constraint_limit,
)
}
CallStack::Evaluate(authorization) => CallStack::Evaluate(authorization.replicate()),
CallStack::Execute(authorization, trace) => {
CallStack::Execute(authorization.replicate(), Arc::new(RwLock::new(trace.read().clone())))
Expand Down
2 changes: 1 addition & 1 deletion synthesizer/process/src/tests/test_credits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@ mod sanity_checks {
// Initialize the assignments.
let assignments = Assignments::<N>::default();
// Initialize the call stack.
let call_stack = CallStack::CheckDeployment(vec![request], *private_key, assignments.clone());
let call_stack = CallStack::CheckDeployment(vec![request], *private_key, assignments.clone(), None);
// Synthesize the circuit.
let _response = stack.execute_function::<A, _>(call_stack, None, rng).unwrap();
// Retrieve the assignment.
Expand Down
2 changes: 1 addition & 1 deletion synthesizer/src/vm/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
let owner = ProgramOwner::new(private_key, deployment_id, rng)?;

// Compute the minimum deployment cost.
let (minimum_deployment_cost, (_, _)) = deployment_cost(&deployment)?;
let (minimum_deployment_cost, _) = deployment_cost(&deployment)?;
// Authorize the fee.
let fee_authorization = match fee_record {
Some(record) => self.authorize_fee_private(
Expand Down
16 changes: 11 additions & 5 deletions synthesizer/src/vm/helpers/cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,25 @@ use synthesizer_program::{Command, Finalize, Instruction};

use std::collections::HashMap;

/// Returns the *minimum* cost in microcredits to publish the given deployment (total cost, (storage cost, namespace cost)).
pub fn deployment_cost<N: Network>(deployment: &Deployment<N>) -> Result<(u64, (u64, u64))> {
/// Returns the *minimum* cost in microcredits to publish the given deployment (total cost, (storage cost, synthesis cost, namespace cost)).
pub fn deployment_cost<N: Network>(deployment: &Deployment<N>) -> Result<(u64, (u64, u64, u64))> {
// Determine the number of bytes in the deployment.
let size_in_bytes = deployment.size_in_bytes()?;
// Retrieve the program ID.
let program_id = deployment.program_id();
// Determine the number of characters in the program ID.
let num_characters = u32::try_from(program_id.name().to_string().len())?;
// Compute the number of combined constraints in the program.
let num_combined_constraints = deployment.num_combined_constraints()?;

// Compute the storage cost in microcredits.
let storage_cost = size_in_bytes
.checked_mul(N::DEPLOYMENT_FEE_MULTIPLIER)
.ok_or(anyhow!("The storage cost computation overflowed for a deployment"))?;

// Compute the synthesis cost in microcredits.
let synthesis_cost = num_combined_constraints * N::SYNTHESIS_FEE_MULTIPLIER;

// Compute the namespace cost in credits: 10^(10 - num_characters).
let namespace_cost = 10u64
.checked_pow(10u32.saturating_sub(num_characters))
Expand All @@ -45,13 +50,14 @@ pub fn deployment_cost<N: Network>(deployment: &Deployment<N>) -> Result<(u64, (

// Compute the total cost in microcredits.
let total_cost = storage_cost
.checked_add(namespace_cost)
.checked_add(synthesis_cost)
.and_then(|x| x.checked_add(namespace_cost))
.ok_or(anyhow!("The total cost computation overflowed for a deployment"))?;

Ok((total_cost, (storage_cost, namespace_cost)))
Ok((total_cost, (storage_cost, synthesis_cost, namespace_cost)))
}

/// Returns the *minimum* cost in microcredits to publish the given execution (total cost, (storage cost, namespace cost)).
/// Returns the *minimum* cost in microcredits to publish the given execution (total cost, (storage cost, finalize cost)).
pub fn execution_cost<N: Network, C: ConsensusStorage<N>>(
vm: &VM<N, C>,
execution: &Execution<N>,
Expand Down
Loading