Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat: support abi generated from forge #740

Merged
merged 1 commit into from
Jan 8, 2022

Conversation

thasarito
Copy link
Contributor

@thasarito thasarito commented Dec 25, 2021

Motivation

Binding generated by AbiGen which bind output of forge build throws this error #683

But it work If I change the format of ABI json to only be the content of abi

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@thasarito thasarito changed the title [WIP] feat: support abi generated from forge feat: support abi generated from forge Dec 26, 2021
@thasarito thasarito marked this pull request as ready for review December 26, 2021 02:23
@gakonst
Copy link
Owner

gakonst commented Dec 26, 2021

Hey - I think this does not exactly work? A few errors in CI - could you take another look?

@thasarito
Copy link
Contributor Author

@mattsse
Copy link
Collaborator

mattsse commented Dec 27, 2021

looks like https://github.com/thasarito/ethers-rs/blob/8c63eef62669e5a13a5adac44166f2348913310d/ethers-contract/ethers-contract-abigen/src/contract.rs#L159 breaks a lot of test.

Can you give me some direction how to fix this? @mattsse @gakonst

abi parsing is a bit horrible rn:

  • the ethabi::Contract type does not really deserialize to the same json object solc spits out, I noticed it omits some fields for the constructor.
  • Also we currently have a system in place that handles human readable abi differently, instead of the json abi we parse the human readable abi during runtime as well, which is perhaps no longer necessary, I think

So how to mitigate this here:
short term is, update the abi_str only in this else branch, and use the RawAbi to deserialize it

serde_json::from_str::<RawAbi>(&abi_str)
.ok()
.map(InternalStructs::new)
.unwrap_or_default()
};

@gakonst
Copy link
Owner

gakonst commented Jan 1, 2022

hey @thasarito - happy new year! any plans to get this over the line?

@thasarito
Copy link
Contributor Author

hey @thasarito - happy new year! any plans to get this over the line?

I'm working on it but it might take a while.

@gakonst
Copy link
Owner

gakonst commented Jan 1, 2022

word - just checking! no rush.

@thasarito
Copy link
Contributor Author

@gakonst

I checked to change the abi_str in case that it starts with {.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants