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

The call_builder macro to provide a more developer-friendly way to work with CallBuilder #1858

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Jul 26, 2023

Overview

Implemented the call_builder! macro from the proposal.

The macro return the CallBuilder type that supports hinting and highlighting in the IDE.
The usage is the same as for standard cross-contract calls, where the call is wrapped into the call_builder macro:

#[ink::trait_definition]
pub trait Increment {
    /// Increments the current value of the implementer by one (1).
    #[ink(message)]
    fn inc_by_delta(&mut self, delta_1: u64, delta_2: u32);

    /// Returns the current value of the implementer.
    #[ink(message)]
    fn get(&self) -> u64;
}

#[ink(message)]
fn get(&self) -> u64 {
    self.incrementer.get();
    call_builder!(self.incrementer.get()).invoke();
    call_builder!(::dyn_traits::Increment::get(&self.incrementer)).invoke();
    call_builder!(<_ as ::dyn_traits::Increment>::get(&self.incrementer))
        .invoke();
    call_builder!(Increment::get(&self.incrementer)).invoke();
    call_builder!(::dyn_traits::Increment::get(&self.incrementer)).invoke();

    call_builder!(self.incrementer().get()).invoke()
}

#[ink(message)]
fn inc_by_delta(&mut self, delta_1: u64, delta_2: u32) -> u64 {
    self.incrementer.inc_by_delta(delta_1, delta_2);
    call_builder!(self.incrementer.inc_by_delta(delta_1, delta_2)).invoke();
    call_builder!(::dyn_traits::Increment::inc_by_delta(&mut self.incrementer, delta_1, delta_2)).invoke();
    call_builder!(<_ as ::dyn_traits::Increment>::inc_by_delta(&mut self.incrementer, delta_1, delta_2))
        .invoke();
    call_builder!(Increment::inc_by_delta(&mut self.incrementer, delta_1, delta_2)).invoke();
    call_builder!(::dyn_traits::Increment::inc_by_delta(&mut self.incrementer, delta_1, delta_2)).invoke();
    let call_builder =
        call_builder!(self.incrementer.inc_by_delta(delta_1, delta_2));

    call_builder.invoke()
}

How it looks in the IDE:
image
image

Follow-up

  • In the same way, we can add the create_builder! macro for constructors in the follow-up PR. It would simplify instantiation because all functions will be highlighted. The macro would be much simpler because it always requires an explicit type name.
  • Add mesage_descriptor! macro that returns the MessageDescription. Everyone would be able to get the method's selector(and other properties of the message) like in the solidity message_descriptor!(self.balanec_of).selector().
  • Remove the TraitCallBuilder and replace its functionality with a new macro. Remove the {method_ident}Output associated type from the trait definition.

@xgreenx xgreenx force-pushed the feature/call-builder-macro branch from a1aa597 to 2af6bf9 Compare July 26, 2023 15:07
@SkymanOne
Copy link
Contributor

I am not fun of the macro's interface. Wrapping every single call into the macro just to get the syntax highlighting for the .invoke() or .try_invoke() commands doesn't seem very idiomatic IMHO.

Perhaps, if we could do something like:

self.incrementer.get().invoke()

It would be much clearer to the developer. I am open to other proposals too.

@xgreenx
Copy link
Collaborator Author

xgreenx commented Jul 26, 2023

It is a syntax for a rare case when the developers need to work with a result of the cross-contract call. Primarily, you work with methods directly like self.incrementer.get().

The idea of this change is to remove the {method_ident}Output alias from the trait in the follow-up PR. So it is impossible to support self.incrementer.get().invoke().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants