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

Use getsourcecode endpoint from etherscan api instead of anyabi+ proxy detection logic #157

Merged
merged 12 commits into from
Oct 3, 2024

Conversation

portdeveloper
Copy link
Member

@portdeveloper portdeveloper commented Sep 16, 2024

Description

This PR

-> Uses getsourcecode api, which can be found here: https://docs.etherscan.io/api-endpoints/contracts
-> Removes the use of anyabi and proxy detection logic

To-Do

  • Need to update the tests accordingly
    (We probably need to add all those env vars to github actions for the tests to work, and i don't have access to that)

Related Issues

Fixes #141

Copy link

vercel bot commented Sep 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
abi-ninja-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 6:01am

@portdeveloper portdeveloper changed the title Use getsourcecode endpoint from etherscan api instead of heimdall + proxy detection logic Use getsourcecode endpoint from etherscan api instead of anyabi+ proxy detection logic Sep 23, 2024
Copy link
Member

@Pabl0cks Pabl0cks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWESOME job @portdeveloper!!

Tested nicely with different use cases and is working nicely! And implementation contracts are working great now.

Just missing to verify one use case (custom chains). Tried using your custom chain example and is not working for me:

parex chain details:
chainId: 322202
rpc: https://mainnet-rpc.parex.network
verified contract: 0x6058518142C6AD506530F5A62dCc58050bf6fC28
block explorer: https://scan.parex.network/

I'm getting Address is not a contract, are you sure you are on the correct chain? message. Tried with 0x34f04Ed4550DC66798517C454e2ee7DF6B88f45d and i'm getting the "Decompile (beta)" / "Import ABI" options.

@portdeveloper
Copy link
Member Author

portdeveloper commented Sep 27, 2024

@Pabl0cks Thanks for the review!

Unfortunately, parex network is down and Shiv found another chain:

  it("should add Viction as a custom chain and interact with a contract by submitting an ABI manually", () => {
    cy.visit("http://localhost:3000");
    cy.selectNetwork("Add custom chain");
    cy.get("#add-custom-chain-modal").should("be.visible");
    cy.addCustomChain({
      id: "88",
      name: "Viction",
      nativeCurrencyName: "VIC",
      nativeCurrencySymbol: "VIC",
      nativeCurrencyDecimals: "18",
      rpcUrl: "https://rpc.viction.xyz",
      blockExplorer: "https://tomoscan.io/",
    });
    cy.get("#react-select-container").should("contain", "Viction");
    cy.get('input[placeholder="Contract address"]').type("0x381B31409e4D220919B2cFF012ED94d70135A59e");
    cy.fixture("viction_abi").then(victionABI => {
      cy.importABI(JSON.stringify(victionABI));
    });
    cy.url().should("include", "/0x381B31409e4D220919B2cFF012ED94d70135A59e/88");
    cy.get(".loading-spinner", { timeout: 10000 }).should("not.exist");
    cy.interactWithMethod("balanceOf", "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045");
  });

And if you use Viction, you successfully reach addressAbi tab and import an ABI manually and reach the interaction page.

@technophile-04
Copy link
Member

Ohh don't know how I missed this PR, will try it over weekend 🙌

@Pabl0cks
Copy link
Member

And if you use Viction, you successfully reach addressAbi tab and import an ABI manually and reach the interaction page.

TYSM! Approving the PR now 🙌

Copy link
Member

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tysm @portdeveloper!! Great find on getsourcode enpoint! removes a lot of burden and network request to check for proxies! Just tested it out looks good! Added a couple of small comments


Regarding GH actions tests now if someone outsider makes a PR to abi.ninja they would always fails since their fork/branch won't be able to access our variables.

A manual solution for now would be we(reviewer) run yarn cypress:run locally and see if that PR passes the test before merging. I think we had chat about it in TG and also tried digging about it technophile-04/abininja-fork@5a2a48b but it seems to doesn't work but yeah in long run it would be great if we get it solved not at all top-priority though

packages/nextjs/utils/abi.ts Outdated Show resolved Hide resolved
packages/nextjs/utils/abi.ts Outdated Show resolved Hide resolved
@technophile-04
Copy link
Member

Just pushed a couple of commits(hacky) to fix the GH action, the problem seems even after adding those variables to github secrets they were not getting picked by nextjs so added a step where we echo .env.local inside packages/nexjts not the solution I am proud off :/ but yeah I think we should solve this too while tinkering with second part of #157 (review)

@portdeveloper
Copy link
Member Author

@technophile-04 thanks for you feedback, fixed!

also about the automated tests problem, i wonder if playwright is easier to configure?

Copy link
Member

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tysm @portdeveloper !!

@technophile-04 technophile-04 merged commit b7a03b0 into main Oct 3, 2024
4 checks passed
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.

Problems with anyabi + two stepped detection
3 participants