Adding ISafe interface for CompatibilityFallbackHandler
#722
Conversation
Pull Request Test Coverage Report for Build 7541457163
💛 - Coveralls |
| @@ -3,7 +3,7 @@ pragma solidity >=0.7.0 <0.9.0; | |||
|
|
|||
| import {TokenCallbackHandler} from "./TokenCallbackHandler.sol"; | |||
| import {ISignatureValidator} from "../interfaces/ISignatureValidator.sol"; | |||
| import {Safe} from "../Safe.sol"; | |||
| import {ISafe} from "../interfaces/ISafe.sol"; | |||
There was a problem hiding this comment.
Could we import it as Safe? then it would not trigger any additional code changes (same was done for Enum)
There was a problem hiding this comment.
I think keeping it with an I is significant to notify the user that it is an interface and to follow a usual standard of interfaces. Also, using the same name will lead to HardhatError: HH701: There are multiple artifacts for contract "Safe", please use a fully qualified name. in tests (though it could be avoided by using the entire path to the contract I believe).
For Enum, the decision was taken to avoid the changes within the contract like Enum.Operation operation -> IEnum.Operation operation
| * @title ISafe - A multisignature wallet interface with support for confirmations using signed messages based on EIP-712. | ||
| * @author @safe-global/safe-protocol | ||
| */ | ||
| interface ISafe is IModuleManager, IOwnerManager, IFallbackManager { |
There was a problem hiding this comment.
Nice! I was going to say all functions in the interface should be marked as external, but you beat me to it! 🙌
TODO:
ISafeinterface along with other Safe dependency interfaces.ISafeinterface for Tests.codesizescript for checking the codesize of contracts.Closes #701