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

Add no_floating_points feature flag. #747

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

Conversation

mpapierski
Copy link

@mpapierski mpapierski commented Mar 8, 2023

We're exploring the usage of halo2 in the context of ACTUS standard implementation and on-chain proof verification. Since our chain, Casper, doesn't support floats we needed these changes to do it. These changes may be useful more broadly since many other blockchains do not support floating points. This contribution should make the halo2_proofs code more universal across different operating systems and architectures.

With this flag turned on, the halo2_proofs crate can be compiled into Wasm, and the resulting binary will not use any floating point operations.

The code to generate the table is here https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cb4b6564b60d0ad65243679f3203fa77 - it's using a binary search to find the lowest argument to the ceil(ln(x)) function in 1..2^64-1 space, that produces the exponent it's looking for. The linked code and this commit contain relevant tests to ensure the lookup table is correct.

With this flag turned on, the halo2_proofs crate can be compiled into
Wasm, and the resulting binary will not use any floating point
operations.
@@ -115,6 +116,67 @@ fn multiexp_serial<C: CurveAffine>(coeffs: &[C::Scalar], bases: &[C], acc: &mut
}
}

#[cfg(feature = "no_floating_points")]
const LN_CEIL_LOOKUP_TABLE: &[usize] = &[
Copy link
Contributor

Choose a reason for hiding this comment

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

We can definitely simplify this lookup table:

  • It can resolve down to c = 0, but that is not necessary because we only use it with bases.len() >= 32. So we can drop the first 5 rows of the table (and change idx.saturating_sub(1) to idx + 4).
  • It can resolve up to c = 45, but is not necessary for it to be nearly this large, because the C type will be at least 64 bytes in memory (it's an affine curve point, so for a 255-bit base field like Pallas or Vesta you'll have two 32-byte field elements for coordinates). If bases.len() > 86593400423994057 in this table, then a 64-bit memory address space will likely be exhausted; in practice for machines with 16 GiB of RAM, bases.len >= 134217728 would consume it in entirety. If we set a maximum likely memory usage of 1 TiB for curves with 255-bit base fields, then we can drop the last 22 rows of the table.

The above would reduce the table size from 46 rows to 19. But we should also consider whether we need to use / emulate f64::ln here at all, or whether we can instead use f64::log2. If that would work, then we could replace all of this (for all targets) with usize::next_power_of_two and usize::trailing_zeros, using something like:

    let num_bases = bases.len();
    let c = if num_bases < 4 {
        1
    } else if num_bases < 32 {
        3
    } else {
        num_bases.next_power_of_two().trailing_zeros() - 2 // or whatever adjustment makes sense
    }

@ebfull was there any specific reason for using f64::ln here?

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.

2 participants