Skip to content

feat: constrain call #13758

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

Merged
IlyasRidhuan merged 3 commits into from
Apr 25, 2025
Merged

feat: constrain call #13758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c09a2f
feat: constrain call
IlyasRidhuan Apr 14, 2025
a06524c
feat: update context stack
IlyasRidhuan Apr 24, 2025
027e713
address comments
IlyasRidhuan Apr 25, 2025
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
68 changes: 65 additions & 3 deletions barretenberg/cpp/pil/vm2/context.pil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,79 @@
// This is a virtual gadget, which is part of the execution trace.
namespace execution;

Comment thread
fcarreiro marked this conversation as resolved.
// Useful to define some opcodes within this pil file
// Constrained to be boolean by execution instruction spec table
pol commit call_sel;

@fcarreiro fcarreiro Apr 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

didn't we just discuss _sel versus sel_ :P

maybe we want sel_op_.... ?

@IlyasRidhuan IlyasRidhuan Apr 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm ok i get confused by our naming conventions..i dont have a strong opinion outside of it being easy for me to remember the convention 😅 .

We have some that are _sel (i.e {namespace}_sel) for the main selectors in each subtraces - these kinda map to opcodes.
I've used sel_ (e.g. sel_alu, sel_bitwise) for selectors that weren't "opcodes".

@fcarreiro fcarreiro Apr 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say we use sel_ everywhere for everything that is a selector, and after that you can add what you want to make it clear that it is or not an opcode. Easy to remember! always sel_ ;)

@fcarreiro fcarreiro Apr 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

BTW when I say they all start with sel, I mean in PIL. Of course in tracegen etc you'll have them all prefixed by namespace.

@fcarreiro fcarreiro Apr 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Won't you also have this in the execution trace eventually?

@IlyasRidhuan IlyasRidhuan Apr 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is actually the one from the execution trace..but you are right that perhaps i should move this to the execution.pil rather than this virtual trace.

I had it in here because my intuition is that most of the relations involving call_sel will be in this file.

pol commit static_call_sel;
pol CALL = call_sel + static_call_sel; // Guaranteed to be mutually exclusive
// CALL & precomputed.first_row are NAND
CALL * precomputed.first_row = 0;
Comment thread
fcarreiro marked this conversation as resolved.

// Context columns
pol commit context_id;
pol commit parent_id;
pol commit pc;
pol commit next_pc;
pol commit msg_sender;
pol commit contract_address;

pol commit is_static;
is_static * (1 - is_static) = 0;
pol commit is_static; // Constrained boolean by tx trace
Comment thread
IlyasRidhuan marked this conversation as resolved.
Outdated

pol commit parent_calldata_offset_addr;
pol commit parent_calldata_size_addr;

pol commit last_child_returndata_offset_addr;
pol commit last_child_returndata_size_addr;
pol commit last_child_success; // Careful with this for now...

// ==== Helper columns ====
// TODO: These context modifiers also need to factor in when a new enqueued call occurs
// REPLACE prefixed precomputed.first_row in relations with actual phase / enqueued call change

// next_context_id increments with each invocation of an external call or new enqueued call
pol commit next_context_id; // Can be replaced by clk
// The initial next_context_id = 2, in row = 1
#[INCR_CONTEXT_ID]
(1- precomputed.first_row) * sel' * (next_context_id' - (next_context_id + CALL)) = 0;
Comment thread
IlyasRidhuan marked this conversation as resolved.
Outdated

// CALL = 1 <==> context_id' = next_context_id
// CALL = 0 <==> context_id' = context_id
Comment thread
IlyasRidhuan marked this conversation as resolved.
#[NEXT_CONTEXT_ID]
(1- precomputed.first_row) * sel' * ((next_context_id - context_id) * CALL + context_id + precomputed.first_row - context_id') = 0;

// CALL = 1 <==> parent_id' = context_id
// CALL = 0 <==> parent_id' = parent_id
Comment thread
IlyasRidhuan marked this conversation as resolved.
#[NEXT_PARENT_ID]
(1- precomputed.first_row) * sel' * ((context_id - parent_id) * (CALL + precomputed.first_row) + parent_id - parent_id') = 0;

// CALL = 1 <==> pc' = 0
// CALL = 0 <==> pc' = next_pc
#[NEXT_PC]
(1- precomputed.first_row) * sel' * (pc' - ((1 - CALL) * next_pc)) = 0;

// CALL = 1 <==> msg_sender' = contract_address
// CALL = 0 <==> msg_sender' = msg_sender
#[NEXT_MSG_SENDER]
(1- precomputed.first_row) * sel' * ((contract_address - msg_sender) * CALL + msg_sender - msg_sender') = 0;

// CALL = 1 <==> contract_address' = reg3 (intermediate register 3 from execution trace)
// CALL = 0 <==> contract_address' = contract_address
#[NEXT_CONTRACT_ADDR]
(1- precomputed.first_row) * sel' * ((reg3 - contract_address) * CALL + contract_address - contract_address') = 0;

// CALL = 1 && static_call = 1 <==> is_static' = 1
// CALL = 1 && static_call = 0 <==> is_static' = 0
// CALL = 0 && static_call = 0 <==> is_static' = is_static
#[NEXT_IS_STATIC]
(1- precomputed.first_row) * sel' * (is_static' - (static_call_sel + (1 - CALL) * is_static)) = 0;

// CALL = 1 <==> parent_calldata_offset_addr' = rop4 (resolved operand 4 from execution trace)
// CALL = 0 <==> parent_calldata_offset_addr' = parent_calldata_offset_addr
#[NEXT_CD_OFFSET]
(1- precomputed.first_row) * sel' * ((rop4 - parent_calldata_offset_addr) * CALL + parent_calldata_offset_addr - parent_calldata_offset_addr') = 0;

// CALL = 1 <==> parent_calldata_size_addr' = rop5 (resolved operand 5 from execution trace)
// CALL = 0 <==> parent_calldata_size_addr' = parent_calldata_size_addr
#[NEXT_CD_SIZE]
(1- precomputed.first_row) * sel' * ((rop5 - parent_calldata_size_addr) * CALL + parent_calldata_size_addr - parent_calldata_size_addr') = 0;


45 changes: 45 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/execution.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ using tracegen::TestTraceContainer;
using FF = AvmFlavorSettings::FF;
using C = Column;
using execution = bb::avm2::execution<FF>;
using context = bb::avm2::context<FF>;

TEST(ExecutionConstrainingTest, Basic)
{
Expand Down Expand Up @@ -43,6 +44,50 @@ TEST(ExecutionConstrainingTest, Continuity)
check_relation<execution>(trace, execution::SR_TRACE_CONTINUITY_1, execution::SR_TRACE_CONTINUITY_2);
}

TEST(ExecutionConstrainingTest, ContextSwitchingCall)
{
TestTraceContainer trace({ {
{ C::execution_next_context_id, 0 },
{ C::precomputed_first_row, 1 },
},
// Dummy Row
{ { C::execution_sel, 1 },
{ C::execution_pc, 0 },
{ C::execution_next_pc, 1 },
{ C::execution_context_id, 1 },
{ C::execution_next_context_id, 2 } },
// CALL
{
{ C::execution_sel, 1 },
{ C::execution_pc, 1 },
{ C::execution_next_pc, 2 },
{ C::execution_call_sel, 1 },
{ C::execution_context_id, 1 },
{ C::execution_next_context_id, 2 },
{ C::execution_rop4, /*cd offset=*/10 },
{ C::execution_rop5, /*cd size=*/1 },
{ C::execution_reg3, /*contract address=*/0xdeadbeef },
},
// Dummy Row in new context
{
{ C::execution_sel, 1 },
{ C::execution_pc, 0 }, // pc=0 because it is after a CALL
{ C::execution_next_pc, 20 },
{ C::execution_context_id, 2 }, // Previous row next_context_id
{ C::execution_next_context_id, 3 }, // Incremented due to previous call
{ C::execution_parent_id, 1 }, // Previous row context id
{ C::execution_contract_address, 0xdeadbeef },
{ C::execution_parent_calldata_offset_addr, 10 },
{ C::execution_parent_calldata_size_addr, 1 },
},
{
{ C::execution_sel, 0 },
{ C::execution_last, 1 },
} });

check_relation<context>(trace);
}

TEST(ExecutionConstrainingTest, ContinuityBrokenFirstRow)
{
// clang-format off
Expand Down
10 changes: 5 additions & 5 deletions barretenberg/cpp/src/barretenberg/vm2/generated/columns.hpp
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm2/generated/flavor_variables.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ namespace bb::avm2 {

struct AvmFlavorVariables {
static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 70;
static constexpr size_t NUM_WITNESS_ENTITIES = 2071;
static constexpr size_t NUM_SHIFTED_ENTITIES = 135;
static constexpr size_t NUM_WITNESS_ENTITIES = 2076;
static constexpr size_t NUM_SHIFTED_ENTITIES = 144;
static constexpr size_t NUM_WIRES = NUM_WITNESS_ENTITIES + NUM_PRECOMPUTED_ENTITIES;
static constexpr size_t NUM_ALL_ENTITIES = 2276;
static constexpr size_t NUM_ALL_ENTITIES = 2290;

// Need to be templated for recursive verifier
template <typename FF_>
Expand Down
115 changes: 112 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm2/generated/relations/context.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,100 @@ template <typename FF_> class contextImpl {
public:
using FF = FF_;

static constexpr std::array<size_t, 1> SUBRELATION_PARTIAL_LENGTHS = { 3 };
static constexpr std::array<size_t, 10> SUBRELATION_PARTIAL_LENGTHS = { 3, 4, 5, 5, 5, 5, 5, 5, 5, 5 };

template <typename ContainerOverSubrelations, typename AllEntities>
void static accumulate(ContainerOverSubrelations& evals,
const AllEntities& new_term,
[[maybe_unused]] const RelationParameters<FF>&,
[[maybe_unused]] const FF& scaling_factor)
{
const auto execution_CALL = new_term.execution_call_sel + new_term.execution_static_call_sel;

{
using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>;
auto tmp = new_term.execution_is_static * (FF(1) - new_term.execution_is_static);
auto tmp = execution_CALL * new_term.precomputed_first_row;
tmp *= scaling_factor;
std::get<0>(evals) += typename Accumulator::View(tmp);
}
{ // INCR_CONTEXT_ID
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
auto tmp =
(FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(new_term.execution_next_context_id_shift - (new_term.execution_next_context_id + execution_CALL));
tmp *= scaling_factor;
std::get<1>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_CONTEXT_ID
using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_next_context_id - new_term.execution_context_id) * execution_CALL +
new_term.execution_context_id + new_term.precomputed_first_row) -
new_term.execution_context_id_shift);
tmp *= scaling_factor;
std::get<2>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_PARENT_ID
using Accumulator = typename std::tuple_element_t<3, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_context_id - new_term.execution_parent_id) *
(execution_CALL + new_term.precomputed_first_row) +
new_term.execution_parent_id) -
new_term.execution_parent_id_shift);
tmp *= scaling_factor;
std::get<3>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_PC
using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(new_term.execution_pc_shift - (FF(1) - execution_CALL) * new_term.execution_next_pc);
tmp *= scaling_factor;
std::get<4>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_MSG_SENDER
using Accumulator = typename std::tuple_element_t<5, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_contract_address - new_term.execution_msg_sender) * execution_CALL +
new_term.execution_msg_sender) -
new_term.execution_msg_sender_shift);
tmp *= scaling_factor;
std::get<5>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_CONTRACT_ADDR
using Accumulator = typename std::tuple_element_t<6, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_reg3 - new_term.execution_contract_address) * execution_CALL +
new_term.execution_contract_address) -
new_term.execution_contract_address_shift);
tmp *= scaling_factor;
std::get<6>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_IS_STATIC
using Accumulator = typename std::tuple_element_t<7, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(new_term.execution_is_static_shift -
(new_term.execution_static_call_sel + (FF(1) - execution_CALL) * new_term.execution_is_static));
tmp *= scaling_factor;
std::get<7>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_CD_OFFSET
using Accumulator = typename std::tuple_element_t<8, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_rop4 - new_term.execution_parent_calldata_offset_addr) * execution_CALL +
new_term.execution_parent_calldata_offset_addr) -
new_term.execution_parent_calldata_offset_addr_shift);
tmp *= scaling_factor;
std::get<8>(evals) += typename Accumulator::View(tmp);
}
{ // NEXT_CD_SIZE
using Accumulator = typename std::tuple_element_t<9, ContainerOverSubrelations>;
auto tmp = (FF(1) - new_term.precomputed_first_row) * new_term.execution_sel_shift *
(((new_term.execution_rop5 - new_term.execution_parent_calldata_size_addr) * execution_CALL +
new_term.execution_parent_calldata_size_addr) -
new_term.execution_parent_calldata_size_addr_shift);
tmp *= scaling_factor;
std::get<9>(evals) += typename Accumulator::View(tmp);
}
}
};

Expand All @@ -36,9 +115,39 @@ template <typename FF> class context : public Relation<contextImpl<FF>> {

static std::string get_subrelation_label(size_t index)
{
switch (index) {}
switch (index) {
case 1:
return "INCR_CONTEXT_ID";
case 2:
return "NEXT_CONTEXT_ID";
case 3:
return "NEXT_PARENT_ID";
case 4:
return "NEXT_PC";
case 5:
return "NEXT_MSG_SENDER";
case 6:
return "NEXT_CONTRACT_ADDR";
case 7:
return "NEXT_IS_STATIC";
case 8:
return "NEXT_CD_OFFSET";
case 9:
return "NEXT_CD_SIZE";
}
return std::to_string(index);
}

// Subrelation indices constants, to be used in tests.
static constexpr size_t SR_INCR_CONTEXT_ID = 1;
static constexpr size_t SR_NEXT_CONTEXT_ID = 2;
static constexpr size_t SR_NEXT_PARENT_ID = 3;
static constexpr size_t SR_NEXT_PC = 4;
static constexpr size_t SR_NEXT_MSG_SENDER = 5;
static constexpr size_t SR_NEXT_CONTRACT_ADDR = 6;
static constexpr size_t SR_NEXT_IS_STATIC = 7;
static constexpr size_t SR_NEXT_CD_OFFSET = 8;
static constexpr size_t SR_NEXT_CD_SIZE = 9;
};

} // namespace bb::avm2
46 changes: 28 additions & 18 deletions barretenberg/cpp/src/barretenberg/vm2/simulation/context.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class ContextInterface {
virtual void set_next_pc(uint32_t new_next_pc) = 0;
virtual bool halted() const = 0;
virtual void halt() = 0;

virtual uint32_t get_context_id() const = 0;

// Environment.
Expand Down Expand Up @@ -166,16 +165,20 @@ class EnqueuedCallContext : public BaseContext {
// Event Emitting
ContextEvent serialize_context_event() override
{
return { .id = get_context_id(),
.pc = get_pc(),
.msg_sender = get_msg_sender(),
.contract_addr = get_address(),
.is_static = get_is_static(),
.parent_cd_addr = 0,
.parent_cd_size_addr = 0,
.last_child_rd_addr = get_last_rd_offset(),
.last_child_rd_size_addr = get_last_rd_size(),
.last_child_success = get_last_success() };
return {
.id = get_context_id(),
.parent_id = 0,

@IlyasRidhuan IlyasRidhuan Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the main thing that changed (besides formatting)

.pc = get_pc(),
.next_pc = get_next_pc(),
.msg_sender = get_msg_sender(),
.contract_addr = get_address(),
.is_static = get_is_static(),
.parent_cd_addr = 0,
.parent_cd_size_addr = 0,
.last_child_rd_addr = get_last_rd_offset(),
.last_child_rd_size_addr = get_last_rd_size(),
.last_child_success = get_last_success(),
};
};

// Input / Output
Expand Down Expand Up @@ -220,13 +223,20 @@ class NestedContext : public BaseContext {
// Event Emitting
ContextEvent serialize_context_event() override
{
return { .id = get_context_id(),
.pc = get_pc(),
.msg_sender = get_msg_sender(),
.contract_addr = get_address(),
.is_static = get_is_static(),
.parent_cd_addr = parent_cd_offset,
.parent_cd_size_addr = parent_cd_size };
return {
.id = get_context_id(),
.parent_id = parent_context.get_context_id(),

@IlyasRidhuan IlyasRidhuan Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same here

.pc = get_pc(),
.next_pc = get_next_pc(),
.msg_sender = get_msg_sender(),
.contract_addr = get_address(),
.is_static = get_is_static(),
.parent_cd_addr = parent_cd_offset,
.parent_cd_size_addr = parent_cd_size,
.last_child_rd_addr = get_last_rd_offset(),
.last_child_rd_size_addr = get_last_rd_size(),
.last_child_success = get_last_success(),
};
};

// Input / Output
Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm2/simulation/events/context_events.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ namespace bb::avm2::simulation {

struct ContextEvent {
uint32_t id;
// uint32_t parent_id;
uint32_t parent_id;

// State
uint32_t pc;
uint32_t next_pc;
AztecAddress msg_sender;
AztecAddress contract_addr;
bool is_static;
Expand Down
3 changes: 3 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm2/simulation/events/execution_event.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ struct ExecutionEvent {
// todo(ilyas): this is a vector because GETCONTRACTINSTANCE has 2 outputs, we should change this to 1
std::vector<TaggedValue> output;

// Context Id for the next context.
uint32_t next_context_id;

@fcarreiro fcarreiro Apr 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need, for every event, the NEXT context id? Why not the current and why an id at all?

(I'm not asking for a change, but an explanation :) )

@IlyasRidhuan IlyasRidhuan Apr 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah good question (this holds for enqueued calls but it's easier just to consider nested calls for now).

The current id is stored within the context event - it's needed to uniquely identify a context when interacting with the stack AND it is used as the space_id for memory (we should really deprecate the use of space_id IMO).

When we encounter a call we need to assign a new unique id to the new context. We cannot just increment the current id because we could have hit a RETURN and are actually in a prior context.

  1. Start in initial context: ID = 1
  2. Run CALL, creating new context with ID = 2
  3. Return from ID=2 back to context ID = 1
  4. A new context from a new CALL/enqueued call would need to be assigned ID = 3

If we just incremented the current ID = 1, we would get a duplicate context with ID = 2. In order to know that we the next ID is 3 we have to track it at a global level (which is why this doesnt live in context) and this was the easiest way i could think of - maybe we could do some trick with the stack?

@fcarreiro fcarreiro Apr 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah I get it, it's the "next available context id". I thought it was something weird like "ok this is the context id of the call i'm about to go in to". Consider name change (or comment) but not needed.


// Sub-events.
AddressingEvent addressing_event;
ContextEvent context_event;
Expand Down
Loading