From a2c4060589f8ea06e3c7ddb9ff895b4fa06d38ca Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 10 May 2023 17:02:39 +0000 Subject: [PATCH 1/8] Add way to make verifiers data valid by replacing zeroes with valid public keys and signatures Co-authored-by: Zachary James Williamson --- .../dsl/acir_format/acir_format.cpp | 6 +-- .../dsl/acir_format/ecdsa_secp256k1.cpp | 52 ++++++++++++++++++- .../dsl/acir_format/ecdsa_secp256k1.hpp | 4 ++ .../dsl/acir_format/ecdsa_secp256k1.test.cpp | 26 ++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index a8c707943c..ad2107bb9e 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -234,7 +234,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -320,7 +320,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, std:: // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -404,7 +404,7 @@ void create_circuit_with_witness(Composer& composer, const acir_format& constrai // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp index 5a9caaa699..ed31b92f41 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp @@ -5,6 +5,7 @@ namespace acir_format { using namespace proof_system::plonk; +using curve = proof_system::plonk::stdlib::secp256k1; crypto::ecdsa::signature ecdsa_convert_signature(Composer& composer, std::vector signature) { @@ -84,9 +85,56 @@ witness_ct ecdsa_index_to_witness(Composer& composer, uint32_t index) return { &composer, value }; } +template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input) { - + { + std::vector pub_x_indices_; + std::vector pub_y_indices_; + std::vector signature_; + signature_.resize(64); + if constexpr (has_witness) { + for (size_t i = 0; i < 32; ++i) { + uint32_t x_wit = composer.add_variable(composer.get_variable(input.pub_x_indices[i])); + uint32_t y_wit = composer.add_variable(composer.get_variable(input.pub_y_indices[i])); + uint32_t r_wit = composer.add_variable(composer.get_variable(input.signature[i])); + uint32_t s_wit = composer.add_variable(composer.get_variable(input.signature[i + 32])); + pub_x_indices_.emplace_back(x_wit); + pub_y_indices_.emplace_back(y_wit); + signature_[i] = r_wit; + signature_[i + 32] = s_wit; + } + } else { + crypto::ecdsa::key_pair account; + account.private_key = 10; + account.public_key = curve::g1::one * account.private_key; + uint256_t pub_x_value = account.public_key.x; + uint256_t pub_y_value = account.public_key.y; + std::string message_string = "Instructions unclear, ask again later."; + crypto::ecdsa::signature signature = + crypto::ecdsa::construct_signature(message_string, + account); + for (size_t i = 0; i < 32; ++i) { + uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t r_wit = composer.add_variable(signature.r[i]); + uint32_t s_wit = composer.add_variable(signature.s[i]); + pub_x_indices_.emplace_back(x_wit); + pub_y_indices_.emplace_back(y_wit); + signature_[i] = r_wit; + signature_[i + 32] = s_wit; + } + } + for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { + composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); + } + for (size_t i = 0; i < input.pub_y_indices.size(); ++i) { + composer.assert_equal(pub_y_indices_[i], input.pub_y_indices[i]); + } + for (size_t i = 0; i < input.signature.size(); ++i) { + composer.assert_equal(signature_[i], input.signature[i]); + } + } auto new_sig = ecdsa_convert_signature(composer, input.signature); auto message = ecdsa_vector_of_bytes_to_byte_array(composer, input.hashed_message); @@ -126,5 +174,7 @@ void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Con bool_ct signature_result_normalized = signature_result.normalize(); composer.assert_equal(signature_result_normalized.witness_index, input.result); } +template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); +template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); } // namespace acir_format diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp index 93a87d386c..96ff9c3f95 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp @@ -26,8 +26,12 @@ struct EcdsaSecp256k1Constraint { friend bool operator==(EcdsaSecp256k1Constraint const& lhs, EcdsaSecp256k1Constraint const& rhs) = default; }; +template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); +extern template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); +extern template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); + template inline void read(B& buf, EcdsaSecp256k1Constraint& constraint) { using serialize::read; diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp index eae9b7dcd0..8da70204ca 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp @@ -40,6 +40,7 @@ size_t generate_ecdsa_constraint(acir_format::EcdsaSecp256k1Constraint& ecdsa_co const auto byte = static_cast(hashed_message[i]); witness_values.emplace_back(byte); } + std::cout << message_in.size() << std::endl; offset += message_in.size(); for (size_t i = 0; i < 32; ++i) { @@ -110,6 +111,31 @@ TEST(ECDSASecp256k1, TestECDSAConstraintSucceed) EXPECT_EQ(verifier.verify_proof(proof), true); } +TEST(ECDSASecp256k1, TestECDSAConstraintSucceedSeparate) +{ + acir_format::EcdsaSecp256k1Constraint ecdsa_constraint; + std::vector witness_values; + size_t num_variables = generate_ecdsa_constraint(ecdsa_constraint, witness_values); + acir_format::acir_format constraint_system{ + .varnum = static_cast(num_variables), + .public_inputs = {}, + .fixed_base_scalar_mul_constraints = {}, + .logic_constraints = {}, + .range_constraints = {}, + .schnorr_constraints = {}, + .ecdsa_constraints = { ecdsa_constraint }, + .sha256_constraints = {}, + .blake2s_constraints = {}, + .keccak_constraints = {}, + .hash_to_field_constraints = {}, + .pedersen_constraints = {}, + .compute_merkle_root_constraints = {}, + .constraints = {}, + }; + auto crs_factory = std::make_unique(); + auto composer = create_circuit(constraint_system, std::move(crs_factory)); +} + TEST(ECDSASecp256k1, TestECDSAConstraintFail) { acir_format::EcdsaSecp256k1Constraint ecdsa_constraint; From 67420575d00f9d164e7dbd864a72f9704215444f Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 17:26:21 +0100 Subject: [PATCH 2/8] Update cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp --- cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp index 8da70204ca..f09b09f032 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp @@ -40,7 +40,6 @@ size_t generate_ecdsa_constraint(acir_format::EcdsaSecp256k1Constraint& ecdsa_co const auto byte = static_cast(hashed_message[i]); witness_values.emplace_back(byte); } - std::cout << message_in.size() << std::endl; offset += message_in.size(); for (size_t i = 0; i < 32; ++i) { From 160559c1ceecdfb03b31dfb6a856e0e2494b883f Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 17:13:56 +0000 Subject: [PATCH 3/8] replace templates with concrete methods --- .../dsl/acir_format/acir_format.cpp | 48 +++++++++++++++-- .../dsl/acir_format/ecdsa_secp256k1.cpp | 51 +------------------ .../dsl/acir_format/ecdsa_secp256k1.hpp | 4 -- 3 files changed, 46 insertions(+), 57 deletions(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index ad2107bb9e..7d8cbea5f3 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -1,5 +1,6 @@ #include "acir_format.hpp" #include "barretenberg/common/log.hpp" +#include "barretenberg/crypto/ecdsa/ecdsa.hpp" namespace acir_format { @@ -10,6 +11,45 @@ void read_witness(Composer& composer, std::vector witness) composer.variables[i + 1] = witness[i]; } } +using curve = proof_system::plonk::stdlib::secp256k1; + +void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input) +{ + + std::vector pub_x_indices_; + std::vector pub_y_indices_; + std::vector signature_; + signature_.resize(64); + + crypto::ecdsa::key_pair account; + account.private_key = 10; + account.public_key = curve::g1::one * account.private_key; + uint256_t pub_x_value = account.public_key.x; + uint256_t pub_y_value = account.public_key.y; + std::string message_string = "Instructions unclear, ask again later."; + crypto::ecdsa::signature signature = + crypto::ecdsa::construct_signature(message_string, account); + for (size_t i = 0; i < 32; ++i) { + uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t r_wit = composer.add_variable(signature.r[i]); + uint32_t s_wit = composer.add_variable(signature.s[i]); + pub_x_indices_.emplace_back(x_wit); + pub_y_indices_.emplace_back(y_wit); + signature_[i] = r_wit; + signature_[i + 32] = s_wit; + } + + for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { + composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); + } + for (size_t i = 0; i < input.pub_y_indices.size(); ++i) { + composer.assert_equal(pub_y_indices_[i], input.pub_y_indices[i]); + } + for (size_t i = 0; i < input.signature.size(); ++i) { + composer.assert_equal(signature_[i], input.signature[i]); + } +} void create_circuit(Composer& composer, const acir_format& constraint_system) { @@ -62,6 +102,7 @@ void create_circuit(Composer& composer, const acir_format& constraint_system) // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { + dummy_ecdsa_constraint(composer, constraint); create_ecdsa_verify_constraints(composer, constraint); } @@ -145,6 +186,7 @@ Composer create_circuit(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { + dummy_ecdsa_constraint(composer, constraint); create_ecdsa_verify_constraints(composer, constraint); } @@ -234,7 +276,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -320,7 +362,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, std:: // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -404,7 +446,7 @@ void create_circuit_with_witness(Composer& composer, const acir_format& constrai // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp index ed31b92f41..6de8d6be0b 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp @@ -85,56 +85,9 @@ witness_ct ecdsa_index_to_witness(Composer& composer, uint32_t index) return { &composer, value }; } -template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input) { - { - std::vector pub_x_indices_; - std::vector pub_y_indices_; - std::vector signature_; - signature_.resize(64); - if constexpr (has_witness) { - for (size_t i = 0; i < 32; ++i) { - uint32_t x_wit = composer.add_variable(composer.get_variable(input.pub_x_indices[i])); - uint32_t y_wit = composer.add_variable(composer.get_variable(input.pub_y_indices[i])); - uint32_t r_wit = composer.add_variable(composer.get_variable(input.signature[i])); - uint32_t s_wit = composer.add_variable(composer.get_variable(input.signature[i + 32])); - pub_x_indices_.emplace_back(x_wit); - pub_y_indices_.emplace_back(y_wit); - signature_[i] = r_wit; - signature_[i + 32] = s_wit; - } - } else { - crypto::ecdsa::key_pair account; - account.private_key = 10; - account.public_key = curve::g1::one * account.private_key; - uint256_t pub_x_value = account.public_key.x; - uint256_t pub_y_value = account.public_key.y; - std::string message_string = "Instructions unclear, ask again later."; - crypto::ecdsa::signature signature = - crypto::ecdsa::construct_signature(message_string, - account); - for (size_t i = 0; i < 32; ++i) { - uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); - uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); - uint32_t r_wit = composer.add_variable(signature.r[i]); - uint32_t s_wit = composer.add_variable(signature.s[i]); - pub_x_indices_.emplace_back(x_wit); - pub_y_indices_.emplace_back(y_wit); - signature_[i] = r_wit; - signature_[i + 32] = s_wit; - } - } - for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { - composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); - } - for (size_t i = 0; i < input.pub_y_indices.size(); ++i) { - composer.assert_equal(pub_y_indices_[i], input.pub_y_indices[i]); - } - for (size_t i = 0; i < input.signature.size(); ++i) { - composer.assert_equal(signature_[i], input.signature[i]); - } - } + auto new_sig = ecdsa_convert_signature(composer, input.signature); auto message = ecdsa_vector_of_bytes_to_byte_array(composer, input.hashed_message); @@ -174,7 +127,5 @@ void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Con bool_ct signature_result_normalized = signature_result.normalize(); composer.assert_equal(signature_result_normalized.witness_index, input.result); } -template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); -template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); } // namespace acir_format diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp index 96ff9c3f95..93a87d386c 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp @@ -26,12 +26,8 @@ struct EcdsaSecp256k1Constraint { friend bool operator==(EcdsaSecp256k1Constraint const& lhs, EcdsaSecp256k1Constraint const& rhs) = default; }; -template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); -extern template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); -extern template void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); - template inline void read(B& buf, EcdsaSecp256k1Constraint& constraint) { using serialize::read; From cbe01404ed41f10e37acc964007c641692a8811e Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 17:15:33 +0000 Subject: [PATCH 4/8] add comment --- cpp/src/barretenberg/dsl/acir_format/acir_format.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 7d8cbea5f3..72ca5bd13b 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -13,6 +13,11 @@ void read_witness(Composer& composer, std::vector witness) } using curve = proof_system::plonk::stdlib::secp256k1; +// Add dummy constraints for ECDSA because when the verifier creates the +// constraint system, they usually use zeroes for witness values. +// +// This does not work for ECDSA as the signature, r, s and public key need +// to be valid. void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input) { From a27c87aa7198a5b323b84bc0eb42d63548b9ac02 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 20:11:07 +0000 Subject: [PATCH 5/8] PR review --- cpp/src/barretenberg/dsl/acir_format/acir_format.cpp | 8 ++++---- cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp | 1 - .../barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp | 5 ++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 72ca5bd13b..3cdab8d52b 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -11,7 +11,6 @@ void read_witness(Composer& composer, std::vector witness) composer.variables[i + 1] = witness[i]; } } -using curve = proof_system::plonk::stdlib::secp256k1; // Add dummy constraints for ECDSA because when the verifier creates the // constraint system, they usually use zeroes for witness values. @@ -26,14 +25,15 @@ void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& std::vector signature_; signature_.resize(64); - crypto::ecdsa::key_pair account; + crypto::ecdsa::key_pair account; account.private_key = 10; - account.public_key = curve::g1::one * account.private_key; + account.public_key = secp256k1_ct::g1::one * account.private_key; uint256_t pub_x_value = account.public_key.x; uint256_t pub_y_value = account.public_key.y; std::string message_string = "Instructions unclear, ask again later."; crypto::ecdsa::signature signature = - crypto::ecdsa::construct_signature(message_string, account); + crypto::ecdsa::construct_signature( + message_string, account); for (size_t i = 0; i < 32; ++i) { uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp index 6de8d6be0b..5a9caaa699 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp @@ -5,7 +5,6 @@ namespace acir_format { using namespace proof_system::plonk; -using curve = proof_system::plonk::stdlib::secp256k1; crypto::ecdsa::signature ecdsa_convert_signature(Composer& composer, std::vector signature) { diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp index f09b09f032..4e2e25174a 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp @@ -110,7 +110,10 @@ TEST(ECDSASecp256k1, TestECDSAConstraintSucceed) EXPECT_EQ(verifier.verify_proof(proof), true); } -TEST(ECDSASecp256k1, TestECDSAConstraintSucceedSeparate) +// Test that the verifier can create an ECDSA circuit. +// The ECDSA circuit requires that certain dummy data is valid +// even though we are just building the circuit. +TEST(ECDSASecp256k1, TestECDSACompilesForVerifier) { acir_format::EcdsaSecp256k1Constraint ecdsa_constraint; std::vector witness_values; From 64d3d00fd6e950790455e3cd2dd665a34e1db87f Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 20:17:16 +0000 Subject: [PATCH 6/8] add comments --- cpp/src/barretenberg/dsl/acir_format/acir_format.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 3cdab8d52b..7bb2247379 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -25,6 +25,7 @@ void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& std::vector signature_; signature_.resize(64); + // Create a valid signature with a valid public key crypto::ecdsa::key_pair account; account.private_key = 10; account.public_key = secp256k1_ct::g1::one * account.private_key; @@ -34,6 +35,10 @@ void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& crypto::ecdsa::signature signature = crypto::ecdsa::construct_signature( message_string, account); + + // Create new variables which will reference the valid public key and signature. + // We don't use them in a gate, so when we call assert_equal, they will be + // replaced as if they never existed. for (size_t i = 0; i < 32; ++i) { uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); @@ -45,6 +50,7 @@ void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& signature_[i + 32] = s_wit; } + // Call assert_equal(from, to) to replace the value in `to` by the value in `from` for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); } @@ -68,7 +74,6 @@ void create_circuit(Composer& composer, const acir_format& constraint_system) if (std::find(constraint_system.public_inputs.begin(), constraint_system.public_inputs.end(), i) != constraint_system.public_inputs.end()) { composer.add_public_variable(0); - } else { composer.add_variable(0); } From abfa269050913294d43f1a4a8cdae7b97a846bd1 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 20:25:04 +0000 Subject: [PATCH 7/8] change to use boolean flag, so dummy_ecdsa method lives in ecdsa --- .../dsl/acir_format/acir_format.cpp | 63 ++----------------- .../dsl/acir_format/ecdsa_secp256k1.cpp | 58 ++++++++++++++++- .../dsl/acir_format/ecdsa_secp256k1.hpp | 6 +- 3 files changed, 67 insertions(+), 60 deletions(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index 7bb2247379..c73e3eeda2 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -1,6 +1,5 @@ #include "acir_format.hpp" #include "barretenberg/common/log.hpp" -#include "barretenberg/crypto/ecdsa/ecdsa.hpp" namespace acir_format { @@ -12,56 +11,6 @@ void read_witness(Composer& composer, std::vector witness) } } -// Add dummy constraints for ECDSA because when the verifier creates the -// constraint system, they usually use zeroes for witness values. -// -// This does not work for ECDSA as the signature, r, s and public key need -// to be valid. -void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input) -{ - - std::vector pub_x_indices_; - std::vector pub_y_indices_; - std::vector signature_; - signature_.resize(64); - - // Create a valid signature with a valid public key - crypto::ecdsa::key_pair account; - account.private_key = 10; - account.public_key = secp256k1_ct::g1::one * account.private_key; - uint256_t pub_x_value = account.public_key.x; - uint256_t pub_y_value = account.public_key.y; - std::string message_string = "Instructions unclear, ask again later."; - crypto::ecdsa::signature signature = - crypto::ecdsa::construct_signature( - message_string, account); - - // Create new variables which will reference the valid public key and signature. - // We don't use them in a gate, so when we call assert_equal, they will be - // replaced as if they never existed. - for (size_t i = 0; i < 32; ++i) { - uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); - uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); - uint32_t r_wit = composer.add_variable(signature.r[i]); - uint32_t s_wit = composer.add_variable(signature.s[i]); - pub_x_indices_.emplace_back(x_wit); - pub_y_indices_.emplace_back(y_wit); - signature_[i] = r_wit; - signature_[i + 32] = s_wit; - } - - // Call assert_equal(from, to) to replace the value in `to` by the value in `from` - for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { - composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); - } - for (size_t i = 0; i < input.pub_y_indices.size(); ++i) { - composer.assert_equal(pub_y_indices_[i], input.pub_y_indices[i]); - } - for (size_t i = 0; i < input.signature.size(); ++i) { - composer.assert_equal(signature_[i], input.signature[i]); - } -} - void create_circuit(Composer& composer, const acir_format& constraint_system) { if (constraint_system.public_inputs.size() > constraint_system.varnum) { @@ -112,8 +61,7 @@ void create_circuit(Composer& composer, const acir_format& constraint_system) // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - dummy_ecdsa_constraint(composer, constraint); - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint, false); } // Add blake2s constraints @@ -196,8 +144,7 @@ Composer create_circuit(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - dummy_ecdsa_constraint(composer, constraint); - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint, false); } // Add blake2s constraints @@ -286,7 +233,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint, true); } // Add blake2s constraints @@ -372,7 +319,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, std:: // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint, true); } // Add blake2s constraints @@ -456,7 +403,7 @@ void create_circuit_with_witness(Composer& composer, const acir_format& constrai // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint); + create_ecdsa_verify_constraints(composer, constraint, true); } // Add blake2s constraints diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp index 5a9caaa699..31cddb3e2f 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.cpp @@ -84,9 +84,15 @@ witness_ct ecdsa_index_to_witness(Composer& composer, uint32_t index) return { &composer, value }; } -void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input) +void create_ecdsa_verify_constraints(Composer& composer, + const EcdsaSecp256k1Constraint& input, + bool has_valid_witness_assignments) { + if (has_valid_witness_assignments == false) { + dummy_ecdsa_constraint(composer, input); + } + auto new_sig = ecdsa_convert_signature(composer, input.signature); auto message = ecdsa_vector_of_bytes_to_byte_array(composer, input.hashed_message); @@ -127,4 +133,54 @@ void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Con composer.assert_equal(signature_result_normalized.witness_index, input.result); } +// Add dummy constraints for ECDSA because when the verifier creates the +// constraint system, they usually use zeroes for witness values. +// +// This does not work for ECDSA as the signature, r, s and public key need +// to be valid. +void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input) +{ + + std::vector pub_x_indices_; + std::vector pub_y_indices_; + std::vector signature_; + signature_.resize(64); + + // Create a valid signature with a valid public key + crypto::ecdsa::key_pair account; + account.private_key = 10; + account.public_key = secp256k1_ct::g1::one * account.private_key; + uint256_t pub_x_value = account.public_key.x; + uint256_t pub_y_value = account.public_key.y; + std::string message_string = "Instructions unclear, ask again later."; + crypto::ecdsa::signature signature = + crypto::ecdsa::construct_signature( + message_string, account); + + // Create new variables which will reference the valid public key and signature. + // We don't use them in a gate, so when we call assert_equal, they will be + // replaced as if they never existed. + for (size_t i = 0; i < 32; ++i) { + uint32_t x_wit = composer.add_variable(pub_x_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t y_wit = composer.add_variable(pub_y_value.slice(248 - i * 8, 256 - i * 8)); + uint32_t r_wit = composer.add_variable(signature.r[i]); + uint32_t s_wit = composer.add_variable(signature.s[i]); + pub_x_indices_.emplace_back(x_wit); + pub_y_indices_.emplace_back(y_wit); + signature_[i] = r_wit; + signature_[i + 32] = s_wit; + } + + // Call assert_equal(from, to) to replace the value in `to` by the value in `from` + for (size_t i = 0; i < input.pub_x_indices.size(); ++i) { + composer.assert_equal(pub_x_indices_[i], input.pub_x_indices[i]); + } + for (size_t i = 0; i < input.pub_y_indices.size(); ++i) { + composer.assert_equal(pub_y_indices_[i], input.pub_y_indices[i]); + } + for (size_t i = 0; i < input.signature.size(); ++i) { + composer.assert_equal(signature_[i], input.signature[i]); + } +} + } // namespace acir_format diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp index 93a87d386c..bd578f2036 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp @@ -26,7 +26,11 @@ struct EcdsaSecp256k1Constraint { friend bool operator==(EcdsaSecp256k1Constraint const& lhs, EcdsaSecp256k1Constraint const& rhs) = default; }; -void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input); +void create_ecdsa_verify_constraints(Composer& composer, + const EcdsaSecp256k1Constraint& input, + bool has_valid_witness_assignments); + +void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input); template inline void read(B& buf, EcdsaSecp256k1Constraint& constraint) { From 5d19d8ca2d1078a175bc0400db4ecdccbebaa8cb Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 11 May 2023 20:40:36 +0000 Subject: [PATCH 8/8] ad true as default --- cpp/src/barretenberg/dsl/acir_format/acir_format.cpp | 6 +++--- cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp index c73e3eeda2..0e2e510f4d 100644 --- a/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp +++ b/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp @@ -233,7 +233,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint, true); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -319,7 +319,7 @@ Composer create_circuit_with_witness(const acir_format& constraint_system, std:: // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint, true); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints @@ -403,7 +403,7 @@ void create_circuit_with_witness(Composer& composer, const acir_format& constrai // Add ECDSA constraints for (const auto& constraint : constraint_system.ecdsa_constraints) { - create_ecdsa_verify_constraints(composer, constraint, true); + create_ecdsa_verify_constraints(composer, constraint); } // Add blake2s constraints diff --git a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp index bd578f2036..59d1fe48e0 100644 --- a/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp +++ b/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.hpp @@ -28,7 +28,7 @@ struct EcdsaSecp256k1Constraint { void create_ecdsa_verify_constraints(Composer& composer, const EcdsaSecp256k1Constraint& input, - bool has_valid_witness_assignments); + bool has_valid_witness_assignments = true); void dummy_ecdsa_constraint(Composer& composer, EcdsaSecp256k1Constraint const& input);