From d47b342c815de76e937a0910b10ae3ec4310f199 Mon Sep 17 00:00:00 2001 From: Jules Doumeche <30329843+julio4@users.noreply.github.com> Date: Wed, 13 Dec 2023 22:32:29 +0900 Subject: [PATCH] Feat: Component storage collision example + `Switchable` component refactor (#153) * feat: Components How-To with simple `Switch` Component * chore: remove `ch0*` prefix for improved modularity * Refactor switch component to switchable component * feat: Component storage collisions --- .../components/src/contracts.cairo | 4 ++ .../components/src/contracts/switch.cairo | 23 +++++---- .../src/contracts/switch_collision.cairo | 49 +++++++++++++++++++ .../components/src/contracts/tests.cairo | 1 + .../tests/switch_collision_tests.cairo | 44 +++++++++++++++++ .../applications/components/src/lib.cairo | 4 +- .../src/{switch.cairo => switchable.cairo} | 22 ++++----- src/SUMMARY.md | 1 + src/components/collisions.md | 30 ++++++++++++ src/components/how_to.md | 18 +++++-- 10 files changed, 167 insertions(+), 29 deletions(-) create mode 100644 listings/applications/components/src/contracts/switch_collision.cairo create mode 100644 listings/applications/components/src/contracts/tests.cairo create mode 100644 listings/applications/components/src/contracts/tests/switch_collision_tests.cairo rename listings/applications/components/src/{switch.cairo => switchable.cairo} (63%) create mode 100644 src/components/collisions.md diff --git a/listings/applications/components/src/contracts.cairo b/listings/applications/components/src/contracts.cairo index ae0de39b..45b334e6 100644 --- a/listings/applications/components/src/contracts.cairo +++ b/listings/applications/components/src/contracts.cairo @@ -1 +1,5 @@ mod switch; +mod switch_collision; + +#[cfg(test)] +mod tests; diff --git a/listings/applications/components/src/contracts/switch.cairo b/listings/applications/components/src/contracts/switch.cairo index 5d1a9c37..8846fbd6 100644 --- a/listings/applications/components/src/contracts/switch.cairo +++ b/listings/applications/components/src/contracts/switch.cairo @@ -1,20 +1,18 @@ // ANCHOR: contract #[starknet::contract] mod SwitchContract { - use components::switch::switch_component; - // This is needed to be able to use internal functions of the switch component. - use components::switch::switch_component::InternalSwitchImpl; + use components::switchable::switchable_component; - component!(path: switch_component, storage: switch, event: SwitchEvent); + component!(path: switchable_component, storage: switch, event: SwitchableEvent); #[abi(embed_v0)] - impl SwitchImpl = switch_component::Switch; - impl SwitchInternalImpl = switch_component::InternalSwitchImpl; + impl SwitchableImpl = switchable_component::Switchable; + impl SwitchableInternalImpl = switchable_component::InternalImpl; #[storage] struct Storage { #[substorage(v0)] - switch: switch_component::Storage, + switch: switchable_component::Storage, } #[constructor] @@ -25,15 +23,16 @@ mod SwitchContract { #[event] #[derive(Drop, starknet::Event)] enum Event { - SwitchEvent: switch_component::Event + SwitchableEvent: switchable_component::Event, } } // ANCHOR_END: contract #[cfg(test)] mod tests { - use components::switch::switch_component::InternalSwitchTrait; - use components::switch::ISwitchComponent; + use components::switchable::switchable_component::InternalTrait; + use components::switchable::ISwitchable; + use core::starknet::storage::StorageMemberAccessTrait; use super::SwitchContract; @@ -64,10 +63,10 @@ mod tests { #[available_gas(2000000)] fn test_value() { let mut state = STATE(); - assert(state.value() == state.switch.value.read(), 'Wrong value'); + assert(state.value() == state.switch.switchable_value.read(), 'Wrong value'); state.switch.switch(); - assert(state.value() == state.switch.value.read(), 'Wrong value'); + assert(state.value() == state.switch.switchable_value.read(), 'Wrong value'); } #[test] diff --git a/listings/applications/components/src/contracts/switch_collision.cairo b/listings/applications/components/src/contracts/switch_collision.cairo new file mode 100644 index 00000000..69e6c0ad --- /dev/null +++ b/listings/applications/components/src/contracts/switch_collision.cairo @@ -0,0 +1,49 @@ +// ANCHOR: interface +#[starknet::interface] +trait ISwitchCollision { + fn set(ref self: TContractState, value: bool); + fn get(ref self: TContractState) -> bool; +} +// ANCHOR_END: interface + +#[starknet::contract] +mod SwitchCollisionContract { + use components::switchable::switchable_component; + + component!(path: switchable_component, storage: switch, event: SwitchableEvent); + + #[abi(embed_v0)] + impl SwitchableImpl = switchable_component::Switchable; + impl SwitchableInternalImpl = switchable_component::InternalImpl; + + // ANCHOR: storage + #[storage] + struct Storage { + switchable_value: bool, + #[substorage(v0)] + switch: switchable_component::Storage, + } + // ANCHOR_END: storage + + #[constructor] + fn constructor(ref self: ContractState) { + self.switch._off(); + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + SwitchableEvent: switchable_component::Event, + } + + #[external(v0)] + impl SwitchCollisionContract of super::ISwitchCollision { + fn set(ref self: ContractState, value: bool) { + self.switchable_value.write(value); + } + + fn get(ref self: ContractState) -> bool { + self.switchable_value.read() + } + } +} diff --git a/listings/applications/components/src/contracts/tests.cairo b/listings/applications/components/src/contracts/tests.cairo new file mode 100644 index 00000000..8b3bd47d --- /dev/null +++ b/listings/applications/components/src/contracts/tests.cairo @@ -0,0 +1 @@ +mod switch_collision_tests; diff --git a/listings/applications/components/src/contracts/tests/switch_collision_tests.cairo b/listings/applications/components/src/contracts/tests/switch_collision_tests.cairo new file mode 100644 index 00000000..939cbd54 --- /dev/null +++ b/listings/applications/components/src/contracts/tests/switch_collision_tests.cairo @@ -0,0 +1,44 @@ +mod switch_collision_tests { + use components::switchable::switchable_component::InternalTrait; + use components::switchable::{ISwitchable, ISwitchableDispatcher, ISwitchableDispatcherTrait}; + + use components::contracts::switch_collision::{ + SwitchCollisionContract, ISwitchCollisionDispatcher, ISwitchCollisionDispatcherTrait + }; + + use core::starknet::storage::StorageMemberAccessTrait; + use starknet::deploy_syscall; + + fn deploy() -> (ISwitchCollisionDispatcher, ISwitchableDispatcher) { + let (contract_address, _) = deploy_syscall( + SwitchCollisionContract::TEST_CLASS_HASH.try_into().unwrap(), 0, array![].span(), false + ) + .unwrap(); + + ( + ISwitchCollisionDispatcher { contract_address }, + ISwitchableDispatcher { contract_address }, + ) + } + + #[test] + #[available_gas(2000000)] + // ANCHOR: collision + fn test_collision() { + let (mut contract, mut contract_iswitch) = deploy(); + + assert(contract.get() == false, 'value !off'); + assert(contract_iswitch.value() == false, 'switch !off'); + + contract_iswitch.switch(); + assert(contract_iswitch.value() == true, 'switch !on'); + assert(contract.get() == true, 'value !on'); + + // `collision` between component storage 'value' and contract storage 'value' + assert(contract.get() == contract_iswitch.value(), 'value != switch'); + + contract.set(false); + assert(contract.get() == contract_iswitch.value(), 'value != switch'); + } +// ANCHOR_END: collision +} diff --git a/listings/applications/components/src/lib.cairo b/listings/applications/components/src/lib.cairo index 83aadd38..8e1825f4 100644 --- a/listings/applications/components/src/lib.cairo +++ b/listings/applications/components/src/lib.cairo @@ -1,2 +1,4 @@ +// Components +mod switchable; + mod contracts; -mod switch; diff --git a/listings/applications/components/src/switch.cairo b/listings/applications/components/src/switchable.cairo similarity index 63% rename from listings/applications/components/src/switch.cairo rename to listings/applications/components/src/switchable.cairo index 82f0cc16..334c7a83 100644 --- a/listings/applications/components/src/switch.cairo +++ b/listings/applications/components/src/switchable.cairo @@ -1,14 +1,14 @@ #[starknet::interface] -trait ISwitchComponent { +trait ISwitchable { fn value(self: @TContractState) -> bool; fn switch(ref self: TContractState); } #[starknet::component] -mod switch_component { +mod switchable_component { #[storage] struct Storage { - value: bool, + switchable_value: bool, } #[derive(Drop, starknet::Event)] @@ -20,26 +20,26 @@ mod switch_component { SwitchEvent: SwitchEvent, } - #[embeddable_as(Switch)] - impl SwitchImpl< + #[embeddable_as(Switchable)] + impl SwitchableImpl< TContractState, +HasComponent - > of super::ISwitchComponent> { + > of super::ISwitchable> { fn value(self: @ComponentState) -> bool { - self.value.read() + self.switchable_value.read() } fn switch(ref self: ComponentState) { - self.value.write(!self.value.read()); + self.switchable_value.write(!self.switchable_value.read()); self.emit(Event::SwitchEvent(SwitchEvent {})); } } #[generate_trait] - impl InternalSwitchImpl< + impl InternalImpl< TContractState, +HasComponent - > of InternalSwitchTrait { + > of InternalTrait { fn _off(ref self: ComponentState) { - self.value.write(false); + self.switchable_value.write(false); } } } diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 7af583f3..8a81ace3 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -34,6 +34,7 @@ Summary # Components - [Components How-To](./components/how_to.md) + - [Storage Collisions](./components/collisions.md) # Applications diff --git a/src/components/collisions.md b/src/components/collisions.md new file mode 100644 index 00000000..a2125e73 --- /dev/null +++ b/src/components/collisions.md @@ -0,0 +1,30 @@ +# Component-Contract Storage Collision + +Components can declare their own storage variables. + +When a contract use a component, the component storage is merged with the contract storage. +The storage layout is only determined by the variables names, so variables with the same name will collide. + +> In a future release, the `#[substorage(v1)]` will determine the storage layout based on the component as well, so collisions will be avoided. + +A good practice is to prefix the component storage variables with the component name, as shown in the [Switchable component example](./how_to.md). + +#### Example + +Here's an example of a collision on the `switchable_value` storage variable of the `Switchable` component. + +Interface: +```rust +{{#include ../../listings/applications/components/src/contracts/switch_collision.cairo:interface}} +``` + +Here's the storage of the contract (you can expand the code snippet to see the full contract): +```rust +{{#rustdoc_include ../../listings/applications/components/src/contracts/switch_collision.cairo:storage}} +``` + +Both the contract and the component have a `switchable_value` storage variable, so they collide: + +```rust +{{#rustdoc_include ../../listings/applications/components/src/contracts/tests/switch_collision_tests.cairo:collision}} +``` diff --git a/src/components/how_to.md b/src/components/how_to.md index 0f3855d8..9f29bd67 100644 --- a/src/components/how_to.md +++ b/src/components/how_to.md @@ -11,25 +11,33 @@ Key characteristics: ## How to create a component -The following example shows a simple `Switch` component that can be used to turn a boolean on or off. -It contains a storage variable `value`, a function `switch` and an event `Switch`. +The following example shows a simple `Switchable` component that can be used to add a switch that can be either on or off. +It contains a storage variable `switchable_value`, a function `switch` and an event `Switch`. + +> It is a good practice to prefix the component storage variables with the component name to [avoid collisions](./collisions.md). ```rust -{{#include ../../listings/applications/components/src/switch.cairo}} +{{#include ../../listings/applications/components/src/switchable.cairo}} ``` A component in itself is really similar to a contract, it *can* also have: -- An interface defining entrypoints (`ISwitchComponent`) +- An interface defining entrypoints (`ISwitchableComponent`) - A Storage struct - Events - Internal functions It don't have a constructor, but you can create a `_init` internal function and call it from the contract's constructor. In the previous example, the `_off` function is used this way. +> It's currently not possible to use the same component multiple times in the same contract. +> This is a known limitation that may be lifted in the future. +> +> For now, you can view components as an implementation of a specific interface/feature (`Ownable`, `Upgradeable`, ... `~able`). +> This is why we called it `Switchable` and not `Switch`; The contract *is switchable*, not *has a switch*. + ## How to use a component Now that we have a component, we can use it in a contract. -The following contract incorporates the `Switch` component: +The following contract incorporates the `Switchable` component: ```rust {{#include ../../listings/applications/components/src/contracts/switch.cairo:contract}}