Skip to content

Commit

Permalink
Merge pull request #1707 from AztecProtocol/defi-bridge-project
Browse files Browse the repository at this point in the history
* Attempt to fix production deployment (#1683)

* Add stefan to mainframe. Deployment publishes bb.js and blockchain to npm.

* Fix block scanning (#1692)

* Attempt to fix deploy.

* Fix dai-permit issues. Providers did not offset `v` properly (#1698)

* fix: perform offset to `sig.v if 0 or 1 for `signTypedData`

* fix: remove git-submodules

* Force to -O2 with binaryen installed to prevent aes bug.

* Updated fork block number (#1685)

* Updated fork block number

* Updated fork block and element test tranche

* Removed no longer used env variable

* Updated Faucet index page amounts

* [zk-money] Jcf/aave (#1642)

* Configure aave defi cards

* copy-paste error

* Increase test timeout

Co-authored-by: PhilWindle <[email protected]>

* fix aave token addresses (#1701)

* Fuzzer-found bugs that are not related to bigfield (#1605)

* Added a regression test to detect original parallelized asm bugs

* FIxed parallelized SQR asm bugs

* Added comment that Emerson wanted

* Fix for create_range_constraint in the constant case

* Various safe_uint bug fixes having to do with not handling constant cases

* Fixed the comparison issue for the point at infinity

* Added regression tests and fixed 3 more missing flags in optimized SQR

* Fixed tabulation in asm_macros.hpp

* Fixed Montgomery Issue that Adrian Found

* Fixed issue with negative zero found by Guido

* FIxed same self_neg bug found by Guido

* Renamed 1 test

* Fixed one more SQR bug

* Added extra regression tests

* Slightly moved one test

* Added TODO comments into field_impl for potential optimisations

* Addressed Zac's comments

Co-authored-by: zac-williamson <[email protected]>

* Fixing errors in addition and subtraction of field elements with moduli > 254 bits (#1702)

* Added test

* Added bugfixes

* Added comments

* Use 400k gas bridge for aave (#1705)

* Add versioning to falafel status endpoint. Refresh zk-money on version mismatch. (#1674)

* all get/post reqs from sdk to falafel now include  header

* falafel expects all endpoint requests to be given a 'version' header. SDK now gives all reqs a version. If falafel sees a req with version that does not match falafel version, it responds with error 402. ServerRollupProvider now emits versionMismatch whenever one occurs. zk-money promotes this event and forces refresh of browser.

* cleanup new FALAFEL_VERSION constant in falafel - using configurator everywhere instead of using constant directly

* fix prettier errors

* prettier server.ts

* sdk version to 0

* version is now a str like 2.1.0 in falafel and sdk. moved getRollupProviderStatus into SDK so that it doesn't need to accept sdk_version as arg

* prettier sdk index.ts

* zk-money alerts on version mismatch. Falafel allows requests without version header, but if version header is present it enforces a match.

* server rollup provider and block source shouldn't NEED a  to interact with falafel.

* prettier rollup provider ts

* block source / provider fix version default undefined changed to ''

Co-authored-by: joss-aztec <[email protected]>
Co-authored-by: Charlie Lye <[email protected]>
Co-authored-by: Lasse Herskind <[email protected]>
Co-authored-by: Innokentii Sennovskii <[email protected]>
Co-authored-by: zac-williamson <[email protected]>
Co-authored-by: David Banks <[email protected]>
  • Loading branch information
7 people authored Nov 8, 2022
2 parents 40476bb + 875bda3 commit 714da2b
Show file tree
Hide file tree
Showing 15 changed files with 180 additions and 38 deletions.
7 changes: 6 additions & 1 deletion src/aztec/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,15 @@ if(WASM)
$<TARGET_OBJECTS:rollup_proofs_claim_objects>
)

# With binaryen installed, it seems its wasm backend optimiser gets invoked automatically.
# Due to either a bug in the optimiser, or non-standards compliant c++ in crypto/aes, tests start failing with
# -O3 level optimisations. We force down to -O2 for current workaround.
# Presumably the -O3 when compiling the object files is fine as it's llvms IR optimiser.
# The backend optimiser is presumably triggered after linking.
target_link_options(
barretenberg.wasm
PRIVATE
-nostartfiles -Wl,--no-entry -Wl,--export-dynamic -Wl,--import-memory -Wl,--allow-undefined -Wl,--stack-first -Wl,-z,stack-size=1048576
-nostartfiles -O2 -Wl,--no-entry -Wl,--export-dynamic -Wl,--import-memory -Wl,--allow-undefined -Wl,--stack-first -Wl,-z,stack-size=1048576
)

endif()
26 changes: 26 additions & 0 deletions src/aztec/ecc/curves/bn254/fq.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,29 @@ TEST(fq, pow_regression_check)
EXPECT_EQ(zero.pow(uint256_t(0)), one);
}
// 438268ca91d42ad f1e7025a7b654e1f f8d9d72e0438b995 8c422ec208ac8a6e

TEST(fq, sqr_regression)
{
uint256_t values[] = { uint256_t(0xbdf876654b0ade1b, 0x2c3a66c64569f338, 0x2cd8bf2ec1fe55a3, 0x11c0ea9ee5693ede),
uint256_t(0x551b14ec34f2151c, 0x62e472ed83a2891e, 0xf208d5e5c9b5b3fb, 0x14315aeaf6027d8c),
uint256_t(0xad39959ae8013750, 0x7f1d2c709ab84cbb, 0x408028b80a60c2f1, 0x1dcd116fc26f856e),
uint256_t(0x95e967d30dcce9ce, 0x56139274241d2ea1, 0x85b19c1c616ec456, 0x1f1780cf9bf045b4),
uint256_t(0xbe841c861d8eb80e, 0xc5980d67a21386c0, 0x5fd1f1afecddeeb5, 0x24dbb8c1baea0250),
uint256_t(0x3ae4b3a27f05d6e3, 0xc5f6785b12df8d29, 0xc3a6c5f095103046, 0xd6b94cb2cc1fd4b),
uint256_t(0xc003c71932a6ced5, 0x6302a413f68e26e9, 0x2ed4a9b64d69fad, 0xfe61ffab1ae227d) };
for (auto& value : values) {
fq element(value);
EXPECT_EQ(element.sqr(), element * element);
}
}

TEST(fq, neg_and_self_neg_0_cmp_regression)
{
fq a = 0;
fq a_neg = -a;
EXPECT_EQ((a == a_neg), true);
a = 0;
a_neg = 0;
a_neg.self_neg();
EXPECT_EQ((a == a_neg), true);
}
12 changes: 12 additions & 0 deletions src/aztec/ecc/curves/secp256k1/secp256k1.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,4 +494,16 @@ TEST(secp256k1, derive_generators)
}
}
*/

TEST(secp256k1, neg_and_self_neg_0_cmp_regression)
{
secp256k1::fq a = 0;
secp256k1::fq a_neg = -a;
EXPECT_EQ((a == a_neg), true);
a = 0;
a_neg = 0;
a_neg.self_neg();
EXPECT_EQ((a == a_neg), true);
}

} // namespace test_secp256k1
20 changes: 20 additions & 0 deletions src/aztec/ecc/curves/secp256r1/secp256r1.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,26 @@ TEST(secp256r1, msb_bug_regression_check)
EXPECT_EQ(result, expected_result);
}

/**
* @brief We had an issue where we added field elements and subtracted a prime depending on the 2²⁵⁶ overflow. This
* was incorrect. Sometimes we need to subtract the prime twice. The same is true for subtractions
*
*/
TEST(secp256r1, addition_subtraction_regression_check)
{
secp256r1::fq fq1(uint256_t{ 0xfffffe0000000200, 0x200fffff9ff, 0xfffffbfffffffe00, 0xfffffbff00000400 });
secp256r1::fq fq2(uint256_t{ 0xfffffe0000000200, 0x200fffff9ff, 0xfffffbfffffffe00, 0xfffffbff00000400 });
secp256r1::fq fq3(0);
secp256r1::fq fq4(0);
fq1 += secp256r1::fq(secp256r1::fq::modulus_minus_two);
fq1 += secp256r1::fq(2);

fq3 -= fq1;
fq4 -= fq2;
EXPECT_EQ(fq1 + fq1, fq2 + fq2);
EXPECT_EQ(fq3, fq4);
}

/* TODO (#LARGE_MODULUS_AFFINE_POINT_COMPRESSION): Rewrite this test after designing point compression for p>2^255
TEST(secp256r1, derive_generators)
{
Expand Down
20 changes: 14 additions & 6 deletions src/aztec/ecc/fields/asm_macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
"mulxq %[modulus_1], %%rdi, %%rcx \n\t" /* (t[2], t[3]) <- (modulus[1] * k) */ \
"adcq %%rcx, %%r10 \n\t" /* r[2] += t[3] + flag_c */ \
"adcq $0, %%r11 \n\t" /* r[4] += flag_c */ \
/* Partial fix "adcq $0, %%r12 \n\t"*/ /* r[4] += flag_c */ \
/* Partial fix "adcq $0, %%r12 \n\t"*/ /* r[4] += flag_c */ \
"addq %%rdi, %%r9 \n\t" /* r[1] += t[2] */ \
"mulxq %[modulus_2], %%rdi, %%rcx \n\t" /* (t[0], t[1]) <- (modulus[3] * k) */ \
"mulxq %[modulus_3], %%r8, %%rdx \n\t" /* (t[2], t[3]) <- (modulus[2] * k) */ \
Expand Down Expand Up @@ -540,6 +540,7 @@
"adcxq %%rcx, %%r13 \n\t" /* r[5] += t[4] + flag_o */ \
"adoxq %[zero_reference], %%r13 \n\t" /* r[5] += flag_o */ \
"adcxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_c */ \
"adoxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_o */ \
\
/* double result registers */ \
"adoxq %%r9, %%r9 \n\t" /* r[1] = 2r[1] */ \
Expand Down Expand Up @@ -573,10 +574,12 @@
"mulxq %[modulus_0], %%rdi, %%rcx \n\t" /* (t[0], t[1]) <- (modulus[0] * k) */ \
"adoxq %%rdi, %%r8 \n\t" /* r[0] += t[0] (%r8 now free) */ \
"mulxq %[modulus_3], %%r8, %%rdi \n\t" /* (t[2], t[3]) <- (modulus[2] * k) */ \
"adcxq %%rdi, %%r12 \n\t" /* r[4] += t[3] + flag_o */ \
"adoxq %%rcx, %%r9 \n\t" /* r[1] += t[1] + flag_c */ \
"adcxq %[zero_reference], %%r13 \n\t" /* r[5] += flag_o */ \
"adcxq %%rdi, %%r12 \n\t" /* r[4] += t[3] + flag_c */ \
"adoxq %%rcx, %%r9 \n\t" /* r[1] += t[1] + flag_o */ \
"adcxq %[zero_reference], %%r13 \n\t" /* r[5] += flag_c */ \
"adcxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_c */ \
"mulxq %[modulus_1], %%rdi, %%rcx \n\t" /* (t[2], t[3]) <- (modulus[1] * k) */ \
"adcxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_c */ \
"adoxq %%rcx, %%r10 \n\t" /* r[2] += t[3] + flag_o */ \
"adcxq %%rdi, %%r9 \n\t" /* r[1] += t[2] */ \
"adoxq %%r8, %%r11 \n\t" /* r[3] += t[2] + flag_o */ \
Expand All @@ -594,6 +597,9 @@
"adoxq %%rcx, %%r13 \n\t" /* r[5] += t[3] + flag_o */ \
"adcxq %[zero_reference], %%r13 \n\t" /* r[5] += flag_c */ \
"adoxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_o */ \
"adcxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_c */ \
"adoxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_o */ \
"adcxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_c */ \
"mulxq %[modulus_0], %%r8, %%rcx \n\t" /* (t[0], t[1]) <- (modulus[0] * k) */ \
"adcxq %%r8, %%r9 \n\t" /* r[1] += t[0] (%r9 now free) */ \
"adoxq %%rcx, %%r10 \n\t" /* r[2] += t[1] + flag_c */ \
Expand All @@ -614,12 +620,14 @@
"adcxq %%r8, %%r13 \n\t" /* r[5] += t[2] + flag_c */ \
"adoxq %%r9, %%r14 \n\t" /* r[6] += t[3] + flag_c */ \
"adcxq %[zero_reference], %%r14 \n\t" /* r[6] += flag_o */ \
"adoxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_c */ \
"adoxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_o */ \
"adcxq %[zero_reference], %%r15 \n\t" /* r[7] += flag_c */ \
"mulxq %[modulus_0], %%r8, %%r9 \n\t" /* (t[0], t[1]) <- (modulus[0] * k) */ \
"adcxq %%r8, %%r10 \n\t" /* r[2] += t[0] (%r10 now free) */ \
"adoxq %%r9, %%r11 \n\t" /* r[3] += t[1] + flag_c */ \
"adcxq %%rdi, %%r11 \n\t" /* r[3] += t[2] */ \
"adoxq %[zero_reference], %%r12 \n\t" /* r[4] += flag_c */ \
"adoxq %[zero_reference], %%r12 \n\t" /* r[4] += flag_o */ \
"adoxq %[zero_reference], %%r13 \n\t" /* r[5] += flag_o */ \
\
/* perform modular reduction: r[3] */ \
"movq %%r11, %%rdx \n\t" /* move r11 into %rdx */ \
Expand Down
1 change: 1 addition & 0 deletions src/aztec/ecc/fields/field.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
namespace barretenberg {
template <class Params> struct alignas(32) field {
public:
// We don't initialize data by default since we'd lose a lot of time on pointless initializations.
field() noexcept {}

constexpr field(const uint256_t& input) noexcept
Expand Down
33 changes: 28 additions & 5 deletions src/aztec/ecc/fields/field_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,25 @@ template <class T> constexpr field<T> field<T>::operator-() const noexcept
constexpr field p{ modulus.data[0], modulus.data[1], modulus.data[2], modulus.data[3] };
return p - *this; // modulus - *this;
}

// TODO: there are 3 ways we can make this more efficient
// 1: we subtract `p` from `*this` instead of `2p`
// 2: instead of `p - *this`, we use an asm block that does `p - *this` without the assembly reduction step
// 3: we replace `(p - *this).reduce_once()` with an assembly block that is equivalent to `p - *this`,
// but we call `REDUCE_FIELD_ELEMENT` with `not_twice_modulus` instead of `twice_modulus`
// not sure which is faster and whether any of the above might break something!
//
// More context below:
// the operator-(a, b) method's asm implementation has a sneaky was to check underflow.
// if `a - b` underflows we need to add in `2p`. Instead of conditional branching which would cause pipeline
// flushes, we add `2p` into the result of `a - b`. If the result triggers the overflow flag, then we know we are
// correcting an *underflow* produced from computing `a - b`. Finally...we use the overflow flag to conditionally
// move data into registers such that we end up with either `a - b` or `2p + (a - b)` (this is branchless). OK! So
// what's the problem? Well we assume that every field element lies between 0 and 2p - 1. But we are computing `2p -
// *this`! If *this = 0 then we exceed this bound hence the need for the extra reduction step. HOWEVER, we also know
// that 2p - *this won't underflow so we could skip the underflow check present in the assembly code
constexpr field p{ twice_modulus.data[0], twice_modulus.data[1], twice_modulus.data[2], twice_modulus.data[3] };
return p - *this; // modulus - *this;
return (p - *this).reduce_once(); // modulus - *this;
}

template <class T> constexpr field<T> field<T>::operator-=(const field& other) noexcept
Expand All @@ -179,7 +196,7 @@ template <class T> constexpr void field<T>::self_neg() noexcept
*this = p - *this;
} else {
constexpr field p{ twice_modulus.data[0], twice_modulus.data[1], twice_modulus.data[2], twice_modulus.data[3] };
*this = p - *this;
*this = (p - *this).reduce_once();
}
}

Expand Down Expand Up @@ -236,9 +253,15 @@ template <class T> constexpr field<T> field<T>::to_montgomery_form() const noexc
constexpr field r_squared{ T::r_squared_0, T::r_squared_1, T::r_squared_2, T::r_squared_3 };

field result = *this;
result.reduce_once();
result.reduce_once();
result.reduce_once();
// TODO: are these reductions needed?
// Rationale: We want to take any 256-bit input and be able to convert into montgomery form.
// A basic heuristic we use is that any input into the `*` operator must be between [0, 2p - 1]
// to prevent overflows in the asm algorithm.
// However... r_squared is already reduced so perhaps we can relax this requirement?
// (would be good to identify a failure case where not calling self_reduce triggers an error)
result.self_reduce_once();
result.self_reduce_once();
result.self_reduce_once();
return (result * r_squared).reduce_once();
}

Expand Down
21 changes: 19 additions & 2 deletions src/aztec/ecc/fields/field_impl_generic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,15 @@ template <class T> constexpr field<T> field<T>::add(const field& other) const no
r1 = sbb(r1, modulus.data[1], b, b);
r2 = sbb(r2, modulus.data[2], b, b);
r3 = sbb(r3, modulus.data[3], b, b);
// Since both values are in [0, 2**256), the result is in [0, 2**257-2]. Subtracting one p might not be
// enough. We need to ensure that we've underflown the 0 and that might require subtracting an additional p
if (!b) {
b = 0;
r0 = sbb(r0, modulus.data[0], b, b);
r1 = sbb(r1, modulus.data[1], b, b);
r2 = sbb(r2, modulus.data[2], b, b);
r3 = sbb(r3, modulus.data[3], b, b);
}
}
return { r0, r1, r2, r3 };
} else {
Expand Down Expand Up @@ -245,8 +254,16 @@ template <class T> constexpr field<T> field<T>::subtract(const field& other) con
uint64_t carry = r0 < (modulus.data[0] & borrow);
r1 = addc(r1, modulus.data[1] & borrow, carry, carry);
r2 = addc(r2, modulus.data[2] & borrow, carry, carry);
r3 += (modulus.data[3] & borrow) + carry;

r3 = addc(r3, (modulus.data[3] & borrow), carry, carry);
// The value being subtracted is in [0, 2**256), if we subtract 0 - 2*255 and then add p, the value will stay
// negative. If we are adding p, we need to check that we've overflown 2**256. If not, we should add p again
if (!carry) {
r0 += (modulus.data[0] & borrow);
uint64_t carry = r0 < (modulus.data[0] & borrow);
r1 = addc(r1, modulus.data[1] & borrow, carry, carry);
r2 = addc(r2, modulus.data[2] & borrow, carry, carry);
r3 = addc(r3, (modulus.data[3] & borrow), carry, carry);
}
return { r0, r1, r2, r3 };
}

Expand Down
13 changes: 12 additions & 1 deletion src/aztec/ecc/groups/affine_element.test.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <ecc/curves/bn254/g1.hpp>
#include <ecc/curves/secp256k1/secp256k1.hpp>
#include <common/test.hpp>
#include <fstream>
#include <common/serialize.hpp>
Expand Down Expand Up @@ -28,11 +29,21 @@ TEST(affine_element, read_write_buffer)

// Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie
// on the curve, depending on the y-coordinate.
TEST(affine_element, infinity_regression)
TEST(affine_element, infinity_equality_regression)
{
g1::affine_element P;
P.self_set_infinity();
g1::affine_element R(0, P.y);
ASSERT_FALSE(P == R);
}

// Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie
// on the curve, depending on the y-coordinate.
TEST(affine_element, infinity_ordering_regression)
{
secp256k1::g1::affine_element P(0, 1), Q(0, 1);

P.self_set_infinity();
EXPECT_NE(P < Q, Q < P);
}
} // namespace test_affine_element
9 changes: 9 additions & 0 deletions src/aztec/ecc/groups/affine_element_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,19 @@ constexpr bool affine_element<Fq, Fr, T>::operator==(const affine_element& other

/**
* Comparison operators (for std::sort)
*
* @details CAUTION!! Don't use this operator. It has no meaning other than for use by std::sort.
**/
template <class Fq, class Fr, class T>
constexpr bool affine_element<Fq, Fr, T>::operator>(const affine_element& other) const noexcept
{
// We are setting point at infinity to always be the lowest element
if (is_point_at_infinity()) {
return false;
} else if (other.is_point_at_infinity()) {
return true;
}

if (x > other.x) {
return true;
} else if (x == other.x && y > other.y) {
Expand Down
3 changes: 2 additions & 1 deletion src/aztec/numeric/uint256/uint256.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ class alignas(32) uint256_t {

uint64_t data[4];

constexpr std::pair<uint256_t, uint256_t> divmod(const uint256_t& b) const;

private:
constexpr std::pair<uint64_t, uint64_t> mul_wide(const uint64_t a, const uint64_t b) const;
constexpr std::pair<uint64_t, uint64_t> addc(const uint64_t a, const uint64_t b, const uint64_t carry_in) const;
Expand All @@ -162,7 +164,6 @@ class alignas(32) uint256_t {
const uint64_t b,
const uint64_t c,
const uint64_t carry_in) const;
constexpr std::pair<uint256_t, uint256_t> divmod(const uint256_t& b) const;
};

inline std::ostream& operator<<(std::ostream& os, uint256_t const& a)
Expand Down
13 changes: 7 additions & 6 deletions src/aztec/stdlib/primitives/field/field.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -575,14 +575,15 @@ void field_t<ComposerContext>::create_range_constraint(const size_t num_bits, st
{
if (num_bits == 0) {
assert_is_zero("0-bit range_constraint on non-zero field_t.");
}
if (is_constant()) {
ASSERT(uint256_t(get_value()).get_msb() < num_bits);
} else {
if constexpr (ComposerContext::type == waffle::ComposerType::PLOOKUP) {
context->decompose_into_default_range(normalize().get_witness_index(), num_bits, msg);
if (is_constant()) {
ASSERT(uint256_t(get_value()).get_msb() < num_bits);
} else {
context->decompose_into_base4_accumulators(normalize().get_witness_index(), num_bits, msg);
if constexpr (ComposerContext::type == waffle::ComposerType::PLOOKUP) {
context->decompose_into_default_range(normalize().get_witness_index(), num_bits, msg);
} else {
context->decompose_into_base4_accumulators(normalize().get_witness_index(), num_bits, msg);
}
}
}
}
Expand Down
Loading

0 comments on commit 714da2b

Please sign in to comment.