Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: replace usage of vector in keccakf1600 input with array #9350

Merged
merged 10 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,9 +1012,9 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
..Default::default()
});
}
BlackBoxOp::Keccakf1600 { message, output } => {
let message_offset = message.pointer.to_usize();
let message_size_offset = message.size.to_usize();
BlackBoxOp::Keccakf1600 { input, output } => {
let input_offset = input.pointer.to_usize();
assert_eq!(input.size, 25, "Keccakf1600 input size must be 25!");
let dest_offset = output.pointer.to_usize();
assert_eq!(output.size, 25, "Keccakf1600 output size must be 25!");

Expand All @@ -1023,14 +1023,12 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
indirect: Some(
AddressingModeBuilder::default()
.indirect_operand(&output.pointer)
.indirect_operand(&message.pointer)
.direct_operand(&message.size)
.indirect_operand(&input.pointer)
.build(),
),
operands: vec![
AvmOperand::U16 { value: dest_offset as u16 },
AvmOperand::U16 { value: message_offset as u16 },
AvmOperand::U16 { value: message_size_offset as u16 },
AvmOperand::U16 { value: input_offset as u16 },
],
..Default::default()
});
Expand Down
12 changes: 6 additions & 6 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ struct BlackBoxOp {
};

struct Keccakf1600 {
Program::HeapVector message;
Program::HeapArray input;
Program::HeapArray output;

friend bool operator==(const Keccakf1600&, const Keccakf1600&);
Expand Down Expand Up @@ -424,8 +424,8 @@ struct BlackBoxOp {
};

struct Sha256Compression {
Program::HeapVector input;
Program::HeapVector hash_values;
Program::HeapArray input;
Program::HeapArray hash_values;
Program::HeapArray output;

friend bool operator==(const Sha256Compression&, const Sha256Compression&);
Expand Down Expand Up @@ -4061,7 +4061,7 @@ namespace Program {

inline bool operator==(const BlackBoxOp::Keccakf1600& lhs, const BlackBoxOp::Keccakf1600& rhs)
{
if (!(lhs.message == rhs.message)) {
if (!(lhs.input == rhs.input)) {
return false;
}
if (!(lhs.output == rhs.output)) {
Expand Down Expand Up @@ -4094,7 +4094,7 @@ template <typename Serializer>
void serde::Serializable<Program::BlackBoxOp::Keccakf1600>::serialize(const Program::BlackBoxOp::Keccakf1600& obj,
Serializer& serializer)
{
serde::Serializable<decltype(obj.message)>::serialize(obj.message, serializer);
serde::Serializable<decltype(obj.input)>::serialize(obj.input, serializer);
serde::Serializable<decltype(obj.output)>::serialize(obj.output, serializer);
}

Expand All @@ -4104,7 +4104,7 @@ Program::BlackBoxOp::Keccakf1600 serde::Deserializable<Program::BlackBoxOp::Kecc
Deserializer& deserializer)
{
Program::BlackBoxOp::Keccakf1600 obj;
obj.message = serde::Deserializable<decltype(obj.message)>::deserialize(deserializer);
obj.input = serde::Deserializable<decltype(obj.input)>::deserialize(deserializer);
obj.output = serde::Deserializable<decltype(obj.output)>::deserialize(deserializer);
return obj;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ TEST_F(AvmExecutionTests, keccakf1600OpCode)

std::string bytecode_preamble;
// Set operations for keccak state
for (uint8_t i = 0; i < 25; i++) {
for (uint8_t i = 0; i < KECCAKF1600_INPUT_SIZE; i++) {
bytecode_preamble += to_hex(OpCode::SET_64) + // opcode SET
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U64) + to_hex<uint64_t>(state[i]) + // val i
Expand All @@ -1012,13 +1012,8 @@ TEST_F(AvmExecutionTests, keccakf1600OpCode)
to_hex(OpCode::SET_8) + // opcode SET for indirect src (input)
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
"01" // value 1 (i.e. where the src will be read from)
"24" // input_offset 36
+ to_hex(OpCode::SET_8) + //
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
"19" // value 25 (i.e. where the length parameter is stored)
"25" // input_offset 37
"01" // value 1 (i.e. where the src will be read from)
"24" // input_offset 36
+ to_hex(OpCode::SET_16) + // opcode SET for indirect dst (output)
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
Expand All @@ -1028,7 +1023,6 @@ TEST_F(AvmExecutionTests, keccakf1600OpCode)
"03" // Indirect flag (first 2 operands indirect)
"0023" // output offset (indirect 35)
"0024" // input offset (indirect 36)
"0025" // length offset 37
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"0100" // ret offset 256
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
{ OpCode::POSEIDON2PERM, { OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16 } },
{ OpCode::SHA256COMPRESSION,
{ OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16 } },
{ OpCode::KECCAKF1600, { OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16 } },
{ OpCode::KECCAKF1600, { OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16 } },
// TEMP ECADD without relative memory
{ OpCode::ECADD,
{ OperandType::INDIRECT16,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,7 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
case OpCode::KECCAKF1600:
trace_builder.op_keccakf1600(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint16_t>(inst.operands.at(1)),
std::get<uint16_t>(inst.operands.at(2)),
std::get<uint16_t>(inst.operands.at(3)));
std::get<uint16_t>(inst.operands.at(2)));

break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,27 @@ void AvmKeccakTraceBuilder::reset()
keccak_trace.shrink_to_fit(); // Reclaim memory.
}

std::array<uint64_t, 25> AvmKeccakTraceBuilder::keccakf1600(uint32_t clk, std::array<uint64_t, 25> input)
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> AvmKeccakTraceBuilder::keccakf1600(
uint32_t clk, std::array<uint64_t, KECCAKF1600_INPUT_SIZE> input)
{
// BB's Eth hash function uses C-style arrays, while we like to use std::array
// We do a few conversions for here but maybe we will update later.
uint64_t state[25] = {};
uint64_t state[KECCAKF1600_INPUT_SIZE] = {};
std::copy(input.begin(), input.end(), state);
std::vector<uint64_t> input_vector(input.begin(), input.end());
// This function mutates state
ethash_keccakf1600(state);
std::array<uint64_t, 25> output = {};
for (size_t i = 0; i < 25; i++) {
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> output = {};
for (size_t i = 0; i < KECCAKF1600_INPUT_SIZE; i++) {
output[i] = state[i];
}
std::vector<uint64_t> output_vector(output.begin(), output.end());
keccak_trace.push_back(KeccakTraceEntry{
.clk = clk,
.input = input_vector,
.output = output_vector,
.input_size = 25,
.output_size = 25,
.input_size = KECCAKF1600_INPUT_SIZE,
.output_size = KECCAKF1600_INPUT_SIZE,
});
return output;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

namespace bb::avm_trace {

constexpr uint32_t KECCAKF1600_INPUT_SIZE = 25;

class AvmKeccakTraceBuilder {
public:
struct KeccakTraceEntry {
Expand All @@ -23,7 +25,8 @@ class AvmKeccakTraceBuilder {
// Finalize the trace
std::vector<KeccakTraceEntry> finalize();

std::array<uint64_t, 25> keccakf1600(uint32_t clk, std::array<uint64_t, 25> input);
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> keccakf1600(uint32_t clk,
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> input);

private:
std::vector<KeccakTraceEntry> keccak_trace;
Expand Down
25 changes: 8 additions & 17 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "barretenberg/vm/avm/trace/fixed_gas.hpp"
#include "barretenberg/vm/avm/trace/fixed_powers.hpp"
#include "barretenberg/vm/avm/trace/gadgets/cmp.hpp"
#include "barretenberg/vm/avm/trace/gadgets/keccak.hpp"
#include "barretenberg/vm/avm/trace/gadgets/slice_trace.hpp"
#include "barretenberg/vm/avm/trace/helper.hpp"
#include "barretenberg/vm/avm/trace/opcode.hpp"
Expand Down Expand Up @@ -2950,27 +2951,17 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect,

/**
* @brief Keccakf1600 with direct or indirect memory access.
* This function temporarily has the same interface as the kecccak opcode for compatibility, when the keccak
* migration is complete (to keccakf1600) We will update this function call as we will not likely need
* input_size_offset
* @param indirect byte encoding information about indirect/direct memory access.
* @param output_offset An index in memory pointing to where the first u64 value of the output array should be
* stored.
* @param input_offset An index in memory pointing to the first u64 value of the input array to be used in the next
* instance of poseidon2 permutation.
* @param input_size offset An index in memory pointing to the size of the input array. Temporary while we maintain
* the same interface as keccak (this is fixed to 25)
* instance of keccakf1600.
*/
void AvmTraceBuilder::op_keccakf1600(uint8_t indirect,
uint32_t output_offset,
uint32_t input_offset,
[[maybe_unused]] uint32_t input_size_offset)
void AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, uint32_t input_offset)
{
// What happens if the input_size_offset is > 25 when the state is more that that?
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;
auto [resolved_output_offset, resolved_input_offset, _] =
Addressing<3>::fromWire(indirect, call_ptr)
.resolve({ output_offset, input_offset, input_size_offset }, mem_trace_builder);
auto [resolved_output_offset, resolved_input_offset] =
Addressing<2>::fromWire(indirect, call_ptr).resolve({ output_offset, input_offset }, mem_trace_builder);
auto input_read = constrained_read_from_memory(
call_ptr, clk, resolved_input_offset, AvmMemoryTag::U64, AvmMemoryTag::FF, IntermRegister::IA);
auto output_read = constrained_read_from_memory(
Expand Down Expand Up @@ -3002,13 +2993,13 @@ void AvmTraceBuilder::op_keccakf1600(uint8_t indirect,
// Array input is fixed to 1600 bits
std::vector<uint64_t> input_vec;
// Read results are written to input array
read_slice_from_memory<uint64_t>(resolved_input_offset, 25, input_vec);
std::array<uint64_t, 25> input = vec_to_arr<uint64_t, 25>(input_vec);
read_slice_from_memory<uint64_t>(resolved_input_offset, KECCAKF1600_INPUT_SIZE, input_vec);
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> input = vec_to_arr<uint64_t, KECCAKF1600_INPUT_SIZE>(input_vec);

// Now that we have read all the values, we can perform the operation to get the resulting witness.
// Note: We use the keccak_op_clk to ensure that the keccakf1600 operation is performed at the same clock cycle
// as the main trace that has the selector
std::array<uint64_t, 25> result = keccak_trace_builder.keccakf1600(clk, input);
std::array<uint64_t, KECCAKF1600_INPUT_SIZE> result = keccak_trace_builder.keccakf1600(clk, input);
// Write the result to memory after
write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result);
}
Expand Down
3 changes: 1 addition & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ class AvmTraceBuilder {
std::vector<FF> op_revert(uint8_t indirect, uint32_t ret_offset, uint32_t ret_size);

// Gadgets
void op_keccak(uint8_t indirect, uint32_t output_offset, uint32_t input_offset, uint32_t input_size_offset);
void op_poseidon2_permutation(uint8_t indirect, uint32_t input_offset, uint32_t output_offset);
void op_ec_add(uint16_t indirect,
uint32_t lhs_x_offset,
Expand All @@ -167,7 +166,7 @@ class AvmTraceBuilder {

// Future Gadgets -- pending changes in noir
void op_sha256_compression(uint8_t indirect, uint32_t output_offset, uint32_t state_offset, uint32_t inputs_offset);
void op_keccakf1600(uint8_t indirect, uint32_t output_offset, uint32_t input_offset, uint32_t input_size_offset);
void op_keccakf1600(uint8_t indirect, uint32_t output_offset, uint32_t input_offset);

std::vector<Row> finalize();
void reset();
Expand Down
12 changes: 6 additions & 6 deletions noir/noir-repo/acvm-repo/acir/codegen/acir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ namespace Program {
};

struct Keccakf1600 {
Program::HeapVector message;
Program::HeapArray input;
Program::HeapArray output;

friend bool operator==(const Keccakf1600&, const Keccakf1600&);
Expand Down Expand Up @@ -424,8 +424,8 @@ namespace Program {
};

struct Sha256Compression {
Program::HeapVector input;
Program::HeapVector hash_values;
Program::HeapArray input;
Program::HeapArray hash_values;
Program::HeapArray output;

friend bool operator==(const Sha256Compression&, const Sha256Compression&);
Expand Down Expand Up @@ -3498,7 +3498,7 @@ Program::BlackBoxOp::Blake3 serde::Deserializable<Program::BlackBoxOp::Blake3>::
namespace Program {

inline bool operator==(const BlackBoxOp::Keccakf1600 &lhs, const BlackBoxOp::Keccakf1600 &rhs) {
if (!(lhs.message == rhs.message)) { return false; }
if (!(lhs.input == rhs.input)) { return false; }
if (!(lhs.output == rhs.output)) { return false; }
return true;
}
Expand All @@ -3523,15 +3523,15 @@ namespace Program {
template <>
template <typename Serializer>
void serde::Serializable<Program::BlackBoxOp::Keccakf1600>::serialize(const Program::BlackBoxOp::Keccakf1600 &obj, Serializer &serializer) {
serde::Serializable<decltype(obj.message)>::serialize(obj.message, serializer);
serde::Serializable<decltype(obj.input)>::serialize(obj.input, serializer);
serde::Serializable<decltype(obj.output)>::serialize(obj.output, serializer);
}

template <>
template <typename Deserializer>
Program::BlackBoxOp::Keccakf1600 serde::Deserializable<Program::BlackBoxOp::Keccakf1600>::deserialize(Deserializer &deserializer) {
Program::BlackBoxOp::Keccakf1600 obj;
obj.message = serde::Deserializable<decltype(obj.message)>::deserialize(deserializer);
obj.input = serde::Deserializable<decltype(obj.input)>::deserialize(deserializer);
obj.output = serde::Deserializable<decltype(obj.output)>::deserialize(deserializer);
return obj;
}
Expand Down
6 changes: 3 additions & 3 deletions noir/noir-repo/acvm-repo/brillig/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub enum BlackBoxOp {
},
/// Keccak Permutation function of 1600 width
Keccakf1600 {
message: HeapVector,
input: HeapArray,
output: HeapArray,
},
/// Verifies a ECDSA signature over the secp256k1 curve.
Expand Down Expand Up @@ -102,8 +102,8 @@ pub enum BlackBoxOp {
len: MemoryAddress,
},
Sha256Compression {
input: HeapVector,
hash_values: HeapVector,
input: HeapArray,
hash_values: HeapArray,
output: HeapArray,
},
ToRadix {
Expand Down
8 changes: 4 additions & 4 deletions noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
memory.write_slice(memory.read_ref(output.pointer), &to_value_vec(&bytes));
Ok(())
}
BlackBoxOp::Keccakf1600 { message, output } => {
let state_vec: Vec<u64> = read_heap_vector(memory, message)
BlackBoxOp::Keccakf1600 { input, output } => {
let state_vec: Vec<u64> = read_heap_array(memory, input)
.iter()
.map(|&memory_value| memory_value.try_into().unwrap())
.collect();
Expand Down Expand Up @@ -292,7 +292,7 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
}
BlackBoxOp::Sha256Compression { input, hash_values, output } => {
let mut message = [0; 16];
let inputs = read_heap_vector(memory, input);
let inputs = read_heap_array(memory, input);
if inputs.len() != 16 {
return Err(BlackBoxResolutionError::Failed(
BlackBoxFunc::Sha256Compression,
Expand All @@ -303,7 +303,7 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
message[i] = input.try_into().unwrap();
}
let mut state = [0; 8];
let values = read_heap_vector(memory, hash_values);
let values = read_heap_array(memory, hash_values);
if values.len() != 8 {
return Err(BlackBoxResolutionError::Failed(
BlackBoxFunc::Sha256Compression,
Expand Down
Loading
Loading