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

fix: TypeId in ContextTag not stable across builds #217

Merged
merged 3 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions halo2-base/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ During `synthesize()`, the advice values of all `Context`s are concatenated into

For parallel witness generation, multiple `Context`s are created for each parallel operation. After parallel witness generation, these `Context`'s are combined to form a single "virtual column" as above. Note that while the witness generation can be multi-threaded, the ordering of the contents in each `Context`, and the order of the `Context`s themselves, must be deterministic.

**Warning:** If you create your own `Context` in a new virtual region not provided by our libraries, you must ensure that the `type_id: &str` of the context is a globally unique identifier for the virtual region, distinct from the other `type_id` strings used to identify other virtual regions. In the future we will introduce a macro to check this uniqueness at compile time.

### [**AssignedValue**](./src/lib.rs):

Despite the name, an `AssignedValue` is a **virtual cell**. It contains the actual witness value as well as a pointer to the location of the virtual cell within a virtual region. The pointer is given by type `ContextCell`. We only store the pointer when not in witness generation only mode as an optimization.
Expand Down
15 changes: 6 additions & 9 deletions halo2-base/src/gates/flex_gate/threads/single_phase.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{any::TypeId, cell::RefCell};
use std::cell::RefCell;

use getset::CopyGetters;

Expand All @@ -13,10 +13,7 @@ use crate::{
Context, ContextCell,
};
use crate::{
halo2_proofs::{
circuit::{Region, Value},
plonk::{FirstPhase, SecondPhase, ThirdPhase},
},
halo2_proofs::circuit::{Region, Value},
virtual_region::manager::VirtualRegionManager,
};

Expand Down Expand Up @@ -114,11 +111,11 @@ impl<F: ScalarField> SinglePhaseCoreManager<F> {
}

/// A distinct tag for this particular type of virtual manager, which is different for each phase.
pub fn type_of(&self) -> TypeId {
pub fn type_of(&self) -> &'static str {
match self.phase {
0 => TypeId::of::<(Self, FirstPhase)>(),
1 => TypeId::of::<(Self, SecondPhase)>(),
2 => TypeId::of::<(Self, ThirdPhase)>(),
0 => "SinglePhaseCoreManager: FirstPhase",
1 => "SinglePhaseCoreManager: SecondPhase",
2 => "SinglePhaseCoreManager: ThirdPhase",
_ => panic!("Unsupported phase"),
}
}
Expand Down
22 changes: 12 additions & 10 deletions halo2-base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#![warn(clippy::default_numeric_fallback)]
#![warn(missing_docs)]

use std::any::TypeId;

use getset::CopyGetters;
use itertools::Itertools;
// Different memory allocator options:
Expand Down Expand Up @@ -104,14 +102,16 @@ impl<F: ScalarField> QuantumCell<F> {
}
}

/// Unique tag for a context across all virtual regions
pub type ContextTag = (TypeId, usize);
/// Unique tag for a context across all virtual regions.
/// In the form `(type_id, context_id)` where `type_id` should be a unique identifier
/// for the virtual region this context belongs to, and `context_id` is a counter local to that virtual region.
pub type ContextTag = (&'static str, usize);

/// Pointer to the position of a cell at `offset` in an advice column within a [Context] of `context_id`.
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct ContextCell {
/// The [TypeId] of the virtual region that this cell belongs to.
pub type_id: TypeId,
/// The unique string identifier of the virtual region that this cell belongs to.
pub type_id: &'static str,
/// Identifier of the [Context] that this cell belongs to.
pub context_id: usize,
/// Relative offset of the cell within this [Context] advice column.
Expand All @@ -120,7 +120,7 @@ pub struct ContextCell {

impl ContextCell {
/// Creates a new [ContextCell] with the given `type_id`, `context_id`, and `offset`.
pub fn new(type_id: TypeId, context_id: usize, offset: usize) -> Self {
pub fn new(type_id: &'static str, context_id: usize, offset: usize) -> Self {
Self { type_id, context_id, offset }
}
}
Expand Down Expand Up @@ -174,9 +174,11 @@ pub struct Context<F: ScalarField> {
/// The challenge phase that this [Context] will map to.
#[getset(get_copy = "pub")]
phase: usize,
/// Identifier for what virtual region this context is in
/// Identifier for what virtual region this context is in.
/// Warning: the circuit writer must ensure that distinct virtual regions have distinct names as strings to prevent possible errors.
/// We do not use [std::any::TypeId] because it is not stable across rust builds or dependencies.
#[getset(get_copy = "pub")]
type_id: TypeId,
type_id: &'static str,
/// Identifier to reference cells from this [Context].
context_id: usize,

Expand Down Expand Up @@ -204,7 +206,7 @@ impl<F: ScalarField> Context<F> {
pub fn new(
witness_gen_only: bool,
phase: usize,
type_id: TypeId,
type_id: &'static str,
context_id: usize,
copy_manager: SharedCopyConstraintManager<F>,
) -> Self {
Expand Down
3 changes: 1 addition & 2 deletions halo2-base/src/virtual_region/copy_constraints.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::any::TypeId;
use std::collections::{BTreeMap, HashMap};
use std::ops::DerefMut;
use std::sync::{Arc, Mutex, OnceLock};
Expand Down Expand Up @@ -87,7 +86,7 @@ impl<F: Field + Ord> CopyConstraintManager<F> {
}

fn load_external_cell_impl(&mut self, cell: Option<Cell>) -> ContextCell {
let context_cell = ContextCell::new(TypeId::of::<Cell>(), 0, self.external_cell_count);
let context_cell = ContextCell::new("External Raw Halo2 Cell", 0, self.external_cell_count);
self.external_cell_count += 1;
if let Some(cell) = cell {
self.assigned_advices.insert(context_cell, cell);
Expand Down
Loading