-
Notifications
You must be signed in to change notification settings - Fork 82
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
base: stylus
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
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 asIERC20Burnable
). 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 animpl
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 anERC20
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 theIERC20
trait, which contains all the methods that are part of the public spec.Similarly, the
ownable.rs
crate exposesOwnable
struct with attached internal methods and anIOwnable
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 anerc20
field and anownable
field, which are typed asERC20
andOwnable
respectively.We also implement the traits we want our contract to have, in this case
IOwnable
andIERC20
, and we use the internal methods provided byOwnable
andERC20
to handle the logic unless customization is needed.We additionally add a
mint
method to theMyToken
struct itself. Here, we can see the composition at work, by using internal methods from both libraries to validate ownership before executing the internalmint
method provided byERC20
.Interested in thoughts as to the approach. I think it's much cleaner than what we currently have, while not being radically different either.