search

OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.75k stars 11.75k forks source link

SignatureChecker should check for contract method first. #4567

Open drortirosh opened 1 year ago

drortirosh commented 1 year ago

The SignatureChecker.isValidSignatureNow() performs 2 separate checks:

  1. if the account is an EOA, attempt to perform ecrecover()
  2. if the account is a contract, call the isValidSignature on that contract.

Currently, it performs them in the above order. While currently it is quite fine, this ordering is not "future-proof": If at any point in the future we would allow an EOA to be upgraded to a contract (e.g. via EIP-7377 ) , the old signed messages (created before it was migrated) will still be valid.

A better way to implement the SignatureChecker is first try to treat it as a contract, and only if it fails, try ecrecover and assume this is an EOA.

frangio commented 1 year ago

I mentioned this in the EIP-7377 discussion too. I mostly support this proposal but I'm hesitant to make changes now given that the future of account migration is so unclear. There is also EIP-6492 which we might want to implement.