web3.py: Invoke ambiguous contract functions

@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.

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

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. 😄

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. 😃

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.

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!

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…

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)

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.

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

@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).

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(...)

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.