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

Experiment in composition #129

Draft
wants to merge 2 commits into
base: stylus
Choose a base branch
from
Draft

Experiment in composition #129

wants to merge 2 commits into from

Conversation

chrisco512
Copy link
Contributor

@chrisco512 chrisco512 commented Jun 30, 2024

This is an attempt to outline a potential design for contracts that would allow more Rust-like patterns for polymorphism and encapsulation. In the present design of the SDK we provide some macros to imitate Solidity-style inheritance for library authors, but it breaks down for multiple inheritance. Instead of going down this route, we should embrace idiomatic Rust that provides for composition over inheritance.

See Rust Is Beyond Object-Oriented for a primer on this topic and how Rust differs from OOP.

Ideally we'd like library authors to be able to define various traits that align with ERC standards (such as IERC20) and allow for plugins or additional behavior to be defined as well (such as IERC20Burnable). They should also be able to provide a default storage schema and internal methods (that abstract most of the complexity) for developers to use for implementing those traits.

One of the problems in implementing Rust-style composition with the current SDK is our implementation of the Router trait (which is abstracted away from the user as an #[external] macro attached to an impl block for the #[entrypoint] struct). The Router trait is necessary for us to align with Solidity's ABI. At compile-time we use the #[external] macro to convert the methods in that block to Solidity-compatible 4-byte method selectors that we use to route requests to the proper method.

In order to support Rust-style composition and allow for multiple #[external] blocks of methods to be defined, we need to refactor the Router.

In the attached, you'll see a potential design for smart contract composition that could be enabled if this were enabled. It requires less generics, no PhantomData needed, no #[borrow] needed, and no #[inherit].

The erc20.rs crate exposes an ERC20 struct that contains the scoped storage fields for a stripped down ERC20 example. Attached to the struct are internal methods that abstract away the complexity for the implementation of the IERC20 trait, which contains all the methods that are part of the public spec.

Similarly, the ownable.rs crate exposes Ownable struct with attached internal methods and an IOwnable trait that defines the methods that must be implemented by a consumer contract.

In the lib.rs, you can see the top-level contract logic. The #[entrypoint] struct contains an erc20 field and an ownable field, which are typed as ERC20 and Ownable respectively.

We also implement the traits we want our contract to have, in this case IOwnable and IERC20, and we use the internal methods provided by Ownable and ERC20 to handle the logic unless customization is needed.

We additionally add a mint method to the MyToken struct itself. Here, we can see the composition at work, by using internal methods from both libraries to validate ownership before executing the internal mint method provided by ERC20.

Interested in thoughts as to the approach. I think it's much cleaner than what we currently have, while not being radically different either.

@chrisco512 chrisco512 closed this Jun 30, 2024
@chrisco512 chrisco512 reopened this Jun 30, 2024
@chrisco512 chrisco512 marked this pull request as draft June 30, 2024 02:35
Copy link

@qalisander qalisander left a comment

Choose a reason for hiding this comment

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

Great that this problem is prioritized!

Curious how call to the external contract will look like. At current approach contract should be passed by trait TopLevelStorage to function that need to call other contract externally. How we can get reference to a smart contract itself without #[borrow] attributes and BorrowMut restriction? (e.g erc721 calls external contract and updates state of itself after successful call)

And few other thoughts that I have in comments:


#[solidity_storage]
#[entrypoint]
struct MyToken {

Choose a reason for hiding this comment

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

Thinking same, it is not radically different from currently existing approach at the stylus sdk e.g.:
At open zeppelin contracts ownable example looks like that:

sol_storage! {
    #[entrypoint]
    struct OwnableExample {
        #[borrow]
        Erc20 erc20;
        #[borrow]
        Ownable ownable;
    }
}

#[external]
#[inherit(Erc20, Ownable)]
impl OwnableExample {
    pub fn transfer(
        &mut self,
        to: Address,
        value: U256,
    ) -> Result<(), Vec<u8>> {
        self.ownable.only_owner()?;
        self.erc20.transfer(to, value)?;
        Ok(())
    }
}

Erc20 and Ownable contracts implement Router trait (i.e have #[external] attributes).
But I can see that currently existing approach requires less code for user (I'm assuming that library's user code resides at lib.rs file in your example). And allows also to add custom logic for the external erc20 transfer function (It will just add another selector for transfer to the Router impl that will be called instead of previous one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can reduce code for the user by providing a proc_macro_derive for each library trait. IERC20 and IOwnable in this case, would have default implementations as proc macros. And attaching #[derive(IERC20, IOwnable)] to the #[entrypoint] struct would fill that in. Users would still be able to override on the top-level impl block for the struct (as Rust inherently accommodates this).

Copy link

@qalisander qalisander Jul 1, 2024

Choose a reason for hiding this comment

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

Cool. Derive macro makes sense. But when library user wants to modify just the only one method of IERC20 interface he will be forced to write boilerplate code of other methods he doesn't need to change. Here rust auto trait impls could help but tough to say how to organize it..

}
}

pub trait IOwnable {

Choose a reason for hiding this comment

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

Having traits for extensions looks like a good target to aim.
Besides that, we can use auto impl for trait functions that can help in emulation of inheritance model and reduce user's boilerplate.
It is not applicable directly to your approach since #[external] macro computes selector based on explicitly defined functions. But still is a great rust feature we can consider for extensibility.
p.s type restriction for traits like this: trait IErc20Ownable: IOwnable + IErc20 can help if we're relying on auto impl for traits.

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