Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] Support multicall v2 #308

Merged
merged 2 commits into from
Feb 15, 2023
Merged

[ADD] Support multicall v2 #308

merged 2 commits into from
Feb 15, 2023

Conversation

tungnguyen20
Copy link
Contributor

@tungnguyen20 tungnguyen20 commented Feb 14, 2023

The multicall v2 supported contracts can be found here

@DarthMike
Copy link
Member

@tungnguyen20 Thanks for the PR! Please address the linting issue by running [prepareForPush.sh](https://github.com/argentlabs/web3.swift/blob/develop/scripts/prepareForPush.sh) and also we'll need an integration test against a Goerli contract to be sure it works fine and it won't break with future changes

@DarthMike DarthMike self-requested a review February 14, 2023 10:41
@tungnguyen20
Copy link
Contributor Author

@DarthMike I just run the script and add test cases. Please help me continue.

@DarthMike
Copy link
Member

Some tests will fail (see #311). But we can run anyway for now

@tungnguyen20
Copy link
Contributor Author

Yeah I see :D

@DarthMike
Copy link
Member

@tungnguyen20 You can merge develop and tests should pass now

@tungnguyen20
Copy link
Contributor Author

I merged the develop branch. Please help me merge this pull request. Thank you.

@DarthMike
Copy link
Member

@tungnguyen20 strangely this PR is now failing in other tests even though there's nothing changed that would affect it.
I run CI on develop and it's passing correctly (https://github.com/argentlabs/web3.swift/actions/runs/4182648660) , trying again

@tungnguyen20
Copy link
Contributor Author

tungnguyen20 commented Feb 15, 2023

Looks like the private key is missing. Please check it.
Screen Shot 2023-02-15 at 18 19 09

@DarthMike
Copy link
Member

Hm I don't understand why, but pulled the changes manually to see if it changes will work in CI
#313

@tungnguyen20
Copy link
Contributor Author

Hmm. Don't know why. It passed 😂

@tungnguyen20
Copy link
Contributor Author

I think I will close this PR and reopen a new one which is checked out from latest develop

@DarthMike
Copy link
Member

DarthMike commented Feb 15, 2023

@tungnguyen20 you can update from latest develop, I found out why (040ad81)

@tungnguyen20
Copy link
Contributor Author

tungnguyen20 commented Feb 15, 2023

@DarthMike Yah I updated. Hope it will work 😁

@DarthMike DarthMike merged commit cf700b7 into argentlabs:develop Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants