ApeWorX / ethpm-types

Implementation of EIP-2678
Apache License 2.0
14 stars 8 forks source link

"internal_type" vs "internalType" in ABIs #116

Closed cygnusv closed 3 months ago

cygnusv commented 8 months ago

Environment

$ ape --version
0.7.7

$ ape plugins list
Installed Plugins
  etherscan    0.7.0
  infura       0.7.0
  polygon      0.7.1
  solidity     0.7.1

What went wrong?

We've been collecting deployment ABIs by reading my_contract.contract_type.abi, but we're noticing that with v0.7 there's some new elements that are different than before. Compare the output of v0.7.7:

                "inputs": [
                        {
                            "name": "_token",
                            "type": "address",
                            "components": null,
                            "internal_type": "contract IERC20"
                        },

with previous versions (I think this was 0.6.19):

                "inputs": [
                        {
                            "name": "_token",
                            "type": "address",
                            "internalType": "contract IERC20"
                        },

Two differences:

My suspicion is that there may be a better, more standard way to get the ABI for a contract you just deployed? If not, was this change intentional?

linear[bot] commented 8 months ago

APE-1686 "internal_type" vs "internalType" in ABIs

linear[bot] commented 8 months ago

APE-1687 "internal_type" vs "internalType" in ABIs

fubuloubu commented 8 months ago

transferred this to the proper repo

derekpierre commented 3 months ago

@fubuloubu Any update on this issue?

antazoey commented 3 months ago

I am unable to reproduce, please supply more info for reproduction steps.

Here is what I am doing:

ct = project.TestContractSol.contract_type  # Using Ape but can get a ContractType anyway you want.
abi = ct.mode_dump()["abi"]

The ABI it is showing has the correct casing everywhere for internalType.

...
 {'type': 'function',
  'name': 'theAddress',
  'stateMutability': 'view',
  'inputs': [],
  'outputs': [{'name': '', 'type': 'address', 'internalType': 'address'}]}]
...

It works because of this code here: https://github.com/ApeWorX/ethpm-types/blob/main/ethpm_types/base.py#L10-L15

derekpierre commented 3 months ago

Thanks for checking.

It seems that we are doing the following:

def _get_abi(contract_instance: ContractInstance) -> ABI:
    """Returns the ABI of a contract instance."""
    contract_abi = list()
    for entry in contract_instance.contract_type.abi:
        contract_abi.append(entry.dict())
    return contract_abi

The .dict() call seems to now be deprecated ... 🤔 ... I guess we should be using .model_dump() instead?

derekpierre commented 3 months ago

@antazoey ran some tests and that clarifies things - we caused the issue 😅 ! Thanks again for following-up. Much appreciated!

antazoey commented 3 months ago

Oh yay!

Yes, dict() is deprecated but I can make the fix apply there as well. Thank you!