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

Implement Basic Bus #1566

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement Basic Bus #1566

wants to merge 1 commit into from

Conversation

onurinanc
Copy link
Collaborator

Related to the issue of implementing basic bus (#1497), I have implemented basic bus together with an example (permutation_via_bus.asm) as specified inside the issue.

Currently, test_data/std/bus_permutation_via_challenges.asm works as intended (To make it sound, stage(1) witness columns need to be exposed publicly and verifier needs to check such as out_z1 + out_z2 = 0) We can now check using RUST_LOG=trace and adding the final z1 and z2 is equal to 0.

However, test_data/std/bus_permutation_via_challenges_ext.asm is not working correctly as intended. This will be fixed with the following commits.

Comment on lines +15 to +18
let<T: Add + Mul + FromLiteral> fingerprint_with_id: T, T[], Fp2<T> -> Fp2<T> = |id, expr_array, alpha| fold(
expr_array,
from_base(id),
|sum_acc, el| add_ext(mul_ext(alpha, sum_acc), from_base(el))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let<T: Add + Mul + FromLiteral> fingerprint_with_id: T, T[], Fp2<T> -> Fp2<T> = |id, expr_array, alpha| fold(
expr_array,
from_base(id),
|sum_acc, el| add_ext(mul_ext(alpha, sum_acc), from_base(el))
let<T: Add + Mul + FromLiteral> fingerprint_with_id: T, T[], Fp2<T> -> Fp2<T> = |id, expr_array, alpha| fingerprint([id] + expr_array, alpha);

This should work and be equivalent, no?

Comment on lines +57 to +59
let (acc_1, acc_2) = unpack_ext(acc_ext);
let (expr_1, expr_2) = unpack_ext(update_expr);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let (acc_1, acc_2) = unpack_ext(acc_ext);
let (expr_1, expr_2) = unpack_ext(update_expr);

Doesn't seem to be used.

Comment on lines +85 to +105
let compute_next_z_receive: expr, expr, expr[], expr, Fp2<expr>, Fp2<expr>, Fp2<expr> -> fe[] = query |is_first, id, tuple, multiplicity, acc, alpha, beta| {
// Implemented as: folded = (beta - fingerprint(id, tuple...));
// `multiplicity / (beta - fingerprint(id, tuple...))` to `acc`
let folded = sub_ext(beta, fingerprint_with_id(id, tuple, alpha));
let folded_next = next_ext(folded);

let m_ext = from_base(multiplicity);
let m_ext_next = next_ext(m_ext);

let is_first_next = from_base(is_first');

// acc' = acc * (1 - is_first') - multiplicity / fingerprint_with_id(id, (a1', a2'))
let res = sub_ext(
mul_ext(eval_ext(acc), eval_ext(sub_ext(from_base(1), is_first_next))),
mul_ext(eval_ext(m_ext_next), inv_ext(eval_ext(folded_next)))
);

match res {
Fp2::Fp2(a0_fe, a1_fe) => [a0_fe, a1_fe]
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let compute_next_z_receive: expr, expr, expr[], expr, Fp2<expr>, Fp2<expr>, Fp2<expr> -> fe[] = query |is_first, id, tuple, multiplicity, acc, alpha, beta| {
// Implemented as: folded = (beta - fingerprint(id, tuple...));
// `multiplicity / (beta - fingerprint(id, tuple...))` to `acc`
let folded = sub_ext(beta, fingerprint_with_id(id, tuple, alpha));
let folded_next = next_ext(folded);
let m_ext = from_base(multiplicity);
let m_ext_next = next_ext(m_ext);
let is_first_next = from_base(is_first');
// acc' = acc * (1 - is_first') - multiplicity / fingerprint_with_id(id, (a1', a2'))
let res = sub_ext(
mul_ext(eval_ext(acc), eval_ext(sub_ext(from_base(1), is_first_next))),
mul_ext(eval_ext(m_ext_next), inv_ext(eval_ext(folded_next)))
);
match res {
Fp2::Fp2(a0_fe, a1_fe) => [a0_fe, a1_fe]
}
};
let compute_next_z_receive: expr, expr, expr[], expr, Fp2<expr>, Fp2<expr>, Fp2<expr> -> fe[] = query |is_first, id, tuple, multiplicity, acc, alpha, beta| compute_next_z_send(is_first, id, tuple, -multiplicity, acc, alpha, beta);

Right?

Comment on lines +72 to +78
let is_first_next = from_base(is_first');

// acc' = acc * (1 - is_first') + multiplicity / fingerprint_with_id(id, (a1', a2'))
let res = add_ext(
mul_ext(eval_ext(acc), eval_ext(sub_ext(from_base(1), is_first_next))),
mul_ext(eval_ext(m_ext_next), inv_ext(eval_ext(folded_next)))
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let is_first_next = from_base(is_first');
// acc' = acc * (1 - is_first') + multiplicity / fingerprint_with_id(id, (a1', a2'))
let res = add_ext(
mul_ext(eval_ext(acc), eval_ext(sub_ext(from_base(1), is_first_next))),
mul_ext(eval_ext(m_ext_next), inv_ext(eval_ext(folded_next)))
);
let is_first_next = eval(is_first');
let current_acc = if is_first_next == 1 { from_base(0) } else { eval_ext(acc) };
// acc' = current_acc + multiplicity / fingerprint_with_id(id, (a1', a2'))
let res = add_ext(
current_acc,
mul_ext(eval_ext(m_ext_next), inv_ext(eval_ext(folded_next)))
);

This fixes the extension test! Previously, the hint was never executed, because the argument acc was never known. What happens now is this:

  1. The hint is run on the last row. Because is_first' evaluates to 1, current_acc == from_base(0) and acc (which is unknown) never needs to be evaluated.
  2. This sets z_next in the last row, which sets z in the the first row.
  3. When evaluating the hint on the first row, is_first' evaluates to 0, so acc (a.k.a z) needs to be evaluated. But we've just set it, so it works.

In your previous code, you would have evaluated acc in step 1 and then multiply by 0. That is correct, but witgen is not smart enough to know that the when multiplying by 0 it doesn't need to evaluate the other factor.

Also, this code should be more efficient ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants