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.

Sign up for GitHub

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

refactor(avm): type aliasing for VmPublicInputs #8884

Merged
merged 1 commit into from
Oct 10, 2024
Merged

refactor(avm): type aliasing for VmPublicInputs #8884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/dsl/acir_format/avm_recursion_constraint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ using namespace bb;
using field_ct = stdlib::field_t<Builder>;
using bn254 = stdlib::bn254<Builder>;
using aggregation_state_ct = bb::stdlib::recursion::aggregation_state<bn254>;
using VmPublicInputs = avm_trace::VmPublicInputs_<field_ct>;

namespace {
/**
Expand Down Expand Up @@ -184,7 +185,7 @@ AggregationObjectIndices create_avm_recursion_constraints(Builder& builder,
const auto public_inputs_flattened = fields_from_witnesses(input.public_inputs);

auto it = public_inputs_flattened.begin();
avm_trace::VmPublicInputs<field_ct> vm_public_inputs =
VmPublicInputs vm_public_inputs =
avm_trace::convert_public_inputs(std::vector(it, it + PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH));
it += PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH;
std::vector<field_ct> calldata(it, it + AVM_PUBLIC_COLUMN_MAX_SIZE);
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/tests/helpers.test.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ using FF = bb::AvmFlavorSettings::FF;
using Row = bb::AvmFullRow<bb::fr>;
using ThreeOpParam = std::array<FF, 3>;
using ThreeOpParamRow = std::tuple<ThreeOpParam, bb::avm_trace::AvmMemoryTag>;
using VmPublicInputsNT = bb::avm_trace::VmPublicInputs<FF>;
using VmPublicInputsNT = bb::avm_trace::VmPublicInputs_<FF>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could rename this one to VmPublicInputs or alternatively we are explicit everywhere by defining VmPublicInputsNT and VmPublicInputsCT (which personally I would prefer).

Copy link
Contributor

@jeanmon jeanmon Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could say that the default is native and for the circuit type we are explicit in the name:
using VmPublicInputs = bb::avm_trace::VmPublicInputs_<FF>;
using VmPublicInputsCT = bb::avm_trace::VmPublicInputs_<field_t<.....>>;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah this isnt too bad in the future once we decide which naming scheme we want to use. I just did a mass rename so i didnt run into painful merge conflicts 🫠


// If the test is expecting a relation to fail, then use validate_trace_check_circuit.
// Otherwise, use validate_trace with a single argument. If the proving needs to be
Expand Down
11 changes: 7 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ using FF = AvmFlavorSettings::FF;

// There are 4 public input columns, 1 for context inputs, and 3 for emitting side effects
template <typename FF_>
using VmPublicInputs = std::tuple<std::array<FF_, KERNEL_INPUTS_LENGTH>, // Input: Kernel context inputs
std::array<FF_, KERNEL_OUTPUTS_LENGTH>, // Output: Kernel outputs data
std::array<FF_, KERNEL_OUTPUTS_LENGTH>, // Output: Kernel outputs side effects
std::array<FF_, KERNEL_OUTPUTS_LENGTH>>; // Output: Kernel outputs metadata
using VmPublicInputs_ = std::tuple<std::array<FF_, KERNEL_INPUTS_LENGTH>, // Input: Kernel context inputs
std::array<FF_, KERNEL_OUTPUTS_LENGTH>, // Output: Kernel outputs data
std::array<FF_, KERNEL_OUTPUTS_LENGTH>, // Output: Kernel outputs side effects
std::array<FF_, KERNEL_OUTPUTS_LENGTH>>; // Output: Kernel outputs metadata

// This is the VmPublicInputs type for the current AVM flavor
using VmPublicInputs = VmPublicInputs_<FF>;
// Constants for indexing into the tuple above
static const size_t KERNEL_INPUTS = 0;
static const size_t KERNEL_OUTPUTS_VALUE = 1;
Expand Down
6 changes: 3 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void show_trace_info(const auto& trace)
} // namespace

// Needed for dependency injection in tests.
Execution::TraceBuilderConstructor Execution::trace_builder_constructor = [](VmPublicInputs<FF> public_inputs,
Execution::TraceBuilderConstructor Execution::trace_builder_constructor = [](VmPublicInputs public_inputs,
ExecutionHints execution_hints,
uint32_t side_effect_counter,
std::vector<FF> calldata) {
Expand Down Expand Up @@ -252,7 +252,7 @@ bool Execution::verify(AvmFlavor::VerificationKey vk, HonkProof const& proof)
std::copy(returndata_offset, raw_proof_offset, std::back_inserter(returndata));
std::copy(raw_proof_offset, proof.end(), std::back_inserter(raw_proof));

VmPublicInputs<FF> public_inputs = convert_public_inputs(public_inputs_vec);
VmPublicInputs public_inputs = convert_public_inputs(public_inputs_vec);
std::vector<std::vector<FF>> public_inputs_columns =
copy_public_inputs_columns(public_inputs, calldata, returndata);
return verifier.verify_proof(raw_proof, public_inputs_columns);
Expand Down Expand Up @@ -291,7 +291,7 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
vinfo("------- GENERATING TRACE -------");
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/6718): construction of the public input columns
// should be done in the kernel - this is stubbed and underconstrained
VmPublicInputs<FF> public_inputs = convert_public_inputs(public_inputs_vec);
VmPublicInputs public_inputs = convert_public_inputs(public_inputs_vec);
uint32_t start_side_effect_counter =
!public_inputs_vec.empty() ? static_cast<uint32_t>(public_inputs_vec[START_SIDE_EFFECT_COUNTER_PCPI_OFFSET])
: 0;
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace bb::avm_trace {
class Execution {
public:
static constexpr size_t SRS_SIZE = 1 << 22;
using TraceBuilderConstructor = std::function<AvmTraceBuilder(VmPublicInputs<FF> public_inputs,
using TraceBuilderConstructor = std::function<AvmTraceBuilder(VmPublicInputs public_inputs,
ExecutionHints execution_hints,
uint32_t side_effect_counter,
std::vector<FF> calldata)>;
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ std::string to_hex(bb::avm_trace::AvmMemoryTag tag)
* @param public_inputs Public inputs structure
* @param trace The execution trace
*/
void inject_end_gas_values(VmPublicInputs<FF>& public_inputs, std::vector<Row>& trace)
void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector<Row>& trace)
{
auto execution_end_row =
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_execution_end == FF(1); });
Expand Down
8 changes: 4 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ bool is_operand_indirect(uint8_t ind_value, uint8_t operand_idx);
* @param public_inputs_vec
* @return VmPublicInputs
*/
template <typename FF_> VmPublicInputs<FF_> convert_public_inputs(std::vector<FF_> const& public_inputs_vec)
template <typename FF_> VmPublicInputs_<FF_> convert_public_inputs(std::vector<FF_> const& public_inputs_vec)
{
VmPublicInputs<FF_> public_inputs;
VmPublicInputs_<FF_> public_inputs;

// Case where we pass in empty public inputs - this will be used in tests where they are not required
if (public_inputs_vec.empty()) {
Expand Down Expand Up @@ -173,7 +173,7 @@ template <typename FF_> VmPublicInputs<FF_> convert_public_inputs(std::vector<FF
// rather than the fixed length arrays that are used during circuit building. This method copies each array
// into a vector to be used by the verifier.
template <typename FF_>
std::vector<std::vector<FF_>> copy_public_inputs_columns(VmPublicInputs<FF_> const& public_inputs,
std::vector<std::vector<FF_>> copy_public_inputs_columns(VmPublicInputs_<FF_> const& public_inputs,
std::vector<FF_> const& calldata,
std::vector<FF_> const& returndata)
{
Expand Down Expand Up @@ -219,6 +219,6 @@ std::string to_hex(T value)
std::string to_hex(bb::avm_trace::AvmMemoryTag tag);

// Mutate the inputs
void inject_end_gas_values(VmPublicInputs<FF>& public_inputs, std::vector<Row>& trace);
void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector<Row>& trace);

} // namespace bb::avm_trace
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class AvmKernelTraceBuilder {
// optimise this to just hardcode the counter to be the same as the lookup selector value!!!
std::unordered_map<uint32_t, uint32_t> kernel_output_selector_counter;

AvmKernelTraceBuilder(uint32_t initial_side_effect_counter, VmPublicInputs<FF> public_inputs, ExecutionHints hints)
AvmKernelTraceBuilder(uint32_t initial_side_effect_counter, VmPublicInputs public_inputs, ExecutionHints hints)
: public_inputs(std::move(public_inputs))
, initial_side_effect_counter(initial_side_effect_counter)
, hints(std::move(hints))
Expand Down Expand Up @@ -98,7 +98,7 @@ class AvmKernelTraceBuilder {
void op_emit_l2_to_l1_msg(uint32_t clk, uint32_t side_effect_counter, const FF& l2_to_l1_msg, const FF& recipient);

// This is temporarily made public so we can access PIs
VmPublicInputs<FF> public_inputs;
VmPublicInputs public_inputs;

private:
std::vector<KernelTraceEntry> kernel_trace;
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ void AvmTraceBuilder::finalise_mem_trace_lookup_counts()
* @brief Constructor of a trace builder of AVM. Only serves to set the capacity of the
* underlying traces and initialize gas values.
*/
AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs<FF> public_inputs,
AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs,
ExecutionHints execution_hints_,
uint32_t side_effect_counter,
std::vector<FF> calldata)
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct AddressWithMode {
class AvmTraceBuilder {

public:
AvmTraceBuilder(VmPublicInputs<FF> public_inputs = {},
AvmTraceBuilder(VmPublicInputs public_inputs = {},
ExecutionHints execution_hints = {},
uint32_t side_effect_counter = 0,
std::vector<FF> calldata = {});
Expand Down
Loading