diff --git a/src/config/component.cairo b/src/config/component.cairo index 7ca2320..c2471e8 100644 --- a/src/config/component.cairo +++ b/src/config/component.cairo @@ -5,6 +5,8 @@ /// Errors. mod errors { const INVALID_CALLER: felt252 = 'Config: not owner or operator'; + const ALREADY_REGISTERED: felt252 = 'Config: already operator'; + const NOT_OPERATOR: felt252 = 'Config: not operator'; } /// Configuration component. @@ -23,9 +25,8 @@ mod config_cpt { #[storage] struct Storage { - /// Appchain operator that is allowed to update the state. - // TODO(#9): Multiple Operators - operator: ContractAddress, + /// Appchain operators that are allowed to update the state. + operators: LegacyMap, /// Program info (StarknetOS), with program hash and config hash. program_info: (felt252, felt252), /// Facts registry contract address. @@ -53,13 +54,20 @@ mod config_cpt { +HasComponent, impl Ownable: ownable_cpt::HasComponent > of IConfig> { - fn set_operator(ref self: ComponentState, address: ContractAddress) { + fn register_operator(ref self: ComponentState, address: ContractAddress) { get_dep_component!(@self, Ownable).assert_only_owner(); - self.operator.write(address) + assert(!self.operators.read(address), errors::ALREADY_REGISTERED); + self.operators.write(address, true); } - fn get_operator(self: @ComponentState) -> ContractAddress { - self.operator.read() + fn unregister_operator(ref self: ComponentState, address: ContractAddress) { + get_dep_component!(@self, Ownable).assert_only_owner(); + assert(self.operators.read(address), errors::NOT_OPERATOR); + self.operators.write(address, false); + } + + fn is_operator(self: @ComponentState, address: ContractAddress) -> bool { + self.operators.read(address) } fn set_program_info( @@ -118,7 +126,7 @@ mod config_cpt { ref self: ComponentState, address: ContractAddress ) -> bool { let owner = get_dep_component!(@self, Ownable).owner(); - address == owner || address == self.operator.read() + address == owner || self.is_operator(address) } } } diff --git a/src/config/interface.cairo b/src/config/interface.cairo index 183ec90..f83c547 100644 --- a/src/config/interface.cairo +++ b/src/config/interface.cairo @@ -5,19 +5,28 @@ use starknet::ContractAddress; #[starknet::interface] trait IConfig { - /// Sets the operator that is in charge to push state updates. + /// Registers an operator that is in charge to push state updates. + /// Multiple operators can be registered. + /// # Arguments /// + /// * `address` - The account address to register as an operator. + fn register_operator(ref self: T, address: ContractAddress); + + /// Unregisters an operator. /// # Arguments /// - /// * `address` - The operator account address. - fn set_operator(ref self: T, address: ContractAddress); + /// * `address` - The operator account address to unregister. + fn unregister_operator(ref self: T, address: ContractAddress); - /// Gets the operator address. + /// Verifies if the given address is an operator. + /// # Arguments + /// + /// * `address` - The address to verify. /// /// # Returns /// - /// The operator's address. - fn get_operator(self: @T) -> ContractAddress; + /// 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). diff --git a/src/config/tests/test_config.cairo b/src/config/tests/test_config.cairo index 216b985..5d5e2eb 100644 --- a/src/config/tests/test_config.cairo +++ b/src/config/tests/test_config.cairo @@ -15,24 +15,70 @@ fn deploy_mock() -> IConfigDispatcher { } #[test] -fn config_set_operator_ok() { +fn config_register_operator_ok() { let mock = deploy_mock(); - assert(mock.get_operator() == c::ZERO(), 'expect 0 addr'); + assert(!mock.is_operator(c::OPERATOR()), 'expect not operator'); snf::start_prank(CheatTarget::One(mock.contract_address), c::OWNER()); - mock.set_operator(c::OPERATOR()); - assert(mock.get_operator() == c::OPERATOR(), 'expect operator'); + mock.register_operator(c::OPERATOR()); + assert(mock.is_operator(c::OPERATOR()), 'expect operator'); +} + +#[test] +fn config_register_multiple_operators_ok() { + let mock = deploy_mock(); + assert(!mock.is_operator(c::OPERATOR()), 'expect not operator'); + assert(!mock.is_operator(c::OTHER()), 'expect not operator'); + + snf::start_prank(CheatTarget::One(mock.contract_address), c::OWNER()); + + mock.register_operator(c::OPERATOR()); + mock.register_operator(c::OTHER()); + + assert(mock.is_operator(c::OPERATOR()), 'expect operator'); + assert(mock.is_operator(c::OTHER()), 'expect operator'); +} + +#[test] +fn config_unregister_operator_ok() { + let mock = deploy_mock(); + + snf::start_prank(CheatTarget::One(mock.contract_address), c::OWNER()); + + mock.register_operator(c::OPERATOR()); + assert(mock.is_operator(c::OPERATOR()), 'expect operator'); + + mock.unregister_operator(c::OPERATOR()); + assert(!mock.is_operator(c::OPERATOR()), 'expect not operator'); +} + +#[test] +fn config_unregister_multiple_operators_ok() { + let mock = deploy_mock(); + + snf::start_prank(CheatTarget::One(mock.contract_address), c::OWNER()); + + mock.register_operator(c::OPERATOR()); + mock.register_operator(c::OTHER()); + + assert(mock.is_operator(c::OPERATOR()), 'expect operator'); + assert(mock.is_operator(c::OTHER()), 'expect operator'); + + mock.unregister_operator(c::OPERATOR()); + mock.unregister_operator(c::OTHER()); + assert(!mock.is_operator(c::OPERATOR()), 'expect not operator'); + assert(!mock.is_operator(c::OTHER()), 'expect not operator'); } #[test] #[should_panic(expected: ('Caller is not the owner',))] fn config_set_operator_unauthorized() { let mock = deploy_mock(); - assert(mock.get_operator() == c::ZERO(), 'expect 0 addr'); + assert(!mock.is_operator(c::OPERATOR()), 'expect 0 addr'); - mock.set_operator(c::OPERATOR()); - assert(mock.get_operator() == c::OPERATOR(), 'expect operator'); + mock.register_operator(c::OPERATOR()); + assert(mock.is_operator(c::OPERATOR()), 'expect operator'); } #[test] @@ -45,7 +91,7 @@ fn config_set_program_info_ok() { mock.set_program_info(0x1, 0x2); assert(mock.get_program_info() == (0x1, 0x2), 'expect correct hashes'); - mock.set_operator(c::OPERATOR()); + mock.register_operator(c::OPERATOR()); // Operator can also set the program info. snf::start_prank(CheatTarget::One(mock.contract_address), c::OPERATOR()); @@ -75,7 +121,7 @@ fn config_set_facts_registry_ok() { mock.set_facts_registry(facts_registry_address); assert(mock.get_facts_registry() == facts_registry_address, 'expect valid address'); - mock.set_operator(c::OPERATOR()); + mock.register_operator(c::OPERATOR()); // Operator can also set the program info. snf::start_prank(CheatTarget::One(mock.contract_address), c::OPERATOR());