Skip to content

Commit

Permalink
fix libsnark build with optimisations on gcc/g++ 11.x
Browse files Browse the repository at this point in the history
It always was default behavior to build KomodoOcean with `-DUSE_ASM=1`
which is set by default through `config/bitcoin-config.h`. This flag
used not only by `libsnark`, but also by the hardrware SHA256 optimisations
in other sources, etc. However, it turns out that on g++ 11.3.0 enabled
optimisations for libsnark makes ibzcash::PHGRProof (SproutProofVerifier)
not working, and as a result when users tried to sync old chains which
had `sprout` transactions in history, like `SUPERNET`, the verification
of valid transactions throw an error:

```
2023-07-11 15:01:07 ERROR: CheckTransaction(): joinsplit does not verify
2023-07-11 15:01:07 ERROR: CheckBlock: CheckTransaction failed
```

While on gcc/g++ 10.x there was no such error. That's why we decided to
disable `libsnark` optimisations at all, by "shadowing" `USE_ASM` in
internal libnark implementation and rename it to `USE_ASM_SNARK`, to
avoid build with ASM optimisations, which are "unstable" on modern compilers.

More details here:

KomodoPlatform/komodo#591
  • Loading branch information
DeckerSU committed Jul 11, 2023
1 parent be4333c commit 197475f
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/snark/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ CURVE = BN128
OPTFLAGS = -O2 -march=x86-64 -g -mtune=x86-64

ifneq ($(PLATFORM),darwin)
FEATUREFLAGS = -DUSE_ASM -DMONTGOMERY_OUTPUT
FEATUREFLAGS = -DUSE_ASM_SNARK -DMONTGOMERY_OUTPUT
else
FEATUREFLAGS = -DUSE_ASM -DMONTGOMERY_OUTPUT -D__SIZE_TYPE__="unsigned long long"
FEATUREFLAGS = -DUSE_ASM_SNARK -DMONTGOMERY_OUTPUT -D__SIZE_TYPE__="unsigned long long"
endif

# Initialize this using "CXXFLAGS=... make". The makefile appends to that.
Expand Down
4 changes: 2 additions & 2 deletions src/snark/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ The following flags change the behavior of the compiled code.
of the corresponding algebraic objects. This option works for all
curves except bn128.

* define `USE_ASM` (on by default)
* define `USE_ASM_SNARK` (on by default)

Use unrolled assembly routines for F[p] arithmetic and faster heap in
multi-exponentiation. (When not set, use GMP's `mpn_*` routines instead.)
Expand Down Expand Up @@ -472,7 +472,7 @@ with respect to portability. Specifically:
tested with g++ 4.7, g++ 4.8, and clang 3.4.

6. On x86-64, we by default use highly optimized assembly implementations for some
operations (see `USE_ASM` above). On other architectures we fall back to a
operations (see `USE_ASM_SNARK` above). On other architectures we fall back to a
portable C++ implementation, which is slower.

Tested configurations include:
Expand Down
8 changes: 4 additions & 4 deletions src/snark/libsnark/algebra/fields/fp.tcc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ template<mp_size_t n, const bigint<n>& modulus>
void Fp_model<n,modulus>::mul_reduce(const bigint<n> &other)
{
/* stupid pre-processor tricks; beware */
#if defined(__x86_64__) && defined(USE_ASM)
#if defined(__x86_64__) && defined(USE_ASM_SNARK)
if (n == 3)
{ // Use asm-optimized Comba multiplication and reduction
mp_limb_t res[2*n];
Expand Down Expand Up @@ -293,7 +293,7 @@ Fp_model<n,modulus>& Fp_model<n,modulus>::operator+=(const Fp_model<n,modulus>&
#ifdef PROFILE_OP_COUNTS
this->add_cnt++;
#endif
#if defined(__x86_64__) && defined(USE_ASM)
#if defined(__x86_64__) && defined(USE_ASM_SNARK)
if (n == 3)
{
__asm__
Expand Down Expand Up @@ -406,7 +406,7 @@ Fp_model<n,modulus>& Fp_model<n,modulus>::operator-=(const Fp_model<n,modulus>&
#ifdef PROFILE_OP_COUNTS
this->sub_cnt++;
#endif
#if defined(__x86_64__) && defined(USE_ASM)
#if defined(__x86_64__) && defined(USE_ASM_SNARK)
if (n == 3)
{
__asm__
Expand Down Expand Up @@ -579,7 +579,7 @@ Fp_model<n,modulus> Fp_model<n,modulus>::squared() const
this->mul_cnt--; // zero out the upcoming mul
#endif
/* stupid pre-processor tricks; beware */
#if defined(__x86_64__) && defined(USE_ASM)
#if defined(__x86_64__) && defined(USE_ASM_SNARK)
if (n == 3)
{ // use asm-optimized Comba squaring
mp_limb_t res[2*n];
Expand Down
4 changes: 2 additions & 2 deletions src/snark/libsnark/algebra/fields/fp_aux.tcc
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/** @file
*****************************************************************************
Assembly code snippets for F[p] finite field arithmetic, used by fp.tcc .
Specific to x86-64, and used only if USE_ASM is defined.
On other architectures or without USE_ASM, fp.tcc uses a portable
Specific to x86-64, and used only if USE_ASM_SNARK is defined.
On other architectures or without USE_ASM_SNARK, fp.tcc uses a portable
C++ implementation instead.
*****************************************************************************
* @author This file is part of libsnark, developed by SCIPR Lab
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public:

bool operator<(const ordered_exponent<n> &other) const
{
#if defined(__x86_64__) && defined(USE_ASM)
#if defined(__x86_64__) && defined(USE_ASM_SNARK)
if (n == 3)
{
int64_t res;
Expand Down

0 comments on commit 197475f

Please sign in to comment.