Introduce impl Contract { .. } and add to it free functions from std like contract_id()

[Braqzen]

Summary

Currently the Contract does not behave like an object that takes self as a parameter meaning it does not allow the user to do something like self.contract_id() where contract_id() is a function defined in our standard library that returns a pointer to the Tx ID in an external context and the contract id in an internal context.

There are many functions in the standard library that arguably should be moved into the context of the Contract rather than have them exist in the standard library where the user must explicitly import them. Perhaps Contract may be a library in and of itself.

There's a trade-off between unwanted bloat and additional out-of-the-box functionality that should be evaluated but it's unlikely to be a huge issue unless we strictly follow the standard from Rust where the user does not pay for what they do not use.

That being said, here is a list of possible functions that could be added into Contract:

• msg_sender()
• msg_asset_id()
• msg_amount()
• balance() or this_balance()
• contract_id()
• balance_of()
• reentrancy_guard()
• revert()
• require()
• Any function related to minting / burning / transferring

Here are some additional ones that may receive more debate

• assert()
• code_size()
• first_param()
• second_param()
• height() (and potentially a timestamp() once that is implemented)
• store()
• get()
• Functions related to transactions?
• log()
• global_gas()
• context_gas()

[Braqzen]

Thoughts @mohammadfawaz @mitchmindtree @nfurfaro ?

[mohammadfawaz]

Thanks! Looks great. Just updated the title a bit; I hope you're okay with that! And yeah we're going to have to make decisions on what goes in and what does not but the core work will be the same regardless... Some compiler work will be required to actually move Contract from a compiler-generated type to a library type and then allowing abis to be implemented for a non-compiler generated type (but only restrict it to Contract probably)

[otrho]

Related: #2429

[mohammadfawaz]

Yup! Hoping that if we go with this approach, then there would be fewer concerns with the impl MyAbi for Contract we can close #2429

[mohammadfawaz]

Just copying the wallet example from the Sway book to see how it would look like. Turned everything into a method to see if that makes sense:

contract;

use std::{
    address::Address,
    assert::assert,
    constants::BASE_ASSET_ID,
    contract_id::ContractId,
    identity::Identity,
    result::Result,
    revert::revert,
    contract::Contract, // New import
};

const OWNER_ADDRESS: b256 = 0x8900c5bec4ca97d4febf9ceb4754a60d782abbf3cd815836c1872116f203f861;

storage {
    balance: u64 = 0,
}

abi Wallet {
    #[storage(read, write)] 
    fn receive_funds(self);

    #[storage(read, write)] 
    fn send_funds(self, amount_to_send: u64, recipient_address: Address);
}

impl Wallet for Contract {
    #[storage(read, write)]
    fn receive_funds(self) {
        if self.msg_asset_id() == BASE_ASSET_ID {
            storage.balance += self.msg_amount();
        }
    }

    #[storage(read, write)]
    fn send_funds(self, amount_to_send: u64, recipient_address: Address) {
        let sender = self.msg_sender();
        match sender.unwrap() {
            Identity::Address(addr) => {
                assert(addr == ~Address::from(OWNER_ADDRESS));
            },
            _ => {
                revert(0);
            },
        };

        let current_balance = storage.balance;
        assert(current_balance >= amount_to_send);

        storage.balance = current_balance - amount_to_send;
        self.transfer_to_output(amount_to_send, BASE_ASSET_ID, recipient_address);
    }
}

And when calling the contract methods from scripts, maybe the abi cast should return a Contract:

fn main() -> bool {
    let wallet_contract: Contract = abi(Wallet, 0x417e8ee99a538fb03b032862bedf70ccd28dcec4a0fb455c72700f5234467f48);

    wallet_contract.receive_funds {
        coins: 42,
    } ();

    wallet_contract.send_funds(30, ~Address::from(0x0101010101010101010101010101010101010101010101010101010101010101));
}

Maybe this would help replacing the ContractCaller type (see #1261) with an actual library type Contract!

[nfurfaro]

Maybe this would help replacing the ContractCaller type (see #1261) with an actual library type Contract

I like this last point. Intuitively I tent to think of an instance of a ContractCaller as an in-memory proxy for the contract itself.

[nfurfaro]

I think that adding methods to the Contract type makes sense for some, (but not all) of the listed functions above.
i.e: getting the balance or contractId of the current contract seems reasonable, but getting the balance of another contract seems more suited to a free function as it doesn't even touch the current contract.
Similarly, some of the functions related to minting/burning/transferring tokens make sense as methods as only a contract may mint, burn or transfer its own tokens, but msg_sender() & msg_amount() don't belong on the contract type IMO.

[mitchmindtree]

but msg_sender() & msg_amount() don't belong on the contract type IMO.

I think the reason msg_sender() and msg_amount() in particular belong on the contract is that their values may be unique per invocation of any of the contract's methods. Neither of them are meaningful outside of a call to a contract, and as a result their access should be restricted to the contract "context", which having them as contract methods (or method arguments, whether via self or not) would enforce nicely without having to introduce any new language features.

Having msg_sender() and msg_amount() be free functions would mean that any function they're called within would become side-effectful, I.e. the function may behave differently even if the arguments to that function are the same. The more side-effectful functions we expose publicly and encourage the user to adopt, the more we might limit ourselves from performing certain kinds of optimisations or verification in the future that are much easier to perform when your functions are referentially transparent.

If we are to keep them as free functions, we'll also require an extra language feature to isolate their use to contracts (like this one FuelLabs/sway-rfcs#9), which I think would unnecessarily complicate the language when we already have the language constructs we need to isolate their use (i.e. contract methods or contract method arguments).

[mitchmindtree]

On a related note: perhaps one alternative might be to have a msg method on self, that returned something like a ContractMsg type to more clearly distinguish the set of methods relevant to the contract call message from the contract itself. It might also make it easier to pass around the contract call's msg data without passing around the entire contract? E.g.

self.msg().sender()

    #[storage(read, write)]
    fn receive_funds(self) {
        let msg = self.msg();
        if msg.asset_id() == BASE_ASSET_ID {
            storage.balance += msg.amount();
        }
    }

[nfurfaro]

perhaps one alternative might be to have a msg method on self

This sounds promising.
I was also thinking that there maybe a place for a Message type with methods like message.sender, message.amount, etc... But I hadn't considered the approach you describe above @mitchmindtree

One thing we should try to avoid is any confusion between this hypothetical Message/ContractMessage type, and Message Inputs/Outputs.

We use the termmessage for existing functions like msg_sender(), msg_amount() & msg_asset_id() as they have very similar counterparts in the Solidity world, but I wonder if there is a better term to describe this, i.e: is it more correct to call it a ContractCall, or just a Call?

[mohammadfawaz]

I just noticed that most likely the type Contract won't have any actual data fields and so passing in self to its methods is not particularly useful (and should probably be a warning). What this means is that we're going to have to call those methods using the ~Contract:: syntax. Example:

~Contract::msg_amount()
~Contract::msg_sender()

We may also want to think about introducing other types that could hold some of the standard library functions that don't belong to Contract such as height where we might want to have:

~Block::height()

If we agree with the above, then it might be time to really get rid of that ~ (ref #1436)

[mitchmindtree]

I just noticed that most likely the type Contract won't have any actual data fields and so passing in self to its methods is not particularly useful

While the Contract type itself doesn't have any fields, self does represent access to data that is unique to the contract call (e.g. msg_amount, msg_sender, etc).

Having the user pass self is useful for restricting the usage of these contract methods to the contract call context with simple, existing language features (function arguments and methods). In my mind, this restriction is one of the main motivations of this proposal and is part of what helps us to avoid needing to tack on something like FuelLabs/sway-rfcs#9.

Passing self (even if Contract is technically a zero sized type today) also gives us the freedom to add other Contract fields/methods in the future (e.g. providing access to storage via self as mentioned here).

If we don't feel comfortable using Contract or self, we could consider requiring a call: Call argument instead so that it's clear that these methods are specific to the contract call and not just the contract (similar to @nfurfaro's suggestion above).

Whatever we go with, I think it's important that we treat data that is unique to each contract method call (e.g. sender, amount, asset ID) as inputs to the contract method, rather than treating them as global state accessible by global functions and then trying to use attributes in an ad-hoc attempt to restrain where these global functions can be used when they shouldn't really be global in the first place.

[mohammadfawaz]

Thanks @mitchmindtree, this makes sense to me. I certainly do prefer self and I agree with all the reasons you presented above. The only reason I brought this up is because it felt odd having to write all the methods for Contract and require them to take in self when they don't actually use self. But then again, I now remember that this exactly is how I wrote insert and get for StorageMap so not too odd after all 😊.

Your comment about potential future fields for Contract is also important; we didn't finish our discussion about putting storage in there so that's something that we should pick up again soon.