From 341fe9cbb7800f3d8cc4017eeac35c585918f0c4 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 26 Apr 2022 11:27:54 +0200 Subject: [PATCH 1/6] Fixed the ordering of ids. Before it was sorted: inherent_ids traits_ids Now the ordering is not changed. It is important for dispatching logic and wildcard. Dispatching takes the global index of the method but if ordering is changed, it will use wrong id. In the case of wildcard it breaks it at all if trait was implemented before inherent section. --- crates/lang/codegen/src/generator/dispatch.rs | 61 ++++++++----------- .../forward-calls/lib.rs | 53 ++++++++++++---- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/crates/lang/codegen/src/generator/dispatch.rs b/crates/lang/codegen/src/generator/dispatch.rs index acfcc174ea..c5cd6d0b8e 100644 --- a/crates/lang/codegen/src/generator/dispatch.rs +++ b/crates/lang/codegen/src/generator/dispatch.rs @@ -154,48 +154,38 @@ impl Dispatch<'_> { ) -> TokenStream2 { let span = self.contract.module().storage().span(); let storage_ident = self.contract.module().storage().ident(); - let inherent_ids = self + let message_ids = self .contract .module() .impls() - .filter(|item_impl| item_impl.trait_path().is_none()) - .flat_map(|item_impl| item_impl.iter_messages()) - .map(|message| { - let span = message.span(); - message_spans.push(span); - let id = message - .composed_selector() - .into_be_u32() - .hex_padded_suffixed(); - quote_spanned!(span=> #id) - }) - .collect::>(); - let trait_ids = self - .contract - .module() - .impls() - .filter_map(|item_impl| { - item_impl - .trait_path() - .map(|trait_path| { - iter::repeat(trait_path).zip(item_impl.iter_messages()) - }) + .map(|item_impl| { + iter::repeat(item_impl.trait_path()).zip(item_impl.iter_messages()) }) .flatten() .map(|(trait_path, message)| { - let local_id = message.local_id().hex_padded_suffixed(); let span = message.span(); message_spans.push(span); - quote_spanned!(span=> - { - ::core::primitive::u32::from_be_bytes( - <<::ink_lang::reflect::TraitDefinitionRegistry<<#storage_ident as ::ink_lang::reflect::ContractEnv>::Env> - as #trait_path>::__ink_TraitInfo - as ::ink_lang::reflect::TraitMessageInfo<#local_id>>::SELECTOR - ) - } - ) - }); + + if let Some(trait_path) = trait_path { + let local_id = message.local_id().hex_padded_suffixed(); + quote_spanned!(span=> + { + ::core::primitive::u32::from_be_bytes( + <<::ink_lang::reflect::TraitDefinitionRegistry<<#storage_ident as ::ink_lang::reflect::ContractEnv>::Env> + as #trait_path>::__ink_TraitInfo + as ::ink_lang::reflect::TraitMessageInfo<#local_id>>::SELECTOR + ) + } + ) + } else { + let id = message + .composed_selector() + .into_be_u32() + .hex_padded_suffixed(); + quote_spanned!(span=> #id) + } + }) + .collect::>(); quote_spanned!(span=> impl ::ink_lang::reflect::ContractDispatchableMessages<{ <#storage_ident as ::ink_lang::reflect::ContractAmountDispatchables>::MESSAGES @@ -204,8 +194,7 @@ impl Dispatch<'_> { ::core::primitive::u32; <#storage_ident as ::ink_lang::reflect::ContractAmountDispatchables>::MESSAGES ] = [ - #( #inherent_ids , )* - #( #trait_ids ),* + #( #message_ids , )* ]; } ) diff --git a/examples/upgradeable-contracts/forward-calls/lib.rs b/examples/upgradeable-contracts/forward-calls/lib.rs index cb2cfcddb0..5ff82cfec2 100644 --- a/examples/upgradeable-contracts/forward-calls/lib.rs +++ b/examples/upgradeable-contracts/forward-calls/lib.rs @@ -33,23 +33,36 @@ pub mod proxy { admin: AccountId, } - impl Proxy { - /// Instantiate this contract with an address of the `logic` contract. - /// - /// Sets the privileged account to the caller. Only this account may - /// later changed the `forward_to` address. - #[ink(constructor)] - pub fn new(forward_to: AccountId) -> Self { - Self { - admin: Self::env().caller(), - forward_to, - } - } + /// The trait provides managing of forwarder methods. + #[ink_lang::trait_definition] + pub trait Forwarder { + /// Returns `AccountId` of contract that calls are forward to + #[ink(message)] + fn forward_to(&self) -> AccountId; + + /// Returns `AccountId` of administrator of current contract + #[ink(message)] + fn admin(&self) -> AccountId; /// Changes the `AccountId` of the contract where any call that does /// not match a selector of this contract is forwarded to. #[ink(message)] - pub fn change_forward_address(&mut self, new_address: AccountId) { + fn change_forward_address(&mut self, new_address: AccountId); + } + + impl Forwarder for Proxy { + #[ink(message)] + fn forward_to(&self) -> AccountId { + self.forward_to + } + + #[ink(message)] + fn admin(&self) -> AccountId { + self.admin + } + + #[ink(message)] + fn change_forward_address(&mut self, new_address: AccountId) { assert_eq!( self.env().caller(), self.admin, @@ -59,6 +72,20 @@ pub mod proxy { ); self.forward_to = new_address; } + } + + impl Proxy { + /// Instantiate this contract with an address of the `logic` contract. + /// + /// Sets the privileged account to the caller. Only this account may + /// later changed the `forward_to` address. + #[ink(constructor)] + pub fn new(forward_to: AccountId) -> Self { + Self { + admin: Self::env().caller(), + forward_to, + } + } /// Fallback message for a contract call that doesn't match any /// of the other message selectors. From 0be727e9d160443239e4239e787cf7311a4f2a94 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 26 Apr 2022 11:41:08 +0200 Subject: [PATCH 2/6] Make lcippy happy --- crates/lang/codegen/src/generator/dispatch.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/lang/codegen/src/generator/dispatch.rs b/crates/lang/codegen/src/generator/dispatch.rs index c5cd6d0b8e..fee922033e 100644 --- a/crates/lang/codegen/src/generator/dispatch.rs +++ b/crates/lang/codegen/src/generator/dispatch.rs @@ -158,10 +158,9 @@ impl Dispatch<'_> { .contract .module() .impls() - .map(|item_impl| { + .flat_map(|item_impl| { iter::repeat(item_impl.trait_path()).zip(item_impl.iter_messages()) }) - .flatten() .map(|(trait_path, message)| { let span = message.span(); message_spans.push(span); From cd4ec511f2478df4b8b21cad7e385e038384d5ae Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 26 Apr 2022 12:30:26 +0200 Subject: [PATCH 3/6] Update UI test to check that ids have right ordering with trait sections --- .../ui/contract/pass/message-selector.rs | 63 ++++++++++++++++--- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/crates/lang/tests/ui/contract/pass/message-selector.rs b/crates/lang/tests/ui/contract/pass/message-selector.rs index 303c699076..9f883d6f6d 100644 --- a/crates/lang/tests/ui/contract/pass/message-selector.rs +++ b/crates/lang/tests/ui/contract/pass/message-selector.rs @@ -6,20 +6,45 @@ mod contract { #[ink(storage)] pub struct Contract {} + #[ink_lang::trait_definition] + pub trait Messages { + #[ink(message)] + fn message_0(&self); + + #[ink(message, selector = 1)] + fn message_1(&self); + } + + impl Messages for Contract { + #[ink(message)] + fn message_0(&self) {} + + #[ink(message, selector = 1)] + fn message_1(&self) {} + } + impl Contract { #[ink(constructor)] pub fn constructor() -> Self { Self {} } - #[ink(message)] - pub fn message_0(&self) {} - - #[ink(message, selector = 1)] - pub fn message_1(&self) {} - #[ink(message, selector = 0xC0DE_CAFE)] pub fn message_2(&self) {} + + #[ink(message, selector = _)] + pub fn message_3(&self) {} + } + + #[ink_lang::trait_definition] + pub trait Messages2 { + #[ink(message, selector = 0x12345678)] + fn message_4(&self); + } + + impl Messages2 for Contract { + #[ink(message, selector = 0x12345678)] + fn message_4(&self) {} } } @@ -34,7 +59,7 @@ fn main() { >>::IDS[0] }, >>::SELECTOR, - [0x5A, 0x6A, 0xC1, 0x5D], + [0xFB, 0xAB, 0x03, 0xCE], ); assert_eq!( >::SELECTOR, 0xC0DE_CAFE_u32.to_be_bytes(), ); + assert_eq!( + ::MESSAGES + }, + >>::IDS[3] + }, + >>::SELECTOR, + [0xB6, 0xC3, 0x27, 0x49], + ); + assert_eq!( + ::MESSAGES + }, + >>::IDS[4] + }, + >>::SELECTOR, + 0x12345678_u32.to_be_bytes(), + ); } From fa697e73e8d2343af7b1969a3f1dd403d8ac33aa Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 26 Apr 2022 14:22:03 +0200 Subject: [PATCH 4/6] Move the method back to inherent section to fix ink waterfall --- .../forward-calls/lib.rs | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/examples/upgradeable-contracts/forward-calls/lib.rs b/examples/upgradeable-contracts/forward-calls/lib.rs index 5ff82cfec2..febaeb58b9 100644 --- a/examples/upgradeable-contracts/forward-calls/lib.rs +++ b/examples/upgradeable-contracts/forward-calls/lib.rs @@ -33,7 +33,7 @@ pub mod proxy { admin: AccountId, } - /// The trait provides managing of forwarder methods. + /// The trait provides information about `Proxy` #[ink_lang::trait_definition] pub trait Forwarder { /// Returns `AccountId` of contract that calls are forward to @@ -43,11 +43,6 @@ pub mod proxy { /// Returns `AccountId` of administrator of current contract #[ink(message)] fn admin(&self) -> AccountId; - - /// Changes the `AccountId` of the contract where any call that does - /// not match a selector of this contract is forwarded to. - #[ink(message)] - fn change_forward_address(&mut self, new_address: AccountId); } impl Forwarder for Proxy { @@ -60,18 +55,6 @@ pub mod proxy { fn admin(&self) -> AccountId { self.admin } - - #[ink(message)] - fn change_forward_address(&mut self, new_address: AccountId) { - assert_eq!( - self.env().caller(), - self.admin, - "caller {:?} does not have sufficient permissions, only {:?} does", - self.env().caller(), - self.admin, - ); - self.forward_to = new_address; - } } impl Proxy { @@ -87,6 +70,20 @@ pub mod proxy { } } + /// Changes the `AccountId` of the contract where any call that does + /// not match a selector of this contract is forwarded to. + #[ink(message)] + pub fn change_forward_address(&mut self, new_address: AccountId) { + assert_eq!( + self.env().caller(), + self.admin, + "caller {:?} does not have sufficient permissions, only {:?} does", + self.env().caller(), + self.admin, + ); + self.forward_to = new_address; + } + /// Fallback message for a contract call that doesn't match any /// of the other message selectors. /// From b4f6ecf76b5264188620966a5da2a6f1ea096f13 Mon Sep 17 00:00:00 2001 From: GreenBaneling | Supercolony Date: Fri, 29 Apr 2022 23:04:16 +0100 Subject: [PATCH 5/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Müller --- examples/upgradeable-contracts/forward-calls/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/upgradeable-contracts/forward-calls/lib.rs b/examples/upgradeable-contracts/forward-calls/lib.rs index febaeb58b9..c050f3d65c 100644 --- a/examples/upgradeable-contracts/forward-calls/lib.rs +++ b/examples/upgradeable-contracts/forward-calls/lib.rs @@ -33,14 +33,14 @@ pub mod proxy { admin: AccountId, } - /// The trait provides information about `Proxy` + /// The trait provides information about the forward pattern used in this contract. #[ink_lang::trait_definition] pub trait Forwarder { - /// Returns `AccountId` of contract that calls are forward to + /// Returns the `AccountId` of the contract that calls are forward to. #[ink(message)] fn forward_to(&self) -> AccountId; - /// Returns `AccountId` of administrator of current contract + /// Returns the `AccountId` of the admin of the current contract. #[ink(message)] fn admin(&self) -> AccountId; } From 0f1715208cce6208118a99bea97c827284debbee Mon Sep 17 00:00:00 2001 From: xgreenx Date: Mon, 2 May 2022 10:27:48 +0200 Subject: [PATCH 6/6] Remove trait --- .../forward-calls/lib.rs | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/examples/upgradeable-contracts/forward-calls/lib.rs b/examples/upgradeable-contracts/forward-calls/lib.rs index c050f3d65c..cb2cfcddb0 100644 --- a/examples/upgradeable-contracts/forward-calls/lib.rs +++ b/examples/upgradeable-contracts/forward-calls/lib.rs @@ -33,30 +33,6 @@ pub mod proxy { admin: AccountId, } - /// The trait provides information about the forward pattern used in this contract. - #[ink_lang::trait_definition] - pub trait Forwarder { - /// Returns the `AccountId` of the contract that calls are forward to. - #[ink(message)] - fn forward_to(&self) -> AccountId; - - /// Returns the `AccountId` of the admin of the current contract. - #[ink(message)] - fn admin(&self) -> AccountId; - } - - impl Forwarder for Proxy { - #[ink(message)] - fn forward_to(&self) -> AccountId { - self.forward_to - } - - #[ink(message)] - fn admin(&self) -> AccountId { - self.admin - } - } - impl Proxy { /// Instantiate this contract with an address of the `logic` contract. ///