Rebase, and comments

.github/workflows/build.yml

@@ -15,4 +15,4 @@ jobs:
       - uses: actions/checkout@v4
       - uses: asdf-vm/actions/install@v3
       - run: scarb fmt --check
-      - run: scarb build
+      - run: scarb build --all-features

.github/workflows/check.yml

@@ -24,4 +24,4 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: asdf-vm/actions/install@v3
-      - run: snforge test
+      - run: scarb test --all-features

.tool-versions

@@ -1,2 +1,2 @@
-scarb 2.8.0
+scarb 2.8.4
 starknet-foundry 0.27.0

Scarb.toml

@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2023_10"
 
 [dependencies]
-starknet = "2.8.0"
+starknet = "2.8.4"
 openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.16.0" }
 
 [dev-dependencies]
@@ -25,3 +25,5 @@ sierra = true
 [tool.fmt]
 sort-module-level-items = true
 
+[features]
+messaging_test = []

src/appchain.cairo

@@ -130,6 +130,10 @@ mod appchain {
 
     #[abi(embed_v0)]
     impl Appchain of IAppchain<ContractState> {
+        /// This function accepts two outputs due to the dynamic nature of the layout, which
+        /// prevents direct verification of the snos proof.
+        /// To ensure the correctness of `snos_output`, we compare its hash with the corresponding
+        /// hash in `program_output`, which serves as a layout bridge proof.
         fn update_state(
             ref self: ContractState,
             snos_output: Array<felt252>,
@@ -147,7 +151,10 @@ mod appchain {
             let output_hash = poseidon_hash_span(program_output);
 
             let mut snos_output_iter = snos_output.into_iter();
-            let program_output_struct = deserialize_os_output(ref snos_output_iter);
+            let program_output_struct = deserialize_os_output(
+                ref snos_output_iter
+            ); //Custom deserialization function, inspired by 
+            //https://github.com/starkware-libs/cairo-lang/blob/8e11b8cc65ae1d0959328b1b4a40b92df8b58595/src/starkware/starknet/core/aggregator/output_parser.py
 
             let (current_program_hash, current_config_hash): (felt252, felt252) = self
                 .config

src/messaging/component.cairo

@@ -26,7 +26,6 @@
 
 /// Errors.
 mod errors {
-    const INVALID_NONCE: felt252 = 'INVALID_NONCE';
     const INVALID_MESSAGE_TO_CONSUME: felt252 = 'INVALID_MESSAGE_TO_CONSUME';
     const INVALID_MESSAGE_TO_SEAL: felt252 = 'INVALID_MESSAGE_TO_SEAL';
     const NO_MESSAGE_TO_CANCEL: felt252 = 'NO_MESSAGE_TO_CANCEL';
@@ -63,8 +62,8 @@ mod messaging_cpt {
         sn_to_appc_cancellations: Map::<MessageHash, u64>,
         /// The nonce for messages sent to the Appchain from Starknet.
         sn_to_appc_nonce: Nonce,
-        /// Ledger of messages hashes sent from Starknet to the appchain.
-        sn_to_appc_messages: Map::<MessageHash, Nonce>,
+        /// Ledger of messages hashes sent from Starknet to the appchain and their status.
+        sn_to_appc_messages: Map::<MessageHash, MessageToAppchainStatus>,
         /// Ledger of messages hashes registered from the Appchain and a refcount
         /// associated to it to control messages consumption.
         appc_to_sn_messages: Map::<MessageHash, felt252>,
@@ -165,9 +164,14 @@ mod messaging_cpt {
             selector: felt252,
             payload: Span<felt252>
         ) -> (MessageHash, Nonce) {
-            // Starts by +1 to avoid 0 as a valid nonce.
-            let nonce = self.sn_to_appc_nonce.read() + 1;
-            self.sn_to_appc_nonce.write(nonce);
+            // From the what's done from L1 to L2, the first nonce must be 0:
+            // <https://github.com/starkware-libs/cairo-lang/blob/caba294d82eeeccc3d86a158adb8ba209bf2d8fc/src/starkware/starknet/solidity/StarknetMessaging.sol#L117>
+            // The value is read from the storage, and then incremented but the read
+            // value is used at the tx nonce.
+            let mut nonce = self.sn_to_appc_nonce.read();
+
+            // Increment the nonce, but the read value is used at the tx nonce.
+            self.sn_to_appc_nonce.write(nonce + 1);
 
             let from = starknet::get_caller_address();
             let message_hash = hash::compute_message_hash_sn_to_appc(
@@ -179,21 +183,16 @@ mod messaging_cpt {
                     MessageSent { message_hash, from, to: to_address, selector, nonce, payload, }
                 );
 
-            self.sn_to_appc_messages.write(message_hash, nonce);
+            self.sn_to_appc_messages.write(message_hash, MessageToAppchainStatus::Pending(nonce));
             (message_hash, nonce)
         }
 
         fn sn_to_appchain_messages(
             self: @ComponentState<TContractState>, message_hash: MessageHash
         ) -> MessageToAppchainStatus {
-            let nonce = self.sn_to_appc_messages.read(message_hash);
-            if nonce == 0 {
-                return MessageToAppchainStatus::SealedOrNotSent;
-            }
-            return MessageToAppchainStatus::Pending(nonce);
+            self.sn_to_appc_messages.read(message_hash)
         }
 
-
         fn appchain_to_sn_messages(
             self: @ComponentState<TContractState>, message_hash: MessageHash
         ) -> MessageToStarknetStatus {
@@ -235,16 +234,16 @@ mod messaging_cpt {
             payload: Span<felt252>,
             nonce: Nonce,
         ) -> MessageHash {
-            assert(nonce.is_non_zero(), errors::INVALID_NONCE);
             let from = starknet::get_caller_address();
 
             let message_hash = hash::compute_message_hash_sn_to_appc(
                 from, to_address, selector, payload, nonce
             );
 
-            assert(
-                self.sn_to_appc_messages.read(message_hash) == nonce, errors::NO_MESSAGE_TO_CANCEL
-            );
+            match self.sn_to_appc_messages.read(message_hash) {
+                MessageToAppchainStatus::Pending(_) => {},
+                _ => assert(false, errors::NO_MESSAGE_TO_CANCEL),
+            };
 
             self.sn_to_appc_cancellations.write(message_hash, starknet::get_block_timestamp());
 
@@ -272,7 +271,10 @@ mod messaging_cpt {
             );
 
             assert(
-                self.sn_to_appc_messages.read(message_hash) == nonce, errors::NO_MESSAGE_TO_CANCEL
+                self
+                    .sn_to_appc_messages
+                    .read(message_hash) == MessageToAppchainStatus::Pending(nonce),
+                errors::NO_MESSAGE_TO_CANCEL
             );
 
             let request_time = self.sn_to_appc_cancellations.read(message_hash);
@@ -291,10 +293,41 @@ mod messaging_cpt {
                 );
 
             // Once canceled, no more operation can be done on this message.
-            self.sn_to_appc_messages.write(message_hash, 0);
+            self.sn_to_appc_messages.write(message_hash, MessageToAppchainStatus::Cancelled);
 
             return message_hash;
         }
+
+        #[cfg(feature: 'messaging_test')]
+        fn add_messages_hashes_from_appchain(
+            ref self: ComponentState<TContractState>, messages_hashes: Span<felt252>
+        ) {
+            let mut i = 0_usize;
+            loop {
+                if i == messages_hashes.len() {
+                    break;
+                }
+
+                let msg_hash = *messages_hashes[i];
+
+                let count = self.appc_to_sn_messages.read(msg_hash);
+                self.appc_to_sn_messages.write(msg_hash, count + 1);
+
+                // We can't have the detail of the message here, so we emit a dummy event
+                // with at least the message hash.
+                self
+                    .emit(
+                        MessageToStarknetReceived {
+                            message_hash: msg_hash,
+                            from: 0.try_into().unwrap(),
+                            to: 0.try_into().unwrap(),
+                            payload: array![].span()
+                        }
+                    );
+
+                i += 1;
+            };
+        }
     }
 
     #[generate_trait]
@@ -363,12 +396,17 @@ mod messaging_cpt {
                         let message_hash = hash::compute_message_hash_sn_to_appc(
                             from, to, selector, payload, nonce
                         );
-                        assert(
-                            self.sn_to_appc_messages.read(message_hash).is_non_zero(),
-                            errors::INVALID_MESSAGE_TO_SEAL
-                        );
 
-                        self.sn_to_appc_messages.write(message_hash, 0);
+                        match self.sn_to_appc_messages.read(message_hash) {
+                            MessageToAppchainStatus::Pending(_) => {},
+                            _ => assert(false, errors::INVALID_MESSAGE_TO_SEAL),
+                        };
+
+                        // On the L1, they use the Fee in front of the message hash, not the nonce.
+                        // Here, we have an enum to explicitly indicate that the message is sealed.
+                        self
+                            .sn_to_appc_messages
+                            .write(message_hash, MessageToAppchainStatus::Sealed);
 
                         self
                             .emit(

src/messaging/interface.cairo

@@ -116,4 +116,16 @@ trait IMessaging<T> {
         payload: Span<felt252>,
         nonce: felt252,
     ) -> MessageHash;
+
+    /// Manually registers messages hashes as consumable.
+    ///
+    /// This function is mostly used to decorrelate the messages registration from the
+    /// state update step. Since all messages are registered once the proof is verified,
+    /// and the output of SNOS parsed, this function shortcut this step.
+    ///
+    /// The appchain sequencer for testing e2e messaging can then use this function to
+    /// register messages hashes that are considered as ready to be consumed. This can be done
+    /// right after the block is produced, for fast and reliable messaging testing.
+    #[cfg(feature: 'messaging_test')]
+    fn add_messages_hashes_from_appchain(ref self: T, messages_hashes: Span<felt252>);
 }

src/messaging/tests/test_messaging.cairo

@@ -168,7 +168,7 @@ fn send_message_ok() {
     let (message_hash, nonce) = mock.send_message_to_appchain(to, selector, payload.span());
 
     assert(message_hash.is_non_zero(), 'invalid message hash');
-    assert(nonce == 1, 'invalid nonce');
+    assert(nonce == 0, 'invalid nonce');
 
     let expected_event = MessageSent {
         message_hash, from, to, selector, nonce: nonce, payload: payload.span(),
@@ -190,13 +190,10 @@ fn sn_to_appchain_messages_ok() {
 
     // Calculate the message_hash
     let message_hash = hash::compute_message_hash_sn_to_appc(
-        from_address: from, to_address: to, :selector, payload: payload.span(), nonce: 1
+        from_address: from, to_address: to, :selector, payload: payload.span(), nonce: 0
     );
     let is_pending_before = mock.sn_to_appchain_messages(message_hash);
-    assert(
-        is_pending_before == MessageToAppchainStatus::SealedOrNotSent,
-        'Should not be pending before'
-    );
+    assert(is_pending_before == MessageToAppchainStatus::NotSent, 'Should not be pending before');
 
     snf::start_cheat_caller_address(from, from);
     let (message_hash, nonce) = mock.send_message_to_appchain(to, selector, payload.span());
@@ -248,14 +245,14 @@ fn start_cancellation_ok() {
 }
 
 #[test]
-#[should_panic(expected: ('INVALID_NONCE',))]
+#[should_panic(expected: ('NO_MESSAGE_TO_CANCEL',))]
 fn start_cancellation_invalid_nonce() {
     let (mock, _) = deploy_mock();
 
     let to = c::RECIPIENT();
     let selector = selector!("func1");
     let payload = array![1, 2, 3];
-    let nonce = 0;
+    let nonce = 0x800000000000011000000000000000000000000000000000000000000000000;
 
     mock.start_message_cancellation(to, selector, payload.span(), nonce);
 }
@@ -325,7 +322,7 @@ fn cancel_message_no_message_to_cancel() {
     let to = c::RECIPIENT();
     let selector = selector!("func1");
     let payload = array![1, 2, 3];
-    let nonce = 1;
+    let nonce = 0;
 
     mock.cancel_message(to, selector, payload.span(), nonce + 1000);
 }
@@ -407,7 +404,7 @@ fn process_messages_to_appchain_ok() {
     let (_message_hash, _nonce) = mock.send_message_to_appchain(to, selector, payload.span());
 
     let m = MessageToAppchain {
-        from_address: from, to_address: to, nonce: 1, selector, payload: payload.span(),
+        from_address: from, to_address: to, nonce: 0, selector, payload: payload.span(),
     };
 
     let _message_hash = hash::compute_message_hash_sn_to_appc(
@@ -470,7 +467,28 @@ fn appchain_to_sn_messages_ok() {
 
     // Ensure that message is available to consume
     let count_after = mock.appchain_to_sn_messages(message_hash);
-    assert(count_after == MessageToStarknetStatus::ReadyToConsume(1), 'message not be present');
+    assert(count_after == MessageToStarknetStatus::ReadyToConsume(1), 'message not present');
+}
+
+#[cfg(feature: 'messaging_test')]
+#[test]
+fn appchain_to_sn_messages_hashes_test() {
+    let mut mock = mock_state_testing();
+
+    let from = c::SPENDER();
+    let to = starknet::get_contract_address();
+    let payload = array![1, 2, 3].span();
+
+    let message_hash = hash::compute_message_hash_appc_to_sn(from, to, payload);
+
+    let previous_status = mock.appchain_to_sn_messages(message_hash);
+    assert(previous_status == MessageToStarknetStatus::NothingToConsume, 'message already present');
+
+    mock.add_messages_hashes_from_appchain(array![message_hash].span());
+
+    // Ensure that message is available to consume.
+    let count_after = mock.appchain_to_sn_messages(message_hash);
+    assert(count_after == MessageToStarknetStatus::ReadyToConsume(1), 'message not present');
 }
 
 #[test]

src/messaging/types.cairo

@@ -1,9 +1,12 @@
 pub type MessageHash = felt252;
 pub type Nonce = felt252;
 
-#[derive(Serde, Drop, PartialEq)]
+#[derive(Serde, Drop, PartialEq, starknet::Store, Default)]
 pub enum MessageToAppchainStatus {
-    SealedOrNotSent, // sn->appc: The nonce is 0 for the message hash.
+    #[default]
+    NotSent,
+    Sealed,
+    Cancelled,
     Pending: Nonce, // sn->appc: The nonce > 0.
 }
 

src/snos_output.cairo

@@ -89,6 +89,8 @@ fn read_segment(ref input_iter: ArrayIter<felt252>, segment_length: usize) -> Ar
         };
     return segment;
 }
+/// Custom deserialization function, inspired by
+///https://github.com/starkware-libs/cairo-lang/blob/8e11b8cc65ae1d0959328b1b4a40b92df8b58595/src/starkware/starknet/core/aggregator/output_parser.py
 fn deserialize_os_output(ref input_iter: ArrayIter<felt252>) -> StarknetOsOutput {
     let _ = read_segment(ref input_iter, 3);
     let header = read_segment(ref input_iter, HEADER_SIZE);

tests/test_appchain.cairo

@@ -227,9 +227,7 @@ fn update_state_ok() {
     // The state update contains a message to appchain, therefore, before
     // being sealed, it must be sent first.
     // The nonce must be adjusted to ensure the correct message to be sent.
-    snf::store(
-        appchain.contract_address, selector!("sn_to_appc_nonce"), array![1629170 - 1].span()
-    );
+    snf::store(appchain.contract_address, selector!("sn_to_appc_nonce"), array![1629170].span());
 
     snf::start_cheat_caller_address(appchain.contract_address, contract_sn);
     imsg.send_message_to_appchain(contract_appc, selector_appc, payload_sn_to_appc);