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

Consider appending data to a contract's bytecode which decribes which abis the contract implements. #3234

Open
nfurfaro opened this issue Nov 1, 2022 · 9 comments
Labels
ABI Everything to do the ABI, especially the JSON representation bikeshedding For bikeshedding trivialities compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request

Comments

@nfurfaro
Copy link
Contributor

nfurfaro commented Nov 1, 2022

In Solidity it’s faily common to register each interface that a contract implements and provide a getter for this, so that other contracts can query it to see if it can for example receive erc-721 tokens safely. This is done according to https://eips.ethereum.org/EIPS/eip-165 , where “interface detection” is achieved via registering interface signatures in storage manually.
I think we could do better than this. The compiler already knows whether a contract implements an abi or not when compiling; seems like it could append that data somewhere in a common format (like the hash, or the first 4 bytes or whatever).

It was suggested by @adlerjohn @sezna that it could be added to the bytecode:

standardizing it as a blob suffix to the bytecode sounds clean

We would still need to determine the best format to use to represent an abi in a standardized way.
From eip-165 :

We define the interface identifier as the XOR of all function selectors in the interface. This code example shows how to calculate an interface identifier:

pragma solidity ^0.4.20;

interface Solidity101 {
    function hello() external pure;
    function world(int) external pure;
}

contract Selector {
    function calculateSelector() public pure returns (bytes4) {
        Solidity101 i;
        return i.hello.selector ^ i.world.selector;
    }
}

@nfurfaro nfurfaro added enhancement New feature or request bikeshedding For bikeshedding trivialities compiler General compiler. Should eventually become more specific as the issue is triaged labels Nov 1, 2022
@adlerjohn
Copy link
Contributor

adlerjohn commented Nov 1, 2022

We could also just expose an implicit ABI method (that can be toggled on with an annotation) a la supportsInterface from the above-linked EIP. E.g.

fn supports_interface(interface_commitment: u32) -> bool

@nfurfaro
Copy link
Contributor Author

nfurfaro commented Nov 1, 2022

I like that approach.
Any implicit ABI methods we want to expose like this could be achived through a "base contract abi" with default function implementations that all contracts implement by default.

@nfurfaro
Copy link
Contributor Author

nfurfaro commented Dec 5, 2022

I've been thinking more about this in the context of both:
a. the problems it attempts to solve, and
b. FuelVM / Sway specific things that allow us to do better than how this is achieved on Ethereum.

A few points to consider:

  • on Ethereum, the first standard to emerge related to Interface detection/discoverability was erc-165.
    More recently, erc-1820 has been used (an improvement on erc-820), and it includes functionality to make it backwards-compatible with erc-165. A couple of key benefits of erc-1820:
    • provides a "caching" service to help minimize gas costs
    • allows for both contracts and addresses to "delegate" implementors for a given interface (so not just for contracts like erc-165
    • requires a registry contract deployed at the same ContractId on each network, which we can do trivially on Fuel. (This required jumping through some hoops to achieve on ethereum via "Nick's method"
  • We don't need to maintain backward compatibility with erc165, so we could simplify things a fair bit.

Implementation

One way we could support interface detection/ discoverability is as follows:

  1. Append a bytecode blob to the end of each compiled contract (maybe optionally, but done by default).
  • must contain the interface identifier for each interface/abi implemented by the contract
  • must be easy to find the location of this blob, i.e: at a constant offset from the end of the contract bytecode or something
  • ideally this blob could be loaded from another contract with ldc allowing contracts to discover all the interfaces another contract implements without the need for multiple contract calls.
  • an abi in sway should have a method like `.identifier()' which just calculates & returns the identifier for the abi. This is simply calculated as the XOR of all function signatures in a given abi. This way, when doing an abi cast, you get the abi identifier as a bonus, and can then check an external contract's bytecode to see if it implements this abi as well.
  1. Deploy a Fuel-upgraded version of an erc-1820 Registry contract.
  • the caching service provided by this contract on Ethereum is meant to save gas. When the registry is called to inquire about Contract Foo, and whether or not it implements the Bar interface, the local cache in the registry can be checked first to see if Foo has already registered itself as implementing Bar. If so, no need to call Foo to check, if not, then call Foo, ask it if it implements Bar, and then update the cache.
    With ldc + appended bytecode blob, we could skirt the whole need for a cache, or at least make it way cheaper to initialize/update the cache.
  • The main functions of the registry (on Ethereum) are:
    a. caching
    b. erc-165 backwards compatibility
    c. supporting interface lookup (important for token standards like erc-777) for both contracts and Externally-owned Addresses.

Caching is optional, but maybe still beneficial; erc-165 backwards compatibility is not needed.
Supporting interface lookup (important for token standards like erc-777) for both contracts and Externally-owned Addresses is still important, so I think we still want a registry.

cc @adlerjohn @SilentCicero

@adlerjohn
Copy link
Contributor

This seems unnecessarily complicated compared to

#3234 (comment)

the caching service provided by this contract on Ethereum is meant to save gas

ERC-1820 doesn't provide a cache, it provides a registry. It's also not clear how calling the registry then the contract is cheaper than calling the contract then the contract. The registry is itself a contract. The actual motivation for ERC-1820 is:

There have been different approaches to define pseudo-introspection in Ethereum. The first is ERC-165 which has the limitation that it cannot be used by regular accounts. The second attempt is ERC-672 which uses reverse ENS. Using reverse ENS has two issues. First, it is unnecessarily complicated, and second, ENS is still a centralized contract controlled by a multisig. This multisig theoretically would be able to modify the system.

@nfurfaro
Copy link
Contributor Author

nfurfaro commented Dec 6, 2022

This seems unnecessarily complicated

It may be slightly more complicated, but if we separate the core ideas into:

  1. Appending some bytecode to the end of a contract, and
  2. creating an ERC-1820 style registry

then for the purpose of this issue, we're only concerned with number 1. (Number 2 is related, but not necessary for 1 to be useful).

ERC-1820 doesn't provide a cache, it provides a registry.

The ERC-1820 registry contract is indeed first and foremost a registry. It does however have some features meant to provide backward compatibility with ERC-165, and a couple of these functions are caching-related:

This contract also acts as an ERC-165 cache to reduce gas consumption.

Given that I don't think? we need to provide backward compatibility with ERC-165, we can disregard any of my previous comments related to caching.

It's also not clear how calling the registry then the contract is cheaper than calling the contract then the contract.

The beauty of the proposed change here (appending bytecode to a known location) is that we can discover which abis a given contract implements, without ever making a call ! (via ldc).

@nfurfaro nfurfaro closed this as completed Dec 7, 2022
@nfurfaro nfurfaro reopened this Dec 7, 2022
@nfurfaro
Copy link
Contributor Author

nfurfaro commented Dec 7, 2022

Oops, closed by mistake yesterday, so I reopened it.

@nfurfaro
Copy link
Contributor Author

@adlerjohn Any more thoughts on this given my response to your critique above?

@adlerjohn
Copy link
Contributor

A call shouldn't be much more expensive than loading code, if contracts are small. If contracts are larger, then an optimizoooor may well split it up into multiple contracts so that only what's necessary is loaded.

The benefit of a standard code location is being able to get the supported ABI methods without calling (but you still have to load, which as above isn't in all cases meaningfully cheaper). The downside of a standard code location is that it doesn't work with proxies.

@nfurfaro
Copy link
Contributor Author

nfurfaro commented Dec 13, 2022

My thinking here was that when loading code via ldc for the purpose of discovering the abis the target-contract implements, you would only need to load the appended blob of bytes which contain this information, regardless of the size of the contract. This would take advantage of the ability to specify the number of bytes to lode when calling ldc.
So even with a massive contract that implemented 10 different abis, you would only need to load ~40 bytes (10 * 4 bytes per interface-identifier).

cc @sezna

@anton-trunov anton-trunov added the ABI Everything to do the ABI, especially the JSON representation label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Everything to do the ABI, especially the JSON representation bikeshedding For bikeshedding trivialities compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants