3791 - address other review comments

barretenberg/cpp/src/barretenberg/common/utils.cpp

@@ -0,0 +1,17 @@
+#include "./utils.hpp"
+
+namespace bb::utils {
+
+std::vector<uint8_t> hex_to_bytes(const std::string& hex)
+{
+    std::vector<uint8_t> bytes;
+
+    for (unsigned int i = 0; i < hex.length(); i += 2) {
+        std::string byteString = hex.substr(i, 2);
+        bytes.push_back(static_cast<uint8_t>(strtol(byteString.c_str(), nullptr, 16)));
+    }
+
+    return bytes;
+}
+
+} // namespace bb::utils

barretenberg/cpp/src/barretenberg/common/utils.hpp

@@ -0,0 +1,17 @@
+#pragma once
+
+#include <cstdint>
+#include <string>
+#include <vector>
+
+namespace bb::utils {
+
+/**
+ * @brief Routine to transform hexstring to vector of bytes.
+ *
+ * @param Hexadecimal string representation.
+ * @return Vector of uint8_t values.
+ */
+std::vector<uint8_t> hex_to_bytes(const std::string& hex);
+
+} // namespace bb::utils

barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.test.cpp

@@ -1,5 +1,6 @@
 #include "ecdsa.hpp"
 #include "barretenberg/common/serialize.hpp"
+#include "barretenberg/common/utils.hpp"
 #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
 #include "barretenberg/ecc/curves/secp256r1/secp256r1.hpp"
 #include "barretenberg/serialize/test_helper.hpp"
@@ -89,23 +90,10 @@ TEST(ecdsa, recover_public_key_secp256r1_sha256)
     EXPECT_EQ(recovered_public_key, account.public_key);
 }
 
-std::vector<uint8_t> HexToBytes(const std::string& hex)
-{
-    std::vector<uint8_t> 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(), NULL, 16);
-        bytes.push_back(byte);
-    }
-
-    return bytes;
-}
-
 TEST(ecdsa, check_overflowing_r_and_s_are_rejected)
 {
 
-    std::vector<uint8_t> message_vec = HexToBytes("41414141");
+    std::vector<uint8_t> message_vec = utils::hex_to_bytes("41414141");
 
     std::string message(message_vec.begin(), message_vec.end());
     crypto::ecdsa_signature signature;
@@ -181,10 +169,10 @@ TEST(ecdsa, verify_signature_secp256r1_sha256_NIST_1)
     };
 
     crypto::ecdsa_signature sig{ r, s, 27 };
-    std::vector<uint8_t> message_vec =
-        HexToBytes("5905238877c77421f73e43ee3da6f2d9e2ccad5fc942dcec0cbd25482935faaf416983fe165b1a045ee2bcd2e6dca3bdf46"
-                   "c4310a7461f9a37960ca672d3feb5473e253605fb1ddfd28065b53cb5858a8ad28175bf9bd386a5e471ea7a65c17cc934a9"
-                   "d791e91491eb3754d03799790fe2d308d16146d5c9b0d0debd97d79ce8");
+    std::vector<uint8_t> message_vec = utils::hex_to_bytes(
+        "5905238877c77421f73e43ee3da6f2d9e2ccad5fc942dcec0cbd25482935faaf416983fe165b1a045ee2bcd2e6dca3bdf46"
+        "c4310a7461f9a37960ca672d3feb5473e253605fb1ddfd28065b53cb5858a8ad28175bf9bd386a5e471ea7a65c17cc934a9"
+        "d791e91491eb3754d03799790fe2d308d16146d5c9b0d0debd97d79ce8");
     std::string message(message_vec.begin(), message_vec.end());
 
     bool result = crypto::ecdsa_verify_signature<Sha256Hasher, secp256r1::fq, secp256r1::fr, secp256r1::g1>(

barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp

@@ -49,8 +49,6 @@ std::vector<Instruction> Execution::parse(std::vector<uint8_t> const& bytecode)
     size_t pos = 0;
     const auto length = bytecode.size();
 
-    static_assert(sizeof(uint32_t) / sizeof(uint8_t) == AVM_OPERAND_BYTE_LENGTH);
-
     while (pos < length) {
         const uint8_t opcode_byte = bytecode.at(pos);
         pos += AVM_OPCODE_BYTE_LENGTH;

barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.hpp

@@ -15,8 +15,10 @@ class Execution {
     Execution() = default;
 
     static size_t const AVM_OPERAND_BYTE_LENGTH = 4; // Keep in sync with TS code
-    static size_t const AVM_OPCODE_BYTE_LENGTH = 1;  // Keep in sync with TS code
-    static size_t const AVM_IN_TAG_BYTE_LENGTH = 1;  // Keep in sync with TS code
+    static_assert(sizeof(uint32_t) / sizeof(uint8_t) == AVM_OPERAND_BYTE_LENGTH);
+
+    static size_t const AVM_OPCODE_BYTE_LENGTH = 1; // Keep in sync with TS code
+    static size_t const AVM_IN_TAG_BYTE_LENGTH = 1; // Keep in sync with TS code
 
     static std::vector<Instruction> parse(std::vector<uint8_t> const& bytecode);
     static std::vector<Row> gen_trace(std::vector<Instruction> const& instructions, std::vector<FF> const& calldata);

barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp

@@ -1,5 +1,6 @@
 #include "barretenberg/vm/avm_trace/AvmMini_execution.hpp"
 #include "AvmMini_common.test.hpp"
+#include "barretenberg/common/utils.hpp"
 #include "barretenberg/vm/avm_trace/AvmMini_common.hpp"
 #include "barretenberg/vm/avm_trace/AvmMini_helper.hpp"
 #include "barretenberg/vm/avm_trace/AvmMini_opcode.hpp"
@@ -9,25 +10,9 @@
 #include <string>
 #include <utility>
 
-namespace {
-
-// TODO: move to a common utils BB file (following code copied from ecdsa.test.cpp)
-std::vector<uint8_t> hex_to_bytes(const std::string& hex)
-{
-    std::vector<uint8_t> bytes;
-
-    for (unsigned int i = 0; i < hex.length(); i += 2) {
-        std::string byteString = hex.substr(i, 2);
-        bytes.push_back(static_cast<uint8_t>(strtol(byteString.c_str(), nullptr, 16)));
-    }
-
-    return bytes;
-}
-
-} // anonymous namespace
-
 namespace tests_avm {
 using namespace avm_trace;
+using bb::utils::hex_to_bytes;
 
 class AvmMiniExecutionTests : public ::testing::Test {
   public:
@@ -219,7 +204,7 @@ TEST_F(AvmMiniExecutionTests, powerWithMulOpcodes)
 
     auto trace = Execution::gen_trace(instructions, std::vector<FF>{});
 
-    // Find the first row enabling the subtraction selector
+    // Find the first row enabling the multiplication selector and pc = 13
     auto row = std::ranges::find_if(
         trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_mul == 1 && r.avmMini_pc == 13; });
     EXPECT_EQ(row->avmMini_ic, 244140625); // 5^12 = 244140625