From 3bbf856e5fc7200d5e6962981c2e763f3daeaec6 Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 01:26:23 +0200 Subject: [PATCH 1/7] added allow_multiple_var_args annotation and tests for the validate function --- contracts/examples/adder/tests/adder_scenario_rs_test.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/contracts/examples/adder/tests/adder_scenario_rs_test.rs b/contracts/examples/adder/tests/adder_scenario_rs_test.rs index a92de289ff..dc14d01b84 100644 --- a/contracts/examples/adder/tests/adder_scenario_rs_test.rs +++ b/contracts/examples/adder/tests/adder_scenario_rs_test.rs @@ -1,11 +1,7 @@ use multiversx_sc_scenario::*; fn world() -> ScenarioWorld { - let mut blockchain = ScenarioWorld::new(); - blockchain.set_current_dir_from_workspace("contracts/examples/adder"); - - blockchain.register_contract("file:output/adder.wasm", adder::ContractBuilder); - blockchain + todo!() } #[test] From 5e9b27445f0423aee997d5f0fd836b85c52fbe3a Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 01:32:16 +0200 Subject: [PATCH 2/7] commit fix --- framework/base/src/abi/endpoint_abi.rs | 1 + framework/base/src/external_view_contract.rs | 1 + framework/base/src/macros.rs | 5 ++ framework/derive/src/generate/abi_gen.rs | 5 ++ framework/derive/src/model/argument.rs | 1 + framework/derive/src/model/endpoint.rs | 3 ++ framework/derive/src/model/method.rs | 13 +++++ .../derive/src/parse/attributes/attr_names.rs | 1 + .../src/parse/attributes/endpoint_attr.rs | 4 ++ .../parse/attributes/trait_argument_prop.rs | 4 ++ .../src/parse/attributes/trait_prop_names.rs | 1 + framework/derive/src/parse/endpoint_parse.rs | 24 +++++++-- framework/derive/src/parse/method_parse.rs | 5 +- .../derive/src/parse/trait_argument_parse.rs | 12 +++++ .../meta/src/abi_json/endpoint_abi_json.rs | 3 ++ .../contract/output_contract/oc_validate.rs | 51 +++++++++++++++---- 16 files changed, 120 insertions(+), 14 deletions(-) diff --git a/framework/base/src/abi/endpoint_abi.rs b/framework/base/src/abi/endpoint_abi.rs index 1de58313dc..35ee8effd8 100644 --- a/framework/base/src/abi/endpoint_abi.rs +++ b/framework/base/src/abi/endpoint_abi.rs @@ -47,6 +47,7 @@ pub struct EndpointAbi { pub payable_in_tokens: &'static [&'static str], pub inputs: Vec, pub outputs: OutputAbis, + pub allow_multiple_var_args: bool } impl EndpointAbi { diff --git a/framework/base/src/external_view_contract.rs b/framework/base/src/external_view_contract.rs index b97c86f0c7..5f01c27872 100644 --- a/framework/base/src/external_view_contract.rs +++ b/framework/base/src/external_view_contract.rs @@ -49,5 +49,6 @@ pub fn external_view_contract_constructor_abi() -> EndpointAbi { multi_arg: false, }].to_vec(), outputs: OutputAbis::new(), + allow_multiple_var_args: false } } diff --git a/framework/base/src/macros.rs b/framework/base/src/macros.rs index 159b639195..107527c58f 100644 --- a/framework/base/src/macros.rs +++ b/framework/base/src/macros.rs @@ -202,6 +202,11 @@ macro_rules! only_owner { }; } +#[macro_export] +macro_rules! allow_multiple_var_args { + () => {}; +} + /// Converts usize to NonZeroUsize or returns SCError. #[macro_export] macro_rules! non_zero_usize { diff --git a/framework/derive/src/generate/abi_gen.rs b/framework/derive/src/generate/abi_gen.rs index 845d4fafb4..70508b84e4 100644 --- a/framework/derive/src/generate/abi_gen.rs +++ b/framework/derive/src/generate/abi_gen.rs @@ -11,6 +11,7 @@ fn generate_endpoint_snippet( only_admin: bool, mutability: EndpointMutabilityMetadata, endpoint_type: EndpointTypeMetadata, + allow_multiple_var_args: bool, ) -> proc_macro2::TokenStream { let endpoint_docs = &m.docs; let rust_method_name = m.name.to_string(); @@ -65,6 +66,7 @@ fn generate_endpoint_snippet( inputs: multiversx_sc::types::heap::Vec::new(), outputs: multiversx_sc::types::heap::Vec::new(), labels: &[ #(#label_names),* ], + allow_multiple_var_args: #allow_multiple_var_args, }; #(#input_snippets)* #output_snippet @@ -84,6 +86,7 @@ fn generate_endpoint_snippets(contract: &ContractTrait) -> Vec Vec Vec MethodPayableMetadata::NotPayable, } } + + pub fn is_allow_multiple_var_args(&self) -> bool { + match &self.public_role { + PublicRole::Init(init_metadata) => init_metadata.allow_multiple_var_args, + PublicRole::Endpoint(endpoint_metadata) => endpoint_metadata.allow_multiple_var_args, + PublicRole::Callback(callback_metadata) + | PublicRole::CallbackPromise(callback_metadata) => { + callback_metadata.allow_multiple_var_args + }, + PublicRole::CallbackRaw => true, + PublicRole::Private => false, + } + } } diff --git a/framework/derive/src/parse/attributes/attr_names.rs b/framework/derive/src/parse/attributes/attr_names.rs index 412cc95a79..184b170aea 100644 --- a/framework/derive/src/parse/attributes/attr_names.rs +++ b/framework/derive/src/parse/attributes/attr_names.rs @@ -25,3 +25,4 @@ pub(super) static ATTR_STORAGE_IS_EMPTY: &str = "storage_is_empty"; pub(super) static ATTR_STORAGE_CLEAR: &str = "storage_clear"; pub(super) static ATTR_PROXY: &str = "proxy"; pub(super) static ATTR_LABEL: &str = "label"; +pub(super) static ATTR_ALLOW_MULTIPLE_VAR_ARGS: &str = "allow_multiple_var_args"; \ No newline at end of file diff --git a/framework/derive/src/parse/attributes/endpoint_attr.rs b/framework/derive/src/parse/attributes/endpoint_attr.rs index 01d5b61abf..0e8f7b6840 100644 --- a/framework/derive/src/parse/attributes/endpoint_attr.rs +++ b/framework/derive/src/parse/attributes/endpoint_attr.rs @@ -24,6 +24,10 @@ pub fn is_proxy(attr: &syn::Attribute) -> bool { is_attribute_with_no_args(attr, ATTR_PROXY) } +pub fn is_allow_multiple_var_args(attr: &syn::Attribute) -> bool { + is_attribute_with_no_args(attr, ATTR_ALLOW_MULTIPLE_VAR_ARGS) +} + #[derive(Clone, Debug)] pub struct EndpointAttribute { pub endpoint_name: Option, diff --git a/framework/derive/src/parse/attributes/trait_argument_prop.rs b/framework/derive/src/parse/attributes/trait_argument_prop.rs index 1bda25527a..5737deaad7 100644 --- a/framework/derive/src/parse/attributes/trait_argument_prop.rs +++ b/framework/derive/src/parse/attributes/trait_argument_prop.rs @@ -7,3 +7,7 @@ pub fn is_only_owner_prop(attr: &syn::Attribute) -> bool { pub fn is_only_admin_prop(attr: &syn::Attribute) -> bool { is_attribute_with_no_args(attr, PROP_ADMIN_OWNER) } + +pub fn is_allow_multiple_var_args_prop(attr: &syn::Attribute) -> bool { + is_attribute_with_no_args(attr, PROP_ALLOW_MULTIPLE_VAR_ARGS) +} diff --git a/framework/derive/src/parse/attributes/trait_prop_names.rs b/framework/derive/src/parse/attributes/trait_prop_names.rs index 949a3e4b93..3afeeed8b4 100644 --- a/framework/derive/src/parse/attributes/trait_prop_names.rs +++ b/framework/derive/src/parse/attributes/trait_prop_names.rs @@ -1,2 +1,3 @@ pub(super) static PROP_ONLY_OWNER: &str = "only_owner"; pub(super) static PROP_ADMIN_OWNER: &str = "only_admin"; +pub(super) static PROP_ALLOW_MULTIPLE_VAR_ARGS: &str = "allow_multiple_var_args"; diff --git a/framework/derive/src/parse/endpoint_parse.rs b/framework/derive/src/parse/endpoint_parse.rs index 1400ec4167..8c90f86717 100644 --- a/framework/derive/src/parse/endpoint_parse.rs +++ b/framework/derive/src/parse/endpoint_parse.rs @@ -5,9 +5,9 @@ use crate::model::{ use super::{ attributes::{ - is_callback_raw, is_init, is_only_admin, is_only_owner, is_only_user_account, - CallbackAttribute, EndpointAttribute, ExternalViewAttribute, LabelAttribute, - OutputNameAttribute, PromisesCallbackAttribute, ViewAttribute, + is_allow_multiple_var_args, is_callback_raw, is_init, is_only_admin, is_only_owner, + is_only_user_account, CallbackAttribute, EndpointAttribute, ExternalViewAttribute, + LabelAttribute, OutputNameAttribute, PromisesCallbackAttribute, ViewAttribute, }, MethodAttributesPass1, }; @@ -27,6 +27,7 @@ pub fn process_init_attribute( check_single_role(&*method); method.public_role = PublicRole::Init(InitMetadata { payable: pass_1_data.payable.clone(), + allow_multiple_var_args: pass_1_data.allow_multiple_var_args, }); true } else { @@ -34,6 +35,18 @@ pub fn process_init_attribute( } } +pub fn process_allow_multiple_var_args_attribute( + attr: &syn::Attribute, + pass_1_data: &mut MethodAttributesPass1, +) -> bool { + let is_allow_multiple_var_args = is_allow_multiple_var_args(attr); + if is_allow_multiple_var_args { + pass_1_data.allow_multiple_var_args = true; + } + + is_allow_multiple_var_args +} + pub fn process_only_owner_attribute( attr: &syn::Attribute, pass_1_data: &mut MethodAttributesPass1, @@ -86,6 +99,7 @@ pub fn process_endpoint_attribute( only_admin: pass_1_data.only_admin, only_user_account: pass_1_data.only_user_account, mutability: EndpointMutabilityMetadata::Mutable, + allow_multiple_var_args: pass_1_data.allow_multiple_var_args, }); }) .is_some() @@ -110,6 +124,7 @@ pub fn process_view_attribute( only_admin: pass_1_data.only_admin, only_user_account: pass_1_data.only_user_account, mutability: EndpointMutabilityMetadata::Readonly, + allow_multiple_var_args: pass_1_data.allow_multiple_var_args, }); }) .is_some() @@ -134,6 +149,7 @@ pub fn process_external_view_attribute( only_admin: pass_1_data.only_admin, only_user_account: pass_1_data.only_user_account, mutability: EndpointMutabilityMetadata::Readonly, + allow_multiple_var_args: pass_1_data.allow_multiple_var_args, }); }) .is_some() @@ -159,6 +175,7 @@ pub fn process_callback_attribute(attr: &syn::Attribute, method: &mut Method) -> }; method.public_role = PublicRole::Callback(CallbackMetadata { callback_name: callback_ident, + allow_multiple_var_args: method.is_allow_multiple_var_args(), }); }) .is_some() @@ -174,6 +191,7 @@ pub fn process_promises_callback_attribute(attr: &syn::Attribute, method: &mut M }; method.public_role = PublicRole::CallbackPromise(CallbackMetadata { callback_name: callback_ident, + allow_multiple_var_args: method.is_allow_multiple_var_args(), }); }) .is_some() diff --git a/framework/derive/src/parse/method_parse.rs b/framework/derive/src/parse/method_parse.rs index 53175c0a1c..f20c7ce7f4 100644 --- a/framework/derive/src/parse/method_parse.rs +++ b/framework/derive/src/parse/method_parse.rs @@ -11,7 +11,7 @@ use super::{ process_endpoint_attribute, process_external_view_attribute, process_init_attribute, process_label_names_attribute, process_only_admin_attribute, process_only_owner_attribute, process_only_user_account_attribute, process_output_names_attribute, process_payable_attribute, - process_promises_callback_attribute, process_view_attribute, + process_promises_callback_attribute, process_view_attribute, process_allow_multiple_var_args_attribute }; pub struct MethodAttributesPass1 { pub method_name: String, @@ -19,6 +19,7 @@ pub struct MethodAttributesPass1 { pub only_owner: bool, pub only_admin: bool, pub only_user_account: bool, + pub allow_multiple_var_args: bool } pub fn process_method(m: &syn::TraitItemMethod, trait_attributes: &TraitProperties) -> Method { @@ -36,6 +37,7 @@ pub fn process_method(m: &syn::TraitItemMethod, trait_attributes: &TraitProperti only_owner: trait_attributes.only_owner, only_admin: trait_attributes.only_admin, only_user_account: trait_attributes.only_user_account, + allow_multiple_var_args: trait_attributes.allow_multiple_var_args }; let mut first_pass_unprocessed_attributes = Vec::new(); @@ -90,6 +92,7 @@ fn process_attribute_first_pass( || process_only_owner_attribute(attr, first_pass_data) || process_only_admin_attribute(attr, first_pass_data) || process_only_user_account_attribute(attr, first_pass_data) + || process_allow_multiple_var_args_attribute(attr, first_pass_data) } fn process_attributes_second_pass( diff --git a/framework/derive/src/parse/trait_argument_parse.rs b/framework/derive/src/parse/trait_argument_parse.rs index e7d723c23f..ee76b2cfce 100644 --- a/framework/derive/src/parse/trait_argument_parse.rs +++ b/framework/derive/src/parse/trait_argument_parse.rs @@ -20,6 +20,7 @@ fn process_trait_attribute( ) -> bool { process_only_owner_argument(attr, trait_arg_metadata) || process_only_admin_argument(attr, trait_arg_metadata) + || process_allow_multiple_var_args_argument(attr, trait_arg_metadata) } fn process_only_owner_argument(attr: &syn::Attribute, arg_metadata: &mut TraitProperties) -> bool { @@ -37,3 +38,14 @@ fn process_only_admin_argument(attr: &syn::Attribute, arg_metadata: &mut TraitPr } has_attr } + +fn process_allow_multiple_var_args_argument( + attr: &syn::Attribute, + arg_metadata: &mut TraitProperties, +) -> bool { + let has_attr = is_allow_multiple_var_args_prop(attr); + if has_attr { + arg_metadata.allow_multiple_var_args = true; + } + has_attr +} diff --git a/framework/meta/src/abi_json/endpoint_abi_json.rs b/framework/meta/src/abi_json/endpoint_abi_json.rs index 73a72e7954..012293ce56 100644 --- a/framework/meta/src/abi_json/endpoint_abi_json.rs +++ b/framework/meta/src/abi_json/endpoint_abi_json.rs @@ -98,6 +98,8 @@ pub struct EndpointAbiJson { pub outputs: Vec, #[serde(skip_serializing_if = "Vec::is_empty")] pub labels: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub allow_multiple_var_args: Option } impl From<&EndpointAbi> for EndpointAbiJson { @@ -120,6 +122,7 @@ impl From<&EndpointAbi> for EndpointAbiJson { inputs: abi.inputs.iter().map(InputAbiJson::from).collect(), outputs: abi.outputs.iter().map(OutputAbiJson::from).collect(), labels: abi.labels.iter().map(|&label| label.to_owned()).collect(), + allow_multiple_var_args: if abi.allow_multiple_var_args { Some(true) } else { None } } } } diff --git a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs index 1462f15a67..e5ce14933e 100644 --- a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs +++ b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs @@ -25,16 +25,47 @@ fn validate_contract_var_args(abi: &ContractAbi) -> Result<(), String> { } fn validate_endpoint_var_args(endpoint_abi: &EndpointAbi) -> Result<(), String> { - let mut var_args_encountered = false; - for arg in &endpoint_abi.inputs { - if arg.multi_arg { - var_args_encountered = true; - } else if var_args_encountered { - return Err(format!( - "Found regular arguments after var-args in method {}. This is not allowed, because it makes it impossible to parse the arguments.", - &endpoint_abi.rust_method_name)); - } + let num_var_args = endpoint_abi + .inputs + .iter() + .filter(|input| input.multi_arg) + .count(); + if num_var_args > 1usize && !endpoint_abi.allow_multiple_var_args { + return Err(format!( + "Multiple var args found in {}. Use #[allow_multiple_var_args] if you want to enable this feature", + &endpoint_abi.rust_method_name)); } - Ok(()) } + +#[cfg(test)] +mod tests { + use multiversx_sc::abi::{InputAbi, TypeName}; + + use super::*; + + #[test] + fn validate_endpoint_var_args_test() { + let mut endpoint_def = EndpointAbi::default(); + let var_arg_1 = InputAbi { + arg_name: "arg_1", + type_name: TypeName::new(), + multi_arg: true, + }; + let var_arg_2 = InputAbi { + arg_name: "arg_2", + type_name: TypeName::new(), + multi_arg: true, + }; + endpoint_def.inputs.push(var_arg_1); + endpoint_def.inputs.push(var_arg_2); + + assert_eq!(endpoint_def.allow_multiple_var_args, false); + assert_eq!(Err(format!( + "Multiple var args found in {}. Use #[allow_multiple_var_args] if you want to enable this feature", + &endpoint_def.rust_method_name)), validate_endpoint_var_args(&endpoint_def)); + + endpoint_def.allow_multiple_var_args = true; + assert_eq!(Ok(()), validate_endpoint_var_args(&endpoint_def)); + } +} From 76df05bff4d27c5b0d438122aa876a10b7f997bc Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 02:56:41 +0200 Subject: [PATCH 3/7] clippy --- contracts/examples/adder/tests/adder_scenario_rs_test.rs | 6 +++++- .../examples/crypto-kitties/kitty-ownership/src/lib.rs | 1 + contracts/examples/lottery-esdt/src/lottery.rs | 3 +++ contracts/examples/multisig/src/multisig_propose.rs | 2 ++ contracts/examples/nft-minter/src/lib.rs | 1 + contracts/examples/ping-pong-egld/src/ping_pong.rs | 1 + contracts/examples/seed-nft-minter/src/seed_nft_minter.rs | 1 + .../meta/src/cmd/contract/output_contract/oc_validate.rs | 2 +- 8 files changed, 15 insertions(+), 2 deletions(-) diff --git a/contracts/examples/adder/tests/adder_scenario_rs_test.rs b/contracts/examples/adder/tests/adder_scenario_rs_test.rs index dc14d01b84..a92de289ff 100644 --- a/contracts/examples/adder/tests/adder_scenario_rs_test.rs +++ b/contracts/examples/adder/tests/adder_scenario_rs_test.rs @@ -1,7 +1,11 @@ use multiversx_sc_scenario::*; fn world() -> ScenarioWorld { - todo!() + let mut blockchain = ScenarioWorld::new(); + blockchain.set_current_dir_from_workspace("contracts/examples/adder"); + + blockchain.register_contract("file:output/adder.wasm", adder::ContractBuilder); + blockchain } #[test] diff --git a/contracts/examples/crypto-kitties/kitty-ownership/src/lib.rs b/contracts/examples/crypto-kitties/kitty-ownership/src/lib.rs index d0b4d7073a..e3fd696b65 100644 --- a/contracts/examples/crypto-kitties/kitty-ownership/src/lib.rs +++ b/contracts/examples/crypto-kitties/kitty-ownership/src/lib.rs @@ -10,6 +10,7 @@ use random::*; #[multiversx_sc::contract] pub trait KittyOwnership { + #[allow_multiple_var_args] #[init] fn init( &self, diff --git a/contracts/examples/lottery-esdt/src/lottery.rs b/contracts/examples/lottery-esdt/src/lottery.rs index 983317a6d3..24a84bc6be 100644 --- a/contracts/examples/lottery-esdt/src/lottery.rs +++ b/contracts/examples/lottery-esdt/src/lottery.rs @@ -17,6 +17,7 @@ pub trait Lottery { #[init] fn init(&self) {} + #[allow_multiple_var_args] #[endpoint] fn start( &self, @@ -43,6 +44,7 @@ pub trait Lottery { ); } + #[allow_multiple_var_args] #[endpoint(createLotteryPool)] fn create_lottery_pool( &self, @@ -69,6 +71,7 @@ pub trait Lottery { ); } + #[allow_multiple_var_args] #[allow(clippy::too_many_arguments)] fn start_lottery( &self, diff --git a/contracts/examples/multisig/src/multisig_propose.rs b/contracts/examples/multisig/src/multisig_propose.rs index 0ebd6d8aaa..3bdd6a1ae2 100644 --- a/contracts/examples/multisig/src/multisig_propose.rs +++ b/contracts/examples/multisig/src/multisig_propose.rs @@ -75,6 +75,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { /// Can send EGLD without calling anything. /// Can call smart contract endpoints directly. /// Doesn't really work with builtin functions. + #[allow_multiple_var_args] #[endpoint(proposeTransferExecute)] fn propose_transfer_execute( &self, @@ -92,6 +93,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { /// Can use ESDTTransfer/ESDTNFTTransfer/MultiESDTTransfer to send tokens, while also optionally calling endpoints. /// Works well with builtin functions. /// Cannot simply send EGLD directly without calling anything. + #[allow_multiple_var_args] #[endpoint(proposeAsyncCall)] fn propose_async_call( &self, diff --git a/contracts/examples/nft-minter/src/lib.rs b/contracts/examples/nft-minter/src/lib.rs index 9c16cb5084..e65e8a7dff 100644 --- a/contracts/examples/nft-minter/src/lib.rs +++ b/contracts/examples/nft-minter/src/lib.rs @@ -15,6 +15,7 @@ pub trait NftMinter: nft_module::NftModule { #[init] fn init(&self) {} + #[allow_multiple_var_args] #[allow(clippy::too_many_arguments)] #[allow(clippy::redundant_closure)] #[only_owner] diff --git a/contracts/examples/ping-pong-egld/src/ping_pong.rs b/contracts/examples/ping-pong-egld/src/ping_pong.rs index 540db3783a..26b5e74880 100644 --- a/contracts/examples/ping-pong-egld/src/ping_pong.rs +++ b/contracts/examples/ping-pong-egld/src/ping_pong.rs @@ -29,6 +29,7 @@ pub trait PingPong { /// `duration_in_seconds` - how much time (in seconds) until contract expires. /// `opt_activation_timestamp` - optionally specify the contract to only actvivate at a later date. /// `max_funds` - optional funding cap, no more funds than this can be added to the contract. + #[allow_multiple_var_args] #[init] fn init( &self, diff --git a/contracts/examples/seed-nft-minter/src/seed_nft_minter.rs b/contracts/examples/seed-nft-minter/src/seed_nft_minter.rs index 23bb6d289b..5e42c7a765 100644 --- a/contracts/examples/seed-nft-minter/src/seed_nft_minter.rs +++ b/contracts/examples/seed-nft-minter/src/seed_nft_minter.rs @@ -30,6 +30,7 @@ pub trait SeedNftMinter: self.init_distribution(distribution); } + #[allow_multiple_var_args] #[only_owner] #[endpoint(createNft)] fn create_nft( diff --git a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs index e5ce14933e..fe7a701049 100644 --- a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs +++ b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs @@ -60,7 +60,7 @@ mod tests { endpoint_def.inputs.push(var_arg_1); endpoint_def.inputs.push(var_arg_2); - assert_eq!(endpoint_def.allow_multiple_var_args, false); + assert!(!endpoint_def.allow_multiple_var_args); assert_eq!(Err(format!( "Multiple var args found in {}. Use #[allow_multiple_var_args] if you want to enable this feature", &endpoint_def.rust_method_name)), validate_endpoint_var_args(&endpoint_def)); From ffb988b3ec845b1a05dea1a0cc320d3aa80373eb Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 09:42:44 +0200 Subject: [PATCH 4/7] contract examples build fix --- contracts/feature-tests/composability/vault/src/vault.rs | 1 + .../erc-style-contracts/lottery-erc20/src/lottery.rs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/contracts/feature-tests/composability/vault/src/vault.rs b/contracts/feature-tests/composability/vault/src/vault.rs index b5523c3817..d0628e9d20 100644 --- a/contracts/feature-tests/composability/vault/src/vault.rs +++ b/contracts/feature-tests/composability/vault/src/vault.rs @@ -106,6 +106,7 @@ pub trait Vault { .unwrap_or_else(|_| sc_panic!("ESDT transfer failed")); } + #[allow_multiple_var_args] #[label("promises-endpoint")] #[payable("*")] #[endpoint] diff --git a/contracts/feature-tests/erc-style-contracts/lottery-erc20/src/lottery.rs b/contracts/feature-tests/erc-style-contracts/lottery-erc20/src/lottery.rs index 396273e5a0..318563e703 100644 --- a/contracts/feature-tests/erc-style-contracts/lottery-erc20/src/lottery.rs +++ b/contracts/feature-tests/erc-style-contracts/lottery-erc20/src/lottery.rs @@ -23,6 +23,7 @@ pub trait Lottery { self.set_erc20_contract_address(&erc20_contract_address); } + #[allow_multiple_var_args] #[endpoint] fn start( &self, @@ -45,6 +46,7 @@ pub trait Lottery { ) } + #[allow_multiple_var_args] #[endpoint(createLotteryPool)] fn create_lottery_pool( &self, @@ -67,6 +69,7 @@ pub trait Lottery { ) } + #[allow_multiple_var_args] #[allow(clippy::too_many_arguments)] fn start_lottery( &self, From 658c032cfa569b82ce47a86d6dfd0cefe54b3ab6 Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 10:06:14 +0200 Subject: [PATCH 5/7] view build fix --- contracts/feature-tests/basic-features/src/echo.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/feature-tests/basic-features/src/echo.rs b/contracts/feature-tests/basic-features/src/echo.rs index 8fbfff87c4..22324bc605 100644 --- a/contracts/feature-tests/basic-features/src/echo.rs +++ b/contracts/feature-tests/basic-features/src/echo.rs @@ -107,6 +107,7 @@ pub trait EchoTypes { nz } + #[allow_multiple_var_args] #[view] fn echo_some_args_ignore_others( &self, From 991b3fe813e73acf8c5a46aaa3ae598347709208 Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 12:02:06 +0200 Subject: [PATCH 6/7] fmt --- framework/base/src/abi/endpoint_abi.rs | 2 +- framework/base/src/macros.rs | 5 ----- framework/derive/src/model/argument.rs | 2 +- framework/derive/src/model/endpoint.rs | 6 +++--- .../derive/src/parse/attributes/attr_names.rs | 2 +- framework/derive/src/parse/method_parse.rs | 15 ++++++++------- framework/meta/src/abi_json/endpoint_abi_json.rs | 8 ++++++-- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/framework/base/src/abi/endpoint_abi.rs b/framework/base/src/abi/endpoint_abi.rs index 35ee8effd8..4c4e71e3c7 100644 --- a/framework/base/src/abi/endpoint_abi.rs +++ b/framework/base/src/abi/endpoint_abi.rs @@ -47,7 +47,7 @@ pub struct EndpointAbi { pub payable_in_tokens: &'static [&'static str], pub inputs: Vec, pub outputs: OutputAbis, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } impl EndpointAbi { diff --git a/framework/base/src/macros.rs b/framework/base/src/macros.rs index 107527c58f..159b639195 100644 --- a/framework/base/src/macros.rs +++ b/framework/base/src/macros.rs @@ -202,11 +202,6 @@ macro_rules! only_owner { }; } -#[macro_export] -macro_rules! allow_multiple_var_args { - () => {}; -} - /// Converts usize to NonZeroUsize or returns SCError. #[macro_export] macro_rules! non_zero_usize { diff --git a/framework/derive/src/model/argument.rs b/framework/derive/src/model/argument.rs index f87cac3b4e..79d45322a6 100644 --- a/framework/derive/src/model/argument.rs +++ b/framework/derive/src/model/argument.rs @@ -59,5 +59,5 @@ pub struct TraitProperties { pub only_owner: bool, pub only_admin: bool, pub only_user_account: bool, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } diff --git a/framework/derive/src/model/endpoint.rs b/framework/derive/src/model/endpoint.rs index c8f6c15878..5c3d403fdc 100644 --- a/framework/derive/src/model/endpoint.rs +++ b/framework/derive/src/model/endpoint.rs @@ -3,7 +3,7 @@ use super::{EndpointMutabilityMetadata, MethodPayableMetadata}; #[derive(Clone, Debug)] pub struct InitMetadata { pub payable: MethodPayableMetadata, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } #[derive(Clone, Debug)] @@ -14,13 +14,13 @@ pub struct EndpointMetadata { pub only_admin: bool, pub only_user_account: bool, pub mutability: EndpointMutabilityMetadata, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } #[derive(Clone, Debug)] pub struct CallbackMetadata { pub callback_name: syn::Ident, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } /// Method visibility from the point of view of the smart contract diff --git a/framework/derive/src/parse/attributes/attr_names.rs b/framework/derive/src/parse/attributes/attr_names.rs index 184b170aea..0b9bcf9c3c 100644 --- a/framework/derive/src/parse/attributes/attr_names.rs +++ b/framework/derive/src/parse/attributes/attr_names.rs @@ -25,4 +25,4 @@ pub(super) static ATTR_STORAGE_IS_EMPTY: &str = "storage_is_empty"; pub(super) static ATTR_STORAGE_CLEAR: &str = "storage_clear"; pub(super) static ATTR_PROXY: &str = "proxy"; pub(super) static ATTR_LABEL: &str = "label"; -pub(super) static ATTR_ALLOW_MULTIPLE_VAR_ARGS: &str = "allow_multiple_var_args"; \ No newline at end of file +pub(super) static ATTR_ALLOW_MULTIPLE_VAR_ARGS: &str = "allow_multiple_var_args"; diff --git a/framework/derive/src/parse/method_parse.rs b/framework/derive/src/parse/method_parse.rs index f20c7ce7f4..62eeed2470 100644 --- a/framework/derive/src/parse/method_parse.rs +++ b/framework/derive/src/parse/method_parse.rs @@ -7,11 +7,12 @@ use super::{ process_storage_get_attribute, process_storage_is_empty_attribute, process_storage_mapper_attribute, process_storage_set_attribute, }, - extract_method_args, process_callback_attribute, process_callback_raw_attribute, - process_endpoint_attribute, process_external_view_attribute, process_init_attribute, - process_label_names_attribute, process_only_admin_attribute, process_only_owner_attribute, - process_only_user_account_attribute, process_output_names_attribute, process_payable_attribute, - process_promises_callback_attribute, process_view_attribute, process_allow_multiple_var_args_attribute + extract_method_args, process_allow_multiple_var_args_attribute, process_callback_attribute, + process_callback_raw_attribute, process_endpoint_attribute, process_external_view_attribute, + process_init_attribute, process_label_names_attribute, process_only_admin_attribute, + process_only_owner_attribute, process_only_user_account_attribute, + process_output_names_attribute, process_payable_attribute, process_promises_callback_attribute, + process_view_attribute, }; pub struct MethodAttributesPass1 { pub method_name: String, @@ -19,7 +20,7 @@ pub struct MethodAttributesPass1 { pub only_owner: bool, pub only_admin: bool, pub only_user_account: bool, - pub allow_multiple_var_args: bool + pub allow_multiple_var_args: bool, } pub fn process_method(m: &syn::TraitItemMethod, trait_attributes: &TraitProperties) -> Method { @@ -37,7 +38,7 @@ pub fn process_method(m: &syn::TraitItemMethod, trait_attributes: &TraitProperti only_owner: trait_attributes.only_owner, only_admin: trait_attributes.only_admin, only_user_account: trait_attributes.only_user_account, - allow_multiple_var_args: trait_attributes.allow_multiple_var_args + allow_multiple_var_args: trait_attributes.allow_multiple_var_args, }; let mut first_pass_unprocessed_attributes = Vec::new(); diff --git a/framework/meta/src/abi_json/endpoint_abi_json.rs b/framework/meta/src/abi_json/endpoint_abi_json.rs index 012293ce56..41a0addd77 100644 --- a/framework/meta/src/abi_json/endpoint_abi_json.rs +++ b/framework/meta/src/abi_json/endpoint_abi_json.rs @@ -99,7 +99,7 @@ pub struct EndpointAbiJson { #[serde(skip_serializing_if = "Vec::is_empty")] pub labels: Vec, #[serde(skip_serializing_if = "Option::is_none")] - pub allow_multiple_var_args: Option + pub allow_multiple_var_args: Option, } impl From<&EndpointAbi> for EndpointAbiJson { @@ -122,7 +122,11 @@ impl From<&EndpointAbi> for EndpointAbiJson { inputs: abi.inputs.iter().map(InputAbiJson::from).collect(), outputs: abi.outputs.iter().map(OutputAbiJson::from).collect(), labels: abi.labels.iter().map(|&label| label.to_owned()).collect(), - allow_multiple_var_args: if abi.allow_multiple_var_args { Some(true) } else { None } + allow_multiple_var_args: if abi.allow_multiple_var_args { + Some(true) + } else { + None + }, } } } From 729f2acab467dc1af657a357b0747ec3e07d2e93 Mon Sep 17 00:00:00 2001 From: mihaicalinluca Date: Fri, 13 Oct 2023 13:29:02 +0200 Subject: [PATCH 7/7] added order condition and unit tests --- .../contract/output_contract/oc_validate.rs | 54 +++++++++++++++++-- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs index fe7a701049..81dcccc94f 100644 --- a/framework/meta/src/cmd/contract/output_contract/oc_validate.rs +++ b/framework/meta/src/cmd/contract/output_contract/oc_validate.rs @@ -19,12 +19,13 @@ fn check_single_constructor(output_contract: &OutputContract) -> Result<(), Stri /// Note: promise callbacks not included, since they have `#[call_value]` arguments, that are currently not modelled. fn validate_contract_var_args(abi: &ContractAbi) -> Result<(), String> { for endpoint_abi in abi.constructors.iter().chain(abi.endpoints.iter()) { - validate_endpoint_var_args(endpoint_abi)?; + validate_endpoint_var_args_number(endpoint_abi)?; + validate_endpoint_var_args_order(endpoint_abi)?; } Ok(()) } -fn validate_endpoint_var_args(endpoint_abi: &EndpointAbi) -> Result<(), String> { +fn validate_endpoint_var_args_number(endpoint_abi: &EndpointAbi) -> Result<(), String> { let num_var_args = endpoint_abi .inputs .iter() @@ -35,6 +36,22 @@ fn validate_endpoint_var_args(endpoint_abi: &EndpointAbi) -> Result<(), String> "Multiple var args found in {}. Use #[allow_multiple_var_args] if you want to enable this feature", &endpoint_abi.rust_method_name)); } + + Ok(()) +} + +fn validate_endpoint_var_args_order(endpoint_abi: &EndpointAbi) -> Result<(), String> { + let mut var_args_encountered = false; + for arg in &endpoint_abi.inputs { + if arg.multi_arg { + var_args_encountered = true; + } else if var_args_encountered { + return Err(format!( + "Found regular arguments after var-args in method {}. This is not allowed, because it makes it impossible to parse the arguments.", + &endpoint_abi.rust_method_name)); + } + } + Ok(()) } @@ -45,7 +62,7 @@ mod tests { use super::*; #[test] - fn validate_endpoint_var_args_test() { + fn validate_endpoint_var_args_number_test() { let mut endpoint_def = EndpointAbi::default(); let var_arg_1 = InputAbi { arg_name: "arg_1", @@ -63,9 +80,36 @@ mod tests { assert!(!endpoint_def.allow_multiple_var_args); assert_eq!(Err(format!( "Multiple var args found in {}. Use #[allow_multiple_var_args] if you want to enable this feature", - &endpoint_def.rust_method_name)), validate_endpoint_var_args(&endpoint_def)); + &endpoint_def.rust_method_name)), validate_endpoint_var_args_number(&endpoint_def)); endpoint_def.allow_multiple_var_args = true; - assert_eq!(Ok(()), validate_endpoint_var_args(&endpoint_def)); + assert_eq!(Ok(()), validate_endpoint_var_args_number(&endpoint_def)); + } + + #[test] + fn validate_endpoint_var_args_order_test() { + let mut endpoint_def = EndpointAbi::default(); + let arg = InputAbi { + arg_name: "arg_1", + type_name: TypeName::new(), + multi_arg: false, + }; + let var_arg_1 = InputAbi { + arg_name: "arg_2", + type_name: TypeName::new(), + multi_arg: true, + }; + + endpoint_def.inputs.push(var_arg_1.clone()); + endpoint_def.inputs.push(arg.clone()); + assert_eq!(Err(format!( + "Found regular arguments after var-args in method {}. This is not allowed, because it makes it impossible to parse the arguments.", + &endpoint_def.rust_method_name)), validate_endpoint_var_args_order(&endpoint_def)); + + endpoint_def.inputs.clear(); + + endpoint_def.inputs.push(arg); + endpoint_def.inputs.push(var_arg_1); + assert_eq!(Ok(()), validate_endpoint_var_args_order(&endpoint_def)); } }