-
Notifications
You must be signed in to change notification settings - Fork 7
I terminus #53
I terminus #53
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terminus interface has changed in the most recent releases. This needs to be updated.
Given that Terminus is still in flux, I would rather do this kind of work using codegen than manually maintaining ITerminus.sol
. @peersky : Do you want to build the code generator?
It would also be nice to have a tool which could test an interface against an actual ABI to make sure that the interface is complete (or as complete as it needs to be).
import "@openzeppelin-contracts/contracts/token/ERC1155/extensions/IERC1155MetadataURI.sol"; | ||
|
||
interface IERC1155WithTerminusStorage is IERC1155, IERC1155MetadataURI { | ||
function isApprovedForPool(uint256 poolID, address operator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this interface. Might as well roll this into ITerminus
.
pragma solidity ^0.8.0; | ||
import "./IERC1155WithTerminusStorage.sol"; | ||
|
||
interface ITerminus is IERC1155WithTerminusStorage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather this was codegen'd instead of created manually.
Tests are passing on my machine. |
@zomglings there already is hh plugin that does such generation: https://github.com/dmihal/hardhat-interface-generator if we plan to migrate to hardhat than we can use it, otherwise script is quite simple to reapply to our realities Nevertheless, in current PR there are points where source code has to be modified anyway so purely generating interface file is not enough. |
Stale. |
Changes
How to test these changes?
Related issues
#13