feat: ensure snos program hash is checked (#46)

* snos program hash validation

* merge snos info with program info

* regenerate bindings

* Program info struct

* reword comments and cleanup inline documentation

---------

Co-authored-by: glihm <[email protected]>

Author: chudkowsky

bindings/src/lib.rs

@@ -17,18 +17,18 @@ mod appchain {
     use core::poseidon::{Poseidon, PoseidonImpl, HashStateImpl, poseidon_hash_span};
     use openzeppelin::access::ownable::{
         OwnableComponent as ownable_cpt, OwnableComponent::InternalTrait as OwnableInternal,
-        interface::IOwnable
+        interface::IOwnable,
     };
     use openzeppelin::security::reentrancyguard::{
         ReentrancyGuardComponent,
-        ReentrancyGuardComponent::InternalTrait as InternalReentrancyGuardImpl
+        ReentrancyGuardComponent::InternalTrait as InternalReentrancyGuardImpl,
     };
     use openzeppelin::upgrades::{
         UpgradeableComponent as upgradeable_cpt,
-        UpgradeableComponent::InternalTrait as UpgradeableInternal, interface::IUpgradeable
+        UpgradeableComponent::InternalTrait as UpgradeableInternal, interface::IUpgradeable,
     };
     use piltover::components::onchain_data_fact_tree_encoder::{
-        encode_fact_with_onchain_data, DataAvailabilityFact
+        encode_fact_with_onchain_data, DataAvailabilityFact,
     };
     use piltover::config::{config_cpt, config_cpt::InternalTrait as ConfigInternal, IConfig};
     use piltover::fact_registry::{IFactRegistryDispatcher, IFactRegistryDispatcherTrait};
@@ -52,7 +52,7 @@ mod appchain {
     component!(path: messaging_cpt, storage: messaging, event: MessagingEvent);
     component!(path: state_cpt, storage: state, event: StateEvent);
     component!(
-        path: ReentrancyGuardComponent, storage: reentrancy_guard, event: ReentrancyGuardEvent
+        path: ReentrancyGuardComponent, storage: reentrancy_guard, event: ReentrancyGuardEvent,
     );
 
     #[abi(embed_v0)]
@@ -134,42 +134,47 @@ mod appchain {
             snos_output: Span<felt252>,
             program_output: Span<felt252>,
             onchain_data_hash: felt252,
-            onchain_data_size: u256
+            onchain_data_size: u256,
         ) {
             self.reentrancy_guard.start();
             self.config.assert_only_owner_or_operator();
 
+            let program_info = self.config.program_info.read();
+
+            // StarknetOS (SNOS) proof is wrapped in bootloader so 3rd element is the program hash
+            // of bootloaded program, in our case SNOS.
+            let snos_program_hash = snos_output.at(2);
+            assert!(program_info.snos_program_hash == *snos_program_hash);
+
             let snos_output_hash = poseidon_hash_span(snos_output);
+            // Layout bridge program is also bootloaded, and the 5th element is the hash of the
+            // output of the program that has been layout-bridged.
             let snos_output_hash_in_bridge_output = program_output.at(4);
             assert!(snos_output_hash == *snos_output_hash_in_bridge_output);
+
             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 (current_program_hash, current_config_hash): (felt252, felt252) = self
-                .config
-                .program_info
-                .read();
-
             let data_availability_fact: DataAvailabilityFact = DataAvailabilityFact {
-                onchain_data_hash, onchain_data_size
+                onchain_data_hash, onchain_data_size,
             };
             let state_transition_fact: u256 = encode_fact_with_onchain_data(
-                program_output, data_availability_fact
+                program_output, data_availability_fact,
             );
 
             assert(
-                program_output_struct.starknet_os_config_hash == current_config_hash,
-                errors::SNOS_INVALID_CONFIG_HASH
+                program_output_struct.starknet_os_config_hash == program_info.config_hash,
+                errors::SNOS_INVALID_CONFIG_HASH,
             );
 
-            let fact = poseidon_hash_span(array![current_program_hash, output_hash].span());
+            let fact = poseidon_hash_span(array![program_info.program_hash, output_hash].span());
             assert!(
                 *IFactRegistryDispatcher { contract_address: self.config.get_facts_registry() }
                     .get_all_verifications_for_fact_hash(fact)
                     .at(0)
-                    .security_bits > 50
+                    .security_bits > 50,
             );
 
             self.emit(LogStateTransitionFact { state_transition_fact });
@@ -191,7 +196,7 @@ mod appchain {
                         state_root: self.state.state_root.read(),
                         block_number: self.state.block_number.read(),
                         block_hash: self.state.block_hash.read(),
-                    }
+                    },
                 );
         }
     }

src/appchain.cairo

@@ -28,12 +28,28 @@ mod config_cpt {
     struct Storage {
         /// Appchain operators that are allowed to update the state.
         operators: Map<ContractAddress, bool>,
-        /// Program info (StarknetOS), with program hash and config hash.
-        program_info: (felt252, felt252),
+        /// The information of the program verified to apply the state transition.
+        program_info: ProgramInfo,
         /// Facts registry contract address.
         facts_registry: ContractAddress,
     }
 
+    /// Information of the program verified onchain to apply the state transition.
+    ///
+    /// In the current design, the StarknetOS (SNOS) is executed and proven first.
+    /// Since the layout used by SNOS is not verifiable onchain, a bridge layout
+    /// program is also executed on the proof generated from SNOS execution.
+    ///
+    /// For this reason, the program info contains the hash of the SNOS program,
+    /// additionally to the `program_hash`, which in this case is the bridge layout program hash.
+    /// This ensures that the correct programs have been executed.
+    #[derive(starknet::Store, Drop, Serde, Copy, PartialEq)]
+    struct ProgramInfo {
+        program_hash: felt252,
+        config_hash: felt252,
+        snos_program_hash: felt252,
+    }
+
     #[event]
     #[derive(Copy, Drop, starknet::Event)]
     enum Event {
@@ -43,10 +59,8 @@ mod config_cpt {
     #[derive(Copy, Drop, starknet::Event)]
     struct ProgramInfoChanged {
         changed_by: ContractAddress,
-        old_program_hash: felt252,
-        new_program_hash: felt252,
-        old_config_hash: felt252,
-        new_config_hash: felt252,
+        old_program_info: ProgramInfo,
+        new_program_info: ProgramInfo,
     }
 
     #[embeddable_as(ConfigImpl)]
@@ -71,25 +85,21 @@ mod config_cpt {
             self.operators.read(address)
         }
 
-        fn set_program_info(
-            ref self: ComponentState<TContractState>, program_hash: felt252, config_hash: felt252
-        ) {
+        fn set_program_info(ref self: ComponentState<TContractState>, program_info: ProgramInfo) {
             self.assert_only_owner_or_operator();
-            let (old_program_hash, old_config_hash): (felt252, felt252) = self.program_info.read();
-            self.program_info.write((program_hash, config_hash));
+            let old_program_info = self.program_info.read();
+            self.program_info.write(program_info);
             self
                 .emit(
                     ProgramInfoChanged {
                         changed_by: starknet::get_caller_address(),
-                        old_program_hash: old_program_hash,
-                        new_program_hash: program_hash,
-                        old_config_hash: old_config_hash,
-                        new_config_hash: config_hash,
+                        old_program_info,
+                        new_program_info: program_info,
                     }
                 );
         }
 
-        fn get_program_info(self: @ComponentState<TContractState>) -> (felt252, felt252) {
+        fn get_program_info(self: @ComponentState<TContractState>) -> ProgramInfo {
             self.program_info.read()
         }
 

src/config/component.cairo

@@ -2,6 +2,7 @@
 //!
 //! Interface for appchain settlement contract configuration.
 use starknet::ContractAddress;
+use super::component::config_cpt::ProgramInfo;
 
 #[starknet::interface]
 trait IConfig<T> {
@@ -28,22 +29,21 @@ trait IConfig<T> {
     /// True if the address is an operator, false otherwise.
     fn is_operator(self: @T, address: ContractAddress) -> bool;
 
-    /// Sets the information of the program that generates the
-    /// state transition trace (namely StarknetOS).
+    /// Sets the information of the program verified onchain to
+    /// execute the state transition.
     ///
     /// # Arguments
     ///
-    /// * `program_hash` - The program hash.
-    /// * `config_hash` - The program's config hash.
-    fn set_program_info(ref self: T, program_hash: felt252, config_hash: felt252);
+    /// * `program_info` - The program information.
+    fn set_program_info(ref self: T, program_info: ProgramInfo);
 
-    /// Gets the information of the program that generates the
-    /// state transition trace (namely StarknetOS).
+    /// Gets the information of the program verified onchain to
+    /// execute the state transition.
     ///
     /// # Returns
     ///
-    /// The program hash and it's configuration hash.
-    fn get_program_info(self: @T) -> (felt252, felt252);
+    /// The program information.
+    fn get_program_info(self: @T) -> ProgramInfo;
 
     /// Sets the facts registry contract address, which is already
     /// initialized with the verifier information.

src/config/interface.cairo

@@ -1,7 +1,7 @@
 use openzeppelin_testing::constants as c;
 use piltover::config::{
-    config_cpt, config_cpt::InternalTrait as ConfigInternal, IConfig, IConfigDispatcherTrait,
-    IConfigDispatcher, config_mock
+    config_cpt, config_cpt::ProgramInfo, config_cpt::InternalTrait as ConfigInternal, IConfig,
+    IConfigDispatcherTrait, IConfigDispatcher, config_mock
 };
 use snforge_std as snf;
 use snforge_std::ContractClassTrait;
@@ -88,16 +88,20 @@ fn config_set_program_info_ok() {
     snf::start_cheat_caller_address(mock.contract_address, c::OWNER());
 
     // Owner sets the info.
-    mock.set_program_info(0x1, 0x2);
-    assert(mock.get_program_info() == (0x1, 0x2), 'expect correct hashes');
+    let program_info = ProgramInfo { program_hash: 0x1, config_hash: 0x2, snos_program_hash: 0x3, };
+    mock.set_program_info(program_info);
+    assert(mock.get_program_info() == program_info, 'expect correct hashes');
 
     mock.register_operator(c::OPERATOR());
 
     // Operator can also set the program info.
     snf::start_cheat_caller_address(mock.contract_address, c::OPERATOR());
-    mock.set_program_info(0x11, 0x22);
+    let program_info = ProgramInfo {
+        program_hash: 0x11, config_hash: 0x22, snos_program_hash: 0x33,
+    };
+    mock.set_program_info(program_info);
 
-    assert(mock.get_program_info() == (0x11, 0x22), 'expect operator hashes');
+    assert(mock.get_program_info() == program_info, 'expect operator hashes');
 }
 
 #[test]
@@ -106,7 +110,10 @@ fn config_set_program_info_unauthorized() {
     let mock = deploy_mock();
 
     snf::start_cheat_caller_address(mock.contract_address, c::OPERATOR());
-    mock.set_program_info(0x11, 0x22);
+    mock
+        .set_program_info(
+            ProgramInfo { program_hash: 0x1, config_hash: 0x2, snos_program_hash: 0x3, }
+        );
 }
 
 #[test]

src/config/tests/test_config.cairo

@@ -5,7 +5,7 @@ use core::result::ResultTrait;
 //!
 use openzeppelin_testing::constants as c;
 use piltover::appchain::appchain::{Event, LogStateUpdate, LogStateTransitionFact};
-use piltover::config::{IConfig, IConfigDispatcherTrait, IConfigDispatcher};
+use piltover::config::{IConfig, config_cpt::ProgramInfo, IConfigDispatcherTrait, IConfigDispatcher};
 use piltover::fact_registry::{
     IFactRegistryDispatcher, IFactRegistryDispatcherTrait, fact_registry_mock
 };
@@ -171,7 +171,10 @@ fn appchain_owner_ok() {
     let iconfig = IConfigDispatcher { contract_address: appchain.contract_address };
 
     snf::start_cheat_caller_address(appchain.contract_address, c::OWNER());
-    iconfig.set_program_info(0x11, 0x22);
+    iconfig
+        .set_program_info(
+            ProgramInfo { program_hash: 0x11, config_hash: 0x22, snos_program_hash: 0x33 }
+        );
 }
 
 #[test]
@@ -180,7 +183,10 @@ fn appchain_owner_only() {
     let (appchain, _spy) = deploy_with_owner(c::OWNER().into());
 
     let iconfig = IConfigDispatcher { contract_address: appchain.contract_address };
-    iconfig.set_program_info(0x11, 0x22);
+    iconfig
+        .set_program_info(
+            ProgramInfo { program_hash: 0x11, config_hash: 0x22, snos_program_hash: 0x33 }
+        );
 }
 
 #[test]
@@ -219,11 +225,13 @@ fn update_state_ok() {
     snf::start_cheat_caller_address(appchain.contract_address, c::OWNER());
     iconfig
         .set_program_info(
-            program_hash: 0,
-            config_hash: 8868593919264901768958912247765226517850727970326290266005120699201631282
+            ProgramInfo {
+                program_hash: 0,
+                config_hash: 8868593919264901768958912247765226517850727970326290266005120699201631282,
+                snos_program_hash: 3
+            }
         );
     iconfig.set_facts_registry(address: fact_registry_mock.contract_address);
-
     // 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.