Distinction between Identity/Contract/Address

[MujkicA]

From this discussion https://github.com/FuelLabs/fuels-rs/issues/952 it was suggested that the distinction between Identity/Contract/Address represents a risk factor for people building with sway.

Unless the developers have the foresight to use Identity as the type for their admin address, they will not be composable with other contracts.

There seems to be a considerable risk associated with as developers coming from other ecosystems are likely to assume that there is no such limitations when just using Address.

[diyahir]

TLDR: (Highly recommend reading the thread)

1. I was annoyed by not being able to test my contract from a wallet because it was an address and not a contractId.
2. Using Identity caused some casting issues like when using abi(....) since it expects a contractId
3. Realizing that this is actually a much bigger problem because a multisig is a contractId, so if you use address by accident then you are no longer composable with other contracts who may want to open a position / interface with you
4. I look to see if anyone has made this mistake: prominent project SwayLend, a compound fork, has made it (not faulting them at all) but their current implementation is not composable with other contracts https://github.com/compolabs/sway-lend/blob/master/contracts/market/src/main.sw
5. This system is annoying and introduces incompatibility bugs + makes testing contracts much harder

Fundamentally, ANY action done by a wallet should be able to be done by a DAO/multisig (full stop, no exceptions). The current scheme means that developers need to have the foresight to allow contractIds (multisgs) to be composable. You can recommend Identity everywhere sure, but this still introduces the potential for composability bugs + makes testing contracts very hard. If this distinction is not a value ADD then please remove it.

This is a critical mistake, let's ensure that DAOs / Multisigs are not second class citizens in Fuel by removing this distinction. This would actually be a step in the opposite direction with respect to account abstraction, and smart contract wallets.

[K1-R1]

Having read the thread I definitely understand your concerns @diyahir, however I initially find myself agreeing with @MujkicA 's comment:

However, my opinion remains that for a smart contract, any choice of permitting additional functionality/use-cases should always be deliberate instead of implied. I think this is better addressed trough proper documentation.

It seems as though this is a case of understandable user-error. Having said that, both Address and ContractId are just wrappers around a b256:

/// The `ContractId` type, a struct wrapper around the inner `b256` value.
pub struct ContractId {
    value: b256,
}

There's nothing to enforce that the wrapped b256 corresponds to the correct type; you can have the b256 of a multisig wrapped within an Address .

[IGI-111]

Seems like we want three things here:

1. Allow users to express categories of addresses within the type system, to express what is and isn't intended to be used by contracts
2. Prevent common user errors that restrict types in a way that is surprising and not obvious
3. Allow users to use existing contracts in unintended ways if they make it explicit that they want to for testing and compatibility purposes

I would say we should at least:

• Document the distinction better, especially make it clear that Address and ContractId are less permissive than one would expect
• Structure the standard library in a way that holds these types together so the connection is obvious
• Provide From implementations to make the proper conversions easy
• Consider making the improper conversions possible but slightly harder or more explicitly unsafe
• Consider renaming Address to something that is more explicit about the restriction

[diyahir]

These are all a step in the right direction! But this whole distinction is still in my opinion flawed.

Can you name me one concrete example where you do not want a Multi-sig / DAO contract to interface with your protocol? If the answer is no, then just remove this. (I cannot come up with one). This feels a bit over-engineered, no one was asking for this 'feature'.

Secondly, the industry is moving towards account abstraction with smart contract wallets, this decision will mean that there is a chance that my smart contract wallet cannot do all of the things that my normal wallet can? This was never the intention.

Documentation is good, but we're embedding a 'gotcha' into the system, which I have to really push back on. (Oh well you didn't read the one paragraph in the docs, that's a 'you' problem). This is a fundamental error, which is better to find in beta than in prod.

[diyahir]

There's nothing to enforce that the wrapped b256 corresponds to the correct type; you can have the b256 of a multisig wrapped within an Address .

This comment confuses me, I now really don't understand what the whole point of this identity, contracted, address song, and dance was for?

This comment suggests, it's not enforced, it's superficial, and is annoying to deal with, so what exactly are we doing here?..

[IGI-111]

You seem to be of the opinion that there is no circumstance in which distinguishing wallet and contract addresses is useful and therefore that expressing that is pure overhead.

I don't have strong feelings about this but it seems strange to me given the VM treats outputs and contracts differently.

Consider the following code from std:

pub fn transfer(amount: u64, asset_id: AssetId, to: Identity) {
    match to {
        Identity::Address(addr) => transfer_to_address(amount, asset_id, addr),
        Identity::ContractId(id) => force_transfer_to_contract(amount, asset_id, id),
    };
}
// ...
pub fn force_transfer_to_contract(amount: u64, asset_id: AssetId, to: ContractId) {
    asm(r1: amount, r2: asset_id.value, r3: to.value) {
        tr r3 r1 r2;
    }
}
// ...
pub fn transfer_to_address(amount: u64, asset_id: AssetId, to: Address) {
    // maintain a manual index as we only have `while` loops in sway atm:
    let mut index = 0;

    // If an output of type `OutputVariable` is found, check if its `amount` is
    // zero. As one cannot transfer zero coins to an output without a panic, a
    // variable output with a value of zero is by definition unused.
    let number_of_outputs = output_count();
    while index < number_of_outputs {
        if let Output::Variable = output_type(index) {
            if output_amount(index) == 0 {
                asm(r1: to.value, r2: index, r3: amount, r4: asset_id.value) {
                    tro r1 r2 r3 r4;
                };
                return;
            }
        }
        index += 1;
    }

    revert(FAILED_TRANSFER_TO_ADDRESS_SIGNAL);
}

[diyahir]

I'm open to the idea, but no one has provided a compelling example or justification as of yet. The only examples I can come up with are refuted by multisig smart contracts, smart contract wallets, and DAOs should have the same functionality full stop as a wallet, and we don't want to introduce weird casting just to introduce weird casting.

If the VM must know the type, then maybe it should be a hidden fact like in EVM: the fact that it's an EOA is effectively completely abstracted away

The code you're showing me just tells me that places in the stack, have followed the convention introduced, but not that it was a meaningful distinction. This is by definition what tech debt looks like, 'we cannot remove X, because look we´ve used X here and there´.

wallet.force_transfer_to_contract(to, balance, asset_id, tx_parameters).await
wallet.transfer(to, amount, asset_id, tx_parameters).await

With this distinction just transferring tokens is more overhead...this is not useful at all

[IGI-111]

The EVM doesn't use UTXOs though and that's a fundamental design choice. I can't speak to the necessity of having two different transfer instructions, but I doubt we have two for the sake of having two.

So long as transferring funds is a different process for either type we have to be told which type it is. Otherwise it's impossible to deduce what to do, and the safe way to carry this information is through the type system.

I hear that the conversions are annoying and the meaning of the types unclear, hence my previous recommendations to make that less painful. But I can't agree that carrying the information is useless unless it can be demonstrated that the VM has the ability to hide this distinction and retain it's performance characteristics.

In any case, that part of it is not a language design issue.

[diyahir]

Can you tag a few others for more input? @IGI-111

I'm trying to understand the reasoning a bit deeper. To me with freshish eyes, it looks like 'it was just built this way' rather than an intentional value-add distinction.

A compelling example would go a long way to add more flavor to the discussion. Key stakeholders here are normal users, daos, multisigs, smart contract wallets (social recovery etc), and other protocols (like structured product vaults that hook into existing products)

In solidity you can do this: https://ethereum.stackexchange.com/questions/15641/how-does-a-contract-find-out-if-another-address-is-a-contract

To check if contract during run-time, which seems like it would translate to FVM as well