NomicFoundation / hardhat

Hardhat is a development environment to compile, deploy, test, and debug your Ethereum software.
https://hardhat.org
Other
7.18k stars 1.38k forks source link

Add types for ast and compiler output #2901

Open GrandSchtroumpf opened 2 years ago

GrandSchtroumpf commented 2 years ago

Hello, I've created types for the solidity compiler output (included ABI, Yul & Solidity AST) here. I think it would be more efficient to have these types on hardhat. Would you consider a PR adding them to the current CompilerOutput. Best

github-actions[bot] commented 2 years ago

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: edf9eb83-f2e0-4742-b10c-6df9f11c307d

fvictorio commented 2 years ago

Hey @GrandSchtroumpf, thanks for opening this.

We were actually considering using https://github.com/OpenZeppelin/solidity-ast for this. Are you familiar with that project? Can you tell us how/if yours is different?

GrandSchtroumpf commented 2 years ago

Hello @fvictorio, I far as I understand, Openzeppelin focuses on Solidity & Yul AST. My link above focuses on the solc input/output, including the ABI (which is often type as any[] in many libs ^^).

The Yul & Solidity AST is also in my types, but the one from Openzepplin might be better because generated by scripts. I guess a mix of both would work well.

frangio commented 2 years ago

@GrandSchtroumpf solidity-ast is definitely focused on the AST types. Another difference is that the approach is to provide types that are valid across multiple Solidity versions. So for example newer fields will be optional fields so the type still applies for older versions of the AST. I think the types that you're sharing may be focused on being accurate for a specific Solidity version. A project like Hardhat deals with multiple versions of the compiler so I feel that the approach of solidity-ast is better suited for this purpose, and this is the case as well for the projects I develop.

I feel very confident in these types because another aspect of the project is that they are extensively tested against the actual compiler output.

We have some minimal types for the input and output but there is a need for much more than that and I can see that your types are very thorough in that regard.

GrandSchtroumpf commented 2 years ago

I started writing these types because most of the lib are using any[] for the ABI for the last couple of years. My types are not the long term solution, but maybe it can improve the current state on a short term. The optional fields should cover the different versions for now, until someone put some effort to write a script to generate the types dynamically. Or even better, before the solidity team generate them. Honestly if hardhat only implement the types for the ABI I would be very happy already 😀

fvictorio commented 2 years ago

Thanks for the info guys. We definitely need to improve our internal types around this, so we'll use this issue to track that effort. It seems to me that using both libraries somehow could be a good idea.