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

LEM eval completed #617

Merged
merged 47 commits into from
Aug 19, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
821bec9
Implement div
emmorais Aug 9, 2023
e77fd3d
add Op::Lt
arthurpaulino Aug 14, 2023
60df119
Merge branch 'lem-lt' into lem_div
arthurpaulino Aug 14, 2023
c810654
fix Op::Div parameters
arthurpaulino Aug 14, 2023
1eba1f3
Progress on u64
gabriel-barrett Aug 14, 2023
4f7915a
`Op::Lt` finished
gabriel-barrett Aug 14, 2023
9f707aa
slot allocation for is_diff_neg
arthurpaulino Aug 15, 2023
3aee9fa
slot allocation for hide/open
arthurpaulino Aug 15, 2023
29fc20f
Implement enforce_u64
emmorais Aug 15, 2023
110f719
`enforce_u64` -> `implies_u64`
gabriel-barrett Aug 15, 2023
184b13e
Fixed `implies_u64`
gabriel-barrett Aug 15, 2023
6a01555
Progress on eval
gabriel-barrett Aug 15, 2023
f1f9bda
Added equality operator
gabriel-barrett Aug 16, 2023
6148c7f
Fixed div namespace issue
gabriel-barrett Aug 16, 2023
130c0df
rename debug display function
arthurpaulino Aug 16, 2023
6eecd56
`Add` and `Sub` for u64 finished
gabriel-barrett Aug 16, 2023
d574786
Fixed tests
gabriel-barrett Aug 16, 2023
2e71c28
More progress on eval
gabriel-barrett Aug 16, 2023
e518043
Eval finished!
gabriel-barrett Aug 16, 2023
310f1eb
`div_rem` fixed
gabriel-barrett Aug 16, 2023
8868d12
solve conflicts
arthurpaulino Aug 16, 2023
b1f8584
fix nil instantiation in the step function
arthurpaulino Aug 16, 2023
7d6fe20
slight code improvements
arthurpaulino Aug 16, 2023
66e953d
suggested renamings
arthurpaulino Aug 17, 2023
357791e
error when dividing by zero; adding capacity to vector of bits
arthurpaulino Aug 17, 2023
2658f10
Op::Commit -> Op::Hide
arthurpaulino Aug 17, 2023
b408623
Tree -> Tuple
arthurpaulino Aug 17, 2023
dae4c01
Less confusing comment and numbers
gabriel-barrett Aug 17, 2023
bb9f6dc
fix comm
arthurpaulino Aug 18, 2023
e658090
bitwise_and -> truncate, to_bits_le -> to_bits_le_strict
gabriel-barrett Aug 18, 2023
b9c6063
Simplified `Trunc`
gabriel-barrett Aug 18, 2023
53334d4
Added assertions around `Trunc`
gabriel-barrett Aug 18, 2023
6f9d828
Include implie_u64 proptest
emmorais Aug 18, 2023
09a39fa
Rename unit tests
emmorais Aug 18, 2023
b98904b
solve conflicts
arthurpaulino Aug 18, 2023
30a08e4
Merge branch 'lem_div' of github.com:lurk-lab/lurk-rs into lem_div
arthurpaulino Aug 18, 2023
08dd708
remove cargo lock
arthurpaulino Aug 18, 2023
ef37caa
Rename linear to enforce_product_and_sum
emmorais Aug 18, 2023
6ca0102
code suggestions
arthurpaulino Aug 18, 2023
660bd7a
Merge branch 'lem_div' of github.com:lurk-lab/lurk-rs into lem_div
arthurpaulino Aug 18, 2023
e0d2f81
Fmt
emmorais Aug 18, 2023
c075c45
fmt
arthurpaulino Aug 18, 2023
457c5b6
Merge branch 'lem_div' of github.com:lurk-lab/lurk-rs into lem_div
arthurpaulino Aug 18, 2023
4a6fb1e
inline always -> inline
arthurpaulino Aug 18, 2023
99f16c3
Add docstring to implies_u64
emmorais Aug 18, 2023
fd984ff
Review docstrings
emmorais Aug 18, 2023
7a5a7d3
Remove obsolete test
emmorais Aug 18, 2023
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
87 changes: 76 additions & 11 deletions src/circuit/gadgets/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,30 +152,67 @@ pub(crate) fn add_to_lc<F: PrimeField, CS: ConstraintSystem<F>>(
Ok(v_lc)
}

// Enforce v is the bit decomposition of num, therefore we have that 0 <= num < 2ˆ(sizeof(v)).
pub(crate) fn enforce_pack<F: LurkField, CS: ConstraintSystem<F>>(
pub(crate) fn implies_u64<F: LurkField, CS: ConstraintSystem<F>>(
huitseeker marked this conversation as resolved.
Show resolved Hide resolved
mut cs: CS,
premise: &Boolean,
a: &AllocatedNum<F>,
) -> Result<(), SynthesisError> {
let mut a_u64 = a.get_value().and_then(|a| a.to_u64()).unwrap_or(0);

let mut bits: Vec<Boolean> = Vec::with_capacity(64);
for i in 0..64 {
let b = a_u64 & 1;
let b_bool = Boolean::Is(AllocatedBit::alloc(
&mut cs.namespace(|| format!("b.{i}")),
Some(b == 1),
)?);
bits.push(b_bool);

a_u64 /= 2;
}

// premise -> a = sum(bits)
implies_pack(
&mut cs.namespace(|| "u64 bit decomposition check"),
premise,
&bits,
a,
)?;

Ok(())
}

// If premise is true, enforce v is the bit decomposition of num, therefore we have that 0 <= num < 2ˆ(sizeof(v)).
pub(crate) fn implies_pack<F: LurkField, CS: ConstraintSystem<F>>(
mut cs: CS,
premise: &Boolean,
v: &[Boolean],
num: &AllocatedNum<F>,
) -> Result<(), SynthesisError> {
let mut coeff = F::ONE;

let mut v_lc = LinearCombination::<F>::zero();
let mut pack = LinearCombination::<F>::zero();
for b in v {
v_lc = add_to_lc::<F, CS>(b, v_lc, coeff)?;
pack = add_to_lc::<F, CS>(b, pack, coeff)?;
coeff = coeff.double();
}
let diff = |_| pack - num.get_variable();
let premise_lc = |_| premise.lc(CS::one(), F::ONE);
let zero = |lc| lc;

cs.enforce(
|| "pack",
|_| v_lc,
|lc| lc + CS::one(),
|lc| lc + num.get_variable(),
);
cs.enforce(|| "pack", diff, premise_lc, zero);
Comment on lines +198 to +202
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because this logic is tricky and we already abstract it elsewhere for that purpose, I think it would be better to isolate implementation there. Concretely, I think you can use enforce_implication_lc_zero(diff) here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We will also eventually want an allocating version of this (allocating a bit) so we can do erroring type checks. It doesn't have to happen now, but in the interest of thinking about how we want to normalize decomposition of these gadgets, I would suggest that this has an is_pack (maybe there is a better name) part, which is a predicate that returns a linear combination.

Then from that we can create an allocated predicated, an 'implies' function, and an 'enforce' function. Then we could just define our predicates and derive the rest with a proc macro (or more manually, but in a transparent procedural way).


Ok(())
}

// Enforce v is the bit decomposition of num, therefore we have that 0 <= num < 2ˆ(sizeof(v)).
pub(crate) fn enforce_pack<F: LurkField, CS: ConstraintSystem<F>>(
cs: CS,
v: &[Boolean],
num: &AllocatedNum<F>,
) -> Result<(), SynthesisError> {
implies_pack(cs, &Boolean::Constant(true), v, num)
}

/// Adds a constraint to CS, enforcing a difference relationship between the allocated numbers a, b, and difference.
///
/// a - b = difference
Expand Down Expand Up @@ -1282,4 +1319,32 @@ mod tests {

assert!(cs.is_satisfied());
}

#[test]
fn test_enforce_u64() {
Copy link
Member

Choose a reason for hiding this comment

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

This should be renamed to match what it's testing.

let mut cs = TestConstraintSystem::<Fr>::new();

let alloc_num = AllocatedNum::alloc(&mut cs.namespace(|| "num"), || {
Ok(Fr::from_str_vartime("42").unwrap())
})
.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if not a full-on proptest, I would love to see this test made at least a little more convincing. For example, can we call it on 0, 1, and the largest U64?

Copy link
Member

Choose a reason for hiding this comment

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

    proptest!{
        #[test]
        fn test_implies_u64(f in any::<FWrap<Fr>>()) {
            let mut cs = TestConstraintSystem::<Fr>::new();

            let num = AllocatedNum::alloc(cs.namespace(|| "num"), || Ok(f.0)).unwrap();

            let t = Boolean::Constant(true);
            implies_u64(&mut cs.namespace(|| "enforce u64"), &t, &num).unwrap();

            let f_u64_roundtrip: Fr = f.0.to_u64_unchecked().into();
            let was_u64 = f_u64_roundtrip == f.0;
            prop_assert_eq!(was_u64, cs.is_satisfied());
        }
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, just added it


let t = Boolean::Constant(true);
implies_u64(&mut cs.namespace(|| "enforce u64"), &t, &alloc_num).unwrap();
assert!(cs.is_satisfied());
}

#[test]
fn test_enforce_u64_negative() {
let mut cs = TestConstraintSystem::<Fr>::new();

let alloc_num = AllocatedNum::alloc(&mut cs.namespace(|| "num"), || {
Ok(Fr::from_str_vartime("18446744073709551616").unwrap())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also at least test this on 2^64?

Copy link
Contributor

Choose a reason for hiding this comment

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

added a comment to make this edge case clear

})
.unwrap();

let t = Boolean::Constant(true);
implies_u64(&mut cs.namespace(|| "enforce u64"), &t, &alloc_num).unwrap();
assert!(!cs.is_satisfied());
}
}
19 changes: 19 additions & 0 deletions src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,18 @@ pub trait LurkField: PrimeField + PrimeFieldBits {
Some(u64::from_le_bytes(byte_array))
}

/// Attempts to convert the field element to a u64
fn to_u128(&self) -> Option<u128> {
Copy link
Member

Choose a reason for hiding this comment

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

Just impl TryFrom<LurkField> for u128 should be more idiomatic.

Copy link
Member

Choose a reason for hiding this comment

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

This is following the pattern in the rest of the code. If anything, we can create an issue to rewrite all the functions at once

for x in &self.to_repr().as_ref()[16..] {
if *x != 0 {
return None;
}
}
let mut byte_array = [0u8; 16];
byte_array.copy_from_slice(&self.to_repr().as_ref()[0..16]);
Some(u128::from_le_bytes(byte_array))
Comment on lines +151 to +153
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could probably afford to replace these with calls to the 'unchecked' versions generally (not just in this new u128 version). Just an observation.

}

/// Converts the first 4 bytes of the field element to a u32
fn to_u32_unchecked(&self) -> u32 {
let mut byte_array = [0u8; 4];
Expand All @@ -155,6 +167,13 @@ pub trait LurkField: PrimeField + PrimeFieldBits {
u64::from_le_bytes(byte_array)
}

/// Converts the first 16 bytes of the field element to a u128
fn to_u128_unchecked(&self) -> u128 {
let mut byte_array = [0u8; 16];
byte_array.copy_from_slice(&self.to_repr().as_ref()[0..16]);
u128::from_le_bytes(byte_array)
}

/// Constructs a field element from a u64
fn from_u64(x: u64) -> Self {
x.into()
Expand Down
Loading
Loading