Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make cross-contract callee non-optional #1636

Merged
merged 12 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 43 additions & 59 deletions crates/env/src/call/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use crate::{
Error,
};
use core::marker::PhantomData;
use ink_primitives::Clear;
use num_traits::Zero;

/// The final parameters to the cross-contract call.
Expand Down Expand Up @@ -71,10 +70,8 @@ where
E: Environment,
{
/// Returns the account ID of the called contract instance.
///
/// Returns `None` if no account ID has been set for the call.
#[inline]
pub fn callee(&self) -> &Option<E::AccountId> {
pub fn callee(&self) -> &E::AccountId {
&self.call_type.callee
}

Expand Down Expand Up @@ -205,11 +202,9 @@ where
/// # type AccountId = <DefaultEnvironment as Environment>::AccountId;
/// # type Balance = <DefaultEnvironment as Environment>::Balance;
/// build_call::<DefaultEnvironment>()
/// .call_type(
/// Call::new()
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000)
/// .transferred_value(10))
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000)
/// .transferred_value(10)
/// .exec_input(
/// ExecutionInput::new(Selector::new([0xDE, 0xAD, 0xBE, 0xEF]))
/// .push_arg(42u8)
Expand Down Expand Up @@ -241,9 +236,8 @@ where
/// # };
/// # type AccountId = <DefaultEnvironment as Environment>::AccountId;
/// let my_return_value: i32 = build_call::<DefaultEnvironment>()
/// .call_type(Call::new()
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000))
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000)
/// .transferred_value(10)
/// .exec_input(
/// ExecutionInput::new(Selector::new([0xDE, 0xAD, 0xBE, 0xEF]))
Expand All @@ -270,8 +264,7 @@ where
/// # use ink_primitives::Clear;
/// # type AccountId = <DefaultEnvironment as Environment>::AccountId;
/// let my_return_value: i32 = build_call::<DefaultEnvironment>()
/// .call_type(DelegateCall::new()
/// .code_hash(<DefaultEnvironment as Environment>::Hash::CLEAR_HASH))
/// .code_hash(<DefaultEnvironment as Environment>::Hash::CLEAR_HASH)
/// .exec_input(
/// ExecutionInput::new(Selector::new([0xDE, 0xAD, 0xBE, 0xEF]))
/// .push_arg(42u8)
Expand Down Expand Up @@ -306,12 +299,9 @@ where
/// # type AccountId = <DefaultEnvironment as Environment>::AccountId;
/// # type Balance = <DefaultEnvironment as Environment>::Balance;
/// let call_result = build_call::<DefaultEnvironment>()
/// .call_type(
/// Call::new()
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000)
/// .transferred_value(10),
/// )
/// .callee(AccountId::from([0x42; 32]))
/// .gas_limit(5000)
/// .transferred_value(10)
/// .try_invoke()
/// .expect("Got an error from the Contract's pallet.");
///
Expand Down Expand Up @@ -343,41 +333,26 @@ where
/// The default call type for cross-contract calls. Performs a cross-contract call to `callee`
/// with gas limit `gas_limit`, transferring `transferred_value` of currency.
pub struct Call<E: Environment> {
callee: Option<E::AccountId>,
callee: E::AccountId,
gas_limit: Gas,
transferred_value: E::Balance,
}

impl<E: Environment> Default for Call<E> {
fn default() -> Self {
Call {
callee: Default::default(),
impl<E: Environment> Call<E> {
/// Returns a clean builder for [`Call`].
pub fn new(callee: E::AccountId) -> Self {
Self {
callee,
gas_limit: Default::default(),
transferred_value: E::Balance::zero(),
}
}
}

impl<E: Environment> Call<E> {
/// Returns a clean builder for [`Call`].
pub fn new() -> Self {
Default::default()
}
}

impl<E> Call<E>
where
E: Environment,
{
/// Sets the `callee` for the current cross-contract call.
pub fn callee(self, callee: E::AccountId) -> Self {
Call {
callee: Some(callee),
gas_limit: self.gas_limit,
transferred_value: self.transferred_value,
}
}

/// Sets the `gas_limit` for the current cross-contract call.
pub fn gas_limit(self, gas_limit: Gas) -> Self {
Call {
Expand All @@ -404,16 +379,8 @@ pub struct DelegateCall<E: Environment> {

impl<E: Environment> DelegateCall<E> {
/// Returns a clean builder for [`DelegateCall`]
pub const fn new() -> Self {
DelegateCall {
code_hash: E::Hash::CLEAR_HASH,
}
}
}

impl<E: Environment> Default for DelegateCall<E> {
fn default() -> Self {
Self::new()
pub const fn new(code_hash: E::Hash) -> Self {
DelegateCall { code_hash }
}
}

Expand Down Expand Up @@ -521,26 +488,43 @@ where
}
}

impl<E, Args, RetType> CallBuilder<E, Set<Call<E>>, Args, RetType>
impl<E, CallType, Args, RetType> CallBuilder<E, Unset<CallType>, Args, RetType>
where
E: Environment,
{
/// Sets the `callee` for the current cross-contract call.
pub fn callee(self, callee: E::AccountId) -> Self {
let call_type = self.call_type.value();
pub fn callee(
self,
callee: E::AccountId,
) -> CallBuilder<E, Set<Call<E>>, Args, RetType> {
CallBuilder {
call_type: Set(Call {
callee: Some(callee),
gas_limit: call_type.gas_limit,
transferred_value: call_type.transferred_value,
}),
call_type: Set(Call::new(callee)),
call_flags: self.call_flags,
exec_input: self.exec_input,
return_type: self.return_type,
_phantom: Default::default(),
}
}

/// Sets the `code_hash` for the current cross-contract delegate call.
pub fn code_hash(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So @ascjones I guess what you're suggesting is we rename this to delegate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly, so by calling one method or the other we instantiate the builder type for the type of call we want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually maybe the choices are

  1. callee & code_hash
  2. call & delegate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but then we should rename the callee builder method to call

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧠 🤝 🧠

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More like 🥜 🤝 🥜

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deez nuts

self,
code_hash: E::Hash,
) -> CallBuilder<E, Set<DelegateCall<E>>, Args, RetType> {
CallBuilder {
call_type: Set(DelegateCall::new(code_hash)),
call_flags: self.call_flags,
exec_input: self.exec_input,
return_type: self.return_type,
_phantom: Default::default(),
}
}
}

impl<E, Args, RetType> CallBuilder<E, Set<Call<E>>, Args, RetType>
where
E: Environment,
{
/// Sets the `gas_limit` for the current cross-contract call.
pub fn gas_limit(self, gas_limit: Gas) -> Self {
let call_type = self.call_type.value();
Expand Down
6 changes: 1 addition & 5 deletions crates/env/src/engine/on_chain/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,7 @@ impl TypedEnvBackend for EnvInstance {
{
let mut scope = self.scoped_buffer();
let gas_limit = params.gas_limit();
let callee = params
.callee()
.as_ref()
.expect("An account ID must be set in order to call a contract.");
let enc_callee = scope.take_encoded(callee);
let enc_callee = scope.take_encoded(params.callee());
let enc_transferred_value = scope.take_encoded(params.transferred_value());
let call_flags = params.call_flags();
let enc_input = if !call_flags.forward_input() && !call_flags.clone_input() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl CallBuilder<'_> {
#( , #input_bindings : #input_types )*
) -> #output_type {
::ink::env::call::build_call::<Environment>()
.call_type(::ink::env::call::Call::new().callee(::ink::ToAccountId::to_account_id(self)))
.callee(::ink::ToAccountId::to_account_id(self))
.exec_input(
::ink::env::call::ExecutionInput::new(
::ink::env::call::Selector::new([ #( #selector_bytes ),* ])
Expand Down
2 changes: 1 addition & 1 deletion crates/ink/codegen/src/generator/trait_def/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl CallBuilder<'_> {
#( , #input_bindings : #input_types )*
) -> Self::#output_ident {
::ink::env::call::build_call::<Self::Env>()
.call_type(::ink::env::call::Call::new().callee(::ink::ToAccountId::to_account_id(self)))
.callee(::ink::ToAccountId::to_account_id(self))
.exec_input(
::ink::env::call::ExecutionInput::new(
::ink::env::call::Selector::new([ #( #selector_bytes ),* ])
Expand Down
4 changes: 2 additions & 2 deletions examples/erc1155/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,15 +358,15 @@ mod erc1155 {
{
use ink::env::call::{
build_call,
Call,
ExecutionInput,
Selector,
};

// If our recipient is a smart contract we need to see if they accept or
// reject this transfer. If they reject it we need to revert the call.
let result = build_call::<Environment>()
.call_type(Call::new().callee(to).gas_limit(5000))
.callee(to)
.gas_limit(5000)
.exec_input(
ExecutionInput::new(Selector::new(ON_ERC_1155_RECEIVED_SELECTOR))
.push_arg(caller)
Expand Down
5 changes: 2 additions & 3 deletions examples/lang-err-integration-tests/call-builder/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ mod call_builder {
use ink::env::{
call::{
build_call,
Call,
ExecutionInput,
Selector,
},
Expand Down Expand Up @@ -52,7 +51,7 @@ mod call_builder {
selector: [u8; 4],
) -> Option<ink::LangError> {
let result = build_call::<DefaultEnvironment>()
.call_type(Call::new().callee(address))
.callee(address)
.exec_input(ExecutionInput::new(Selector::new(selector)))
.returns::<()>()
.try_invoke()
Expand All @@ -79,7 +78,7 @@ mod call_builder {
use ink::env::call::build_call;

build_call::<DefaultEnvironment>()
.call_type(Call::new().callee(address))
.callee(address)
.exec_input(ExecutionInput::new(Selector::new(selector)))
.returns::<()>()
.invoke()
Expand Down
20 changes: 6 additions & 14 deletions examples/multisig/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ mod multisig {
env::{
call::{
build_call,
Call,
ExecutionInput,
},
CallFlags,
Expand Down Expand Up @@ -310,7 +309,6 @@ mod multisig {
/// use ink::env::{
/// call::{
/// utils::ArgumentList,
/// Call,
/// CallParams,
/// ExecutionInput,
/// Selector,
Expand Down Expand Up @@ -536,12 +534,9 @@ mod multisig {
let t = self.take_transaction(trans_id).expect(WRONG_TRANSACTION_ID);
assert!(self.env().transferred_value() == t.transferred_value);
let result = build_call::<<Self as ::ink::env::ContractEnv>::Env>()
.call_type(
Call::new()
.callee(t.callee)
.gas_limit(t.gas_limit)
.transferred_value(t.transferred_value),
)
.callee(t.callee)
.gas_limit(t.gas_limit)
.transferred_value(t.transferred_value)
.call_flags(CallFlags::default().set_allow_reentry(t.allow_reentry))
.exec_input(
ExecutionInput::new(t.selector.into()).push_arg(CallInput(&t.input)),
Expand Down Expand Up @@ -574,12 +569,9 @@ mod multisig {
self.ensure_confirmed(trans_id);
let t = self.take_transaction(trans_id).expect(WRONG_TRANSACTION_ID);
let result = build_call::<<Self as ::ink::env::ContractEnv>::Env>()
.call_type(
Call::new()
.callee(t.callee)
.gas_limit(t.gas_limit)
.transferred_value(t.transferred_value),
)
.callee(t.callee)
.gas_limit(t.gas_limit)
.transferred_value(t.transferred_value)
.call_flags(CallFlags::default().set_allow_reentry(t.allow_reentry))
.exec_input(
ExecutionInput::new(t.selector.into()).push_arg(CallInput(&t.input)),
Expand Down
10 changes: 3 additions & 7 deletions examples/upgradeable-contracts/forward-calls/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#[ink::contract]
pub mod proxy {
use ink::env::call::Call;

/// A simple proxy contract.
#[ink(storage)]
Expand Down Expand Up @@ -70,12 +69,9 @@ pub mod proxy {
#[ink(message, payable, selector = _)]
pub fn forward(&self) -> u32 {
ink::env::call::build_call::<ink::env::DefaultEnvironment>()
.call_type(
Call::new()
.callee(self.forward_to)
.transferred_value(self.env().transferred_value())
.gas_limit(0),
)
.callee(self.forward_to)
.transferred_value(self.env().transferred_value())
.gas_limit(0)
.call_flags(
ink::env::CallFlags::default()
.set_forward_input(true)
Expand Down