Skip to content

Commit

Permalink
fix: TypeId in ContextTag not stable across builds (#217)
Browse files Browse the repository at this point in the history
* fix: use &str instead of TypeId in ContextTag

* chore: add warning to readme

* chore: fix comment
  • Loading branch information
jonathanpwang authored Nov 13, 2023
1 parent 67e3a0e commit 70d6d8a
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 21 deletions.
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

0 comments on commit 70d6d8a

Please sign in to comment.