feat: updated Solidity interface policy #139
Conversation
Introduces a new proposal to update the Solidity interface policy such that source contracts do not use interfaces at all and interfaces live in an entirely separate folder. Proposal is intended to set us up to fully automate interface generation while still maintaining most of the compilation benefits.
smartcontracts
force-pushed
the
sc/solidity-interfaces
branch
from
38a42ba
to
3aaabd6
Compare
|
|
||
| Proposed solution would increase compilation time for source contracts, but the impact would not be | ||
| nearly as significant as the original compilation time when no interfaces existed at all. We're | ||
| talking about something on the order of 5-20s instead of the current 2-10s. Not ideal but fine. |
There was a problem hiding this comment.
How do we know this? I would rather deal with some manual iface hassles than suffer a major slow down.
There was a problem hiding this comment.
I can run a script to show the exact compile time
| ### Continued Automation Challenges | ||
|
|
||
| Simply removing interface usage will solve the problems that come with using interfaces within the | ||
| source contracts but will not solve all of the challenges to automating interface generation. We do |
There was a problem hiding this comment.
Why is iface generation hard? Can't most of it be done using cast interface <(forge inspect Contract abi)?
There was a problem hiding this comment.
cast interface gets the types all wrong, it can't be used out of the box. Same with forge inspect.
You can use it but you're losing some type safety. For example, anything that takes a specific contract type as input is just an address according to foundry.
Supporting interface generation that's actually good is harder.
| - Contract interfaces don't have source code, so navigating the functions in the source files | ||
| becomes awkward as each external call goes through an interface. |
There was a problem hiding this comment.
Are you referring to functionality like "Go to definition" here, which would take you to the uninformative interface definition?
| - Using interfaces within the source contracts leads to other awkward situations with function | ||
| types that are challenging to resolve in an automated generation script. |
There was a problem hiding this comment.
This is the primary source of pain I have experienced myself.
| crafting contract interfaces. For instance, if `ContractA` depends on `IContractB` and | ||
| `ContractB` depends on `IContractA` but neither contract interface exists yet (new contracts) | ||
| then the contracts cannot be compiled (because neither interface exists) and the interfaces can | ||
| not be generated. Catch 22. |
There was a problem hiding this comment.
IMO the two items above can be fairly easily resolved during development just by manually defining the interface being used in the same file as the contract using it, and when the interface is stable it can be autogenerated into /interfaces.
My concern is that creating a new contract is a rare occurrence vs. compiling the source contracts, maybe something on the order of 1:1000 in terms of compilation cycles, thus even a fairly small slow down in the compilation process requires a strong justification.
There was a problem hiding this comment.
Yeah fair, makes total sense!
|
Copying over some counter-arguments from discord: circular dependency: it is indeed a pain. But it seems to be an edge case that can be solved by writing an empty implementation first, or manually crafting a temporary interface. security: it is a bit misleading to be pointed to a specific implementation rather than just seeing an external call that uses an interface, which forces you to double check "what the hell is being called on here" bonus in-favor argument: |
| locations that interact with the source contracts. Removing interfaces from source contracts | ||
| entirely would also be a much clearer mental model ("just don't use interfaces in src/"). | ||
|
|
||
| In addition, interfaces would live in a new `interfaces` folder at the root of the contracts |
There was a problem hiding this comment.
really like this, since my biggest issue with interface generation right now is I feel it generates way too many files and clutters up src/, and it makes it unclear when you are depending on machine generated interfaces vs beautifully handwritten interfaces
|
Alright it sounds like the latest consensus is to allow usage of interfaces within |
Introduces a new proposal to update the Solidity interface policy such that source contracts do not use interfaces at all and interfaces live in an entirely separate folder. Proposal is intended to set us up to fully automate interface generation while still maintaining most of the compilation benefits.