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

Refactor ethers-contract #80

Closed
gakonst opened this issue Oct 2, 2020 · 4 comments · Fixed by #88
Closed

Refactor ethers-contract #80

gakonst opened this issue Oct 2, 2020 · 4 comments · Fixed by #88
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@gakonst
Copy link
Owner

gakonst commented Oct 2, 2020

Is your feature request related to a problem? Please describe.
Ethers-contract currently requires that it's connected to a Middleware in order to be able to broadcast the transactions.

It'd be nicer if we were able to have a base contract struct which simply takes the ABI and produces ABI encoded data for its functions (and for filters if given a vec of addresses as the filter's address). You'd then wrap that with an address and a middleware, and you'd get the equivalent functionality of the current Contract.

It'd also be nice if we supported human readable ABIs. We could do the tokenization by looking at a function's parentheses

@pawanjay176
Copy link
Contributor

Hi @gakonst, I'd like to take this up to familiarise myself with the codebase a bit :)

So from what I understand, the task is mainly to split the existing Contract struct to

pub struct BaseContract {
    abi: Abi,
    methods: HashMap<Selector, (String, usize)>,
}

pub struct Contract<M> {
    base_contract: BaseContract,
    client: Arc<M>,
    address: Address,
}

impl BaseContract {
    ....
    
    /// Should be upgradeable to a "full" `Contract` struct.
    pub fn into_full_contract<M>(self, address: Address, client: impl Into<Arc<M>>) -> Contract<M> {
        Contract {
            base_contract: self,
            address,
            client: client.into(),
        }
    }
}

Is this roughly what you have in mind?

It'd also be nice if we supported human readable ABIs.

could you expand on "human readable" a bit?

@gakonst
Copy link
Owner Author

gakonst commented Oct 12, 2020

Hey! Yes, exactly what I had in mind. Human readable ABIs, are the ones from Ethers-js https://blog.ricmoo.com/human-readable-contract-abis-in-ethers-js-141902f4d917

@gakonst gakonst added enhancement New feature or request good first issue Good for newcomers labels Oct 13, 2020
@gakonst
Copy link
Owner Author

gakonst commented Oct 19, 2020

hey @pawanjay176! were you able to take a stab at the issue? If there was anything confusing, could I help you in some way?

@pawanjay176
Copy link
Contributor

@gakonst sorry, got busy over the weekend. Have started working on it. Will make some commits in a day or two :)

meetmangukiya pushed a commit to meetmangukiya/ethers-rs that referenced this issue Mar 21, 2022
* feat: fuzz bool

* feat: implement for fixed array

* feat: implement for dynamic array

* fix(evm): do not try to trim gas costs

it seems to result in arithmetic overflows?

* feat: fuzz fixed bytes

* feat: fuzz ints

* feat: fuzz abicoder v2

* fix(solc): get line with pragma solidity not abicoder

* feat: fuzz up to 256 length arrays

* chore: rename test to `test_fuzzing`

* fix: saturating sab when removing extra gas

* chore: clippy lints
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants