brinktrade / brink-core

Core smart contracts for Brink accounts
GNU General Public License v3.0
13 stars 7 forks source link

Audit Issue 9: Change calculation to constructor #19

Closed Kyrrui closed 3 years ago

Kyrrui commented 3 years ago

WARNING: Unit tests fail this change - needs to be looked into

mikec commented 3 years ago

@Kyrrui it's failing because address(this) in the constructor is the address of the canonical deployment of Account.sol. When it's hashed in _recoverSigner, address(this) is the address of the proxy account contract. It needs to be the address of the proxy account contract for the signer recovery to work properly.

We could possibly set the domain hash when the proxy is deployed, but I'm not sure the overall gas savings for doing this are that significant.