fix: check version, chainid and sender for cross-chain l1 to l2 msgs (#…

…3457)

Fixes #3455 by also checking the portal address, version and chainid.
Needed updates in the tests to pass the additional tests.

Existing tests generally seem to be very heavily focused on happy paths,
we need to do something there.

Also ran `nargo fmt`

yarn-project/acir-simulator/src/client/private_execution.test.ts

@@ -467,7 +467,13 @@ describe('Private Execution test suite', () => {
         messageKey,
         secretForL1ToL2MessageConsumption,
       ];
-      const result = await runSimulator({ contractAddress, artifact, args });
+      const result = await runSimulator({
+        contractAddress,
+        artifact,
+        args,
+        portalContractAddress: preimage.sender.sender,
+        txContext: { version: new Fr(1n), chainId: new Fr(1n) },
+      });
 
       // Check a nullifier has been inserted
       const newNullifiers = result.callStackItem.publicInputs.newNullifiers.filter(field => !field.equals(Fr.ZERO));

yarn-project/acir-simulator/src/public/index.test.ts

@@ -386,7 +386,7 @@ describe('ACIR public execution simulator', () => {
       const callContext = CallContext.from({
         msgSender: AztecAddress.random(),
         storageContractAddress: contractAddress,
-        portalContractAddress: EthAddress.random(),
+        portalContractAddress: preimage.sender.sender,
         functionSelector: FunctionSelector.empty(),
         isContractDeployment: false,
         isDelegateCall: false,
@@ -406,7 +406,14 @@ describe('ACIR public execution simulator', () => {
       });
 
       const execution: PublicExecution = { contractAddress, functionData, args, callContext };
-      const result = await executor.simulate(execution, GlobalVariables.empty());
+
+      const gv = new GlobalVariables(
+        new Fr(preimage.sender.chainId),
+        new Fr(preimage.recipient.version),
+        Fr.ZERO,
+        Fr.ZERO,
+      );
+      const result = await executor.simulate(execution, gv);
 
       expect(result.newNullifiers.length).toEqual(1);
     });

yarn-project/aztec-nr/aztec/src/context.nr

@@ -202,7 +202,7 @@ impl PrivateContext {
     ) 
     // docs:end:context_consume_l1_to_l2_message
     {
-        let nullifier = process_l1_to_l2_message(self.block_data.l1_to_l2_messages_tree_root, self.this_address(), msg_key, content, secret);
+        let nullifier = process_l1_to_l2_message(self.block_data.l1_to_l2_messages_tree_root, self.this_address(), self.this_portal_address(), self.chain_id(), self.version(), msg_key, content, secret);
 
         // Push nullifier (and the "commitment" corresponding to this can be "empty")
         self.push_new_nullifier(nullifier, EMPTY_NULLIFIED_COMMITMENT)
@@ -543,7 +543,7 @@ impl PublicContext {
     // Note this returns self to get around an issue where mutable structs do not maintain mutations unless reassigned
     pub fn consume_l1_to_l2_message(&mut self, msg_key: Field, content: Field, secret: Field) {
         let this = (*self).this_address();
-        let nullifier = process_l1_to_l2_message(self.block_data.l1_to_l2_messages_tree_root, this, msg_key, content, secret);
+        let nullifier = process_l1_to_l2_message(self.block_data.l1_to_l2_messages_tree_root, this, self.this_portal_address(), self.chain_id(), self.version(), msg_key, content, secret);
 
         // Push nullifier (and the "commitment" corresponding to this can be "empty")
         self.push_new_nullifier(nullifier, EMPTY_NULLIFIED_COMMITMENT)

yarn-project/aztec-nr/aztec/src/messaging.nr

@@ -7,7 +7,16 @@ use crate::abi::PublicContextInputs;
 use crate::oracle::get_l1_to_l2_message::get_l1_to_l2_message_call;
 
 // Returns the nullifier for the message
-pub fn process_l1_to_l2_message(l1_to_l2_root: Field, storage_contract_address: Field, msg_key: Field, content: Field, secret: Field) -> Field {
+pub fn process_l1_to_l2_message(
+    l1_to_l2_root: Field,
+    storage_contract_address: Field,
+    portal_contract_address: Field,
+    chain_id: Field,
+    version: Field,
+    msg_key: Field,
+    content: Field,
+    secret: Field
+) -> Field {
     let returned_message = get_l1_to_l2_message_call(msg_key);
     let l1_to_l2_message_data = make_l1_to_l2_message_getter_data(returned_message, 0, secret);
 
@@ -17,6 +26,15 @@ pub fn process_l1_to_l2_message(l1_to_l2_root: Field, storage_contract_address:
     // Validate this is the target contract
     assert(l1_to_l2_message_data.message.recipient == storage_contract_address);
 
+    // Validate the sender is the portal contract
+    assert(l1_to_l2_message_data.message.sender == portal_contract_address);
+
+    // Validate the chain id is correct
+    assert(l1_to_l2_message_data.message.chainId == chain_id);
+
+    // Validate the version is correct
+    assert(l1_to_l2_message_data.message.version == version);
+
     // Validate the message hash is correct
     assert(l1_to_l2_message_data.message.content == content);
 

yarn-project/noir-contracts/src/contracts/card_game_contract/src/main.nr

@@ -163,7 +163,7 @@ contract CardGame {
         context.call_public_function(context.this_address(),
             selector,
             [game as Field, player, card.to_field()]);
-         // docs:end:call_public_function
+        // docs:end:call_public_function
     }
 
     #[aztec(public)]

yarn-project/noir-contracts/src/contracts/lending_contract/src/main.nr

@@ -158,9 +158,15 @@ contract Lending {
 
     #[aztec(public)]
     fn deposit_public(amount: Field, nonce: Field, on_behalf_of: Field, collateral_asset: Field) {
-        Token::at(collateral_asset).transfer_public(context, context.msg_sender(), context.this_address(), amount, nonce);
+        Token::at(collateral_asset).transfer_public(context,
+            context.msg_sender(),
+            context.this_address(),
+            amount,
+            nonce);
         let selector = compute_selector("_deposit(Field,Field,Field)");
-        context.call_public_function(context.this_address(), selector, [on_behalf_of, amount, collateral_asset]);
+        context.call_public_function(context.this_address(),
+            selector,
+            [on_behalf_of, amount, collateral_asset]);
     }
 
     #[aztec(public)]
@@ -185,7 +191,9 @@ contract Lending {
     #[aztec(public)]
     fn withdraw_public(to: Field, amount: Field) {
         let selector = compute_selector("_withdraw(Field,Field,Field)");
-        context.call_public_function(context.this_address(), selector, [context.msg_sender(), to, amount]);
+        context.call_public_function(context.this_address(),
+            selector,
+            [context.msg_sender(), to, amount]);
     }
 
     #[aztec(public)]
@@ -201,7 +209,11 @@ contract Lending {
 
         // debt_covered will revert if decrease would leave insufficient collateral to cover debt.
         // or trying to remove more collateral than available
-        let debt_covered = covered_by_collateral(price, asset.loan_to_value, collateral as u120, 0, amount as u120);
+        let debt_covered = covered_by_collateral(price,
+            asset.loan_to_value,
+            collateral as u120,
+            0,
+            amount as u120);
         let debt_returns = debt_updates(asset.interest_accumulator, static_debt as u120, 0, 0);
 
         assert(debt_returns.debt_value < debt_covered);
@@ -223,7 +235,9 @@ contract Lending {
     #[aztec(public)]
     fn borrow_public(to: Field, amount: Field) {
         let selector = compute_selector("_borrow(Field,Field,Field)");
-        context.call_public_function(context.this_address(), selector, [context.msg_sender(), to, amount]);
+        context.call_public_function(context.this_address(),
+            selector,
+            [context.msg_sender(), to, amount]);
     }
 
     #[aztec(public)]
@@ -252,7 +266,9 @@ contract Lending {
         let on_behalf_of = compute_identifier(secret, on_behalf_of, context.msg_sender());
         let _res = Token::at(stable_coin).burn(&mut context, from, amount, nonce);
         let selector = compute_selector("_repay(Field,Field,Field)");
-        context.call_public_function(context.this_address(), selector, [on_behalf_of, amount, stable_coin]);
+        context.call_public_function(context.this_address(),
+            selector,
+            [on_behalf_of, amount, stable_coin]);
     }
 
     #[aztec(public)]

yarn-project/noir-contracts/src/contracts/test_contract/src/interface.nr

@@ -1,5 +1,5 @@
 /* Autogenerated file, do not edit! */
-  
+
 use dep::std;
 use dep::aztec::context::{ PrivateContext, PublicContext };
 use dep::aztec::constants_gen::RETURN_VALUES_LENGTH;
@@ -26,7 +26,6 @@ struct ManyNotesADeepStructTestCodeGenStruct {
   secret_hash: Field,
 }
 
-
 // Interface for calling Test functions from a private context
 struct TestPrivateContextInterface {
   address: Field,
@@ -242,9 +241,6 @@ impl TestPrivateContextInterface {
   }
 
 }
-
-
-
 
 // Interface for calling Test functions from a public context
 struct TestPublicContextInterface {
@@ -330,5 +326,4 @@ impl TestPublicContextInterface {
   }
 
 }
-
-
+

yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/main.nr

@@ -175,5 +175,5 @@ contract TokenBridge {
     unconstrained fn compute_note_hash_and_nullifier(contract_address: Field, nonce: Field, storage_slot: Field, serialized_note: [Field; 0]) -> [Field; 4] {
         [0, 0, 0, 0]
     }
-// docs:end:compute_note_hash_and_nullifier_placeholder
+    // docs:end:compute_note_hash_and_nullifier_placeholder
 }

yarn-project/noir-contracts/src/contracts/token_contract/src/main.nr

@@ -216,7 +216,6 @@ contract Token {
 
         let to_balance = storage.public_balances.at(to.address).read().add(amount);
         storage.public_balances.at(to.address).write(to_balance);
-
     }
     // docs:end:transfer_public