From a824a397bf539854fc67256df3f8c5054c0e34ab Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Mon, 23 Feb 2026 09:22:18 +0000 Subject: [PATCH 1/5] Inject version to correct binary --- barretenberg/cpp/bootstrap.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/bootstrap.sh b/barretenberg/cpp/bootstrap.sh index da73553be6e5..cbe57a1327df 100755 --- a/barretenberg/cpp/bootstrap.sh +++ b/barretenberg/cpp/bootstrap.sh @@ -71,7 +71,11 @@ function build_native { cache_upload barretenberg-$native_preset-$hash.zst build/{bin,lib} fi # Always inject version (even for cached binaries) to ensure correct version on release - inject_version build/bin/bb + if [ "${NATIVE_PRESET:-clang20}" == "clang20" ]; then + inject_version build/bin/bb + elif [ "${NATIVE_PRESET}" == "debug" ]; then + inject_version build-debug/bin/bb + fi if [ -f build/bin/bb-avm ]; then inject_version build/bin/bb-avm fi From 37a026d07c5e6a1dc6004f4afeccdb34d7395086 Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Mon, 23 Feb 2026 12:42:30 +0000 Subject: [PATCH 2/5] feat: Native ECDSA audit (#20658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### ๐Ÿงพ Audit Context Native ECDSA audit: test refactoring, deterministic nonce derivation according to RFC6979, small code refactoring. ### ๐Ÿ› ๏ธ Changes Made - Derivation of the nonce (ephemeral key) is now in accordance with RFC6979 - Testing refactored and extended - Added some Wycherproof testing infrastructure - Code refactoring to make it easier to read - Address TODOs about erasing data in HMAC ### โœ… Checklist - [X] Audited all methods of the relevant module/class - [X] Audited the interface of the module/class with other (relevant) components - [X] Documented existing functionality and any changes made (as per Doxygen requirements) - [X] Resolved and/or closed all issues/TODOs pertaining to the audited files - [ ] Confirmed and documented any security or other issues found (if applicable) - [X] Verified that tests cover all critical paths (and added tests if necessary) - [X] Updated audit tracking for the files audited (check the start of each file you audited) ### ๐Ÿ“Œ Notes for Reviewers --- .../audit_scopes/ecdsa_native_audit_scope.md | 51 --- .../audit_scopes/ecdsa_stdlib_audit_scope.md | 24 +- .../src/barretenberg/crypto/CMakeLists.txt | 1 + .../src/barretenberg/crypto/ecdsa/ecdsa.hpp | 49 ++- .../barretenberg/crypto/ecdsa/ecdsa.test.cpp | 365 +++++++++++++----- .../barretenberg/crypto/ecdsa/ecdsa_impl.hpp | 177 +++++---- .../crypto/ecdsa/ecdsa_tests_data.hpp | 136 +++++++ .../crypto/hashers/CMakeLists.txt | 1 + .../barretenberg/crypto/hashers/hashers.hpp | 12 +- .../crypto/hashers/hashers.test.cpp | 51 +++ .../cpp/src/barretenberg/crypto/hmac/hmac.hpp | 193 ++++++--- .../barretenberg/crypto/hmac/hmac.test.cpp | 79 +++- .../acir_format/avm2_recursion_constraint.hpp | 2 +- .../stdlib/encryption/ecdsa/ecdsa.test.cpp | 18 - .../encryption/ecdsa/ecdsa_tests_data.hpp | 22 ++ .../barretenberg/vm2/constraining/flavor.cpp | 2 +- .../barretenberg/vm2/constraining/flavor.hpp | 2 +- .../barretenberg/vm2/constraining/prover.cpp | 2 +- .../barretenberg/vm2/constraining/prover.hpp | 2 +- .../recursion/recursive_flavor.hpp | 2 +- .../recursion/recursive_verifier.cpp | 2 +- .../recursion/recursive_verifier.hpp | 2 +- .../vm2/constraining/verifier.cpp | 2 +- .../vm2/constraining/verifier.hpp | 2 +- .../vm2/dsl/avm2_recursion_constraint.cpp | 2 +- .../vm2/dsl/avm2_recursion_constraint.hpp | 2 +- 26 files changed, 902 insertions(+), 301 deletions(-) delete mode 100644 barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_native_audit_scope.md create mode 100644 barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_tests_data.hpp create mode 100644 barretenberg/cpp/src/barretenberg/crypto/hashers/CMakeLists.txt create mode 100644 barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.test.cpp diff --git a/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_native_audit_scope.md b/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_native_audit_scope.md deleted file mode 100644 index 49420277683c..000000000000 --- a/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_native_audit_scope.md +++ /dev/null @@ -1,51 +0,0 @@ -# ECDSA Audit Scope - -Repository: https://github.com/AztecProtocol/aztec-packages -Commit hash: 4a956ceb179c2fe855e4f1fd78f2594e7fc3f5ea - -### Files to audit - -#### Native implementation -1. ```barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp``` -2. ```barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp``` -3. ```barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.hpp``` -4. ```barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.hpp``` - -#### Tests -5. ```barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp``` - - Test vectors for ECDSA signature generation and verification -6. ```barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.test.cpp``` - - Test vectors for HMAC implementation - - - -### Summary of the module - -#### ECDSA -ECDSA (Elliptic Curve Digital Signature Algorithm) is a digital signature scheme based on elliptic curve cryptography. This implementation supports both secp256k1 and secp256r1 curves. - -The implementation provides: -- Key pair generation -- Signature creation (sign operation) -- Signature verification -- Support for multiple hash functions via hasher wrappers (SHA256, BLAKE2s, Keccak) -- Recovery of public key from signature (recover_public_key) - -Key components: -- `ecdsa_key_pair`: Contains private key and public key -- `ecdsa_signature`: Contains r, s components and recovery id (v) -- Template-based design supporting different curves (secp256k1, secp256r1) - -#### HMAC -HMAC (Hash-based Message Authentication Code) is a cryptographic authentication mechanism. It is used within ECDSA for deterministic nonce generation (RFC 6979) to prevent vulnerabilities from weak random number generators. - -Key features: -- Implements HMAC-DRBG for deterministic k-value generation in ECDSA -- Template-based design supporting various hash functions - -#### Hashers -The hashers module provides uniform wrapper interfaces around different hash function implementations (SHA256, BLAKE2s, Keccak256). These wrappers allow ECDSA and other signature schemes to be templated on the hash function type, providing consistent `hash()` interfaces and metadata (BLOCK_SIZE, OUTPUT_SIZE) across different hash algorithms. - -### Documentation - -ECDSA specification: https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm diff --git a/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_stdlib_audit_scope.md b/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_stdlib_audit_scope.md index 1d4530fe25fa..80d14cade5c2 100644 --- a/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_stdlib_audit_scope.md +++ b/barretenberg/cpp/scripts/audit/audit_scopes/ecdsa_stdlib_audit_scope.md @@ -10,17 +10,39 @@ Commit hash: 05a381f8b31ae4648e480f1369e911b148216e8b 2. `dsl/acir_format/ecdsa_constraints.cpp` 3. `stdlib/encryption/ecdsa/ecdsa.hpp` 4. `stdlib/encryption/ecdsa/ecdsa_impl.hpp` +5. `crypto/ecdsa/ecdsa.hpp` +6. `crypto/ecdsa/ecdsa_impl.hpp` +7. `crypto/hmac/hmac.hpp` +8. `crypto/hashers/hashers.hpp` + ## Brief Summary of Module -The files above contain our implementation of the ECDSA verification algorithm and its exposure to Noir. For more details about the algorithm see the documentation in `stdlib/encryption/ecdsa/ecdsa_impl.hpp`, while for details about the usage via Noir see the documentation in `dsl/acir_format/ecdsa_constraints.hpp` and `dsl/acir_format/ecdsa_constraints.cpp` +The files 1. to 4. above contain our implementation of the ECDSA verification algorithm and its exposure to Noir. For more details about the algorithm see the documentation in `stdlib/encryption/ecdsa/ecdsa_impl.hpp`, while for details about the usage via Noir see the documentation in `dsl/acir_format/ecdsa_constraints.hpp` and `dsl/acir_format/ecdsa_constraints.cpp` Files 3. and 4. implement ECDSA verification, while 1. and 2. expose usage of the algorithm to Noir. +Files 5. to 6. contain our native implementation of: +- ECDSA signature algorithm +- ECDSA verification algorithm +- ECDSA public key recovery algorithm + +#### HMAC +HMAC (Hash-based Message Authentication Code) is a cryptographic authentication mechanism. It is used within ECDSA for deterministic nonce generation (RFC 6979) to prevent vulnerabilities from weak random number generators. + +File 7. contains our native implementation of HMAC and of deterministic nonce derivation following RFC6979, see links in the code. + +#### Hashers +The hashers module provides uniform wrapper interfaces around different hash function implementations (SHA256, BLAKE2s, Keccak256). These wrappers allow ECDSA and other signature schemes to be templated on the hash function type, providing consistent `hash()` interfaces and metadata (BLOCK_SIZE, OUTPUT_SIZE) across different hash algorithms. + + ## Test Files 1. `dsl/acir_format/ecdsa_constraints.test.cpp` 2. `stdlib/encryption/ecdsa/ecdsa.test.cpp` 3. `stdlib/encryption/ecdsa/ecdsa_tests_data.hpp` +4. `crypto/hmac/hmac.test.cpp` +5. `crypto/ecdsa/ecdsa.test.cpp` +5. `crypto/ecdsa/hashers.test.cpp` ## Security Mechanisms diff --git a/barretenberg/cpp/src/barretenberg/crypto/CMakeLists.txt b/barretenberg/cpp/src/barretenberg/crypto/CMakeLists.txt index a1f7703c0c5c..a2911b6b6876 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/CMakeLists.txt +++ b/barretenberg/cpp/src/barretenberg/crypto/CMakeLists.txt @@ -1,4 +1,5 @@ add_subdirectory(hmac) +add_subdirectory(hashers) add_subdirectory(blake2s) add_subdirectory(blake3s) add_subdirectory(generators) diff --git a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp index bcf031b2fde5..2d2446cadf14 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -15,9 +15,14 @@ #include namespace bb::crypto { + +static constexpr int ECDSA_RECOVERY_ID_OFFSET = 27; +static constexpr int ECDSA_R_FINITENESS_OFFSET = 2; + template struct ecdsa_key_pair { Fr private_key; - typename G1::affine_element public_key; + G1::affine_element public_key; + // For serialization, update with any new fields MSGPACK_FIELDS(private_key, public_key); }; @@ -26,22 +31,58 @@ struct ecdsa_signature { std::array r; std::array s; uint8_t v; + // For serialization, update with any new fields MSGPACK_FIELDS(r, s, v); }; +/** + * @brief Generate the ECDSA for the message using the provided account key pair and hash function + * + */ template ecdsa_signature ecdsa_construct_signature(const std::string& message, const ecdsa_key_pair& account); +/** + * @brief Given a message and a valid signature with recovery_id, recover the public key whose private key was used to + * sign the message + * + * @details Given \f$(m, (r, s))\f$ a valid signature for message m, we can recover up to four public keys for which + * this is a valid signature: a public key P is valid if \f$H(m) \cdot s^{-1} G + r \cdot s^{-1} P = \pm R\f$, where R + * is the point on the curve with x-coordinate equal to r. Given r, there can be two points R with that x-coordinate. + * Solving the equation for \f$P\f$, gives \f$P = r^{-1} \cdot s \cdot (- H(m) \cdot s^{-1} G + \pm R)$, which means we + * have up to four possibilities for \f$P\f$. We use a recovery_id \f$v\f$ to select which of the four public keys to + * get. The mapping is: recovery_id = offset (=27) + + * y is even && x < |Fr| --> 0 + * y is odd && x < |Fr| --> 1 + * y is even && |Fr| <= x < |Fq| --> 2 + * y is odd && |Fr| <= x < |Fq| --> 3 + * + * @note We enforce the signature to have low-s: meaning s < (Fr::modulus + 1) / 2. This means that if s is flipped, the + * effective nonce point R becomes -R, which has the opposite y-parity. We take this into account when generating the + * recovery_id. + */ template typename G1::affine_element ecdsa_recover_public_key(const std::string& message, const ecdsa_signature& sig); -// TODO(https://github.com/AztecProtocol/barretenberg/issues/659) +/** + * @brief Verify the ECDSA signature for the message using the provided public key and hash function + * + */ template bool ecdsa_verify_signature(const std::string& message, const typename G1::affine_element& public_key, const ecdsa_signature& signature); +/** + * @brief Hash the message and trim the hash output if required + * + * @details Following https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf, if the length of the hash output is + * longer than the bit length of the modulus of the scalar field, we trim the hash output. + * + */ +template Fr ecdsa_hash_message(const std::string& message); + inline bool operator==(ecdsa_signature const& lhs, ecdsa_signature const& rhs) { return lhs.r == rhs.r && lhs.s == rhs.s && lhs.v == rhs.v; @@ -55,4 +96,4 @@ inline std::ostream& operator<<(std::ostream& os, ecdsa_signature const& sig) } // namespace bb::crypto -#include "./ecdsa_impl.hpp" +#include "ecdsa_impl.hpp" diff --git a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp index cdf781b57a9a..b8b460f68343 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp @@ -1,7 +1,9 @@ #include "ecdsa.hpp" #include "barretenberg/common/serialize.hpp" #include "barretenberg/common/utils.hpp" +#include "barretenberg/crypto/ecdsa/ecdsa_tests_data.hpp" #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp" +#include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" #include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" #include "barretenberg/serialize/test_helper.hpp" #include @@ -9,136 +11,319 @@ using namespace bb; using namespace bb::crypto; -TEST(ecdsa, msgpack) +// Templated test fixture for ECDSA operations on different curves +template class EcdsaNativeTests : public ::testing::Test { + public: + using Curve = typename EcdsaTestParams::CurveType; + using Hasher = typename EcdsaTestParams::Hasher; + using Fr = typename Curve::ScalarField; + using Fq = typename Curve::BaseField; + using G1 = typename Curve::Group; + using AffineElement = typename Curve::AffineElement; + + // Generate a random keypair for the curve + static ecdsa_key_pair generate_keypair() + { + ecdsa_key_pair account; + account.private_key = Fr::random_element(); + account.public_key = G1::one * account.private_key; + return account; + } + + // Create a valid signature for the given message and account + static ecdsa_signature create_valid_signature(const std::string& message, const ecdsa_key_pair& account) + { + return ecdsa_construct_signature(message, account); + } + + // Verify a signature + static bool verify_signature(const std::string& message, + const AffineElement& public_key, + const ecdsa_signature& sig) + { + return ecdsa_verify_signature(message, public_key, sig); + } + + // Recover public key from signature (only works for curves with recovery support) + static AffineElement recover_public_key(const std::string& message, const ecdsa_signature& sig) + { + return ecdsa_recover_public_key(message, sig); + } + + // Fetch Wycherproof test cases for the curve + template + static auto get_wycheproof_test_cases() + requires(T::has_wycheproof_tests) + { + if constexpr (std::is_same_v) { + return secp256k1_tests; + } else if constexpr (std::is_same_v) { + return secp256r1_tests; + } + } +}; + +// Define curve wrapper structs to match the pattern +struct secp256k1_curve { + using Type = bb::curve::SECP256K1; + using ScalarField = secp256k1::fr; + using BaseField = secp256k1::fq; + using Group = secp256k1::g1; + using AffineElement = secp256k1::g1::affine_element; + static constexpr bool supports_recovery = true; + static constexpr bool has_wycheproof_tests = true; +}; + +struct secp256r1_curve { + using Type = bb::curve::SECP256R1; + using ScalarField = secp256r1::fr; + using BaseField = secp256r1::fq; + using Group = secp256r1::g1; + using AffineElement = secp256r1::g1::affine_element; + static constexpr bool supports_recovery = true; + static constexpr bool has_wycheproof_tests = true; +}; + +struct grumpkin_curve { + using ScalarField = grumpkin::fr; + using BaseField = grumpkin::fq; + using Group = grumpkin::g1; + using AffineElement = grumpkin::g1::affine_element; + static constexpr bool supports_recovery = false; + static constexpr bool has_wycheproof_tests = false; +}; + +template struct EcdsaTestParams { + public: + using CurveType = Curve; + using Hasher = Hasher_; +}; + +// Define the list of curve types to test +using Params = ::testing::Types, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams, + EcdsaTestParams>; + +// Register the test suite +TYPED_TEST_SUITE(EcdsaNativeTests, Params); + +// ================================================================================ +// POSITIVE TESTS: Valid signatures should pass verification +// ================================================================================ + +TYPED_TEST(EcdsaNativeTests, VerifyValidSignature) { - auto [actual, expected] = msgpack_roundtrip(ecdsa_signature{}); - EXPECT_EQ(actual, expected); + std::string message = "The quick brown dog jumped over the lazy fox."; + + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); + bool result = TestFixture::verify_signature(message, account.public_key, signature); + + EXPECT_TRUE(result); } -TEST(ecdsa, verify_signature_grumpkin_sha256) +TYPED_TEST(EcdsaNativeTests, RecoverPublicKey) { - std::string message = "The quick brown dog jumped over the lazy fox."; + using Curve = TypeParam::CurveType; - ecdsa_key_pair account; - account.private_key = grumpkin::fr::random_element(); - account.public_key = grumpkin::g1::one * account.private_key; + std::string message = "The quick brown dog jumped over the lazy fox."; - ecdsa_signature signature = - ecdsa_construct_signature(message, account); + if constexpr (Curve::supports_recovery) { + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); - bool result = ecdsa_verify_signature( - message, account.public_key, signature); + // Verify the signature is valid + bool result = TestFixture::verify_signature(message, account.public_key, signature); + EXPECT_TRUE(result); - EXPECT_EQ(result, true); + // Recover the public key and check it matches + auto recovered_public_key = TestFixture::recover_public_key(message, signature); + EXPECT_EQ(recovered_public_key, account.public_key); + } else { + GTEST_SKIP() << "Public key recovery not supported for this curve"; + } } -TEST(ecdsa, verify_signature_secp256r1_sha256) -{ - std::string message = "The quick brown dog jumped over the lazy fox."; +// ================================================================================ +// NEGATIVE TESTS: Invalid signatures should be rejected +// ================================================================================ - ecdsa_key_pair account; - account.private_key = secp256r1::fr::random_element(); - account.public_key = secp256r1::g1::one * account.private_key; +TYPED_TEST(EcdsaNativeTests, RejectZeroR) +{ + using serialize::write; - ecdsa_signature signature = - ecdsa_construct_signature(message, account); + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); - bool result = ecdsa_verify_signature( - message, account.public_key, signature); + // Set r = 0 + uint256_t zero_r = 0; + auto* r_ptr = &signature.r[0]; + write(r_ptr, zero_r); - EXPECT_EQ(result, true); + bool result = TestFixture::verify_signature(message, account.public_key, signature); + EXPECT_FALSE(result); } -TEST(ecdsa, recover_public_key_secp256k1_sha256) +TYPED_TEST(EcdsaNativeTests, RejectROverflowModulus) { - std::string message = "The quick brown dog jumped over the lazy fox."; + using serialize::read; + using serialize::write; + using Fr = typename TestFixture::Fr; + + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); + + // Set r = 1 + Fr::modulus (overflow) + uint256_t overflowing_r = uint256_t(1) + uint256_t(Fr::modulus); + auto* r_write_ptr = &signature.r[0]; + write(r_write_ptr, overflowing_r); - ecdsa_key_pair account; - account.private_key = secp256k1::fr::random_element(); - account.public_key = secp256k1::g1::one * account.private_key; + bool result = TestFixture::verify_signature(message, account.public_key, signature); + EXPECT_FALSE(result); +} - ecdsa_signature signature = - ecdsa_construct_signature(message, account); +TYPED_TEST(EcdsaNativeTests, RejectZeroS) +{ + using serialize::write; - bool result = ecdsa_verify_signature( - message, account.public_key, signature); + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); - auto recovered_public_key = - ecdsa_recover_public_key(message, signature); + // Set s = 0 + uint256_t zero_s = 0; + auto* s_ptr = &signature.s[0]; + write(s_ptr, zero_s); - EXPECT_EQ(result, true); - EXPECT_EQ(recovered_public_key, account.public_key); + bool result = TestFixture::verify_signature(message, account.public_key, signature); + EXPECT_FALSE(result); } -TEST(ecdsa, recover_public_key_secp256r1_sha256) +TYPED_TEST(EcdsaNativeTests, RejectHighS) { - std::string message = "The quick brown dog jumped over the lazy fox."; + using serialize::read; + using serialize::write; + using Fr = typename TestFixture::Fr; - ecdsa_key_pair account; - account.private_key = secp256r1::fr::random_element(); - account.public_key = secp256r1::g1::one * account.private_key; + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); - ecdsa_signature signature = - ecdsa_construct_signature(message, account); + // Set s to high s (should be rejected) + Fr s = Fr::serialize_from_buffer(&signature.s[0]); + Fr::serialize_to_buffer(-s, &signature.s[0]); - bool result = ecdsa_verify_signature( - message, account.public_key, signature); + bool result = TestFixture::verify_signature(message, account.public_key, signature); + EXPECT_FALSE(result); +} - auto recovered_public_key = - ecdsa_recover_public_key(message, signature); +TYPED_TEST(EcdsaNativeTests, RejectInvalidPublicKey) +{ + using Fq = typename TestFixture::Fq; + using AffineElement = typename TestFixture::AffineElement; - EXPECT_EQ(result, true); - EXPECT_EQ(recovered_public_key, account.public_key); + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); + + // Create a point not on the curve by taking a valid point and modifying y + AffineElement invalid_pubkey = account.public_key; + invalid_pubkey.y = invalid_pubkey.y + Fq::one(); + + bool result = TestFixture::verify_signature(message, invalid_pubkey, signature); + EXPECT_FALSE(result); } -TEST(ecdsa, check_overflowing_r_and_s_are_rejected) +TYPED_TEST(EcdsaNativeTests, RejectInfinityPublicKey) { + using AffineElement = typename TestFixture::AffineElement; - std::vector message_vec = utils::hex_to_bytes("41414141"); + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); - std::string message(message_vec.begin(), message_vec.end()); - ecdsa_signature signature; - grumpkin::fr private_key; - grumpkin::g1::affine_element public_key; - ecdsa_key_pair key_pair; - // We create a private and public key and a signature - private_key = grumpkin::fr::random_element(); - public_key = grumpkin::g1::affine_element((grumpkin::g1::one * private_key).normalize()); - key_pair = { private_key, public_key }; - signature = ecdsa_construct_signature(message, key_pair); - // Check that the signature is correct - bool result = - ecdsa_verify_signature(message, public_key, signature); - EXPECT_TRUE(result); - using serialize::read; + // Use point at infinity as public key + AffineElement infinity_pubkey = AffineElement::infinity(); - const auto* p_r = &signature.r[0]; - uint256_t old_r, new_r, old_s, new_s; - read(p_r, old_r); - new_r = old_r; - // We update r so it is larger than the modulus, but has the same value modulo fr::modulus - new_r = new_r + grumpkin::fr::modulus; - using serialize::write; - auto* p_r_m = &signature.r[0]; - write(p_r_m, new_r); - result = - ecdsa_verify_signature(message, public_key, signature); - // Signature verification should decline this signature, since it breaks specification + bool result = TestFixture::verify_signature(message, infinity_pubkey, signature); EXPECT_FALSE(result); - // Do the same for s, restore r - const auto* p_s = &signature.s[0]; - read(p_s, old_s); - new_s = old_s; - new_s = new_s + grumpkin::fr::modulus; - using serialize::write; - auto* p_r_s = &signature.s[0]; - write(p_r_m, old_r); - write(p_r_s, new_s); - result = - ecdsa_verify_signature(message, public_key, signature); +} + +TYPED_TEST(EcdsaNativeTests, RejectInfinityResult) +{ + using Fr = typename TestFixture::Fr; + using G1 = typename TestFixture::G1; + + std::string message = "Test message"; + auto account = TestFixture::generate_keypair(); + ecdsa_signature signature = TestFixture::create_valid_signature(message, account); + + // Compute H(m) + std::vector buffer; + std::ranges::copy(message, std::back_inserter(buffer)); + auto hash = Sha256Hasher::hash(buffer); + + // Override the public key: new public key is (-hash) * r^{-1} * G + Fr fr_hash = Fr::serialize_from_buffer(hash.data()); + Fr r = Fr::serialize_from_buffer(&signature.r[0]); + Fr r_inverse = r.invert(); + Fr modified_private_key = r_inverse * (-fr_hash); + account.public_key = G1::one * modified_private_key; + + // Verify that the result is the point at infinity + auto P = G1::one * fr_hash + account.public_key * r; + BB_ASSERT_EQ(P.is_point_at_infinity(), true); + + bool result = TestFixture::verify_signature(message, account.public_key, signature); EXPECT_FALSE(result); } +TYPED_TEST(EcdsaNativeTests, Wycherproof) +{ + using Curve = TypeParam::CurveType; + using AffineElement = TestFixture::AffineElement; + using Fr = TestFixture::Fr; + + if constexpr (Curve::has_wycheproof_tests) { + for (const auto& test_case : TestFixture::template get_wycheproof_test_cases()) { + std::string message_string(test_case.message.begin(), test_case.message.end()); + std::array r; + std::array s; + Fr::serialize_to_buffer(test_case.r, &r[0]); + Fr::serialize_to_buffer(test_case.s, &s[0]); + ecdsa_signature sig = { r, s, ECDSA_RECOVERY_ID_OFFSET }; + + bool is_signature_valid = ecdsa_verify_signature( + message_string, AffineElement(test_case.x, test_case.y), sig); + + EXPECT_EQ(is_signature_valid, test_case.is_valid_signature) << "Test case: " << test_case.comment; + } + } else { + GTEST_SKIP() << "Wycheproof tests not available for this curve"; + } +} + +// ================================================================================ +// STANDALONE TESTS: Non-templated tests for specific scenarios +// ================================================================================ + +TEST(ecdsa, msgpack) +{ + auto [actual, expected] = msgpack_roundtrip(ecdsa_signature{}); + EXPECT_EQ(actual, expected); +} + TEST(ecdsa, verify_signature_secp256r1_sha256_NIST_1) { /* diff --git a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp index 8e4f4f4f5d3f..9f41a87021bc 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp @@ -1,14 +1,17 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== #pragma once +#include + #include "../hmac/hmac.hpp" #include "barretenberg/common/serialize.hpp" #include "barretenberg/numeric/uint256/uint256.hpp" +#include "ecdsa.hpp" namespace bb::crypto { @@ -17,43 +20,47 @@ ecdsa_signature ecdsa_construct_signature(const std::string& message, const ecds { ecdsa_signature sig; - // use HMAC in PRF mode to derive 32-byte secret `k` + // Hash the message and convert it to an element of Fr + Fr z = ecdsa_hash_message(message); + + // Derived secret `k` using deterministic derivation according to RFC6979 std::vector pkey_buffer; write(pkey_buffer, account.private_key); - Fr k = crypto::get_unbiased_field_from_hmac(message, pkey_buffer); + Fr k = crypto::deterministic_nonce_rfc6979(message, pkey_buffer); + secure_erase(pkey_buffer); + // Compute R = k * G typename G1::affine_element R(G1::one * k); - Fq::serialize_to_buffer(R.x, &sig.r[0]); - std::vector message_buffer; - std::copy(message.begin(), message.end(), std::back_inserter(message_buffer)); - auto ev = Hash::hash(message_buffer); + // Compute the signature + Fr r = Fr(R.x); + Fr s = (z + r * account.private_key) / k; + secure_erase_bytes(&k, sizeof(k)); - Fr z = Fr::serialize_from_buffer(&ev[0]); - Fr r_fr = Fr::serialize_from_buffer(&sig.r[0]); - Fr s_fr = (z + r_fr * account.private_key) / k; + // Ensure that the value of s is "low", i.e. s := min{ s, (|Fr| - s) } + const bool is_s_low = (uint256_t(s) < (uint256_t(Fr::modulus) + 1) / 2); + s = is_s_low ? s : -s; - // Ensure that the value of s is "low", i.e. s := min{ s_fr, (|Fr| - s_fr) } - const bool is_s_low = (uint256_t(s_fr) < (uint256_t(Fr::modulus) / 2)); - uint256_t s_uint256 = is_s_low ? uint256_t(s_fr) : (uint256_t(Fr::modulus) - uint256_t(s_fr)); + Fr::serialize_to_buffer(r, &sig.r[0]); + Fr::serialize_to_buffer(s, &sig.s[0]); - Fr::serialize_to_buffer(Fr(s_uint256), &sig.s[0]); - - // compute recovery_id: given R = (x, y) + // Compute recovery_id: given R = (x, y) // 0: y is even && x < |Fr| // 1: y is odd && x < |Fr| // 2: y is even && |Fr| <= x < |Fq| // 3: y is odd && |Fr| <= x < |Fq| // v = offset + recovery_id - Fq r_fq = Fq(R.x); - bool is_r_finite = (uint256_t(r_fq) == uint256_t(r_fr)); + bool is_r_finite = (uint256_t(R.x) == uint256_t(r)); bool y_parity = uint256_t(R.y).get_bit(0); - bool recovery_bit = y_parity ^ is_s_low; - constexpr uint8_t offset = 27; + // When s is negated (low-s normalization), the effective nonce point becomes -R, + // which has the opposite y-parity. Flip y_parity accordingly. + bool recovery_id = is_s_low ? y_parity : !y_parity; - int value = offset + recovery_bit + static_cast(2) * !is_r_finite; + int value = is_r_finite ? ECDSA_RECOVERY_ID_OFFSET + recovery_id + : ECDSA_RECOVERY_ID_OFFSET + recovery_id + ECDSA_R_FINITENESS_OFFSET; BB_ASSERT_LTE(value, UINT8_MAX); sig.v = static_cast(value); + return sig; } @@ -61,7 +68,9 @@ template typename G1::affine_element ecdsa_recover_public_key(const std::string& message, const ecdsa_signature& sig) { using serialize::read; - using Point = G1::affine_element; + using AffineElement = G1::affine_element; + + // Read the signature components uint256_t r_uint; uint256_t s_uint; uint8_t v_uint; @@ -73,54 +82,46 @@ typename G1::affine_element ecdsa_recover_public_key(const std::string& message, read(r_buf, r_uint); read(s_buf, s_uint); read(v_buf, v_uint); - - // We need to check that r and s are in Field according to specification - BB_ASSERT((r_uint < mod) && (s_uint < mod), "r or s value exceeds the modulus"); - BB_ASSERT((r_uint != 0) && (s_uint != 0), "r or s value is zero"); - - // Check that the s value is less than |Fr| / 2 - BB_ASSERT_LTE(s_uint * 2, mod, "s value is not less than curve order by 2"); - - // Check that v must either be in {27, 28, 29, 30} Fr r = Fr(r_uint); Fr s = Fr(s_uint); - Fq r_fq = Fq(r_uint); - bool is_r_finite = true; - - if ((v_uint == 27) || (v_uint == 28)) { - BB_ASSERT_EQ(uint256_t(r), uint256_t(r_fq)); - } else if ((v_uint == 29) || (v_uint == 30)) { - BB_ASSERT_LT(uint256_t(r), uint256_t(r_fq)); - is_r_finite = false; - } else { - bb::assert_failure("v value is not in {27, 28, 29, 30}"); - } + + // Validate signature components according to specification + BB_ASSERT_LT(r_uint, mod, "r value exceeds the modulus"); + BB_ASSERT_LT(s_uint, mod, "s value exceeds the modulus"); + BB_ASSERT_LT(s_uint, (mod + 1) / 2, "s value is not less than curve order by 2"); + BB_ASSERT(r_uint != 0, "r value is zero"); + BB_ASSERT(s_uint != 0, "s value is zero"); + + // Check that v is in {27, 28, 29, 30} and then bring it back to the range {0, 1, 2, 3} + BB_ASSERT_GTE(v_uint, ECDSA_RECOVERY_ID_OFFSET, "v value is too small"); + BB_ASSERT_LTE(v_uint, ECDSA_RECOVERY_ID_OFFSET + 3, "v value is too large"); + bool is_r_finite = v_uint < ECDSA_RECOVERY_ID_OFFSET + 2; + v_uint -= ECDSA_RECOVERY_ID_OFFSET; // Decompress the x-coordinate r_uint to get two possible R points // The first uncompressed R point is selected when r < |Fr| // The second uncompressed R point is selected when |Fr| <= r < |Fq| - // Note that the second condition can occur with probability 1/2^128 so its highly unlikely. - auto uncompressed_points = Point::from_compressed_unsafe(r_uint); - Point point_R = uncompressed_points[!is_r_finite]; + // Note that the second condition occurs with very low probability, so it's highly unlikely. + auto uncompressed_points = AffineElement::from_compressed_unsafe(r_uint); + AffineElement point_R = uncompressed_points[is_r_finite ? 0 : 1]; // Negate the y-coordinate point of R based on the parity of v bool y_parity_R = uint256_t(point_R.y).get_bit(0); - if ((v_uint & 1) ^ y_parity_R) { + bool v_parity = v_uint & 1; + // Flip the sign of R.y if either v is even and R.y is odd, or v is odd and R.y is even + if ((v_parity && !y_parity_R) || (!v_parity && y_parity_R)) { point_R.y = -point_R.y; } // Start key recovery algorithm - std::vector message_buffer; - std::copy(message.begin(), message.end(), std::back_inserter(message_buffer)); - auto ev = Hash::hash(message_buffer); - Fr z = Fr::serialize_from_buffer(&ev[0]); + Fr z = ecdsa_hash_message(message); Fr r_inv = r.invert(); Fr u1 = -(z * r_inv); Fr u2 = s * r_inv; - Point recovered_public_key(Point(point_R) * u2 + G1::one * u1); + AffineElement recovered_public_key(point_R * u2 + G1::one * u1); return recovered_public_key; } @@ -130,45 +131,91 @@ bool ecdsa_verify_signature(const std::string& message, const ecdsa_signature& sig) { using serialize::read; - uint256_t r_uint; - uint256_t s_uint; - uint256_t mod = uint256_t(Fr::modulus); + using Element = G1::element; + using AffineElement = G1::affine_element; + + // Validate that the public key is on the curve and not the point at infinity if ((!public_key.on_curve()) || (public_key.is_point_at_infinity())) { return false; } + + // Deserialize the signature and validate the components + uint256_t r_uint; + uint256_t s_uint; + uint256_t mod = uint256_t(Fr::modulus); + const auto* r_buf = &sig.r[0]; const auto* s_buf = &sig.s[0]; read(r_buf, r_uint); read(s_buf, s_uint); + // We need to check that r and s are in Field according to specification - if ((r_uint >= mod) || (s_uint >= mod)) { + if (r_uint >= mod) { + vinfo("The r component of the signature exceeds the modulus of the scalar field of the curve."); + return false; + } + if (s_uint >= (mod + 1) / 2) { + vinfo("The s component of the signature exceeds (modulus + 1) / 2."); return false; } - if ((r_uint == 0) || (s_uint == 0)) { + if (r_uint == 0) { + vinfo("The r component of the signature is zero."); + return false; + } + if (s_uint == 0) { + vinfo("The s component of the signature is zero."); return false; } - // Check that the s value is less than |Fr| / 2 - BB_ASSERT_LT(s_uint, (mod + 1) / 2, "s value is not less than curve order by 2"); + // Hash the message + Fr z = ecdsa_hash_message(message); + // Validate the signature Fr r = Fr(r_uint); Fr s = Fr(s_uint); - std::vector message_buffer; - std::copy(message.begin(), message.end(), std::back_inserter(message_buffer)); - auto ev = Hash::hash(message_buffer); - Fr z = Fr::serialize_from_buffer(&ev[0]); - Fr s_inv = s.invert(); Fr u1 = z * s_inv; Fr u2 = r * s_inv; - typename G1::affine_element R((typename G1::element(public_key) * u2) + (G1::one * u1)); - BB_ASSERT_EQ(R.is_point_at_infinity(), false, "Result of the scalar multiplication is the point at infinity."); + AffineElement R = (G1::one * u1) + (Element(public_key) * u2); + if (R.is_point_at_infinity()) { + vinfo("Result of the scalar multiplication is the point at infinity."); + return false; + } uint256_t Rx(R.x); Fr result(Rx); return result == r; } + +template Fr ecdsa_hash_message(const std::string& message) +{ + using serialize::read; + using serialize::write; + + static_assert(Hash::OUTPUT_SIZE <= 32, "Hash output size is too large for our implementation of ECDSA."); + + constexpr size_t MODULUS_BIT_LENGTH = Fr::modulus.get_msb() + 1; + + std::vector message_buffer; + std::ranges::copy(message, std::back_inserter(message_buffer)); + auto ev = Hash::hash(message_buffer); + + if (Hash::OUTPUT_SIZE * 8 > MODULUS_BIT_LENGTH) { + const uint8_t* ptr = ev.data(); + uint256_t ev_uint; + read(ptr, ev_uint); + + // Bit shift the hash output + ev_uint = ev_uint >> (Hash::OUTPUT_SIZE * 8 - MODULUS_BIT_LENGTH); + + // Write the output to ev + uint8_t* ptr_write = ev.data(); + write(ptr_write, ev_uint); + } + + return Fr::serialize_from_buffer(&ev[0]); +} } // namespace bb::crypto diff --git a/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_tests_data.hpp b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_tests_data.hpp new file mode 100644 index 000000000000..2f25212271f7 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_tests_data.hpp @@ -0,0 +1,136 @@ +#include +#include +#include +#include +#include + +#include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" +#include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" + +namespace bb::crypto { + +using namespace bb; +using namespace bb::curve; + +template struct WycherproofTest { + using Fr = Curve::ScalarField; + using Fq = Curve::BaseField; + + // Public Key + Fq x; + Fq y; + + // Data + std::vector message; + Fr r; + Fr s; + bool is_valid_signature; + std::string comment; +}; + +using WycherproofSecp256k1 = WycherproofTest; +using WycherproofSecp256r1 = WycherproofTest; + +/** + * @brief Test for Secp256k1 ECDSA signatures taken from the Wycherproof project + * + */ +const std::vector secp256k1_tests{ + // Arithmetic error tests + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0x02ef4d6d6cfd5a94f1d7784226e3e2a6c0a436c55839619f38fb4472b5f9ee77"), + .y = WycherproofSecp256k1::Fq("0x7eb4acd4eebda5cd72875ffd2a2f26229c2dc6b46500919a432c86739f3ae866"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256k1::Fr("0x0000000000000000000000000000000000000000000000000000000000000101"), + .s = WycherproofSecp256k1::Fr("0xc58b162c58b162c58b162c58b162c58a1b242973853e16db75c8a1a71da4d39d"), + .is_valid_signature = false, + .comment = "Arithmetic error, s is larger than (n+1)/2", + }, + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0xd6ef20be66c893f741a9bf90d9b74675d1c2a31296397acb3ef174fd0b300c65"), + .y = WycherproofSecp256k1::Fq("0x4a0c95478ca00399162d7f0f2dc89efdc2b28a30fbabe285857295a4b0c4e265"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256k1::Fr("0x00000000000000000000000000000000000000062522bbd3ecbe7c39e93e7c26"), + .s = WycherproofSecp256k1::Fr("0x783266e90f43dafe5cd9b3b0be86de22f9de83677d0f50713a468ec72fcf5d57"), + .is_valid_signature = true, + .comment = "Arithmetic error, r component is small", + }, + // Point duplication tests + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0x79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"), + .y = WycherproofSecp256k1::Fq("0x483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256k1::Fr("0xbb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023"), + .s = WycherproofSecp256k1::Fr("0x2492492492492492492492492492492463cfd66a190a6008891e0d81d49a0952"), + .is_valid_signature = false, + .comment = "Point duplication, public key shares x-coordinates with generator", + }, + // Edge case public key tests + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0x6e823555452914099182c6b2c1d6f0b5d28d50ccd005af2ce1bba541aa40caff"), + .y = WycherproofSecp256k1::Fq("0x00000001060492d5a5673e0f25d8d50fb7e58c49d86d46d4216955e0aa3d40e1"), + .message = { 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65 }, + .r = WycherproofSecp256k1::Fr("0x6d6a4f556ccce154e7fb9f19e76c3deca13d59cc2aeb4ecad968aab2ded45965"), + .s = WycherproofSecp256k1::Fr("0x53b9fa74803ede0fc4441bf683d56c564d3e274e09ccf47390badd1471c05fb7"), + .is_valid_signature = true, + .comment = "Edge case public key, y coordinate is small", + }, + // Modular inverse edge case + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0x9171fec3ca20806bc084f12f0760911b60990bd80e5b2a71ca03a048b20f837e"), + .y = WycherproofSecp256k1::Fq("0x634fd17863761b2958d2be4e149f8d3d7abbdc18be03f451ab6c17fa0a1f8330"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256k1::Fr("0x55555555555555555555555555555554e8e4f44ce51835693ff0ca2ef01215c1"), + .s = WycherproofSecp256k1::Fr("0x2736d76e412246e097148e2bf62915614eb7c428913a58eb5e9cd4674a9423de"), + .is_valid_signature = true, + .comment = "Modular inverse edge case", + }, +}; + +/** + * @brief Test for Secp256r1 ECDSA signatures taken from the Wycherproof project + * + */ +const std::vector secp256r1_tests{ + // Arithmetic error test + WycherproofSecp256r1{ + .x = WycherproofSecp256r1::Fq("0x8d3c2c2c3b765ba8289e6ac3812572a25bf75df62d87ab7330c3bdbad9ebfa5c"), + .y = WycherproofSecp256r1::Fq("0x4c6845442d66935b238578d43aec54f7caa1621d1af241d4632e0b780c423f5d"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256r1::Fr("0x6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296"), + .s = WycherproofSecp256r1::Fr("0x16a4502e2781e11ac82cbc9d1edd8c981584d13e18411e2f6e0478c34416e3bb"), + .is_valid_signature = true, + .comment = "Arithmetic error", + }, + // Point duplication test + WycherproofSecp256r1{ + .x = WycherproofSecp256r1::Fq("0x6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296"), + .y = WycherproofSecp256r1::Fq("0x4fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256r1::Fr("0xbb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023"), + .s = WycherproofSecp256r1::Fr("0x249249246db6db6ddb6db6db6db6db6dad4591868595a8ee6bf5f864ff7be0c2"), + .is_valid_signature = false, + .comment = "Point duplication, public key shares x-coordinates with generator", + }, + // Edge case public key test + WycherproofSecp256r1{ + .x = WycherproofSecp256r1::Fq("0x4f337ccfd67726a805e4f1600ae2849df3807eca117380239fbd816900000000"), + .y = WycherproofSecp256r1::Fq("0xed9dea124cc8c396416411e988c30f427eb504af43a3146cd5df7ea60666d685"), + .message = { 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65 }, + .r = WycherproofSecp256r1::Fr("0x0fe774355c04d060f76d79fd7a772e421463489221bf0a33add0be9b1979110b"), + .s = WycherproofSecp256r1::Fr("0x500dcba1c69a8fbd43fa4f57f743ce124ca8b91a1f325f3fac6181175df55737"), + .is_valid_signature = true, + .comment = "Edge case public key, x-coordinate has many trailing zeros", + }, + // Edge case public key test + WycherproofSecp256r1{ + .x = WycherproofSecp256r1::Fq("0x2927b10512bae3eddcfe467828128bad2903269919f7086069c8c4df6c732838"), + .y = WycherproofSecp256r1::Fq("0xc7787964eaac00e5921fb1498a60f4606766b3d9685001558d1a974e7341513e"), + .message = { 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65 }, + .r = WycherproofSecp256r1::Fr("0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551"), + .s = WycherproofSecp256r1::Fr("0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632550"), + .is_valid_signature = false, + .comment = "Signature with special case values r=n and s=n - 1", + }, +}; +} // namespace bb::crypto diff --git a/barretenberg/cpp/src/barretenberg/crypto/hashers/CMakeLists.txt b/barretenberg/cpp/src/barretenberg/crypto/hashers/CMakeLists.txt new file mode 100644 index 000000000000..751ff59b0943 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/crypto/hashers/CMakeLists.txt @@ -0,0 +1 @@ +barretenberg_module(crypto_hashers crypto_blake2s crypto_keccak crypto_sha256 numeric) diff --git a/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.hpp b/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.hpp index 6bf49d37394c..d6e0bb680624 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -19,12 +19,12 @@ struct KeccakHasher { static constexpr size_t OUTPUT_SIZE = 32; static std::vector hash(const std::vector& message) { - keccak256 hash_result = ethash_keccak256(&message[0], message.size()); + const uint8_t* ptr = message.empty() ? nullptr : message.data(); + keccak256 hash_result = ethash_keccak256(ptr, message.size()); - std::vector output; - output.resize(32); + std::vector output(32); - memcpy((void*)&output[0], (void*)&hash_result.word64s[0], 32); + memcpy(output.data(), &hash_result.word64s[0], output.size()); return output; } }; @@ -41,4 +41,4 @@ struct Blake2sHasher { static constexpr size_t OUTPUT_SIZE = 32; static auto hash(const std::vector& message) { return blake2s(message); } }; -} // namespace bb::crypto \ No newline at end of file +} // namespace bb::crypto diff --git a/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.test.cpp new file mode 100644 index 000000000000..aac8eb6eeb69 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/crypto/hashers/hashers.test.cpp @@ -0,0 +1,51 @@ +#include + +#include "../hashers/hashers.hpp" + +#include +#include +#include +#include + +using namespace bb; +using namespace bb::crypto; + +namespace { + +std::string bytes_to_hex_string(const std::vector& bytes) +{ + static constexpr std::array HEX_CHARS = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + std::string hex(bytes.size() * 2, '\0'); + for (size_t i = 0; i < bytes.size(); ++i) { + hex[2 * i] = HEX_CHARS[bytes[i] >> 4]; + hex[(2 * i) + 1] = HEX_CHARS[bytes[i] & 0x0f]; + } + return hex; +} + +} // namespace + +TEST(Sha256, EmptyHash) +{ + auto hash = Sha256Hasher::hash(std::vector{}); + std::vector hash_vec(hash.begin(), hash.end()); + auto hash_string = bytes_to_hex_string(hash_vec); + EXPECT_EQ(hash_string, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); +} + +TEST(Keccak, EmptyHash) +{ + auto hash = KeccakHasher::hash(std::vector{}); + std::vector hash_vec(hash.begin(), hash.end()); + auto hash_string = bytes_to_hex_string(hash_vec); + EXPECT_EQ(hash_string, "c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"); +} + +TEST(Blake2s, EmptyHash) +{ + auto hash = Blake2sHasher::hash(std::vector{}); + std::vector hash_vec(hash.begin(), hash.end()); + auto hash_string = bytes_to_hex_string(hash_vec); + EXPECT_EQ(hash_string, "69217a3079908094e11121d042354a7c1f55b6482ca1a51e1b250dfd1ed0eef9"); +} diff --git a/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.hpp b/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.hpp index 55b3847a18d6..f5ff50541df0 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -15,15 +15,35 @@ #include namespace bb::crypto { +inline void secure_erase_bytes(void* ptr, size_t size) +{ + volatile uint8_t* p = static_cast(ptr); + while (size-- > 0) { + *p++ = 0; + } +} + +template inline void secure_erase(std::array& buffer) +{ + secure_erase_bytes(buffer.data(), buffer.size() * sizeof(T)); +} + +template inline void secure_erase(std::vector& buffer) +{ + if (!buffer.empty()) { + secure_erase_bytes(buffer.data(), buffer.size() * sizeof(T)); + } +} + /** - * @brief Compute an HMAC given a secret key and a message + * @brief Compute an HMAC given a secret key and a message, see https://datatracker.ietf.org/doc/html/rfc2104 * * @tparam Hash hasher being used * @tparam MessageContainer a byte container (std::vector, std::array, std::string) * @tparam KeyContainer a byte container - * @param message the message! - * @param key the key! - * @return std::array the HMAC output! + * @param message the message + * @param key the key + * @return std::array the HMAC output */ template std::array hmac(const MessageContainer& message, const KeyContainer& key) @@ -40,96 +60,165 @@ std::array hmac(const MessageContainer& message, con // initialize k_prime to 0x00,...,0x00 // copy key or truncated key to start. - // TODO: securely erase `k_prime` std::array k_prime{}; if (key.size() > B) { - const auto truncated_key = Hash::hash(key); + std::vector key_buffer(key.begin(), key.end()); + auto truncated_key = Hash::hash(key_buffer); std::copy(truncated_key.begin(), truncated_key.end(), k_prime.begin()); + secure_erase(key_buffer); + secure_erase(truncated_key); } else { std::copy(key.begin(), key.end(), k_prime.begin()); } - // TODO: securely erase `h1` std::array h1; for (size_t i = 0; i < B; ++i) { h1[i] = k_prime[i] ^ opad[i]; } - // TODO: securely erase `h2` std::array h2; for (size_t i = 0; i < B; ++i) { h2[i] = k_prime[i] ^ ipad[i]; } + secure_erase(k_prime); - // TODO: securely erase copy of `h2` in `message_buffer`, - // ensure `message_buffer` is not re-allocated std::vector message_buffer; + message_buffer.reserve(B + message.size()); std::copy(h2.begin(), h2.end(), std::back_inserter(message_buffer)); std::copy(message.begin(), message.end(), std::back_inserter(message_buffer)); - const auto h3 = Hash::hash(message_buffer); + auto h3 = Hash::hash(message_buffer); + secure_erase(h2); + secure_erase(message_buffer); - // TODO: securely erase copy of `h1` in `hmac_buffer`, - // ensure `hmac_buffer` is not re-allocated std::vector hmac_buffer; + hmac_buffer.reserve(B + Hash::OUTPUT_SIZE); std::copy(h1.begin(), h1.end(), std::back_inserter(hmac_buffer)); std::copy(h3.begin(), h3.end(), std::back_inserter(hmac_buffer)); - const auto hmac_key = Hash::hash(hmac_buffer); + auto hmac_key = Hash::hash(hmac_buffer); + secure_erase(h1); + secure_erase(h3); + secure_erase(hmac_buffer); std::array result; std::copy(hmac_key.begin(), hmac_key.end(), result.begin()); + secure_erase(hmac_key); return result; } /** - * @brief Takes a size-HASH_OUTPUT buffer from HMAC and converts into a field element - * - * @details We assume HASH_OUTPUT = 32. Reducing HMAC(key, message) modulo r would result in an unacceptable bias. - * We hash input with `0` and `1` to produce 64 bytes of input data. This is then converted into a uin512_t, - * which is taken modulo Fr::modulus to produce our field element, where the statistical bias is negligble in - * the security parameter. + * @brief Deterministic nonce derivation according to RFC6979 specification + * (https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf, A.3.3) * * @tparam Hash the hash function we're using * @tparam Fr field type * @tparam MessageContainer a byte container (std::vector, std::array, std::string) * @tparam KeyContainer a byte container - * @param message the input buffer - * @param key key used to derive - * @return Fr output field element as uint512_t( H(10...0 || HMAC(k,m)) || H(00...0 || HMAC(k,m)) ) % r + * @param message the input buffer used to derive the nonce + * @param key key used to derive the nonce */ template -Fr get_unbiased_field_from_hmac(const MessageContainer& message, const KeyContainer& key) +Fr deterministic_nonce_rfc6979(const MessageContainer& message, const KeyContainer& key) requires(Hash::OUTPUT_SIZE == 32) { - // Strong assumption that works for now with our suite of Hashers - static_assert(Hash::BLOCK_SIZE > Hash::OUTPUT_SIZE); - constexpr size_t DOMAIN_SEPARATOR_SIZE = Hash::BLOCK_SIZE - Hash::OUTPUT_SIZE; - - // Domain separators whose size ensures we hash a block of the exact size expected by - // the Hasher. - constexpr std::array KLO_DOMAIN_SEPARATOR{ 0x0 }; - constexpr std::array KHI_DOMAIN_SEPARATOR{ 0x1 }; - - auto input = hmac(message, key); - - // klo = H(00...0 || input) - std::vector lo_buffer(KLO_DOMAIN_SEPARATOR.begin(), KLO_DOMAIN_SEPARATOR.end()); - std::copy(input.begin(), input.end(), std::back_inserter(lo_buffer)); - auto klo = Hash::hash(lo_buffer); - - // khi = H(10...0 || input) - std::vector hi_buffer(KHI_DOMAIN_SEPARATOR.begin(), KHI_DOMAIN_SEPARATOR.end()); - std::copy(input.begin(), input.end(), std::back_inserter(hi_buffer)); - auto khi = Hash::hash(hi_buffer); - - // full_buffer = khi || klo - std::vector full_buffer(khi.begin(), khi.end()); - std::copy(klo.begin(), klo.end(), std::back_inserter(full_buffer)); + using serialize::read; + using serialize::write; + + static_assert(Hash::OUTPUT_SIZE == 32, + "Hash output size must be 32 bytes for our implementation of RFC6979 nonce generation"); + static constexpr size_t INITIAL_BUFFER_SIZE = 32; // Equal to 8 * (Hash::OUTPUT_SIZE + 7/ 8) + static constexpr size_t MODULUS_BIT_LENGTH = Fr::modulus.get_msb() + 1; + + // Hash the message + std::vector message_buffer(message.begin(), message.end()); + auto hashed_message = Hash::hash(message_buffer); + secure_erase(message_buffer); + // Round trip reduces the hash modulo Fr::modulus + Fr hashed_message_fr = Fr::serialize_from_buffer(hashed_message.data()); + hashed_message = {}; + Fr::serialize_to_buffer(hashed_message_fr, &hashed_message[0]); + + // Concatenate the private key and the hashed message + std::vector seed_material; + seed_material.reserve(key.size() + hashed_message.size()); + std::ranges::copy(key, std::back_inserter(seed_material)); + std::ranges::copy(hashed_message, std::back_inserter(seed_material)); + secure_erase(hashed_message); + + // Initialize the buffers V and K + std::array v_buffer; + v_buffer.fill(0x01); + std::array key_buffer; + key_buffer.fill(0x00); + + // Temporary buffer for first HMAC round: V || 0x00 || seed_material + std::vector tmp_buffer; + tmp_buffer.reserve(INITIAL_BUFFER_SIZE + 1 + seed_material.size()); + std::ranges::copy(v_buffer, std::back_inserter(tmp_buffer)); + tmp_buffer.emplace_back(0x00); + std::ranges::copy(seed_material, std::back_inserter(tmp_buffer)); + + // First HMAC round: K = HMAC(K, V || 0x00 || seed_material), V = HMAC(K, V) + key_buffer = hmac, std::array>(tmp_buffer, key_buffer); + v_buffer = hmac, std::array>( + v_buffer, key_buffer); + + // Temporary buffer for second HMAC round: V || 0x01 || seed_material + tmp_buffer.clear(); + tmp_buffer.reserve(INITIAL_BUFFER_SIZE + 1 + seed_material.size()); + std::ranges::copy(v_buffer, std::back_inserter(tmp_buffer)); + tmp_buffer.emplace_back(0x01); + std::ranges::copy(seed_material, std::back_inserter(tmp_buffer)); + + // Second HMAC round: K = HMAC(K, V || 0x01 || seed_material), V = HMAC(K, V) + key_buffer = hmac, std::array>(tmp_buffer, key_buffer); + v_buffer = hmac, std::array>( + v_buffer, key_buffer); + + // Loop until we get a valid k: 0 < k < Fr::modulus + uint256_t k = 0; + while (k == 0 || k >= static_cast(Fr::modulus)) { + v_buffer = hmac, std::array>( + v_buffer, key_buffer); + + // Trim the output if required + if (Hash::OUTPUT_SIZE * 8 > MODULUS_BIT_LENGTH) { + std::vector trimmed_v_buffer(v_buffer.begin(), v_buffer.end()); + // Read the hash output + const uint8_t* ptr = trimmed_v_buffer.data(); + uint256_t trimmed_v; + read(ptr, trimmed_v); + + // Bit shift the output + trimmed_v = trimmed_v >> (Hash::OUTPUT_SIZE * 8 - MODULUS_BIT_LENGTH); + + // Set k + k = trimmed_v; + } else { + const uint8_t* ptr = v_buffer.data(); + read(ptr, k); + } + + if ((k > 0) && (k < static_cast(Fr::modulus))) { + break; + } + + std::vector tmp_buffer; + tmp_buffer.reserve(INITIAL_BUFFER_SIZE + 1); + std::ranges::copy(v_buffer, std::back_inserter(tmp_buffer)); + tmp_buffer.emplace_back(0x00); + key_buffer = hmac, std::array>(tmp_buffer, key_buffer); + secure_erase(tmp_buffer); + v_buffer = hmac, std::array>( + v_buffer, key_buffer); + } - auto field_as_u512 = from_buffer(full_buffer); + secure_erase(seed_material); + secure_erase(tmp_buffer); + secure_erase(v_buffer); + secure_erase(key_buffer); - Fr result((field_as_u512 % Fr::modulus).lo); - return result; + return Fr(k); } } // namespace bb::crypto diff --git a/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.test.cpp index 71340a365564..d9a3bf9d9f39 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/hmac/hmac.test.cpp @@ -2,6 +2,8 @@ #include #include "../hashers/hashers.hpp" +#include "barretenberg/ecc/curves/secp256k1/secp256k1.hpp" +#include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp" #include #include @@ -11,17 +13,21 @@ using namespace bb; using namespace bb::crypto; +namespace { + std::array hex_to_bytes(const std::string& hex) { std::array bytes; for (unsigned int i = 0; i < hex.length(); i += 2) { std::string byteString = hex.substr(i, 2); - uint8_t byte = (uint8_t)strtol(byteString.c_str(), nullptr, 16); + uint8_t byte = static_cast(strtol(byteString.c_str(), nullptr, 16)); bytes[i >> 1] = byte; } return bytes; } +} // namespace + struct TestData { std::string key; std::string message; @@ -113,4 +119,73 @@ TEST(hmac, ValidateHMAC) EXPECT_EQ(result, hex_to_bytes(expected)); } -} \ No newline at end of file +} + +TEST(NonceDerivation, ValidateRFC6979K1) +{ + // Test vectors from + // https://crypto.stackexchange.com/questions/20838/request-for-data-to-test-deterministic-ecdsa-signature-algorithm-for-secp256k1?utm_source=chatgpt.com + using Fr = bb::secp256k1::fr; + using G1 = bb::secp256k1::g1; + using AffineElement = G1::affine_element; + + { + std::string message = "Absence makes the heart grow fonder."; + Fr key = Fr::one(); + Fr expected_x_coordinate = Fr("0xAFFF580595971B8C1700E77069D73602AEF4C2A760DBD697881423DFFF845DE8"); + + std::array key_buffer; + Fr::serialize_to_buffer(key, &key_buffer[0]); + + Fr result = deterministic_nonce_rfc6979(message, key_buffer); + AffineElement R = G1::one * result; + + EXPECT_EQ(static_cast(R.x), static_cast(expected_x_coordinate)); + } + + { + std::string message = "Absence makes the heart grow fonder."; + Fr key = Fr("0x4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a"); + Fr expected_x_coordinate = Fr("0x996D79FBA54B24E9394FC5FAB6BF94D173F3752645075DE6E32574FE08625F77"); + + std::array key_buffer; + Fr::serialize_to_buffer(key, &key_buffer[0]); + + Fr result = deterministic_nonce_rfc6979(message, key_buffer); + AffineElement R = G1::one * result; + + EXPECT_EQ(static_cast(R.x), static_cast(expected_x_coordinate)); + } +} + +TEST(NonceDerivation, ValidateRFC6979R1) +{ + // Test vectors from https://www.rfc-editor.org/rfc/rfc6979.html#appendix-A + using Fr = bb::secp256r1::fr; + + { + std::string message = "sample"; + Fr key = Fr("0xC9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721"); + Fr expected = Fr("0xA6E3C57DD01ABE90086538398355DD4C3B17AA873382B0F24D6129493D8AAD60"); + + std::array key_buffer; + Fr::serialize_to_buffer(key, &key_buffer[0]); + + Fr result = deterministic_nonce_rfc6979(message, key_buffer); + + EXPECT_EQ(result, expected); + } + + { + std::string message = "test"; + Fr key = Fr("0xC9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721"); + Fr expected = Fr("0xD16B6AE827F17175E040871A1C7EC3500192C4C92677336EC2537ACAEE0008E0"); + + std::array key_buffer; + Fr::serialize_to_buffer(key, &key_buffer[0]); + + Fr result = deterministic_nonce_rfc6979(message, key_buffer); + + EXPECT_EQ(result, expected); + } +} diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/avm2_recursion_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/avm2_recursion_constraint.hpp index cba2a95f06a8..47cc7d36f5f6 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/avm2_recursion_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/avm2_recursion_constraint.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa.test.cpp index f6745cea0b15..422a77d1eaac 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa.test.cpp @@ -158,11 +158,6 @@ template class EcdsaTests : public ::testing::Test { // Natively verify that the tampering was successfull bool is_signature_valid = ecdsa_verify_signature( message_string, account.public_key, signature); - if (mode == TamperingMode::HighS || mode == TamperingMode::InfinityScalarMul) { - // If either s >= (n+1)/2 or the result of the scalar multiplication is the point at infinity, then the - // verification function raises an error, we treat it as an invalid signature - is_signature_valid = false; - } if (mode == TamperingMode::XCoordinateOverflow || mode == TamperingMode::YCoordinateOverflow) { // In these tampering modes nothing has changed and the tampering happens in circuit, so we override the // result and set it to false @@ -448,8 +443,6 @@ TYPED_TEST(EcdsaTests, InvalidS) TYPED_TEST(EcdsaTests, HighS) { - // Disable asserts because native ecdsa verification raises an error if s >= (n+1)/2 - BB_DISABLE_ASSERTS(); TestFixture::test_verify_signature(/*random_signature=*/false, TestFixture::TamperingMode::HighS); } @@ -465,24 +458,16 @@ TYPED_TEST(EcdsaTests, ZeroS) TYPED_TEST(EcdsaTests, InvalidPubKey) { - // Disable asserts because `validate_on_curve` raises an error in the `mult_madd` function: - // BB_ASSERT_EQ(remainder_1024.lo, uint512_t(0)) - BB_DISABLE_ASSERTS(); TestFixture::test_verify_signature(/*random_signature=*/false, TestFixture::TamperingMode::InvalidPubKey); } TYPED_TEST(EcdsaTests, InfinityPubKey) { - // Disable asserts to avoid errors trying to invert zero - BB_DISABLE_ASSERTS(); TestFixture::test_verify_signature(/*random_signature=*/false, TestFixture::TamperingMode::InfinityPubKey); } TYPED_TEST(EcdsaTests, InfinityScalarMul) { - // Disable asserts because native ecdsa verification raises an error if the result of the scalar multiplication is - // the point at infinity - BB_DISABLE_ASSERTS(); TestFixture::test_verify_signature(/*random_signature=*/false, TestFixture::TamperingMode::InfinityScalarMul); } @@ -512,9 +497,6 @@ TYPED_TEST(EcdsaTests, SignatureGenerator) TEST(EcdsaTests, Secp256k1PointAtInfinityRegression) { - // Disable asserts because native ecdsa verification raises an error if the result of the scalar multiplication is - // the point at infinity - BB_DISABLE_ASSERTS(); using Curve = stdlib::secp256k1; using FqNative = Curve::fq; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_tests_data.hpp b/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_tests_data.hpp index 44d443fa6c32..8982107dc1c5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_tests_data.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_tests_data.hpp @@ -81,6 +81,17 @@ const std::vector secp256k1_tests{ .is_circuit_satisfied = true, .comment = "Edge case public key, y coordinate is small", }, + // Modular inverse edge case + WycherproofSecp256k1{ + .x = WycherproofSecp256k1::Fq("0x9171fec3ca20806bc084f12f0760911b60990bd80e5b2a71ca03a048b20f837e"), + .y = WycherproofSecp256k1::Fq("0x634fd17863761b2958d2be4e149f8d3d7abbdc18be03f451ab6c17fa0a1f8330"), + .message = { 0x31, 0x32, 0x33, 0x34, 0x30, 0x30 }, + .r = WycherproofSecp256k1::Fr("0x55555555555555555555555555555554e8e4f44ce51835693ff0ca2ef01215c1"), + .s = WycherproofSecp256k1::Fr("0x2736d76e412246e097148e2bf62915614eb7c428913a58eb5e9cd4674a9423de"), + .is_valid_signature = true, + .is_circuit_satisfied = true, + .comment = "Modular inverse edge case", + }, }; /** @@ -122,5 +133,16 @@ const std::vector secp256r1_tests{ .is_circuit_satisfied = true, .comment = "Edge case public key, x-coordinate has many trailing zeros", }, + // Edge case public key test + WycherproofSecp256r1{ + .x = WycherproofSecp256r1::Fq("0x2927b10512bae3eddcfe467828128bad2903269919f7086069c8c4df6c732838"), + .y = WycherproofSecp256r1::Fq("0xc7787964eaac00e5921fb1498a60f4606766b3d9685001558d1a974e7341513e"), + .message = { 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65 }, + .r = WycherproofSecp256r1::Fr("0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551"), + .s = WycherproofSecp256r1::Fr("0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632550"), + .is_valid_signature = false, + .is_circuit_satisfied = true, + .comment = "Signature with special case values r=n and s=n - 1", + }, }; } // namespace bb::stdlib diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.cpp index bad8415dc3da..3c9c652e4f45 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.hpp index 7373b9332536..91e8e4763c3c 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/flavor.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.cpp index 55ed170d8645..003c91c41766 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.hpp index 974df688191d..db82c3d996fb 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/prover.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp index 59f90f8238ab..8dbb4b6f00ee 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_flavor.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp index f0ddbaeb8491..5098b266889a 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.hpp index ef8ec647e7cd..7a91f977b140 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/recursion/recursive_verifier.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.cpp index d9ffb8f95ef9..20441a3998e8 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.hpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.hpp index a0f161a6a387..78cb4268dc45 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/verifier.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: } +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.cpp index 966b78c07b44..c5bb57d28875 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: 54146acfe3568e22f80648f4092e10cb2c8702c2} +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.hpp b/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.hpp index da63bf2667e5..678e47304481 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/dsl/avm2_recursion_constraint.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Completed, auditors: [Federico], commit: 54146acfe3568e22f80648f4092e10cb2c8702c2} +// internal: { status: Completed, auditors: [Federico], commit: 0e37cb8} // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== From a310458e2c1cb0d40d538682181c6c4d66e2c72d Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Mon, 23 Feb 2026 14:23:07 +0000 Subject: [PATCH 3/5] Address comment --- barretenberg/cpp/bootstrap.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/bootstrap.sh b/barretenberg/cpp/bootstrap.sh index cbe57a1327df..b64fcc5ca371 100755 --- a/barretenberg/cpp/bootstrap.sh +++ b/barretenberg/cpp/bootstrap.sh @@ -71,13 +71,10 @@ function build_native { cache_upload barretenberg-$native_preset-$hash.zst build/{bin,lib} fi # Always inject version (even for cached binaries) to ensure correct version on release - if [ "${NATIVE_PRESET:-clang20}" == "clang20" ]; then - inject_version build/bin/bb - elif [ "${NATIVE_PRESET}" == "debug" ]; then - inject_version build-debug/bin/bb - fi - if [ -f build/bin/bb-avm ]; then - inject_version build/bin/bb-avm + inject_version $(scripts/native-preset-build-dir)/bin/bb + + if [ -f $(scripts/native-preset-build-dir)/bin/bb-avm ]; then + inject_version $(scripts/native-preset-build-dir)/bin/bb-avm fi } From 5c4c608499544ffffa7610a6ab0f490d8dcd1a57 Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Mon, 23 Feb 2026 14:29:18 +0000 Subject: [PATCH 4/5] chore: Generic lookup/permutation audit 2 (#20761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### ๐Ÿงพ Audit Context Add testing for generic lookup and permutation relation. ### ๐Ÿ› ๏ธ Changes Made - Add testing framework for generic lookup and permutation relation. ### โœ… Checklist - [ ] Audited all methods of the relevant module/class - [ ] Audited the interface of the module/class with other (relevant) components - [ ] Documented existing functionality and any changes made (as per Doxygen requirements) - [ ] Resolved and/or closed all issues/TODOs pertaining to the audited files - [ ] Confirmed and documented any security or other issues found (if applicable) - [X] Verified that tests cover all critical paths (and added tests if necessary) - [ ] Updated audit tracking for the files audited (check the start of each file you audited) ### ๐Ÿ“Œ Notes for Reviewers --- .../generic_lookup_relation.test.cpp | 1066 +++++++++++++++++ .../generic_permutation_relation.test.cpp | 307 +++++ 2 files changed, 1373 insertions(+) create mode 100644 barretenberg/cpp/src/barretenberg/relations/generic_lookup/generic_lookup_relation.test.cpp create mode 100644 barretenberg/cpp/src/barretenberg/relations/generic_permutation/generic_permutation_relation.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/relations/generic_lookup/generic_lookup_relation.test.cpp b/barretenberg/cpp/src/barretenberg/relations/generic_lookup/generic_lookup_relation.test.cpp new file mode 100644 index 000000000000..e7685b4b7a43 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/relations/generic_lookup/generic_lookup_relation.test.cpp @@ -0,0 +1,1066 @@ +#include "generic_lookup_relation.hpp" +#include "barretenberg/ecc/curves/bn254/fr.hpp" +#include "barretenberg/relations/relation_parameters.hpp" +#include +#include + +using namespace bb; +using FF = bb::fr; + +// ============================================================================ +// Generic Lookup Relation โ€” test overview +// +// Three test environments are defined to cover the main configuration modes: +// +// BasicLookupTest โ€” BASIC_LOOKUP / BASIC_TABLE (LOOKUP_TUPLE_SIZE = 2) +// The relation auto-batches columns into a single term +// via beta-encoding: term = c1*beta + c2 + gamma. +// +// CustomizedLookupTest โ€” CUSTOMIZED_LOOKUP / CUSTOMIZED_TABLE +// The user supplies compute_lookup_term / compute_table_term +// (here: lookup = f^2, table = t). +// +// MixedLookupTest โ€” Two lookup terms (BASIC + CUSTOMIZED) and two table +// terms (BASIC + CUSTOMIZED), exercising both modes +// simultaneously in the same relation instance. +// +// For each environment the following tests are created: +// +// InactiveRow โ€” All-zero row: both subrelations accumulate to zero. +// ValidLookupRow โ€” Correctly-set-up lookup row satisfies subrelation 0 +// (the inverse check: I * prod - inverse_exists = 0). +// ValidTableRow โ€” Correctly-set-up table row satisfies subrelation 0. +// ValidTrace โ€” Two-row trace where the lookup term matches the table +// term: subrelation 1 (the log-derivative sum) equals zero. +// IncorrectInverse โ€” Wrong I value on an active row: subrelation 0 โ‰  0. +// InvalidLookup โ€” Lookup/table term mismatch: subrelation 1 โ‰  0. +// InvalidReadCount โ€” Read count mismatch with a matching term: subrelation 1 โ‰  0. +// ============================================================================ + +// ============================================================================ +// SettingsBasicLookup +// +// Uses BASIC_LOOKUP / BASIC_TABLE so the relation auto-batches polynomial +// columns via: term = col[0]*beta + col[1] + gamma (LOOKUP_TUPLE_SIZE=2) +// +// Polynomial index map (NUM_POLYS = 8): +// [0] Inverse polynomial (I) +// [1] Read count (table term 0) +// [2] Lookup predicate +// [3] Table predicate +// [4] Lookup column f1 +// [5] Lookup column f2 โ†’ lookup_term = f1*beta + f2 + gamma +// [6] Table column t1 +// [7] Table column t2 โ†’ table_term = t1*beta + t2 + gamma +// ============================================================================ +struct SettingsBasicLookup { + static constexpr size_t NUM_LOOKUP_TERMS = 1; + static constexpr size_t NUM_TABLE_TERMS = 1; + static constexpr size_t LOOKUP_TUPLE_SIZE = 2; + static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 2; + + static constexpr std::array LOOKUP_TYPES = { BASIC_LOOKUP }; + static constexpr std::array TABLE_TYPES = { BASIC_TABLE }; + // Degrees are only used for CUSTOMIZED types; for BASIC the relation uses degree=1 internally. + static constexpr std::array LOOKUP_TERM_DEGREES = { 1 }; + static constexpr std::array TABLE_TERM_DEGREES = { 1 }; + + static constexpr size_t NUM_POLYS = 1 + // Inverse + NUM_TABLE_TERMS + // Read counts + NUM_LOOKUP_TERMS + // Lookup predicates + NUM_TABLE_TERMS + // Table predicates + (LOOKUP_TUPLE_SIZE * NUM_LOOKUP_TERMS) + // Lookup columns + (LOOKUP_TUPLE_SIZE * NUM_TABLE_TERMS); // Table columns + + using AllEntities = std::array; + + /** + * @brief Returns true if either predicate is active, meaning inverse must be computed at this row. + */ + static bool inverse_polynomial_is_computed_at_row(const AllEntities& in) + { + return in[2] == FF(1) || in[3] == FF(1); + } + + /** + * @brief OR(lookup_pred, table_pred) via the inclusion-exclusion formula A + B - A*B. + * + * This is a degree-2 polynomial in the predicates, matching INVERSE_EXISTS_POLYNOMIAL_DEGREE=2. + */ + template static Accumulator compute_inverse_exists(const AllEntities& in) + { + return Accumulator(in[2]) + Accumulator(in[3]) - Accumulator(in[2]) * Accumulator(in[3]); + } + + // Both const and nonconst overloads are templated so they accept `const AllEntities&` + // when called from GenericLookupRelationImpl::get_inverse_polynomial (which uses a deduced + // AllEntities&& that can be const). + template static auto get_const_entities(const AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5], in[6], in[7]); + } + + template static auto get_nonconst_entities(AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5], in[6], in[7]); + } +}; + +// ============================================================================ +// SettingsCustomizedLookup +// +// Uses CUSTOMIZED_LOOKUP / CUSTOMIZED_TABLE so no polynomial columns are +// auto-added for batching. Two extra columns are added manually: +// [4] f column โ†’ lookup_term = f^2 (degree 2) +// [5] t column โ†’ table_term = t (degree 1) +// +// Polynomial index map (NUM_POLYS = 6): +// [0] Inverse polynomial (I) +// [1] Read count +// [2] Lookup predicate +// [3] Table predicate +// [4] Custom f column +// [5] Custom t column +// ============================================================================ +struct SettingsCustomizedLookup { + static constexpr size_t NUM_LOOKUP_TERMS = 1; + static constexpr size_t NUM_TABLE_TERMS = 1; + // LOOKUP_TUPLE_SIZE is required by the concept but is unused for CUSTOMIZED types. + static constexpr size_t LOOKUP_TUPLE_SIZE = 1; + static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 2; + + static constexpr std::array LOOKUP_TYPES = { CUSTOMIZED_LOOKUP }; + static constexpr std::array TABLE_TYPES = { CUSTOMIZED_TABLE }; + static constexpr std::array LOOKUP_TERM_DEGREES = { 2 }; // f^2 is degree 2 + static constexpr std::array TABLE_TERM_DEGREES = { 1 }; // t is degree 1 + + // 1 (inv) + 1 (count) + 1 (lookup pred) + 1 (table pred) + 1 (f col) + 1 (t col) + static constexpr size_t NUM_POLYS = 6; + + using AllEntities = std::array; + + static bool inverse_polynomial_is_computed_at_row(const AllEntities& in) + { + return in[2] == FF(1) || in[3] == FF(1); + } + + template static Accumulator compute_inverse_exists(const AllEntities& in) + { + return Accumulator(in[2]) + Accumulator(in[3]) - Accumulator(in[2]) * Accumulator(in[3]); + } + + /** + * @brief Custom lookup term: f^2 (degree 2) + */ + template + static Accumulator compute_lookup_term(const AllEntities& in, [[maybe_unused]] const Parameters& params) + { + using View = typename Accumulator::View; + auto f = Accumulator(View(in[4])); + return f * f; + } + + /** + * @brief Custom table term: t (degree 1) + */ + template + static Accumulator compute_table_term(const AllEntities& in, [[maybe_unused]] const Parameters& params) + { + using View = typename Accumulator::View; + auto t = Accumulator(View(in[5])); + return t; + } + + template static auto get_const_entities(const AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5]); + } + + template static auto get_nonconst_entities(AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5]); + } +}; + +// ============================================================================ +// Test fixtures +// ============================================================================ + +template class GenericLookupRelationTest : public testing::Test { + public: + using Relation = GenericLookupRelationImpl; + using AllEntities = typename Settings::AllEntities; + static constexpr size_t NUM_SUBRELATIONS = 2; + + using Accumulator = std::array; + + /** + * @brief Accumulate a single row into a fresh accumulator. + */ + static Accumulator eval_row(const AllEntities& row, const RelationParameters& params, FF scaling_factor = FF(1)) + { + Accumulator acc{}; + Relation::accumulate(acc, row, params, scaling_factor); + return acc; + } + + /** + * @brief Accumulate multiple rows into one accumulator. + */ + static Accumulator eval_trace(const std::vector& rows, + const RelationParameters& params, + FF scaling_factor = FF(1)) + { + Accumulator acc{}; + for (const auto& row : rows) { + Relation::accumulate(acc, row, params, scaling_factor); + } + return acc; + } +}; + +// ============================================================================ +// Tests for SettingsBasicLookup +// ============================================================================ + +class BasicLookupTest : public GenericLookupRelationTest { + public: + /** + * @brief Build and evaluate a canonical two-row (lookup + table) trace. + * + * Row 0 is always a lookup row with f1=1, f2=2. + * Row 1 is always a table row with f1=9, f2=10 (dummies, lookup_pred=0). + * The table columns for row 1 and the read count are the varying parameters. + * + * The expected subrelation 1 sum is derived directly from the inputs: + * acc[1] = 1/lookup_term0 - read_count/table_term1 + * so passing table columns that match row 0 (t1=1, t2=2) with read_count=1 gives 0. + */ + static void check_two_row_sum(const RelationParameters& params, FF t1_row1, FF t2_row1, FF read_count) + { + const FF beta = params.beta; + const FF gamma = params.gamma; + + auto construct_term = [&](FF col1, FF col2) { return col1 * beta + col2 + gamma; }; + + // Row 0: lookup row (fixed) + const FF f1 = FF(1); + const FF f2 = FF(2); + const FF t1_row0 = FF(3); + const FF t2_row0 = FF(4); + const FF lookup_term0 = construct_term(f1, f2); + const FF table_term_row0 = construct_term(t1_row0, t2_row0); + + AllEntities row0{}; + row0[0] = (lookup_term0 * table_term_row0).invert(); + row0[2] = FF(1); // lookup predicate + row0[4] = f1; + row0[5] = f2; + row0[6] = t1_row0; + row0[7] = t2_row0; + + // Row 1: table row (parametrized table columns and read count) + const FF f1_row1 = FF(9); + const FF f2_row1 = FF(10); + const FF lookup_term_row1 = construct_term(f1_row1, f2_row1); + const FF table_term1 = construct_term(t1_row1, t2_row1); + + AllEntities row1{}; + row1[0] = (lookup_term_row1 * table_term1).invert(); + row1[1] = read_count; // read count + row1[3] = FF(1); // table predicate + row1[4] = f1_row1; + row1[5] = f2_row1; + row1[6] = t1_row1; + row1[7] = t2_row1; + + Accumulator acc{}; + Relation::accumulate(acc, row0, params, FF(1)); + EXPECT_EQ(acc[0], FF(0)); // subrelation 0 satisfied independently per row + Relation::accumulate(acc, row1, params, FF(1)); + + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], FF(1) / lookup_term0 - read_count / table_term1); + } +}; + +/** + * @brief An all-zero row must leave both subrelations at zero. + * + * When no predicate is active, inverse_exists = 0, lookup_inverse = 0: + * subrel_0 = (prod * 0 - 0) * sf = 0 + * subrel_1 += 0 * ... - 0 * ... = 0 + */ +TEST_F(BasicLookupTest, InactiveRow) +{ + const auto params = RelationParameters::get_random(); + AllEntities row{}; // all zeros + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], FF(0)); +} + +/** + * @brief A correctly-set-up lookup row satisfies subrelation 0. + * + * With lookup_predicate=1, table_predicate=0: + * inverse_exists = 1 + * I = 1 / (lookup_term * table_term) + * subrel_0 = I * lookup_term * table_term - 1 = 0 + */ +TEST_F(BasicLookupTest, ValidLookupRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + // Construct lookup and table terms + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF t1 = FF(7); + const FF t2 = FF(11); + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); // I + row[2] = FF(1); // lookup predicate + row[4] = f1; + row[5] = f2; + row[6] = t1; + row[7] = t2; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief A correctly-set-up table row satisfies subrelation 0. + * + * With lookup_predicate=0, table_predicate=1, read_count=1: + * inverse_exists = 1 + * I = 1 / (lookup_term * table_term) + * subrel_0 = I * lookup_term * table_term - 1 = 0 + */ +TEST_F(BasicLookupTest, ValidTableRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + const FF f1 = FF(2); + const FF f2 = FF(4); + const FF t1 = FF(6); + const FF t2 = FF(8); + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); // I + row[1] = FF(1); // read count + row[3] = FF(1); // table predicate + row[4] = f1; + row[5] = f2; + row[6] = t1; + row[7] = t2; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief A two-row trace with matching lookup/table terms satisfies the log-derivative identity. + * + * t1=1, t2=2 on row 1 โ†’ table_term1 = 1*beta + 2 + gamma = lookup_term0, so acc[1] = 0. + */ +TEST_F(BasicLookupTest, ValidTrace) +{ + const auto params = RelationParameters::get_random(); + // t1=1, t2=2 matches the lookup term (f1=1, f2=2) โ†’ valid lookup, sum = 0 + check_two_row_sum(params, /*t1_row1=*/FF(1), /*t2_row1=*/FF(2), /*read_count=*/FF(1)); +} + +/** + * @brief An active lookup row with an incorrect inverse violates subrelation 0. + * + * We set I to a wrong value (not the product-inverse) and confirm subrelation 0 โ‰  0. + */ +TEST_F(BasicLookupTest, IncorrectInverse) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF t1 = FF(7); + const FF t2 = FF(11); + + AllEntities row{}; + row[0] = FF(42); // deliberate wrong inverse + row[2] = FF(1); // lookup predicate + row[4] = f1; + row[5] = f2; + row[6] = t1; + row[7] = t2; + + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + auto acc = eval_row(row, params); + + // Subrelation 0 = (lookup_term * table_term * I_wrong - inverse_exists) * sf + // = (lookup_term * table_term * 42 - 1) which is generically non-zero + const FF expected = lookup_term * table_term * FF(42) - FF(1); + EXPECT_EQ(acc[0], expected); + EXPECT_NE(acc[0], FF(0)); +} + +/** + * @brief Table term mismatch: lookup_term0 = 1*beta+2+gamma, table_term1 = 2*beta+4+gamma โ†’ acc[1] โ‰  0. + */ +TEST_F(BasicLookupTest, InvalidLookup) +{ + const auto params = RelationParameters::get_random(); + // t1=2, t2=4 gives table_term1 โ‰  lookup_term0 (f1=1, f2=2) + check_two_row_sum(params, /*t1_row1=*/FF(2), /*t2_row1=*/FF(4), /*read_count=*/FF(1)); +} + +/** + * @brief Read count mismatch: table_term1 = lookup_term0 but read_count=2 โ†’ acc[1] = -1/lookup_term0 โ‰  0. + */ +TEST_F(BasicLookupTest, InvalidReadCount) +{ + const auto params = RelationParameters::get_random(); + // t1=1, t2=2 matches (table_term1 == lookup_term0) but read_count=2 makes the sum non-zero + check_two_row_sum(params, /*t1_row1=*/FF(1), /*t2_row1=*/FF(2), /*read_count=*/FF(2)); + + // Invalid: more lookups than allowed + check_two_row_sum(params, /*t1_row1=*/FF(1), /*t2_row1=*/FF(2), /*read_count=*/FF(0)); +} + +// ============================================================================ +// Tests for SettingsCustomizedLookup +// ============================================================================ + +class CustomizedLookupTest : public GenericLookupRelationTest { + public: + /** + * @brief Build and evaluate a canonical two-row (lookup + table) trace. + * + * Row 0 is always a lookup row with f=3 (lookup_term = 9). + * Row 1 is always a table row with f=5 (dummy, lookup_pred=0). + * The t column for row 1 and the read count are the varying parameters. + * + * The expected subrelation 1 sum is 1/lookup_term0 - read_count/table_term1, + * so passing table_t_value=9 (== v^2) with read_count=1 gives 0. + */ + static void check_two_row_sum(const RelationParameters& params, FF table_t_value, FF read_count) + { + auto construct_term = [&](FF col) { return col * col; }; + + const FF v = FF(3); + const FF v_sq = construct_term(v); // lookup_term0 = 9 + + // Row 0: lookup row (fixed) + const FF t_row0 = FF(1); // arbitrary table column for row 0 inverse denominator + AllEntities row0{}; + row0[0] = (v_sq * t_row0).invert(); + row0[2] = FF(1); // lookup predicate + row0[4] = v; // f column โ†’ lookup_term = v^2 + row0[5] = t_row0; + + // Row 1: table row (parametrized t column and read count) + const FF f_row1 = FF(5); + const FF lookup_term_row1 = construct_term(f_row1); // 25 + AllEntities row1{}; + row1[0] = (lookup_term_row1 * table_t_value).invert(); + row1[1] = read_count; // read count + row1[3] = FF(1); // table predicate + row1[4] = f_row1; + row1[5] = table_t_value; // t column โ†’ table_term = table_t_value + + Accumulator acc{}; + Relation::accumulate(acc, row0, params, FF(1)); + EXPECT_EQ(acc[0], FF(0)); // subrelation 0 satisfied independently per row + Relation::accumulate(acc, row1, params, FF(1)); + + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], FF(1) / v_sq - read_count / table_t_value); + } +}; + +/** + * @brief All-zero row โ†’ both subrelations are zero. + */ +TEST_F(CustomizedLookupTest, InactiveRow) +{ + const auto params = RelationParameters::get_random(); + AllEntities row{}; + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], FF(0)); +} + +/** + * @brief Correctly-set-up lookup row satisfies subrelation 0. + * + * lookup_term = f^2, f = in[4] + * table_term = t , t = in[5] (arbitrary, since table_pred=0) + */ +TEST_F(CustomizedLookupTest, ValidLookupRow) +{ + const auto params = RelationParameters::get_random(); + + const FF f = FF(3); + const FF t_val = FF(7); // arbitrary table column value + const FF lookup_term = f * f; // f^2 + const FF table_term = t_val; // t + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); // I + row[2] = FF(1); // lookup predicate + row[4] = f; + row[5] = t_val; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief Correctly-set-up table row satisfies subrelation 0. + * + * lookup_term = f^2, f = in[4] (arbitrary) + * table_term = t , t = in[5] + */ +TEST_F(CustomizedLookupTest, ValidTableRow) +{ + const auto params = RelationParameters::get_random(); + + const FF f = FF(5); // arbitrary lookup column + const FF t_val = FF(9); + const FF lookup_term = f * f; + const FF table_term = t_val; + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); + row[1] = FF(1); // read count + row[3] = FF(1); // table predicate + row[4] = f; + row[5] = t_val; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief Two-row trace with matching lookup/table terms satisfies the log-derivative identity. + * + * table_t_value=9 (=v^2) matches lookup_term0=9, so acc[1] = 1/9 - 1/9 = 0. + */ +TEST_F(CustomizedLookupTest, ValidTrace) +{ + const auto params = RelationParameters::get_random(); + check_two_row_sum(params, /*table_t_value=*/FF(9), /*read_count=*/FF(1)); +} + +/** + * @brief Wrong inverse on a customized active row violates subrelation 0. + */ +TEST_F(CustomizedLookupTest, IncorrectInverse) +{ + const auto params = RelationParameters::get_random(); + + const FF f = FF(3); + const FF t_val = FF(7); + const FF lookup_term = f * f; + const FF table_term = t_val; + + AllEntities row{}; + row[0] = FF(13); // deliberate wrong inverse + row[2] = FF(1); // lookup predicate + row[4] = f; + row[5] = t_val; + + auto acc = eval_row(row, params); + + const FF expected = lookup_term * table_term * FF(13) - FF(1); + EXPECT_EQ(acc[0], expected); + EXPECT_NE(acc[0], FF(0)); +} + +/** + * @brief Table term mismatch: lookup_term0=9, table_t_value=8 โ†’ acc[1] = 1/9 - 1/8 โ‰  0. + */ +TEST_F(CustomizedLookupTest, InvalidLookup) +{ + const auto params = RelationParameters::get_random(); + check_two_row_sum(params, /*table_t_value=*/FF(8), /*read_count=*/FF(1)); +} + +/** + * @brief Read count mismatch: table_t_value=9 matches but read_count=2 โ†’ acc[1] = 1/9 - 2/9 โ‰  0. + */ +TEST_F(CustomizedLookupTest, InvalidReadCount) +{ + const auto params = RelationParameters::get_random(); + check_two_row_sum(params, /*table_t_value=*/FF(9), /*read_count=*/FF(2)); + + // Invalid: more lookups than allowed + check_two_row_sum(params, /*table_t_value=*/FF(9), /*read_count=*/FF(0)); +} + +// ============================================================================ +// SettingsMixedLookup +// +// Combines two lookup terms (BASIC_LOOKUP + CUSTOMIZED_LOOKUP) and two table +// terms (BASIC_TABLE + CUSTOMIZED_TABLE), giving two read counts and two table predicates. +// +// Polynomial index map (NUM_POLYS = 15): +// [0] Inverse polynomial +// [1] Read count 0 (table term 0) +// [2] Read count 1 (table term 1) +// [3] Lookup predicate 0 (BASIC_LOOKUP) +// [4] Lookup predicate 1 (CUSTOMIZED_LOOKUP) +// [5] Table predicate 0 +// [6] Table predicate 1 +// [7] Basic lookup col f1 +// [8] Basic lookup col f2 +// [9] Basic lookup col f3 -> lookup_term_0 = f1*beta^2 + f2*beta + f3 + gamma +// [10] Basic table col t1 (table 0) +// [11] Basic table col t2 (table 0) +// [12] Basic table col t3 (table 0) -> table_term_0 = t1*beta^2 + t2*beta + t3 + gamma +// [13] Custom f column -> lookup_term_1 = f^3 (degree 3) +// [14] Custom t column -> table_term_1 = t (degree 1) +// +// ============================================================================ +struct SettingsMixedLookup { + static constexpr size_t NUM_LOOKUP_TERMS = 2; + static constexpr size_t NUM_TABLE_TERMS = 2; + static constexpr size_t LOOKUP_TUPLE_SIZE = 3; + static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 6; + + static constexpr std::array LOOKUP_TYPES = { BASIC_LOOKUP, CUSTOMIZED_LOOKUP }; + static constexpr std::array TABLE_TYPES = { BASIC_TABLE, CUSTOMIZED_TABLE }; + static constexpr std::array LOOKUP_TERM_DEGREES = { 1, 3 }; + static constexpr std::array TABLE_TERM_DEGREES = { 1, 1 }; + + // 1 (inv) + 2 (counts) + 2 (lookup preds) + 2 (table preds) + // + 3 (basic lookup cols) + 3 (basic table 0 cols) + 1 (custom f col) + 1 (custom t col) + static constexpr size_t NUM_POLYS = 15; + + // Index map: + // [0] Inverse, [1] Read count 0, [2] Read count 1 + // [3] Lookup predicate 0 (BASIC) + // [4] Lookup predicate 1 (CUSTOMIZED) + // [5] Table predicate 0, [6] Table predicate 1 + // [7..9] Basic lookup cols f1,f2,f3 + // [10..12] Basic table 0 cols t1,t2,t3 + // [13] Custom f column -> lookup_term_1 = f^3 + // [14] Custom t column -> table_term_1 = t + using AllEntities = std::array; + + /** + * @brief Returns true if any predicate is active, meaning the inverse must be computed at this row. + * + * Active predicates: basic lookup (in[3]), customized lookup (in[4]), + * basic table (in[5]), customized table (in[6]). + */ + static bool inverse_polynomial_is_computed_at_row(const AllEntities& in) + { + return in[3] == FF(1) || in[5] == FF(1) || in[4] == FF(1) || in[6] == FF(1); + } + + /** + * @brief OR of all four predicates via inclusion-exclusion. + * + * Groups basic and customized pairs, then ORs the groups: + * basic_term = OR(lookup_pred_0, table_pred_0) = OR(in[3], in[5]) + * customized_term = OR(lookup_pred_1, table_pred_1) = OR(in[4], in[6]) + * result = OR(basic_term, customized_term) (degree 4) + */ + template static Accumulator compute_inverse_exists(const AllEntities& in) + { + auto basic_term = + Accumulator(in[3]) + Accumulator(in[5]) - Accumulator(in[3]) * Accumulator(in[5]); // OR(in[3], in[5]) + + auto customized_term = + Accumulator(in[4]) + Accumulator(in[6]) - Accumulator(in[4]) * Accumulator(in[6]); // OR(in[4], in[6]) + + return basic_term + customized_term - basic_term * customized_term; // OR(basic_term, customized_term) + } + + /** + * @brief Custom lookup term: f^3 (degree 3), f = in[13]. + * Called only for the CUSTOMIZED_LOOKUP term (lookup_index=1). + */ + template + static Accumulator compute_lookup_term(const AllEntities& in, [[maybe_unused]] const Parameters& params) + { + using View = typename Accumulator::View; + auto f = Accumulator(View(in[13])); + return f * f * f; + } + + /** + * @brief Custom table term: t (degree 1) + */ + template + static Accumulator compute_table_term(const AllEntities& in, [[maybe_unused]] const Parameters& params) + { + using View = typename Accumulator::View; + auto t = Accumulator(View(in[14])); + return t; + } + + template static auto get_const_entities(const AE& in) + { + return std::forward_as_tuple(in[0], + in[1], + in[2], + in[3], + in[4], + in[5], + in[6], + in[7], + in[8], + in[9], + in[10], + in[11], + in[12], + in[13], + in[14]); + } + + template static auto get_nonconst_entities(AE& in) + { + return std::forward_as_tuple(in[0], + in[1], + in[2], + in[3], + in[4], + in[5], + in[6], + in[7], + in[8], + in[9], + in[10], + in[11], + in[12], + in[13], + in[14]); + } +}; + +// ============================================================================ +// Tests for SettingsMixedLookup +// ============================================================================ + +class MixedLookupTest : public GenericLookupRelationTest { + public: + /** + * @brief Build and evaluate a canonical two-row mixed trace. + * + * Row 0: basic lookup (in[3]=1) + customized table (in[6]=1). + * f1=1, f2=2, f3=3 -> lookup_term_basic_row0 = 1*beta^2 + 2*beta + 3 + gamma + * custom_f=2 -> lookup_term_custom_row0 = 8 (used only in the inverse product) + * Dummy basic table cols: 1,1,1 + * custom_t_row0 -> table_term_custom_row0 = custom_t_row0 (default 27) + * read_count_customized = count for the customized table on row 0 + * + * Row 1: customized lookup (in[4]=1) + basic table (in[5]=1). + * Dummy basic lookup cols: 4,5,6 + * custom_f=3 -> lookup_term_custom_row1 = 27 + * t1,t2,t3 -> table_term_basic_row1 = t1*beta^2 + t2*beta + t3 + gamma (parametrized) + * read_count = count for the basic table on row 1 + * + * acc[1] = 1/lookup_term_basic_row0 - read_count/table_term_basic_row1 + * + 1/lookup_term_custom_row1 - read_count_customized/table_term_custom_row0 + * + * Default (valid) parameters: t1=1, t2=2, t3=3 (matching lookup_term_basic_row0), + * custom_t_row0=27 (= 3^3, matching lookup_term_custom_row1), both read_counts=1 -> acc[1]=0. + */ + static void check_two_row_sum(const RelationParameters& params, + FF t1_row1 = FF(1), + FF t2_row1 = FF(2), + FF t3_row1 = FF(3), + FF read_count = FF(1), + FF custom_t_row0 = FF(27), + FF read_count_customized = FF(1)) + { + const FF beta = params.beta; + const FF beta_sq = params.beta_sqr; + const FF gamma = params.gamma; + + auto compute_basic_term = [&](FF f1, FF f2, FF f3) { return f1 * beta_sq + f2 * beta + f3 + gamma; }; + auto compute_custom_term = [&](FF f) { return f * f * f; }; + + // Valid values for the test + const FF valid_t1_row1 = FF(1); + const FF valid_t2_row1 = FF(2); + const FF valid_t3_row1 = FF(3); + const FF valid_custom_f_row_1 = FF(3); + + // Row 0: basic lookup and customized table active (in[3]=1, in[4]=0, in[5]=0, in[6]=1) + const FF custom_f_row0 = FF(2); // Dummy value, predicate is off + const FF dummy_t1_row0 = FF(1); + const FF dummy_t2_row0 = FF(1); + const FF dummy_t3_row0 = FF(1); + + // Construct terms + const FF lookup_term_basic_row0 = compute_basic_term(valid_t1_row1, valid_t2_row1, valid_t3_row1); + const FF lookup_term_custom_row0 = compute_custom_term(custom_f_row0); + const FF table_term_basic_row0 = compute_basic_term(dummy_t1_row0, dummy_t2_row0, dummy_t3_row0); + const FF table_term_custom_row0 = custom_t_row0; + + AllEntities row0{}; + row0[0] = (lookup_term_basic_row0 * lookup_term_custom_row0 * table_term_basic_row0 * table_term_custom_row0) + .invert(); + row0[2] = read_count_customized; // read count customized table + row0[3] = FF(1); // basic lookup predicate + row0[6] = FF(1); // customized table predicate + row0[7] = valid_t1_row1; + row0[8] = valid_t2_row1; + row0[9] = valid_t3_row1; + row0[10] = dummy_t1_row0; + row0[11] = dummy_t2_row0; + row0[12] = dummy_t3_row0; + row0[13] = custom_f_row0; + row0[14] = table_term_custom_row0; + + // Row 1: basic table and customized lookup active (in[3]=0, in[4]=1, in[5]=1, in[6]=0) + const FF dummy_f1_row1 = FF(4); // Dummy value, predicate is off + const FF dummy_f2_row1 = FF(5); // Dummy value, predicate is off + const FF dummy_f3_row1 = FF(6); // Dummy value, predicate is off + const FF dummy_custom_t_row1 = FF(1); // Dummy value, predicate is off + + // Construct terms + const FF lookup_term_basic_row1 = compute_basic_term(dummy_f1_row1, dummy_f2_row1, dummy_f3_row1); + const FF lookup_term_custom_row1 = compute_custom_term(valid_custom_f_row_1); // 3^3 + const FF table_term_basic_row1 = compute_basic_term(t1_row1, t2_row1, t3_row1); + const FF table_term_custom_row1 = dummy_custom_t_row1; + + AllEntities row1{}; + row1[0] = (lookup_term_basic_row1 * lookup_term_custom_row1 * table_term_basic_row1 * table_term_custom_row1) + .invert(); + row1[1] = read_count; // read count basic table + row1[4] = FF(1); // customized lookup predicate + row1[5] = FF(1); // basic table predicate + row1[7] = dummy_f1_row1; + row1[8] = dummy_f2_row1; + row1[9] = dummy_f3_row1; + row1[10] = t1_row1; + row1[11] = t2_row1; + row1[12] = t3_row1; + row1[13] = valid_custom_f_row_1; + row1[14] = dummy_custom_t_row1; + + Accumulator acc{}; + Relation::accumulate(acc, row0, params, FF(1)); + EXPECT_EQ(acc[0], FF(0)); // subrelation 0 satisfied independently per row + Relation::accumulate(acc, row1, params, FF(1)); + + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], + FF(1) / lookup_term_basic_row0 - read_count / table_term_basic_row1 + + FF(1) / lookup_term_custom_row1 - read_count_customized / table_term_custom_row0); + } +}; + +/** + * @brief An all-zero row leaves both subrelations at zero. + * + * All predicates (in[3], in[4], in[5], in[6]) are 0, so + * inverse_exists = OR(OR(0,0), OR(0,0)) = 0 and no inverse is needed. + * subrel_0 = 0*prod - 0 = 0 + * subrel_1 = all predicate contributions = 0 + */ +TEST_F(MixedLookupTest, InactiveRow) +{ + const auto params = RelationParameters::get_random(); + AllEntities row{}; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); // subrelation 0: 0*prod - 0 = 0 + EXPECT_EQ(acc[1], FF(0)); // subrelation 1 +} + +/** + * @brief A correctly-set-up basic lookup row satisfies subrelation 0. in[3]=1 or in[4] = 1, all other predicates 0. + */ +TEST_F(MixedLookupTest, ValidLookupRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF beta_sq = params.beta_sqr; + const FF gamma = params.gamma; + + auto validate_row = [&](const size_t idx) { + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF f3 = FF(7); + const FF custom_f = FF(2); + const FF t0_1 = FF(1); + const FF t0_2 = FF(1); + const FF t0_3 = FF(1); + const FF custom_t = FF(1); + const FF lookup_term_0 = f1 * beta_sq + f2 * beta + f3 + gamma; + const FF lookup_term_1 = custom_f * custom_f * custom_f; // 2^3 + const FF table_term_0 = t0_1 * beta_sq + t0_2 * beta + t0_3 + gamma; + const FF table_term_1 = custom_t; + + AllEntities row{}; + row[0] = (lookup_term_0 * lookup_term_1 * table_term_0 * table_term_1).invert(); + row[idx] = FF(1); // turn on calculation of the inverse + row[7] = f1; + row[8] = f2; + row[9] = f3; + row[10] = t0_1; + row[11] = t0_2; + row[12] = t0_3; + row[13] = custom_f; + row[14] = custom_t; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); + }; + + // Validate basic lookup row + validate_row(3); + + // Validate customized table row + validate_row(4); +} + +/** + * @brief A correctly-set-up table-0 row satisfies subrelation 0. in[5]=1 or in[6] = 1, all other predicates 0 + */ +TEST_F(MixedLookupTest, ValidTableRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF beta_sq = params.beta_sqr; + const FF gamma = params.gamma; + + auto validate_row = [&](const size_t idx) { + const FF f1 = FF(2); + const FF f2 = FF(4); + const FF f3 = FF(6); + const FF custom_f = FF(3); + const FF t0_1 = FF(5); + const FF t0_2 = FF(7); + const FF t0_3 = FF(9); + const FF custom_t = FF(1); + const FF lookup_term_0 = f1 * beta_sq + f2 * beta + f3 + gamma; + const FF lookup_term_1 = custom_f * custom_f * custom_f; // 3^3 + const FF table_term_0 = t0_1 * beta_sq + t0_2 * beta + t0_3 + gamma; + const FF table_term_1 = custom_t; + + AllEntities row{}; + row[0] = (lookup_term_0 * lookup_term_1 * table_term_0 * table_term_1).invert(); + row[idx] = FF(1); // turn on calculation of the inverse + row[7] = f1; + row[8] = f2; + row[9] = f3; + row[10] = t0_1; + row[11] = t0_2; + row[12] = t0_3; + row[13] = custom_f; + row[14] = custom_t; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); + }; + + // Validate basic table row + validate_row(5); + + // Validate customized table row + validate_row(6); +} + +/** + * @brief A two-row trace where the basic lookup matches table 0 satisfies the log-derivative identity. + * + */ +TEST_F(MixedLookupTest, ValidTrace) +{ + const auto params = RelationParameters::get_random(); + check_two_row_sum(params); +} + +/** + * @brief Wrong inverse on an active basic lookup row violates subrelation 0. + */ +TEST_F(MixedLookupTest, IncorrectInverse) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF beta_sq = params.beta_sqr; + const FF gamma = params.gamma; + + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF f3 = FF(7); + const FF custom_f = FF(2); // lookup_term_1 = 8 + const FF t0_1 = FF(1); + const FF t0_2 = FF(1); + const FF t0_3 = FF(1); + const FF custom_t = FF(1); + const FF lookup_term_0 = f1 * beta_sq + f2 * beta + f3 + gamma; + const FF lookup_term_1 = FF(8); // 2^3 + const FF table_term_0 = t0_1 * beta_sq + t0_2 * beta + t0_3 + gamma; + const FF table_term_1 = custom_t; + + AllEntities row{}; + row[0] = FF(42); // deliberate wrong inverse + row[3] = FF(1); // basic lookup predicate + row[7] = f1; + row[8] = f2; + row[9] = f3; + row[10] = t0_1; + row[11] = t0_2; + row[12] = t0_3; + row[13] = custom_f; + row[14] = custom_t; + + auto acc = eval_row(row, params); + + const FF expected = lookup_term_0 * lookup_term_1 * table_term_0 * table_term_1 * FF(42) - FF(1); + EXPECT_EQ(acc[0], expected); + EXPECT_NE(acc[0], FF(0)); +} + +/** + * @brief Table term mismatch. + */ +TEST_F(MixedLookupTest, InvalidLookup) +{ + const auto params = RelationParameters::get_random(); + + // Invalid basic lookup + check_two_row_sum(params, FF(2), FF(4), FF(6), FF(1)); + + // Invalid customized lookup + check_two_row_sum(params, FF(1), FF(2), FF(3), FF(1), FF(10)); +} + +/** + * @brief Read count mismatch. + */ +TEST_F(MixedLookupTest, InvalidReadCount) +{ + const auto params = RelationParameters::get_random(); + + // Invalid basic lookup + check_two_row_sum(params, FF(1), FF(2), FF(3), FF(2)); + + // Invalid: more basic lookups than allowed + check_two_row_sum(params, FF(1), FF(2), FF(3), FF(0)); + + // Invalid customized lookup + check_two_row_sum(params, FF(1), FF(2), FF(3), FF(1), FF(27), FF(2)); + + // Invalid: more customized lookups than allowed + check_two_row_sum(params, FF(1), FF(2), FF(3), FF(1), FF(27), FF(0)); +} diff --git a/barretenberg/cpp/src/barretenberg/relations/generic_permutation/generic_permutation_relation.test.cpp b/barretenberg/cpp/src/barretenberg/relations/generic_permutation/generic_permutation_relation.test.cpp new file mode 100644 index 000000000000..584d0d65937b --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/relations/generic_permutation/generic_permutation_relation.test.cpp @@ -0,0 +1,307 @@ +#include "generic_permutation_relation.hpp" +#include "barretenberg/ecc/curves/bn254/fr.hpp" +#include "barretenberg/relations/relation_parameters.hpp" +#include +#include + +using namespace bb; +using FF = bb::fr; + +// ============================================================================ +// Generic Permutation Relation +// +// We define a simple permutation relation with lookup/table terms composed of the batching of two columns each. Then, +// we test that: +// - Correctly set up lookup and table rows satisfy the first subrelation (inverse check) +// - A two row trace is satisfied if the lookup/table values are correct. +// - A two row trace is not satisfied if the lookup/table terms do not match. +struct SettingsBasicPermutation { + static constexpr size_t COLUMNS_PER_SET = 2; + static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 2; + + static constexpr size_t NUM_POLYS = 1 + // Inverse + 1 + // Lookup predicates + 1 + // Table predicates + COLUMNS_PER_SET + // Lookup columns + COLUMNS_PER_SET; // Table columns + + using AllEntities = std::array; + + /** + * @brief Returns true if either predicate is active, meaning inverse must be computed at this row. + */ + static bool inverse_polynomial_is_computed_at_row(const AllEntities& in) + { + return in[1] == FF(1) || in[2] == FF(1); + } + + /** + * @brief OR(lookup_pred, table_pred) via the inclusion-exclusion formula A + B - A*B. + * + * This is a degree-2 polynomial in the predicates, matching INVERSE_EXISTS_POLYNOMIAL_DEGREE=2. + */ + template static Accumulator compute_inverse_exists(const AllEntities& in) + { + return Accumulator(in[1]) + Accumulator(in[2]) - Accumulator(in[1]) * Accumulator(in[2]); + } + + // Both const and nonconst overloads are templated so they accept `const AllEntities&` + // when called from GenericLookupRelationImpl::get_inverse_polynomial (which uses a deduced + // AllEntities&& that can be const). + template static auto get_const_entities(const AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5], in[6]); + } + + template static auto get_nonconst_entities(AE& in) + { + return std::forward_as_tuple(in[0], in[1], in[2], in[3], in[4], in[5], in[6]); + } +}; + +// ============================================================================ +// Test fixtures +// ============================================================================ + +template class GenericPermutationRelationTest : public testing::Test { + public: + using Relation = GenericPermutationRelationImpl; + using AllEntities = typename Settings::AllEntities; + static constexpr size_t NUM_SUBRELATIONS = 2; + + using Accumulator = std::array; + + /** + * @brief Accumulate a single row into a fresh accumulator. + */ + static Accumulator eval_row(const AllEntities& row, const RelationParameters& params, FF scaling_factor = FF(1)) + { + Accumulator acc{}; + Relation::accumulate(acc, row, params, scaling_factor); + return acc; + } + + /** + * @brief Accumulate multiple rows into one accumulator. + */ + static Accumulator eval_trace(const std::vector& rows, + const RelationParameters& params, + FF scaling_factor = FF(1)) + { + Accumulator acc{}; + for (const auto& row : rows) { + Relation::accumulate(acc, row, params, scaling_factor); + } + return acc; + } +}; + +// ============================================================================ +// Tests for SettingsBasicPermutation +// ============================================================================ + +class BasicPermutationTest : public GenericPermutationRelationTest { + public: + /** + * @brief Build and evaluate a canonical two-row (lookup + table) trace. + * + * Row 0 is always a lookup row with f1=1, f2=2. + * Row 1 is always a table rows, optionally a lookup row + * The table columns and the lookup predicates for row 1 are the varying parameters. + * + */ + static void check_two_row_sum(const RelationParameters& params, + FF t1_row1, + FF t2_row1, + FF lookup_predicate_row1) + { + const FF beta = params.beta; + const FF gamma = params.gamma; + + auto construct_term = [&](FF col1, FF col2) { return col1 * beta + col2 + gamma; }; + + // Valid lookup terms + const FF valid_f1 = FF(1); + const FF valid_f2 = FF(2); + const FF valid_lookup_term = construct_term(valid_f1, valid_f2); + + // Row 0: lookup row (fixed) + const FF t1_row0 = FF(3); + const FF t2_row0 = FF(4); + const FF table_term_row0 = construct_term(t1_row0, t2_row0); + + AllEntities row0{}; + row0[0] = (valid_lookup_term * table_term_row0).invert(); + row0[1] = FF(1); // lookup predicate + row0[3] = valid_f1; + row0[4] = valid_f2; + row0[5] = t1_row0; + row0[6] = t2_row0; + + // Row 1: table row (parametrized table columns and read count) + const FF lookup_term_row1 = construct_term(valid_f1, valid_f2); + const FF table_term1 = construct_term(t1_row1, t2_row1); + + AllEntities row1{}; + row1[0] = (lookup_term_row1 * table_term1).invert(); + row1[1] = lookup_predicate_row1; // lookup predicate + row1[2] = FF(1); // table predicate + row1[3] = valid_f1; + row1[4] = valid_f2; + row1[5] = t1_row1; + row1[6] = t2_row1; + + Accumulator acc{}; + Relation::accumulate(acc, row0, params, FF(1)); + EXPECT_EQ(acc[0], FF(0)); // subrelation 0 satisfied independently per row + Relation::accumulate(acc, row1, params, FF(1)); + + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], (FF(1) + lookup_predicate_row1) / valid_lookup_term - FF(1) / table_term1); + } +}; + +/** + * @brief An all-zero row must leave both subrelations at zero. + * + * When no predicate is active, inverse_exists = 0, lookup_inverse = 0: + * subrel_0 = (prod * 0 - 0) * sf = 0 + * subrel_1 += 0 * ... - 0 * ... = 0 + */ +TEST_F(BasicPermutationTest, InactiveRow) +{ + const auto params = RelationParameters::get_random(); + AllEntities row{}; // all zeros + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); + EXPECT_EQ(acc[1], FF(0)); +} + +/** + * @brief A correctly-set-up lookup row satisfies subrelation 0. + * + * With lookup_predicate=1, table_predicate=0: + * inverse_exists = 1 + * I = 1 / (lookup_term * table_term) + * subrel_0 = I * lookup_term * table_term - 1 = 0 + */ +TEST_F(BasicPermutationTest, ValidLookupRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + // Construct lookup and table terms + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF t1 = FF(7); + const FF t2 = FF(11); + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); // I + row[1] = FF(1); // lookup predicate + row[3] = f1; + row[4] = f2; + row[5] = t1; + row[6] = t2; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief A correctly-set-up table row satisfies subrelation 0. + * + * With lookup_predicate=0, table_predicate=1, read_count=1: + * inverse_exists = 1 + * I = 1 / (lookup_term * table_term) + * subrel_0 = I * lookup_term * table_term - 1 = 0 + */ +TEST_F(BasicPermutationTest, ValidTableRow) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + const FF f1 = FF(2); + const FF f2 = FF(4); + const FF t1 = FF(6); + const FF t2 = FF(8); + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + AllEntities row{}; + row[0] = (lookup_term * table_term).invert(); // I + row[1] = FF(1); // read count + row[2] = FF(1); // lookup predicate + row[3] = f1; + row[4] = f2; + row[5] = t1; + row[6] = t2; + + auto acc = eval_row(row, params); + EXPECT_EQ(acc[0], FF(0)); +} + +/** + * @brief A two-row trace with matching lookup/table terms satisfies the log-derivative identity. + * + * t1=1, t2=2 on row 1 โ†’ table_term1 = 1*beta + 2 + gamma = lookup_term0, so acc[1] = 0. + */ +TEST_F(BasicPermutationTest, ValidTrace) +{ + const auto params = RelationParameters::get_random(); + // t1=1, t2=2 matches the lookup term (f1=1, f2=2) โ†’ valid lookup, sum = 0 + check_two_row_sum(params, /*t1_row1=*/FF(1), /*t2_row1=*/FF(2), /*lookup_predicate_row1=*/FF(0)); +} + +/** + * @brief An active lookup row with an incorrect inverse violates subrelation 0. + * + * We set I to a wrong value (not the product-inverse) and confirm subrelation 0 โ‰  0. + */ +TEST_F(BasicPermutationTest, IncorrectInverse) +{ + const auto params = RelationParameters::get_random(); + const FF beta = params.beta; + const FF gamma = params.gamma; + + const FF f1 = FF(3); + const FF f2 = FF(5); + const FF t1 = FF(7); + const FF t2 = FF(11); + + AllEntities row{}; + row[0] = FF(42); // deliberate wrong inverse + row[1] = FF(1); // lookup predicate + row[3] = f1; + row[4] = f2; + row[5] = t1; + row[6] = t2; + + const FF lookup_term = f1 * beta + f2 + gamma; + const FF table_term = t1 * beta + t2 + gamma; + + auto acc = eval_row(row, params); + + // Subrelation 0 = (lookup_term * table_term * I_wrong - inverse_exists) * sf + // = (lookup_term * table_term * 42 - 1) which is generically non-zero + const FF expected = lookup_term * table_term * FF(42) - FF(1); + EXPECT_EQ(acc[0], expected); + EXPECT_NE(acc[0], FF(0)); +} + +/** + * @brief Table term mismatch: lookup_term0 = 1*beta+2+gamma, table_term1 = 2*beta+4+gamma โ†’ acc[1] โ‰  0. + */ +TEST_F(BasicPermutationTest, InvalidLookup) +{ + const auto params = RelationParameters::get_random(); + // t1=2, t2=4 gives table_term1 โ‰  lookup_term0 (f1=1, f2=2) + check_two_row_sum(params, /*t1_row1=*/FF(2), /*t2_row1=*/FF(4), /*lookup_predicate_row1=*/FF(0)); + + // Invalid: more lookups than allowed + check_two_row_sum(params, /*t1_row1=*/FF(1), /*t2_row1=*/FF(2), /*lookup_predicate_row1=*/FF(1)); +} From f2c8bd3ee99d9c5595f560017600b5bd064d3925 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 24 Feb 2026 11:30:51 +0000 Subject: [PATCH 5/5] feat: automatic VK regeneration via VK-UPDATE commit convention (#20158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary When a PR changes protocol VKs, CI now requires the author to explicitly acknowledge the change by adding a commit with a `VK-UPDATE: ` message. CI then auto-regenerates and commits the updated VKs. This replaces the previous label-based approach (`ci-update-vks`) to: - Avoid label proliferation - Force authors to **explain** why VKs changed (explanation lives in git history) - Require explicit acknowledgment before VKs are regenerated ## How it works 1. CI runs and the VK test fails with a clear message: ``` VK changes detected! To acknowledge and auto-regenerate, add a commit to your PR: git commit --allow-empty -m "VK-UPDATE: " ``` 2. Author adds the acknowledgment commit: ``` git commit --allow-empty -m "VK-UPDATE: changed public inputs in rollup circuit" git push ``` 3. CI reruns โ†’ VK test fails again โ†’ "Handle VK Update" step fires: - Detects the `VK-UPDATE:` commit message - Regenerates VKs and uploads to S3 - Commits `chore: regenerate chonk VKs` with the author's explanation - Pushes โ†’ next CI run passes ## Implementation - New `.github/ci3_vk_update.sh` runs as a separate step with `if: failure()` - Scans PR commit messages for `VK-UPDATE:` prefix - No label needed โ€” no changes to `ci3_labels_to_env.sh` - VK test failure message updated with instructions Closes https://github.com/AztecProtocol/barretenberg/issues/1485 --- ...est_chonk_standalone_vks_havent_changed.sh | 11 +++- bootstrap.sh | 8 ++- scripts/ci_vk_update.sh | 64 +++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) create mode 100755 scripts/ci_vk_update.sh diff --git a/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh index c4f081a01633..94ab351fd659 100755 --- a/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh @@ -16,7 +16,7 @@ cd .. pinned_short_hash="189f0026" pinned_chonk_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-chonk-inputs-${pinned_short_hash}.tar.gz" -script_path="$(cd "$(dirname "${BASH_SOURCE[0]}")/scripts" && pwd)/$(basename "${BASH_SOURCE[0]}")" +script_path="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" function update_pinned_hash_in_script { local new_hash=$1 @@ -229,7 +229,14 @@ else echo "No VK changes detected. Short hash is: ${pinned_short_hash}" elif [[ $exit_code -eq 1 ]]; then # All flows had VK changes - echo "VK changes detected. Please re-run the script with --update_inputs" + echo "" + echo "VK changes detected!" + echo "To acknowledge and auto-regenerate, add a commit to your PR:" + echo ' git commit --allow-empty -m "VK-UPDATE: "' + echo "" + echo "CI will automatically regenerate and commit the updated VKs on the next ci-fast/ci-full run." + echo "If running ci-barretenberg, add the ci-full label to trigger a full CI run." + echo "To update locally instead, re-run with --update_inputs." exit 1 else # At least one real error diff --git a/bootstrap.sh b/bootstrap.sh index 125fd48dff09..87d8e313a2bc 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -570,14 +570,18 @@ case "$cmd" in export USE_TEST_CACHE=1 export CI_FULL=0 build - test + if ! test; then + scripts/ci_vk_update.sh + fi ;; "ci-full") export CI=1 export USE_TEST_CACHE=1 export CI_FULL=1 build - test + if ! test; then + scripts/ci_vk_update.sh + fi bench ;; "ci-full-no-test-cache") diff --git a/scripts/ci_vk_update.sh b/scripts/ci_vk_update.sh new file mode 100755 index 000000000000..fd47df6d98be --- /dev/null +++ b/scripts/ci_vk_update.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Handles automatic VK regeneration when CI fails and the author has acknowledged +# the VK change via a VK-UPDATE commit message. +# +# Flow: +# 1. CI fails (potentially due to VK staleness) +# 2. This step checks if any commit in the PR contains "VK-UPDATE: " +# 3. If found, regenerates VKs, commits the update, and pushes +# 4. The subsequent CI run should pass with updated VKs +# +# If no VK-UPDATE commit is found, this step is a no-op (the CI failure stands). +set -euo pipefail + +NO_CD=1 source $(git rev-parse --show-toplevel)/ci3/source + +function main { + echo_header "VK Update Check" + + # Scan PR commit messages for VK-UPDATE acknowledgment. + local vk_update_message + vk_update_message=$(git log --format=%B -n "${PR_COMMITS:-50}" HEAD | grep -m1 "^VK-UPDATE:" || true) + + if [ -z "$vk_update_message" ]; then + echo "No VK-UPDATE commit found. If VKs need updating, add a commit with:" + echo ' git commit --allow-empty -m "VK-UPDATE: "' + echo "CI failure stands." + return + fi + + local explanation="${vk_update_message#VK-UPDATE:}" + explanation="${explanation# }" # trim leading space + + if [ -z "$explanation" ]; then + echo "Found VK-UPDATE commit but no explanation provided." + echo "Please include an explanation:" + echo ' git commit --allow-empty -m "VK-UPDATE: changed public inputs in rollup circuit"' + echo "CI failure stands." + return + fi + + echo "Found VK-UPDATE acknowledgment: $explanation" + echo "Proceeding with VK regeneration..." + + local github_repository + github_repository=$(git remote get-url origin | sed -E 's|.*github\.com[/:]([^/]+/[^/]+)(\.git)?$|\1|') + + # Reauth the git repo with our GITHUB_TOKEN + git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${github_repository}" + + # Generate fresh IVC inputs, upload to S3, and verify VKs + # Run from barretenberg/cpp in a subshell so script_path resolves correctly + (cd barretenberg/cpp && scripts/test_chonk_standalone_vks_havent_changed.sh --update_inputs) + + # Commit and push the updated pinned hash + git add barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh + git commit -m "chore: regenerate chonk VKs + +VK-UPDATE acknowledged: ${explanation}" + git push origin HEAD + + echo "VK update completed. A new CI run will be triggered." +} + +main