From ee7b45d4a9d50a4599f6f4e7a4bc06c718b98c95 Mon Sep 17 00:00:00 2001 From: prxgr4mmer Date: Thu, 13 Jul 2023 11:19:11 +0300 Subject: [PATCH 1/7] Fix reentrancy example --- docs/docs/smart-contracts/example/data.md | 24 +------------------ .../contracts/flip_on_me/lib.rs | 12 +++++++--- .../reentrancy_guard/contracts/flipper/lib.rs | 9 ++++++- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/docs/docs/smart-contracts/example/data.md b/docs/docs/smart-contracts/example/data.md index 86dfae3ec..9cd10862b 100644 --- a/docs/docs/smart-contracts/example/data.md +++ b/docs/docs/smart-contracts/example/data.md @@ -35,12 +35,6 @@ For example, if in your default implementation you need to have `psp22::extensio you can add bounds `T: Storage + Storage`. It allows you to work with two independent storage. -`openbrush::traits::Storage` trait requires that the inner data implements the -`openbrush::traits::OccupyStorage` trait. -It is because each storage in the smart contract should occupy a unique storage key. -Overlapping of those keys can cause unexpected bugs. Derive macro provided by -OpenBrush to simplify the implementation of the `Storage` trait also checks that -the storage key from the `OccupyStorage ` trait is unique. ### Data of the trait @@ -53,23 +47,7 @@ pub struct PointData { } ``` -If you want to use `openbrush::traits::Storage` then you also need to implement `openbrush::traits::OccupyStorage` -with unique storage key. - -```rust -pub struct PointData { - pub x: u32, - pub y: u32, -} - -impl openbrush::traits::OccupyStorage for PointData { - // You can specify your unique key manually like `0x123` or you can use macro - const KEY: u32 = openbrush::storage_unique_key!(PointData); -} -``` - -Also, you can use the `openbrush::upgradeable_storage` macro that implements that trait by default, -and also prepare the storage to be upgradeable. +You can use the `openbrush::storage_item` macro that implements that trait by default. ```rust #[openbrush::storage_item] diff --git a/examples/reentrancy_guard/contracts/flip_on_me/lib.rs b/examples/reentrancy_guard/contracts/flip_on_me/lib.rs index 03c343229..7217a53ca 100644 --- a/examples/reentrancy_guard/contracts/flip_on_me/lib.rs +++ b/examples/reentrancy_guard/contracts/flip_on_me/lib.rs @@ -2,8 +2,10 @@ #[openbrush::contract] pub mod flip_on_me { + use flipper::traits::flipper::*; use flipper::traits::flip_on_me::*; - use ink::codegen::Env; + use ink::env::CallFlags; + use openbrush::traits::DefaultEnv; #[ink(storage)] #[derive(Default)] @@ -19,14 +21,18 @@ pub mod flip_on_me { impl FlipOnMe for FlipOnMeContract { #[ink(message)] fn flip_on_me(&mut self) -> Result<(), ReentrancyGuardError> { - let caller = self.env().caller(); + let caller = Self::env().caller(); self.flip_on_target(caller) } #[ink(message)] fn flip_on_target(&mut self, callee: AccountId) -> Result<(), ReentrancyGuardError> { // This method does a cross-contract call to caller contract and calls the `flip` method. - flipper::traits::flipper::FlipperRef::flip(&callee) + FlipperRef::flip_builder(&callee) + .call_flags(CallFlags::default().set_allow_reentry(true)) + .invoke() + .unwrap(); + Ok(()) } } } diff --git a/examples/reentrancy_guard/contracts/flipper/lib.rs b/examples/reentrancy_guard/contracts/flipper/lib.rs index d795bfcc0..9d994c0e9 100644 --- a/examples/reentrancy_guard/contracts/flipper/lib.rs +++ b/examples/reentrancy_guard/contracts/flipper/lib.rs @@ -3,10 +3,12 @@ #[openbrush::contract] pub mod my_flipper_guard { use flipper::traits::flipper::*; + use flipper::traits::flip_on_me::*; use openbrush::{ modifiers, traits::Storage, }; + use ink::env::CallFlags; #[ink(storage)] #[derive(Default, Storage)] @@ -43,7 +45,12 @@ pub mod my_flipper_guard { // Callee contract during execution of `flip_on_me` will call `flip` of this contract. // `call_flip_on_me` and `flip` are marked with `non_reentrant` modifier. It means, // that call of `flip` after `call_flip_on_me` must fail. - flipper::traits::flip_on_me::FlipOnMeRef::flip_on_me(&callee) + //FlipOnMeRef::flip_on_me(&callee).call_flags(CallFlags::default().set_allow_reentry(true)); + FlipOnMeRef::flip_on_me_builder(&callee) + .call_flags(CallFlags::default().set_allow_reentry(true)) + .invoke() + .unwrap(); + Ok(()) } } } From b724215210332361a2776c9e541850ac599e08fc Mon Sep 17 00:00:00 2001 From: prxgr4mmer Date: Thu, 13 Jul 2023 11:40:11 +0300 Subject: [PATCH 2/7] Fix reentrancy example --- examples/reentrancy_guard/contracts/flipper/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/reentrancy_guard/contracts/flipper/lib.rs b/examples/reentrancy_guard/contracts/flipper/lib.rs index 9d994c0e9..1ab57e00b 100644 --- a/examples/reentrancy_guard/contracts/flipper/lib.rs +++ b/examples/reentrancy_guard/contracts/flipper/lib.rs @@ -45,7 +45,6 @@ pub mod my_flipper_guard { // Callee contract during execution of `flip_on_me` will call `flip` of this contract. // `call_flip_on_me` and `flip` are marked with `non_reentrant` modifier. It means, // that call of `flip` after `call_flip_on_me` must fail. - //FlipOnMeRef::flip_on_me(&callee).call_flags(CallFlags::default().set_allow_reentry(true)); FlipOnMeRef::flip_on_me_builder(&callee) .call_flags(CallFlags::default().set_allow_reentry(true)) .invoke() From d39dc26a3470398802a6a1399b2724fb233f4249 Mon Sep 17 00:00:00 2001 From: prxgr4mmer Date: Thu, 13 Jul 2023 13:05:59 +0300 Subject: [PATCH 3/7] Revert changes in docs/docs/example/data.md --- docs/docs/smart-contracts/example/data.md | 44 +++++++++++++++++------ 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/docs/docs/smart-contracts/example/data.md b/docs/docs/smart-contracts/example/data.md index 9cd10862b..f758323d6 100644 --- a/docs/docs/smart-contracts/example/data.md +++ b/docs/docs/smart-contracts/example/data.md @@ -7,11 +7,11 @@ title: Data and derive macro Rust doesn't have inheritance like OOP languages. If you want to "inherit" some fields, you can use structural composition. -If you want to "inherit" some implementation, you can use traits. +If you want to "inherit" some implementation, you can use traits. Traits can have a [default implementation](https://doc.rust-lang.org/book/ch10-02-traits.html#default-implementations) or a [generic implementation](https://doc.rust-lang.org/book/ch10-02-traits.html#using-trait-bounds-to-conditionally-implement-methods). The traits in Rust can't contain fields, it is pure interfaces. -Based on that information we propose you the following concept of smart contract +Based on that information we propose you the following concept of smart contract development: ### Storage trait @@ -31,14 +31,20 @@ pub trait PointStorage { Or you can use `openbrush::traits::Storage` trait from OpenBrush. `Storage` is a generic trait, so you can use it to work with different storage. -For example, if in your default implementation you need to have `psp22::extensions::metadata::Data` and `psp22::Data`, +For example, if in your default implementation you need to have `psp22::extensions::metadata::Data` and `psp22::Data`, you can add bounds `T: Storage + Storage`. It allows you to work with two independent storage. +`openbrush::traits::Storage` trait requires that the inner data implements the +`openbrush::traits::OccupyStorage` trait. +It is because each storage in the smart contract should occupy a unique storage key. +Overlapping of those keys can cause unexpected bugs. Derive macro provided by +OpenBrush to simplify the implementation of the `Storage` trait also checks that +the storage key from the `OccupyStorage ` trait is unique. ### Data of the trait -That trait returns some data with fields that can be used in the implementation. +That trait returns some data with fields that can be used in the implementation. The data is a simple struct with fields. Later that struct can be embedded into the contract struct. ```rust pub struct PointData { @@ -47,7 +53,23 @@ pub struct PointData { } ``` -You can use the `openbrush::storage_item` macro that implements that trait by default. +If you want to use `openbrush::traits::Storage` then you also need to implement `openbrush::traits::OccupyStorage` +with unique storage key. + +```rust +pub struct PointData { + pub x: u32, + pub y: u32, +} + +impl openbrush::traits::OccupyStorage for PointData { + // You can specify your unique key manually like `0x123` or you can use macro + const KEY: u32 = openbrush::storage_unique_key!(PointData); +} +``` + +Also, you can use the `openbrush::upgradeable_storage` macro that implements that trait by default, +and also prepare the storage to be upgradeable. ```rust #[openbrush::storage_item] @@ -59,7 +81,7 @@ pub struct PointData { ### Default implementation -Define the default or generic implementation for your main trait with the restriction that `Self` +Define the default or generic implementation for your main trait with the restriction that `Self` should also implement storage trait. A default implementation: @@ -68,11 +90,11 @@ pub trait Point: PointStorage { fn x(&self) -> u32 { PointStorage::get(self).x } - + fn y(&self) -> u32 { PointStorage::get(self).y } - + fn name(&self) -> String { "AlphaPoint".to_string() } @@ -110,11 +132,11 @@ pub trait Point: openbrush::traits::Storage { fn x(&self) -> u32 { self.data().x } - + fn y(&self) -> u32 { self.data().y } - + fn name(&self) -> String { "AlphaPoint".to_string() } @@ -148,7 +170,7 @@ impl> Point for T { ### "Inheritance" of the implementation -When someone wants to "inherit" implementation and fields, he can embed the data structure, +When someone wants to "inherit" implementation and fields, he can embed the data structure, implement the storage trait, and define an impl section of the main trait: ```rust struct PointContract { From 464f47353544da21bbdcddd6085abfb423116ab0 Mon Sep 17 00:00:00 2001 From: prxgr4mmer Date: Thu, 13 Jul 2023 22:16:19 +0300 Subject: [PATCH 4/7] Delete flip_on_me function --- examples/reentrancy_guard/contracts/flip_on_me/lib.rs | 8 ++------ examples/reentrancy_guard/contracts/flipper/lib.rs | 6 +++++- examples/reentrancy_guard/traits/flip_on_me.rs | 3 --- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/examples/reentrancy_guard/contracts/flip_on_me/lib.rs b/examples/reentrancy_guard/contracts/flip_on_me/lib.rs index 7217a53ca..340b7bdc6 100644 --- a/examples/reentrancy_guard/contracts/flip_on_me/lib.rs +++ b/examples/reentrancy_guard/contracts/flip_on_me/lib.rs @@ -1,5 +1,7 @@ #![cfg_attr(not(feature = "std"), no_std, no_main)] +pub use openbrush::examples::contracts::reentrancy_guard::flip_on_me::*; + #[openbrush::contract] pub mod flip_on_me { use flipper::traits::flipper::*; @@ -19,12 +21,6 @@ pub mod flip_on_me { } impl FlipOnMe for FlipOnMeContract { - #[ink(message)] - fn flip_on_me(&mut self) -> Result<(), ReentrancyGuardError> { - let caller = Self::env().caller(); - self.flip_on_target(caller) - } - #[ink(message)] fn flip_on_target(&mut self, callee: AccountId) -> Result<(), ReentrancyGuardError> { // This method does a cross-contract call to caller contract and calls the `flip` method. diff --git a/examples/reentrancy_guard/contracts/flipper/lib.rs b/examples/reentrancy_guard/contracts/flipper/lib.rs index 1ab57e00b..581117019 100644 --- a/examples/reentrancy_guard/contracts/flipper/lib.rs +++ b/examples/reentrancy_guard/contracts/flipper/lib.rs @@ -1,5 +1,7 @@ #![cfg_attr(not(feature = "std"), no_std, no_main)] +pub use openbrush::examples::contracts::reentrancy_guard::my_flipper_guard::*; + #[openbrush::contract] pub mod my_flipper_guard { use flipper::traits::flipper::*; @@ -9,6 +11,8 @@ pub mod my_flipper_guard { traits::Storage, }; use ink::env::CallFlags; + use openbrush::traits::DefaultEnv; + #[ink(storage)] #[derive(Default, Storage)] @@ -45,7 +49,7 @@ pub mod my_flipper_guard { // Callee contract during execution of `flip_on_me` will call `flip` of this contract. // `call_flip_on_me` and `flip` are marked with `non_reentrant` modifier. It means, // that call of `flip` after `call_flip_on_me` must fail. - FlipOnMeRef::flip_on_me_builder(&callee) + FlipOnMeRef::flip_on_target_builder(&callee, Self::env().account_id()) .call_flags(CallFlags::default().set_allow_reentry(true)) .invoke() .unwrap(); diff --git a/examples/reentrancy_guard/traits/flip_on_me.rs b/examples/reentrancy_guard/traits/flip_on_me.rs index 1472c260b..74e1f1a1c 100644 --- a/examples/reentrancy_guard/traits/flip_on_me.rs +++ b/examples/reentrancy_guard/traits/flip_on_me.rs @@ -6,9 +6,6 @@ pub type FlipOnMeRef = dyn FlipOnMe; #[openbrush::trait_definition] pub trait FlipOnMe { - #[ink(message)] - fn flip_on_me(&mut self) -> Result<(), ReentrancyGuardError>; - #[ink(message)] fn flip_on_target(&mut self, callee: AccountId) -> Result<(), ReentrancyGuardError>; } From b1d162657b75e6936b3c7c3582cb711c77e549ce Mon Sep 17 00:00:00 2001 From: Artemka374 Date: Thu, 20 Jul 2023 13:56:08 +0300 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0993e3f88..ed37b7e94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,17 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased v4.0.0] +## [v4.0.0-beta] +## Changes ### Added -- [*BREAKING*] `implementation`, `override` macros: [78](https://github.com/Brushfam/openbrush-contracts/pull/78) -- [*BREAKING*] `storage_item` macro, new OB feature: `Upgradeable`, which implements `set_code_hash` functionality [99](https://github.com/Brushfam/openbrush-contracts/pull/99) -- `UI` tests [77](https://github.com/Brushfam/openbrush-contracts/pull/77) -- `openbrush::accessors` macro for automatic generation of getters/setters for storage items: [66](https://github.com/Brushfam/openbrush-contracts/pull/66) and [61](https://github.com/Brushfam/openbrush-contracts/pull/61) +- [*BREAKING*] `implementation`, `override`, `default_impl` macros: [#78](https://github.com/Brushfam/openbrush-contracts/pull/78) +- [*BREAKING*] `storage_item` macro, which implements `#[ink::storage_item]` macro, but also allows to make field of struct upgradeable by using `#[lazy]` attribute. For all fields that + are either `Lazy`/`Mapping`/`MultiMapping` is generated it's own constant storage key. Also it allows OpenBrush to work correctly with every default implementation [#99](https://github.com/Brushfam/openbrush-contracts/pull/99) +- New OB feature: `Upgradeable`, which implements `set_code_hash` functionality [#99](https://github.com/Brushfam/openbrush-contracts/pull/99) +- `UI` tests for testing different scenarios for macros [#77](https://github.com/Brushfam/openbrush-contracts/pull/77) +- `openbrush::accessors` macro for automatic generation of getters/setters for storage items: [#66](https://github.com/Brushfam/openbrush-contracts/pull/66) and [61](https://github.com/Brushfam/openbrush-contracts/pull/61) ### Removed -- [*BREAKING*] `min_specilization`, now openbrush is `stable`: [78](https://github.com/Brushfam/openbrush-contracts/pull/78) -- `ZERO_ADDRESS`, now using `Option` instead: [98](https://github.com/Brushfam/openbrush-contracts/pull/98) +- [*BREAKING*] `upgradeable_storage` macro, `OccupyStorage` trait [#99](https://github.com/Brushfam/openbrush-contracts/pull/99) +- [*BREAKING*] `min_specilization`, now OpenBrush can be used with `stable` toolchain: [#78](https://github.com/Brushfam/openbrush-contracts/pull/78) +- [*BREAKING*] `ZERO_ADDRESS`, now using `Option` instead: [#98](https://github.com/Brushfam/openbrush-contracts/pull/98) + +### Changed + +- [*BREAKING*] Now every field in OpenBrush's types that is not read/written directly in storage, is wrapped in `Lazy`, so all the types in OpenBrush can be considered upgradeable: [#99](https://github.com/Brushfam/openbrush-contracts/pull/99) ### Fixed - Fixed reentrancy guard problem: [#88](https://github.com/Brushfam/openbrush-contracts/pull/88) +- Updated reentrancy example: [#108](https://github.com/Brushfam/openbrush-contracts/pull/108) \ No newline at end of file From 4d4115a56ce93d53ca5ec43761deb15b852793b1 Mon Sep 17 00:00:00 2001 From: Artemka374 Date: Thu, 20 Jul 2023 14:36:09 +0300 Subject: [PATCH 6/7] fix payment_splitter example --- examples/payment_splitter/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/payment_splitter/lib.rs b/examples/payment_splitter/lib.rs index 9247fb5b6..ccf008750 100644 --- a/examples/payment_splitter/lib.rs +++ b/examples/payment_splitter/lib.rs @@ -3,7 +3,6 @@ #[openbrush::implementation(PaymentSplitter)] #[openbrush::contract] pub mod my_payment_splitter { - use ink::prelude::vec::Vec; use openbrush::traits::Storage; #[ink(storage)] From 25adb8ebb19339cea061e721a63bdf72c618afb9 Mon Sep 17 00:00:00 2001 From: Artemka374 Date: Thu, 20 Jul 2023 14:40:09 +0300 Subject: [PATCH 7/7] fix timelock_controller example --- examples/timelock_controller/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/timelock_controller/lib.rs b/examples/timelock_controller/lib.rs index 77053bcf6..b1f4d312c 100644 --- a/examples/timelock_controller/lib.rs +++ b/examples/timelock_controller/lib.rs @@ -3,7 +3,6 @@ #[openbrush::implementation(AccessControl, TimelockController)] #[openbrush::contract] pub mod my_timelock_controller { - use ink::prelude::vec::Vec; use openbrush::traits::Storage; #[ink(storage)]