fix: Enforce recipients of l2l1msgs having 20 bytes at most

Author: sirasistant

barretenberg/cpp/pil/vm2/constants_gen.pil

@@ -1,5 +1,6 @@
 // GENERATED FILE - DO NOT EDIT, RUN yarn remake-constants in yarn-project/constants
 namespace constants;
+    pol MAX_ETH_ADDRESS_VALUE = 1461501637330902918203684832716283019655932542975;
     pol NOTE_HASH_TREE_HEIGHT = 42;
     pol PUBLIC_DATA_TREE_HEIGHT = 40;
     pol NULLIFIER_TREE_HEIGHT = 42;

barretenberg/cpp/pil/vm2/opcodes/send_l2_to_l1_msg.pil

@@ -39,9 +39,26 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
     #[MAX_WRITES_REACHED]
     sel_execute_send_l2_to_l1_msg * (REMAINING_L2_TO_L1_MSG_WRITES * (sel_l2_to_l1_msg_limit_error * (1 - remaining_l2_to_l1_msgs_inv) + remaining_l2_to_l1_msgs_inv) - 1 + sel_l2_to_l1_msg_limit_error) = 0;
 
-    // The opcode errors if we have reached the limit or if we are in a static context
+    // TODO: We need this temporarily while we do not allow for aliases in the lookup tuple
+    pol commit max_eth_address_value;
+    sel_execute_send_l2_to_l1_msg * (constants.MAX_ETH_ADDRESS_VALUE - max_eth_address_value) = 0;
+
+    pol commit too_large_recipient_error;
+
+    #[RECIPIENT_CHECK]
+    sel_execute_send_l2_to_l1_msg {
+        register[0], // recipient
+        max_eth_address_value,
+        too_large_recipient_error
+    } in ff_gt.sel_gt {
+        ff_gt.a,
+        ff_gt.b,
+        ff_gt.result
+    };
+
+    // The opcode errors if we have reached the limit, if we are in a static context, or if the recipient is too large.
     #[OPCODE_ERROR]
-    sel_execute_send_l2_to_l1_msg * ((1 - sel_l2_to_l1_msg_limit_error) * (1 - is_static) - (1 - sel_opcode_error)) = 0;
+    sel_execute_send_l2_to_l1_msg * ((1 - sel_l2_to_l1_msg_limit_error) * (1 - is_static) * (1 - too_large_recipient_error) - (1 - sel_opcode_error)) = 0;
 
     // =========== LOOKUP PI ===========
     pol commit sel_write_l2_to_l1_msg;

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/simulator.cpp

@@ -194,16 +194,6 @@ bool compare_simulator_results(SimulatorResult& result1, SimulatorResult& result
         result2.output.resize(MAX_RETURN_DATA_SIZE_IN_FIELDS);
     }
 
-    // TODO(avm):
-    // https://linear.app/aztec-labs/issue/AVM-209/l2l1msgs-possible-completeness-issue-with-recipient-20-bytes
-    for (auto& l2_to_l1_msg : result1.public_tx_effect.l2_to_l1_msgs) {
-        l2_to_l1_msg.message.recipient = FF(static_cast<uint256_t>(l2_to_l1_msg.message.recipient).slice(0, 20));
-    }
-
-    for (auto& l2_to_l1_msg : result2.public_tx_effect.l2_to_l1_msgs) {
-        l2_to_l1_msg.message.recipient = FF(static_cast<uint256_t>(l2_to_l1_msg.message.recipient).slice(0, 20));
-    }
-
     return result1.reverted == result2.reverted && result1.output == result2.output &&
            result1.end_tree_snapshots == result2.end_tree_snapshots &&
            result1.public_tx_effect == result2.public_tx_effect;

barretenberg/cpp/src/barretenberg/avm_fuzzer/mutations/basic_types/eth_address.cpp

@@ -0,0 +1,21 @@
+#include "barretenberg/avm_fuzzer/mutations/basic_types/eth_address.hpp"
+
+#include "barretenberg/avm_fuzzer/mutations/basic_types/field.hpp"
+#include "barretenberg/vm2/common/aztec_types.hpp"
+
+using bb::avm2::EthAddress;
+using bb::avm2::FF;
+
+namespace {
+
+EthAddress ff_to_eth_address(FF ff)
+{
+    return EthAddress(static_cast<uint256_t>(ff).slice(0, 20));
+}
+
+} // namespace
+
+EthAddress generate_random_eth_address(std::mt19937_64& rng)
+{
+    return ff_to_eth_address(generate_random_field(rng));
+}

barretenberg/cpp/src/barretenberg/avm_fuzzer/mutations/basic_types/eth_address.hpp

@@ -0,0 +1,7 @@
+#pragma once
+
+#include <random>
+
+#include "barretenberg/vm2/common/aztec_types.hpp"
+
+bb::avm2::EthAddress generate_random_eth_address(std::mt19937_64& rng);

barretenberg/cpp/src/barretenberg/avm_fuzzer/mutations/basic_types/field.hpp

@@ -3,6 +3,7 @@
 #include <random>
 
 #include "barretenberg/avm_fuzzer/mutations/configuration.hpp"
+#include "barretenberg/vm2/common/aztec_types.hpp"
 #include "barretenberg/vm2/common/field.hpp"
 
 bb::avm2::FF generate_random_field(std::mt19937_64& rng);

barretenberg/cpp/src/barretenberg/avm_fuzzer/mutations/tx_types/accumulated_data.cpp

@@ -1,6 +1,7 @@
 #include "barretenberg/avm_fuzzer/mutations/tx_types/accumulated_data.hpp"
 
 #include "barretenberg/avm_fuzzer/fuzz_lib/constants.hpp"
+#include "barretenberg/avm_fuzzer/mutations/basic_types/eth_address.hpp"
 #include "barretenberg/avm_fuzzer/mutations/basic_types/field.hpp"
 #include "barretenberg/avm_fuzzer/mutations/basic_types/vector.hpp"
 #include "barretenberg/avm_fuzzer/mutations/configuration.hpp"
@@ -45,7 +46,8 @@ void mutate_nullifier(FF& nullifier, std::mt19937_64& rng)
 ScopedL2ToL1Message generate_l2_to_l1_message(std::mt19937_64& rng)
 {
     return ScopedL2ToL1Message{
-        .message = L2ToL1Message{ .recipient = generate_random_field(rng), .content = generate_random_field(rng) },
+        .message =
+            L2ToL1Message{ .recipient = generate_random_eth_address(rng), .content = generate_random_field(rng) },
         .contract_address = generate_random_field(rng),
     };
 }
@@ -57,7 +59,7 @@ void mutate_l2_to_l1_msg(ScopedL2ToL1Message& msg, std::mt19937_64& rng)
     switch (choice) {
     case 0:
         // Mutate recipient
-        msg.message.recipient = generate_random_field(rng);
+        msg.message.recipient = generate_random_eth_address(rng);
         break;
     case 1:
         // Mutate content

barretenberg/cpp/src/barretenberg/vm2/common/aztec_constants.hpp

@@ -1,6 +1,7 @@
 // GENERATED FILE - DO NOT EDIT, RUN yarn remake-constants in yarn-project/constants
 #pragma once
 
+#define MAX_ETH_ADDRESS_VALUE "0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff"
 #define ARCHIVE_HEIGHT 30
 #define NOTE_HASH_TREE_HEIGHT 42
 #define PUBLIC_DATA_TREE_HEIGHT 40

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/send_l2_to_l1_msg.test.cpp

@@ -7,21 +7,30 @@
 #include "barretenberg/vm2/constraining/flavor_settings.hpp"
 #include "barretenberg/vm2/constraining/testing/check_relation.hpp"
 #include "barretenberg/vm2/generated/relations/execution.hpp"
+#include "barretenberg/vm2/simulation/gadgets/field_gt.hpp"
+#include "barretenberg/vm2/simulation/testing/mock_range_check.hpp"
 #include "barretenberg/vm2/testing/macros.hpp"
 #include "barretenberg/vm2/testing/public_inputs_builder.hpp"
 #include "barretenberg/vm2/tracegen/execution_trace.hpp"
+#include "barretenberg/vm2/tracegen/field_gt_trace.hpp"
 #include "barretenberg/vm2/tracegen/precomputed_trace.hpp"
 #include "barretenberg/vm2/tracegen/public_inputs_trace.hpp"
 #include "barretenberg/vm2/tracegen/test_trace_container.hpp"
 
 namespace bb::avm2::constraining {
 namespace {
 
+using simulation::EventEmitter;
+using simulation::FieldGreaterThan;
+using simulation::FieldGreaterThanEvent;
+using simulation::MockRangeCheck;
+
 using tracegen::ExecutionTraceBuilder;
 using tracegen::PublicInputsTraceBuilder;
 using tracegen::TestTraceContainer;
 
 using testing::PublicInputsBuilder;
+using tracegen::FieldGreaterThanTraceBuilder;
 
 using FF = AvmFlavorSettings::FF;
 using C = Column;
@@ -32,6 +41,7 @@ TEST(SendL2ToL1MsgConstrainingTest, Positive)
     uint64_t prev_num_l2_to_l1_msgs = MAX_L2_TO_L1_MSGS_PER_TX - 1;
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_register_0_, /*recipient=*/42 },
         { C::execution_register_1_, /*content=*/27 },
         { C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
@@ -54,6 +64,7 @@ TEST(SendL2ToL1MsgConstrainingTest, LimitReached)
     uint64_t prev_num_l2_to_l1_msgs = MAX_L2_TO_L1_MSGS_PER_TX;
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_register_0_, /*recipient=*/42 },
         { C::execution_register_1_, /*content=*/27 },
         { C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
@@ -88,6 +99,7 @@ TEST(SendL2ToL1MsgConstrainingTest, Discard)
     uint64_t prev_num_l2_to_l1_msgs = 0;
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_register_0_, /*recipient=*/42 },
         { C::execution_register_1_, /*content=*/27 },
         { C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
@@ -119,6 +131,12 @@ TEST(SendL2ToL1MsgConstrainingTest, Interactions)
     AztecAddress address = 0xdeadbeef;
     AvmAccumulatedData accumulated_data = {};
 
+    MockRangeCheck mock_range_check;
+    EventEmitter<FieldGreaterThanEvent> ff_gt_event_emitter;
+    FieldGreaterThan field_gt(mock_range_check, ff_gt_event_emitter);
+
+    ASSERT_FALSE(field_gt.ff_gt(recipient, FF(MAX_ETH_ADDRESS_VALUE)));
+
     accumulated_data.l2_to_l1_msgs[0] = {
         .message =
             L2ToL1Message{
@@ -135,6 +153,7 @@ TEST(SendL2ToL1MsgConstrainingTest, Interactions)
 
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_register_0_, recipient },
         { C::execution_register_1_, content },
         { C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
@@ -150,6 +169,9 @@ TEST(SendL2ToL1MsgConstrainingTest, Interactions)
         { C::execution_subtrace_operation_id, AVM_EXEC_OP_ID_SENDL2TOL1MSG },
     } });
 
+    FieldGreaterThanTraceBuilder field_gt_builder;
+    field_gt_builder.process(ff_gt_event_emitter.dump_events(), trace);
+
     PublicInputsTraceBuilder public_inputs_builder;
     public_inputs_builder.process_public_inputs(trace, public_inputs);
     public_inputs_builder.process_public_inputs_aux_precomputed(trace);
@@ -158,13 +180,34 @@ TEST(SendL2ToL1MsgConstrainingTest, Interactions)
     precomputed_builder.process_misc(trace, trace.get_num_rows());
 
     check_relation<send_l2_to_l1_msg>(trace);
-    check_interaction<ExecutionTraceBuilder, lookup_send_l2_to_l1_msg_write_l2_to_l1_msg_settings>(trace);
+    check_interaction<ExecutionTraceBuilder,
+                      lookup_send_l2_to_l1_msg_write_l2_to_l1_msg_settings,
+                      lookup_send_l2_to_l1_msg_recipient_check_settings>(trace);
+}
+
+TEST(SendL2ToL1MsgConstrainingTest, NegativeShouldErrorIfRecipientTooLarge)
+{
+    TestTraceContainer trace({ {
+        { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
+        { C::execution_too_large_recipient_error, 1 },
+        { C::execution_sel_l2_to_l1_msg_limit_error, 0 },
+        { C::execution_is_static, 0 },
+        { C::execution_sel_opcode_error, 1 },
+    } });
+    check_relation<send_l2_to_l1_msg>(trace, send_l2_to_l1_msg::SR_OPCODE_ERROR);
+
+    // Negative test: sel_opcode_error must be on
+    trace.set(C::execution_sel_opcode_error, 0, 0);
+    EXPECT_THROW_WITH_MESSAGE(check_relation<send_l2_to_l1_msg>(trace, send_l2_to_l1_msg::SR_OPCODE_ERROR),
+                              "OPCODE_ERROR");
 }
 
 TEST(SendL2ToL1MsgConstrainingTest, NegativeShouldErrorIfStatic)
 {
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_sel_l2_to_l1_msg_limit_error, 0 },
         { C::execution_is_static, 1 },
         { C::execution_sel_opcode_error, 1 },
@@ -182,6 +225,7 @@ TEST(SendL2ToL1MsgConstrainingTest, NegativeShouldNotWriteIfDiscard)
 {
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_sel_opcode_error, 0 },
         { C::execution_discard, 1 },
         { C::execution_sel_write_l2_to_l1_msg, 0 },
@@ -199,6 +243,7 @@ TEST(SendL2ToL1MsgConstrainingTest, NegativeShouldNumL2ToL1MessagesIncrease)
 {
     TestTraceContainer trace({ {
         { C::execution_sel_execute_send_l2_to_l1_msg, 1 },
+        { C::execution_max_eth_address_value, FF(MAX_ETH_ADDRESS_VALUE) },
         { C::execution_sel_opcode_error, 0 },
         { C::execution_prev_num_l2_to_l1_messages, 0 },
         { C::execution_num_l2_to_l1_messages, 1 },