Invoke ambiguous contract functions

[carver]

What was wrong?

If a contract has multiple functions of the same name, and the arguments are ambiguous, say:

contract AmbiguousDuo {
  function identity(uint256 input, bool uselessFlag) returns (uint256) {
    return input;
  }
  function identity(int256 input, bool uselessFlag) returns (int256) {
    return input;
  }

It is currently impossible to call the identity function on that contract for positive numbers, because web3 cannot identify which one you want:

contract.functions.identity(1).call()

How can it be fixed?

Add a way to unambiguously call a specific method (maybe by providing the function signature). Something like:

identity_unsigned = contract.functions['identity(uint256,bool)']
identity_unsigned(1, True).call()

identity_signed = contract.functions['identity(int256,bool)']
identity_signed(1, True).call()

It should support all these options for identifying the function:

• Full Signature: contract.functions['identity(int256,bool)']
• Byte selector (first 4 bytes of the hash of the full signature), in the form of:
  • bytes -- contract.functions[b'\x8e\xab\x23\x03']
  • int (writable as a hex literal in python) -- contract.functions[0x8eab2303]

[pipermerriam]

might be nice to support both the full function signature and the 4byte selector in those lookups.

[pipermerriam]

Also probably worth some extra convenience APIs surrounding this.

>>> contract.functions.find_by_name('identity')
[<Function identity(int256)>, <Function identity(uint256)>]

[carver]

might be nice to support both the full function signature

Hm, was I suggesting something different? Oh, maybe you thought I meant you could just specify the one type you cared about, like: identity(,,bytes,)? That's interesting too, but I meant the whole signature.

and the 4byte selector in those lookups.

Yeah, that too, but I was punting because it requires an even heavier api (since you have to also specify the ABI definition somehow).

[pipermerriam]

maybe you thought I meant you could just specify the one type you cared about,

Nope, just suggesting what you already proposed (full signature) with the addition of support for 4byte selector.

but I was punting because it requires an even heavier api

No, I just mean supporting contract.functions['0xabcd1234'] assuming 0xabcd1234 was one of the 4byte selectors for a contract functions. Should be quite trivial to implement.

[carver]

No, I just mean supporting contract.functions['0xabcd1234'] assuming 0xabcd1234 was one of the 4byte selectors for a contract functions. Should be quite trivial to implement.

🤦‍♂️ Ah, of course: given that the function info was already specified in the abi (just like with all these other selectors).

[carver]

Hm, and we're back to inferring hex vs text strings. Solidity doesn't allow a function starting with 0, so we could use that to tell the difference, but I'm not sure we can guarantee that all languages that compile to EVM bytecode have that restriction. Maybe we should just require that people supply the prefix in bytes or hex literal (int) to use this esoteric feature.

[pipermerriam]

ok, how about we don't support it with item lookups.

contract.functions['identity']  # only allow function names here.
contract.functions.get_by_signature('identity(uint256')  # only lookup based on signature
contract.functions.get_by_selector('0x1234abcd')  # only lookup by selector.

We could allow for signature lookups using contract.functions['identity(uint256)'] as I don't think that leaves any ambiguity since signatures are uniquely distinguishable from function names.

Also, it might be good to support a full API like the following and make use of it internally.

contract.functions.get_by_<signature/name/selector/arguments>  # returns exactly 1, throws error on multiple matches.
contract.functions.find_by_<signature/name/selector/arguments>  # returns iterable of matches (potentially zero matches

Alternative names for find

• search_by
• filter_by

[carver]

Also, it might be good to support a full API like the following and make use of it internally.

I really like this (but they shouldn't sit inside contract.functions where they might conflict with the contract's functions).

contract.all_functions_by_selector('0xac37eebb')
contract.all_functions_by_selector(0xac37eebb)
contract.all_functions_by_selector(b'\xac7\xee\xbb')
contract.function_by_selector('0xac37eebb')
contract.function_by_selector(0xac37eebb)
contract.function_by_selector(b'\xac7\xee\xbb')

contract.all_functions_by_name('identity')
contract.function_by_name('identity')  # <-- contract.functions.identity aliases to this

contract.all_functions_by_signature('identity(uint256)')  # <-- this one seems a little silly, I don't know how the solidity compiler handles a conflict here... but I guess just in case it doesn't hurt to include it.
contract.function_by_signature('identity(uint256)')

contract.all_functions_by_args(123)
contract.function_by_args(123)

One trick is how do we help people do something useful with the returned functions in an iterator. I guess we add each function's abi to that ContractFunction instance, so users can inspect the types and decide how to filter them further...

[veox]

One trick is how do we help people do something useful with the returned functions in an iterator. I guess we add each function's abi to that ContractFunction instance (...)

Yes, please. ^_^

A case I've had (with LLL, so maybe not so relevant to most) is testing several same-name functions from a contract's ABI where the argument list gets ever-longer (less arguments implied).

The proposed improvement is very far up the pipe from where I'm at now; but "it would be great" if I could

• take a contract,
• get all_functions_by_name('approve') that it implements,
• get function.<abi_selector_incantation> for each, and
• submit a transaction with data of <selector><payload_iterable>

in a test matrix that mutates the <payload_iterable>, to see whether the contract in question correctly implements those function approve() variants that it has in its ABI.

This example is slightly convoluted, but I hope it demonstrates the possible utility of contract.all_functions_by_name().

[carver]

Good to know! This whole feature suite is aimed at power users, so I'm definitely curious to explore it a bit with you.

I don't understand something though: the default selector should be pretty good at differentiating between calls with different numbers of arguments. Why doesn't something like this work for you?

args = (1, True, 'str', b'bytes')
for last_arg_idx in range(len(args)):
    range_end = last_arg_idx + 1
    contract.functions.approve(*args[:range_end]).transact(...)

[veox]

That example of mine was not only "convoluted", but also "contrived": I'm not actually doing this (yet). :D

(EDIT: To clarify: I've gone with "change the name" instead, i.e. the extra-argument function is called approve_timed().)

The snippet you provide would work just fine for testing the functions with their correct arguments and data, i.e.:

function approve(uint256);
function approve(uint256,bool);
function approve(uint256,bool,string);
function approve(uint256,bool,string,bytes);

with a total of 4 tests. What I wrote about should allow for 16 tests: every selector also tested with data that would have been passed to the other selectors in "normal" use.

---

This gets even hairier if you imagine, say, ERC-20 getting "ported" to same-name approach:

function transfer(address,uint256); // ERC-20 transfer()
function transfer(address,address,uint256); // ERC-20 transferFrom()

In this case, the "extra non-implied argument" is prepended, not appended, to the argument list.

---

Again, these examples are contrived, and probably "don't look like the best approach", precisely because I'm not trying to solve an actual problem with them. Maybe I better keep quiet now.

(Having the ABI description available on the ContractFunction instance still seems useful.)

[pipermerriam]

The following API improvements come to mind.

>>> fn = contract.functions.transfer
>>> fn.is_unique
False
>>> fn.selectors
["0x1234abcd", "0xdeadbeef"]
>>> fn.selector
# exception: Multiple selectors available.
>>> fn.abis
[{...}, {...}]
>>> fn.filter_by_args(my_address, 12345)
[<Function transfer(address,uint256): 0x1234abcd]
>>> exact_fn = fn.get_by_args(my_address, 12345)
>>> exact_fn.is_unique
True
>>> exact_fn.selector
"0x1234abcd"
>>> exact_fn.abi
{...}

And probably something like this too.

>>> contract.all_functions
[<Function transfer()>, <Function transfer(address)>, <Function withdraw()>, ...]
>>> contract.all_functions.filter_by_name('transfer')
[...]
>>> contract.all_functions.filter_by_name('transfer').get_by_args(my_address, 12345)
<Function transfer(address,uint256)>

[voith]

I'll take a stab at this over the weekend.

[carver]

Thanks, @voith !

Note that the API here doesn't seem finalized. So there may be quite a bit of churn in the PR. (By posting a PR, you're implicitly proposing a final API)

Alternatively, you could propose a summarized API here for consensus before trying to polish off an implementation.

[voith]

@carver I wanted to ask you if the API was finalized but then I'd thought to myself that I'll get the basic implementation out and then we could rename/add functionality as needed.
In my first take I want to implement the following:

contract.all_functions_by_selector
contract.function_by_selector
contract.all_functions_by_name
contract.function_by_name
contract.all_functions_by_signature
contract.all_functions_by_args
contract.functions_by_args

piper merriam's suggestion seems like it'll need another class to implement the functionality. I'll try to implement that too if time permits(but not in my first attempt).

[voith]

@carver One question, At the moment I'm not super familiar with how contract function names and signatures are fetched(I will figure it out once I start), but from my brief look I can see that it is inferred from the ABI.
So my question is Should I be assuming that the ABI will be available to me or is there any other way of fetching it?

[carver]

It's okay to fail out if contract.abi is not set, hopefully with a nice error message (as requested in #698).

[pipermerriam]

@voith re needing another class. +1 to pushing that out to a subsequent PR. Lets ignore the idea of chaining these filters for now and just implement the top level API, after which we can see about extending it to somthing where you can chain the filtering methods together.

Also, my naming suggestion would be find_functions_by_<thing> and get_function_by_<thing>.

[voith]

piper merriam's suggestion seems like it'll need another class to implement the functionality.

I made this statement without having a complete Knowledge about the existing codebase, So I might be wrong there.

@pipermerriam In fact I gave a thought to the API improvement that you've proposed.
IIUC, In the current implementation ContractFunctions acts like a bridge for every ContractFunction. ContractFunctions stores metadata of each function( function name at the moment) and the ABI's of all functions.
Keeping the current implementation in mind, implementing the code below would need every
ContractFunction to have an instance of ContractFunctions or have the abi's of all other functions, because a ContractFunction doesn't( or shouldn't?) have Knowledge about other existing functions.

>>> fn = contract.functions.transfer
>>> fn.is_unique
False

This reminds me of the Dependency Inversion Principle(Not very sure if it's applicable here).
The alternate API that I'd propose would be:

contract.is_unique_function('function_name')

This again is similar to the API that @carver proposed.

[pipermerriam]

I think we could drop the is_unique API.

[pipermerriam]

I actually think we could drop the is_unique API.

[carver]

Keeping the current implementation in mind, implementing the code below would need every
ContractFunction to have an instance of ContractFunctions or have the abi's of all other functions

Not exactly. ContractFunction is really the set of all functions that match the given function name. So it is arguably a bit misnamed (since it implies exactly one function match). So you could find out is_unique by checking if more than one function has that same name.

[carver]

That's not me necessarily saying we need is_unique in this particular API. I'm not sure exactly what good it would be, since all the find_* functions will be collections, and all the get_* functions will be exactly one function. So is_unique is trivially implementable by users as:

funcs = find_*(...)
unique = (len(funcs) == 1)

[voith]

Update: I just started working on this. Posting here to make sure that no one else is working on this. I don't want to duplicate work!

[voith]

I have a basic working version of this which I'll submit either today or tomorrow. But I have a few questions before I submit.

1. I don't think supporting contract.find_functions_by_signature makes much sense as signature will always be unique(unless I'm missing something). Solidity gives a compilation error if two functions are defined with the same signature.
2. Also I'm not sure if it makes sense to support contract.find_functions_by_selector. Solidity gives collision error if two different functions have the same 4byte_selector . I am not well versed with solidity so correct me if I'm wrong.

I used the following code to test case 2.

pragma solidity ^0.4.21;
contract AmbiguousDuo {
  function blockHashAmphithyronVersify(uint256 input) returns (uint256) {
  	  return input;
  }
  function blockHashAskewLimitary(uint256 input) returns (uint256) {
  	  return input;
  }
}

both function selectors evaluate to 0x00000000. (I borrowed these names from the pyeth-dev gitter channel)

[carver]

Dropping these two is fine by me, if we explicitly check for these conditions in the abi passed into the contract and error out when trying to create the contract object, on failure.

[veox]

Somewhat OT: "Solidity does X" shouldn't be an argument. There are other languages, too. ;) (Some of them don't even have the notion of signatures/selectors...)

---

EDIT: Well, at least when discussing abstract contracts that are being created from user-provided ABI specifications. Checking for collisions while sanitising/validating the ABI would seem desirable indeed. :)

[voith]

There are other languages, too. ;) Some of them don't even have the notion of signatures/selectors...

If that's the case then I assume that collision of selectors would not be an issue for such languages. So should we/Shouldn't error out strictly when collisions are found in the ABI?? OR does web3 plan to support ABI generated by such languages?

Also @veox can you name some languages that don't have the notion of selectors. I've heard about vyper and lll, but haven't explored them.

[veox]

TL;DR: Raising an exception on collisions in the ABI would be a safe bet, and a wise thing to do. :)

It is an edge case, so should be rare unless intentional. My main concern is with the "intentional" part: it's not about what web3 implementations are willing to support, but what is going to be "fed" to them, in some cases maliciously.

---

Very OT:

Yes, I'm talking about LLL mainly. :)

The earliest workflow for me was:

1. write a program where the selectors are pre-calculated and placed as "constants" in code;
2. write a matching interface description of it in Solidity, and generate an ABI spec in JSON using an utility script;
3. write tests that use the ABI spec from step (2) and the compiled bytecode from step (1), basically populating contract.abi and contract.bytecode{,_runtime}.

These days, the order is reversed. So, I do use solc --hashes to generate the same selectors as everybody else, and I place these selectors in the program, - but that's just a convenience, for the sake of compatibility with existing tools.

This is all very flexible, precisely because there are no assumptions of, say, the bytecode having been produced by the same tool as the abi.

OR does web3 plan to support ABI generated by such languages?

For now, everyone's doing just fine pigeonholing themselves into what's already supported. No need to spread thinner. :)

---

Rabbit-hole OT:

According to the web3.0 zeitgeist, a program would not have both:

// selector collision: both 0x00000000
function left_branch_block(uint32);
function overdiffusingness(bytes,uint256,uint256,uint256,uint256);

because Solidity does not allow collisions in function selectors; the ecosystem is optimised for the most popular language, Solidity; "a contract" mentioned in an abstract statement will likely be Solidity. Such a program is inconceivable!..

An LLL program would check the length of CALLDATA, plus maybe the first four bytes of it, and run some macro based on those checks. So, it's definitely conceivable.

Where this is going I don't know, but I'm sure that a general-purpose computer will always present ways to dig ourselves in deeper. :D

[voith]

@veox Thanks for the detailed write up, makes sense! I will stick to jason's advice for erroring out early incase of collisions.

[pipermerriam]

@veox great write-up and really good stuff for us to be aware of because I can see us eventually having some of the following present in the ecosystem.

• Something completely departed from ABI semantics
• ABI with pre-specified function selectors where each function explicitely specifies it's selector.

[carver]

Closed by #841