FuelLabs / sway

🌴 Empowering everyone to build reliable and efficient smart contracts.
https://docs.fuel.network/docs/sway/
Apache License 2.0
62.65k stars 5.36k forks source link

Storage accesses in libraries #2585

Closed mohammadfawaz closed 1 year ago

mohammadfawaz commented 2 years ago

We need a way to support reading and writing storage variables in libraries. Storage variables in a contract behave like global variables. They are accessible from everywhere in the contract and don't need to be passed around. In libraries, this is different. We want to be able to write a library function that is able to read a storage variable or update it. For example, this is a rough idea of what we want:

// Some contract
...
storage {
    var: u64 = 0,
}
...
update_storage_variable(storage.var);
...
// Some library
fn update_storage_variable(storage x: u64) { // storage somewhat implies `ref mut`?
    storage.x = 42;
}

The syntax above is just an example... it may not be the best idea as is.

mohammadfawaz commented 2 years ago

Or,

// Some contract
...
storage {
    var: u64 = 0,
}
...
update_storage_variable(storage.var);
...
// Some library
fn update_storage_variable(ref mut x: u64) { // Just like a regular function that takes a `ref mut` parameter
    x = 42;
}

The fact that we're passing in a storage variable via storage.var means "modify that variable"? The downside here is that the function update_storage_variable will sometimes be pure and sometimes be impure which will complicate storage annotations. Also, the lack of the storage keyword on x = 42 is a deviation from our philosophy of making storage variables and storage access as explicit as possible.

adlerjohn commented 2 years ago

Could we do something like attach storage to a library import?

E.g.

library foo;

// To import this library, you must "attach" storage variables to it.
storage {
    bar: u64,
}

then

contract MyContract;

storage {
    s: u64,
}

// Attach `storage.s` to `bar`, syntax obviously subject to change.
use foo { bar: storage.s };
vaivaswatha commented 2 years ago

Could we do something like attach storage to a library import?

E.g.

library foo;

// To import this library, you must "attach" storage variables to it.
storage {
    bar: u64,
}

then

contract MyContract;

storage {
    s: u64,
}

// Attach `storage.s` to `bar`, syntax obviously subject to change.
use foo { bar: storage.s };

I vote for this approach.

mohammadfawaz commented 2 years ago

To summarize: there seems to be multiple potential directions here:

  1. Function-level storage similar to my initial suggestions where storage is added to a function argument.
  2. John's suggestion of importing storage variables manually and "map" them to an existing storage variable in the contract itself.
  3. One additional suggestion inspired by (2) is to allow importing storage variables, similarly to constants, and without having to "map" to storage variables in the contract. For example:

Library:

library foo;

storage {
    bar: u64 = 0,
}

#[storage(read)]
fn reads_bar() {
    let local_bar = storage.bar;
}

Contract:

contract;

use foo::*; // imports `storage.bar` and `reads_bar()` OR `use foo::{storage.bar, reads_bar}`

storage {
    baz:u64 = 0,
}

// ...

fn multi_storage_access() {
    let local_baz = storage.baz; // From contract

    let local_bar = reads_bar(); // From library via a function
    let local_bar = storage.bar; // From library via direct access
    let local_bar = foo::storage.bar; // From library via a full path (?)
}

This means that when compiling the contract, we have to (effectively) allow multiple storage blocks which is not something we allow today.

We could even introduce visibility qualifiers to storage variables so that one can add pub to storage variable that can be accessed from outside the library.

mohammadfawaz commented 2 years ago

So (3) is a deviation from the initial proposals in sense that it allows storage variables declared in libraries to be accessible in the contract and not the other way around. I don't know yet if it covers all uses though.

adlerjohn commented 2 years ago

One additional suggestion inspired by (2) is to allow importing storage variables, similarly to constants, and without having to "map" to storage variables in the contract.

I would caution against this. It makes importing libraries have opaque side effects and is a source of confusion in Solidity. It's also more like inheritance.

Additionally, for upgradable contracts, storage slots really need to be known. If using a library changes storage slots, then it can make it difficult if not impossible to upgrade a contract without breaking the default formatting.

mohammadfawaz commented 2 years ago

Alright, in that case, it seems that "linking" explicit storage variables in the contract to storage variables in the library is the way to go, just like your earlier suggestion. Following the traditional syntax for use, an alternative syntax to do this may be:

contract MyContract;

storage {
    s: u64,
}

// Import `storage.bar` with an alias that has to match an existing storage variable in the contract. Otherwise, this is a compile error.
use foo::storage.bar as storage.s;
nfurfaro commented 2 years ago

Not a fully thought-out approach,just an idea from the sync call today. In a scenario where storage is a field on the Contract type , in order for a library to operate on storage we could just require passing an instance of a Contract to the library function as a param.

mohammadfawaz commented 1 year ago

Alright, so I've been experimenting with storage references and how we can use them for storage accesses in libraries. For free functions, I got something working for some basic types. Generalizing the work for all types is challenging but doable. That being said, when I tried to create a clone of ownable.sol in Sway, I faced some problems.

We previously said that we don't want to allow storage variables in libraries and that we would have library functions or methods take a StorageRef. Here's how the ownable library would look like:

library ownable;

dep utils; // This is where I currently have all the `StorageRef` stuff

use utils::*;

pub struct OwnershipTransferred {
    previous_owner: b256, 
    new_owner: b256, 
}

abi Ownable { 
    // No methods in the interface. The user shouldn't need to implement anything manually.
} { 
    // These are all default-implemented ABI methods

    #[storage(read)]
    fn owner(owner: StorageRef<b256>) -> b256 { // This seems a bit silly now?
        owner.read()
    }

    #[storage(read)]
    fn only_owner(owner: StorageRef<b256>) {
        assert(std::chain::auth::msg_sender().unwrap() == Identity::Address(~Address::from(owner.read())));
    }

    #[storage(write)]
    fn renounce_ownership(owner: StorageRef<b256>) {
        owner.write(std::constants::ZERO_B256);
    }

    #[storage(read, write)]
    fn transfer_ownership(owner: StorageRef<b256>, new_owner: b256) {
        assert(new_owner != std::constants::ZERO_B256);
        let old_owner = owner.read();
        owner.write(new_owner);
        std::logging::log(OwnershipTransferred {
            previous_owner: old_owner,
            new_owner: new_owner,
        });
    }
}

Note I'm simplifying things a bit by assuming that the owner is just a b256 and not an Address.

Now, in our contract, we can implement this ABI by just writing

impl Ownable for Contract { }

and all the methods of Ownable will be available in the contract.

Note Now, I personally prefer if we use Ownable as a "supertrait" (or really a "superabi") of the main contract's ABI so that the main ABI simply receives all methods of Ownable for free and we then have a single contract handler to use in scripts and in the SDK, instead of two separate ABIs (and two separate abi casts to deal with. So basically, abi MyContract : Ownable { // main ABI methods... }

Now, if we were to call a method of Ownable from a script or the SDK, we need to pass in a StorageRef, and this is where things get ugly. The only sane way to do this is for the main ABI to provide storage reference getters that can be called from a script or the SDK and their results would then be used when calling the methods of Ownable. This seems overly expensive (two contract calls?).

mohammadfawaz commented 1 year ago

What if we allow ABIs to have their own storage blocks? Ownable is an example of an ABI where storage variables are always required because it implements methods in its definition and those implementations require storage. An ABI that doesn't do that should not require a storage block because the need for storage is dependent on the implementation of the ABI. For Ownable, at least a single storage variable called owner is required, regardless of how the ABI is implemented (unless we allow re-implementing default-implemented methods?)

That being said, we also should keep this in mind.

Just brainstorming some ideas here. We need to continue thinking through how to make this as safe and as ergonomic as possible.

mitchmindtree commented 1 year ago

It seems like trying to recreate Solidity's Ownable contract has been the main motivation here, but I think the design of such a contract (which was designed in a context where data inheritance is the norm and baked into the language) is at odds with the Rust/Sway philosophy of separating data and behaviour, and preferring composition.

I think It's also tricky to come up with a strong alternative without seeing the motivating context in which it is needed. However, looking at your example above, perhaps something like the following might be more Rust/Sway-esque alternative to exposing an "ownership" API:

library owned;

use std::chain::auth::msg_sender;
use std::constants::ZERO_B256;

pub struct Owned<T> {
    /// The owner of the asset.
    ///
    /// Can only be `read` via the `owner` method or updated via the
    /// `transfer_ownership` method.
    owner: Identity,
    /// The asset. Can only be accessed via the checked `read` or `write` methods.
    asset: T,
}

pub struct OwnershipTransferred {
    prev: Identity,
    new: Identity,
}

impl<T> Owned<T> {
    pub fn new(owner: Identity, asset: T) -> Self {
        Self { owner, asset }
    }

    /// Returns the owner of the asset.
    pub fn owner(self) -> Identity {
         self.owner
    }

    /// Reverts if the `msg_sender` is not the owner.
    // NOTE: `msg_sender` in libraries is a bad idea due to both its
    // side-effecting nature and the ambiguity of its meaning in non-contract
    // contexts. See #2463. However, for the sake of the solidity example, we
    // match behaviour. Ideally, Contracts should check `is_owner` and pass the
    // identity directly.
    pub fn check_owner(self) {
        check_owner(self.owner);
    }

    /// Returns whether or not the given Identity is the owner.
    pub fn is_owner(self, id: Identity) -> bool {
        self.owner == id
    }

    pub fn transfer_ownership(ref mut self, new: Identity) {
        transfer_ownership(self, new);
    }

    /// Relinquish ownership, sets the owner to a zeroed address.
    // NOTE: If we had Rust's move semantics, we could ensure the original
    // `Owned` cannot be used again without having to write a null address
    // to `owner`. I.e. in Rust, this could be:
    // ```
    // pub fn renounce_ownership(self) -> T { self.asset }
    // ```
    pub fn renounce_onwership(ref mut self) {
        transfer_ownership(self, Identity::Address(~Address::from(ZERO_B256)));
    }

    /// Read the asset.
    pub fn read(self) -> T {
        // NOTE: This assumes only owner the owner can read. Could also imagine
        // different types to distinguish between read-only and write-only assets.
        check_owner(self.owner);
        self.asset
    }

    /// Write the asset.
    pub fn write(ref mut self, asset: T) {
        check_owner(self.owner);
        self.asset = asset;
    }
}

// Abstracted into a free function to avoid the error `No method named
// "check_owner" found for type "Owned<T>" which occurs when trying to call
// `self.check_owner` within another method.
fn check_owner(owner: Identity) {
    assert(owner == msg_sender().unwrap());
}

// Abstracted into a free function to avoid the error `No method
// named "transfer_ownership" found for type "Owned<T>"` which occurs when
// trying to call `self.transfer_ownership` in the `renounce_ownership` method.
fn transfer_ownership<T>(ref mut owned: Owned<T>, new: Identity) {
    check_owner(owned.owner);
    let prev = owned.owner;
    owned.owner = new;
    std::logging::log(OwnershipTransferred { prev, new });
}

Then its downstream use in contracts might look something like:

contract;

use owned::Owned;
use std::constants::ZERO_B256;

struct Asset {}

storage {
    owned: Owned<Asset> = ~Owned::new(Identity::Address(~Address::from(ZERO_B256)), Asset {}),
}

abi MyContract {
} {
    #[storage(read)]
    fn read_asset() -> Asset {
        storage.owned.read() // Reverts if msg sender isn't owner
    }

    #[storage(read, write)]
    fn write_asset(new: Asset) {
        storage.owned.write(new) // Reverts if msg sender isn't owner
    }
}

impl MyContract for Contract {}

ref mut self methods on storage fields?

Curiously, I get the following warning when building the contract above:

warning
  --> /home/mindtree/programming/sway/contract-ownership/src/main.sw:20:5
   |
18 |   
19 |       #[storage(read, write)]
20 |       fn write_asset(new: Asset) {
   |  _____-
21 | |         storage.owned.write(new)
22 | |     }
   | |_____- The 'write' storage declaration for this function is never accessed and can be removed.
23 |   }
   |
____

  Compiled contract "contract-ownership" with 1 warning.
  Bytecode size is 3836 bytes.

This seems to indicate that storage.owned.write(new) is not actually taking a mutable reference to the value in storage, which could be very unintuitive / a dangerous footgun for users. Perhaps something like this would also require some form of storage references. In the meantime, perhaps it's worth opening an issue about methods taking ref mut self on storage fields?

Field privacy/visibility control?

Also curious, if I change the write_asset method in the contract to the following:

    #[storage(read, write)]
    fn write_asset(new: Asset) {
        storage.owned.asset = new;
    }

I don't get any compiler errors about private field access. Do we have an issue open for this? It seems not after a quick look, in which case I'll create one. Being able to make fields private is essential for encapsulation in library APIs. E.g. in this example, the lack of private fields allows the user to bypass the check_owner check entirely, which defeats the purpose of my proposed Owned<T> type :smiling_face_with_tear:

Rust Ownership

As an anecdote designing this small library, I felt like I was seriously missing Rust's ownership semantics here, and the ability to distinguish between moving values or returning by reference or mutable reference. While the reason for Rust's ownership is often cited as memory safety, I'd argue it is much more about enabling expressiveness in API design around resources in general, and this ownership problem is a perfect example of this. E.g. with Rust's ownership model, we could have methods like the following:

    /// Relinquish ownership of the asset.
    // Note that `T` would be *moved*, so `Owned<T>` referred to by
    // `self` could no longer be used.
    pub fn renounce_onwership(self) -> T {
        check_owner(self.owner);
        self.asset
    }

    /// Provide mutable access to the asset if the msg sender is the owner.
    // Here we can return a mutable reference to `T`, almost certainly useful
    // in cases where `T` is some collection like a `StorageMap`.
    pub fn asset_mut(&mut self) -> Option<&mut T> {
        if self.is_owner() {
            Some(&mut self.asset)
        } else {
            None
        }
    }
mohammadfawaz commented 1 year ago

Thank you @mitchmindtree.

Making a note of all the issues you brought up as well.

The above makes a lot of sense for situations where the user wants to access the methods on owner, such as check_owner(), from the main contract directly. Another use case for these methods is accessible from a script. I thought I'd just draw the use case 😆

image

This is why I had added an ABI in the ownable.sw library. Now this more like C++ inheritance indeed, and the question is how to make it look more like Rust. For ABI methods, we could use the equivalent of supertraits like I mentioned earlier and say abi MyContract : Ownable { .. }. For the data itself (i.e. the owner), it gets tricky.

Alternative but Related Proposal

So, Rust allows associated constants for traits, right? Can we have a const StorageRef<T> in the abi maybe?

library ownable;

dep utils; // This is where I currently have all the `StorageRef` stuff

use utils::*;

pub struct OwnershipTransferred {
    previous_owner: b256, 
    new_owner: b256, 
}

abi Ownable { 
    const owner: StorageRef<b256>; // to be set by the implementer of this ABI

    // No methods in the interface. The user shouldn't need to implement anything manually.
} { 
    // These are all default-implemented ABI methods

    #[storage(read)]
    fn owner() -> b256 { // This seems a bit silly now?
        owner.read()
    }

    #[storage(read)]
    fn only_owner() {
        assert(std::chain::auth::msg_sender().unwrap() == Identity::Address(~Address::from(owner.read())));
    }

    #[storage(write)]
    fn renounce_ownership() {
        owner.write(std::constants::ZERO_B256);
    }

    #[storage(read, write)]
    fn transfer_ownership(new_owner: b256) {
        assert(new_owner != std::constants::ZERO_B256);
        let old_owner = owner.read();
        owner.write(new_owner);
        std::logging::log(OwnershipTransferred {
            previous_owner: old_owner,
            new_owner: new_owner,
        });
    }
}

When the contract implements the Ownable ABI, it can just do:

impl Ownable for Contract {
    const owner: StorageRef<b256> = storage.owner.as_ref(); // // This **has** to be set. as_ref() is exposed in utils.rs
    // Nothing else needed here
}

If we want Ownable to a "superabi", then we can do the same thing:

abi MyContract : Ownable {
    // Interface
}

impl MyContract for Contract {
    const owner: StorageRef<b256> = storage.owner.as_ref(); // This **has** to be set

    // Implement the methods in MyContract
}

I think StorageRef can be a const here because you're only allowed to set it once in the contract. The "reference" does not change, and should not change. This could be our way of "linking" to a specific storage variable. Storage variables never change location. Their references are always compile-time constants.

Full example here: https://github.com/mohammadfawaz/storage_in_libraries/tree/main/example_1

mitchmindtree commented 1 year ago

Thanks for sharing a more clear vision of your proposal Mohammad!

I have to head off for an appointment this morning, but just wanted to share a few concerns that come to mind first!

I'll note that, while the alternative Owned<T> type proposal above does imply needing more work around on returning mutable references, it doesn't have any of the issues listed above and otherwise works with existing language features.

The above makes a lot of sense for situations where the user wants to access the methods on owner, such as check_owner(), from the main contract directly. Another use case for these methods is accessible from a script. I thought I'd just draw the use case laughing

In the case that the top-level contract that a script interacts with does wish to expose some ownership-related methods, I'd suggest that that contract provide its own abi methods that expose only what is relevant to the contract - not just dumping all Ownable methods into the ABI by default every time.

mohammadfawaz commented 1 year ago

Thank you @mitchmindtree. These are all very good points.

In the case that the top-level contract that a script interacts with does wish to expose some ownership-related methods, I'd suggest that that contract provide its own abi methods that expose only what is relevant to the contract - not just dumping all Ownable methods into the ABI by default every time.

If we can get away with this, that'll solve a lot of our problems, I agree. I guess it's a matter of defining the requirements behind this feature, and maybe the requirements are: "don't access storage in an abi defined in a library, there is always a workaround" 😄

Looks like it's time for "storage in libraries, part 2" meeting.

mitchmindtree commented 1 year ago

@mohammadfawaz I think a lot of my concerns about your associated const StorageRef idea in my previous comment stemmed from my thinking that the StorageRef could not really be const. After our chat I realise I was wrong on this. I think the re-iteration that StorageRefs are basically slot indices has helped me to follow!

I think another important distinction I was missing is that, we can allow for accessing a storage slot (i.e. StorageRef) in a const manner, while still restricting the act of dereferencing and accessing the value to non-const contexts.

With all this in mind, I'll try and address my concerns below:

Diamond Inheritance

In retrospect, this can't ever be a problem if, like in Rust, we disallow abis from ever defining associated consts, and require that only the implementing type (i.e. the Contract) can provide the definition. E.g.

// ABIs.
abi Foo {
    const FOO: StorageRef<b256>;
}
abi Bar: Foo {} // Cannot define `Foo::FOO`
abi Baz: Foo {} // Cannot define `Foo::FOO`

// Implementations.
storage {
    addr: b256 = 0x1111111111111111111111111111111111111111111111111111111111111111;
}
impl Foo for Contract {
    const FOO: StorageRef<b256> = storage.addr.as_ref(); // The only place that can define `FOO` for `Contract`.
}
impl Bar for Contract {}
impl Baz for Contract {}

If we're restricting associated const definitions to the implementation, there isn't any data inheritance at all, and so my concern around the user needing to choose between data inheritance and nested fields also doesn't really apply.

Moving storage from a global context to the Contract type (#3025)?

If storage is no longer global and can only be accessed via self in Contract methods, then this kind of rules out the associated const approach, as the storage slots would no longer be accessible to the associated const definitions :thinking:

It seems like we have two desires:

  1. storage data should not be globally readable/writable (i.e. #3025), but also
  2. we don't want to unnecessarily restrict StorageRef use to Contract method calls. Ideally they'd be accessible as consts in the Contract's "associated" context.

"Associated storage" on Contract?

Maybe we want something like "associated storage", so that storage access is still restricted to the Contract's implementation, but still allows for referring to the constness of its storage slots?

Then, rather than trying to work out a way of exposing the slots as fields on the Contract methods' self arugment, we could restrict access to values to Contract calls using @sezna's idea of read and write methods, but have them be special methods on the Contract rather than the StorageRef.

Here's @mohammadfawaz's example adapted for an "owned" counter contract that demonstrates the idea:

library ownable;

pub struct OwnershipTransferred {
    previous_owner: b256,
    new_owner: b256,
}

abi Ownable { 
    const OWNER: StorageRef<b256>;
} { 
    fn owner(self) -> b256 {
        self.read(Self::OWNER)
    }

    fn only_owner(self) {
        assert(std::chain::auth::msg_sender().unwrap() == Identity::Address(~Address::from(self.owner())));
    }

    fn renounce_ownership(ref mut self) {
        self.write(Self::OWNER, std::constants::ZERO_B256); // `write` requires self is `ref mut`
    }

    fn transfer_ownership(ref mut self, new_owner: b256) {
        assert(new_owner != std::constants::ZERO_B256);
        let old_owner = self.owner();
        self.write(Self::OWNER, new_owner); // `write` requires self is `ref mut`
        std::logging::log(OwnershipTransferred {
            previous_owner: old_owner,
            new_owner: new_owner,
        });
    }
}
contract;

impl Contract {
    // :-o associated storage!
    storage {
        owner: b256 = 0x1111111111111111111111111111111111111111111111111111111111111111;
        counter: u64 = 0;
    }
}

impl Ownable for Contract {
    const OWNER: StorageRef<b256> = Self::owner; // Referring to contract storage like this *always* produces a `StorageRef`.
}

abi Counter {
    fn next(ref mut self) -> u64;
}

impl Counter for Contract {
    fn next(ref mut self) -> u64 {
        self.only_owner();
        let count = self.read(Self::counter); // read from storage
        self.write(Self::counter, count + 1); // write to storage
        count
    }
}

To clarify, referring to storage directly via Self:: (e.g. Self::owner, Self::counter) only ever produces a StorageRef<T>. To read/write storage, the Contract's read/write method must be called with the relevant StorageRef.

With this approach, we still satisfy #3025 by disallowing side-effectful storage data access in free functions (because the Contract's read/write methods must be used) while also enabling @mohammadfawaz's idea of using associated const StorageRefs to allow abis to provide ergonomic interfaces around expected storage availability.

Where can "associated storage" appear?

My intuition would be to restrict its use solely to the top-level Contract.

If an abi requires access to storage, they can use @mohammadfawaz's idea of declaring a const StorageRef and then use self.read / self.write to access the storage (as in the Ownable abi above).

Where do read and write come from?

I'm imagining these as special methods only available on Contract types. As a result, these are available via the self arg on abi methods, or the self arg on inherent Contract methods. Their signatures might look like this:

fn read<T>(self, slot: StorageRef<T>) -> T { ... }
fn write<T>(ref mut self, slot: StorageRef<T>, value: T) { ... }

What about collections? (e.g. StorageVec<T>?)

One concern that comes to mind is: how do we efficiently read and write to storage collections? That is, if a user wants to change a single value within a collection, how do we avoid requiring that they read the whole collection, change a value, then write the whole collection again?

Let's say we have the following:

impl Contract {
    storage {
        addrs: StorageVec<b256> = StorageVec {};
    }
}

and let's say we have the following ABI that we want to implement:

abi IndexAddr {
    fn index_addr(self, ix: u64) -> b256;
}

The expression Self::addrs produces the value StorageRef<StorageVec<b256>>. Perhaps one approach might be to implement methods for the StorageRef<StorageVec<T>> type that allow for producing a StorageRef<T> (i.e. a storage reference to the element we want)? E.g. the std storage module could provide a method like the following:

impl<T> StorageRef<StorageVec<T>> {
    /// Produces the storage slot of the element at the given index.
    pub fn index(self, index: u64) -> StorageRef<T> {
        // Implementation shouldn't access storage, just returns
        // a new `StorageRef` to the element at the given index
    }
}

Then our GetAddr implementation could look something like this:

impl GetAddr for Contract {
    fn get_addr(self, ix: u64) -> b256 {
        self.read(Self::addrs.index(ix))
    }
}

The general idea would be to first operate on the StorageRef when possible, before actually doing any storage access.

All that said, this still raises a lot of questions, e.g. how to do bounds checking in the index implementation without storage access to get the len? The collection APIs require making multiple synchronous reads/writes to storage, which this doesn't solve.

Maybe Contract's read and write methods could be provided by StorageRead and StorageRight traits that are automatically implemented for Contract (but cannot be safely implemented for any other types, similar to Rust's OIBITs)? This would allow us to take T: StorageRead as an argument and pass the Contract type into functions for APIs like StorageVec that involve multiple sequential storage interactions?

mohammadfawaz commented 1 year ago

Thank you so much @mitchmindtree for the elaborate description. There's a lot to unpack here but I'm glad to see this shaping up and I look forward to discussing these suggestions soon!

sezna commented 1 year ago

Just now getting around to updating this here, but I have one more proposal that we discussed on the call. It seems I may be the only fan of it, but as far as I can tell it is a reasonable solution 😄

Summary

There are a few things we are trying to avoid:

  1. Data inheritence
  2. Storage unsafety/implicit storage access when a library is imported
  3. Unmanageable global behavior

A couple things we want to introduce:

  1. The ability for libraries to reason about storage
  2. Mandatory visibility from contracts into libraries' storage usage

And a few philosophies we'd like to embrace while doing so:

  1. Prefer excess syntax on the library side
  2. Explicit > implicit
  3. Even if complexity is increased on the library writer's side, allow for more elegant API design from the consumer's perspective

Here's my proposal that addresses all of these. There are a few variations on this theme, but all of them allow some way to define an interface on a contract that is not part of the ABI. This is similar to Solidity's private functions but without all the complication of inheritance. What this allows us to do is enforce contract implementations have some interface with the library internally, and then separately allow enforcement of ABI externally. This allows us to utilize getter and setter methods, a comfortable and common pattern, to provide storage access to library ABIs.

Syntax Option 1: Allow Trait Implementations on Contract Types

library my_lib;

trait Point {
  fn get_a(ref self) -> u64;
  fn get_b(ref self) -> u64;
}

abi Multiply: Point {
} { 
  fn multiply(ref self) -> u64 {
    self.get_a() * self.get_b()
  }
}
contract;

storage {
  a: u64 = 0,
  b: u64 = 0
}

impl Point for Contract {
  #[storage(read)]
  fn get_a(ref self) -> u64 { storage.a }

  #[storage(read)]
  fn get_b(ref self) -> u64 { storage.b }
}

impl Multiply for Contract {}

Syntax Option 2: Implicitly Identify Private Methods

Alternatively, the compiler can identify when a contract is calling its own ABI via the self keyword and not generate a contract call, thus creating a private method. I'm not sure I like this as it is more implicit than explicit, but using self.call_fn() is obviously not a contract call when contract calls require abi casting: let my_abi = abi(...); my_abi.call_fn().

Pros

Cons

sezna commented 1 year ago

Recap from the meeting today:

There is a way for us to work and implement new features that will enable storage in libraries within the current paradigms of the Sway language's design. This means no data inheritance, no new syntactical ideas, and trait-based composition. This approach also has the added advantage of being extensible and cohesive with the rest of the ecosystem.

  1. Implement supertraits (and super-ABIs) for ABIs. a. a trait on a contract will be a private method
  2. Add self to ABIs so that an ABI can reason about its own methods (or maybe just use Foo::bar syntax?)

At this point, an abi can define getters and setters, or some kind of API into the consumer's storage, and then can utilize it as private methods on self. This allows for storage in libraries while keeping libraries pure.

The next steps are:

  1. Implement storage refs for deferred execution of storage ops
  2. Implement associated consts across all traits and abis

At this point, we can use associated consts and storage refs to implement storage in libraries as well. We can decide which one to document as the preferred method, likely the associated const version.

  1. Add codegen/macros or other ease-of-use utilities to increase usability of traits.


    Unresolved ambiguities:

  2. A property abstraction could really help here.

  3. Should storage just be treated like a file system with a handler api?