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 KingOfTheHill contract with struct, error and event #514

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

hasparus
Copy link
Contributor

I've added a new contract that's quite short but it has a little bit of everything. We can use it to test TypeChain's behaviour on Solidity structs and errors.

Part of #474 I guess? Treat this PR as a point to discuss instead of a change I wanna make to the code.

Idea: We already emit some runtime for factories. Should we use the information about errors from the ABI to emit type guards for errors?

The following ABI

  {
    "inputs": [
      { "internalType": "uint256", "name": "value", "type": "uint256" },
      {
        "internalType": "uint256",
        "name": "highestBidValue",
        "type": "uint256"
      }
    ],
    "name": "BidNotHighEnough",
    "type": "error"
  }

could become something like

interface BidNotHighEnough extends Error { /* ... */ }
function isBidNotHighEnough(error: unknown): error is BidNotHighEnough { /* ... */ }

try {
  await contract.bid({ value: 1 })
} catch (err) {
  if (isBidNotHighEnough(err)) {
    console.log(err.highestBidValue)
  }
}

(As a side note, we'd need to fiddle with "ethers" dependency a bit, because we currently encounter ethers-io/ethers.js#1493)

@changeset-bot
Copy link

changeset-bot bot commented Oct 12, 2021

⚠️ No Changeset found

Latest commit: 9a2dbe7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hasparus hasparus requested a review from krzkaczor October 12, 2021 15:17
@krzkaczor
Copy link
Member

krzkaczor commented Oct 12, 2021

Idea: We already emit some runtime for factories. Should we use the information about errors from the ABI to emit type guards for errors?

Type guards (like in your example) would be dope as fuck but TBH i have no idea how ethers support this. Do they generate these guards (predicates) already and we will only generate types?

For now, it's good to know that typechain doesn't just bail when encountered "error" abi piece. I think we could merge it as it is and create low priority issue to add error support sometime in future.

@hasparus
Copy link
Contributor Author

For now, it's good to know that typechain doesn't just bail when encountered "error" abi piece. I think we could merge it as it is and create low priority issue to add error support sometime in future.

Shall we merge it then?

@krzkaczor
Copy link
Member

@hasparus yes go ahead.

@hasparus hasparus merged commit 5b4916e into dethcrypto:master Oct 13, 2021
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