From 13dfec1fae8773b22fdee81d17bbf6dc02e9f2c4 Mon Sep 17 00:00:00 2001 From: T-Hax <> Date: Tue, 13 Jun 2023 19:11:23 +0000 Subject: [PATCH] add instance registry to check and spec gas limits Signed-off-by: T-Hax <> --- contracts/instances/ERC20TornadoCloneable.sol | 51 +++++++++++++++---- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/contracts/instances/ERC20TornadoCloneable.sol b/contracts/instances/ERC20TornadoCloneable.sol index 26e444f..d972d1e 100644 --- a/contracts/instances/ERC20TornadoCloneable.sol +++ b/contracts/instances/ERC20TornadoCloneable.sol @@ -11,12 +11,18 @@ import "../core/ERC20Tornado.sol"; interface ITornadoRouter { function relayerRegistry() external view returns (address); + + function instanceRegistry() external view returns (address); } interface IRelayerRegistry { function burn(address, address, address) external; } +interface IInstanceRegistry { + function instanceData(address) external view returns (address, uint80, bool, bool); +} + /* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ CONTRACT ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ /** @@ -94,19 +100,39 @@ contract ERC20TornadoCloneable is ERC20Tornado { } /** - * @notice This function is a permissionless function which, if the registry is dead, immediately downgrades - the address of the registry to address(0), which makes the instance function as any of the older + * @notice This function is a permissionless function which, if the infra is dead, immediately downgrades + the address of the router to address(0), which makes the instance function as any of the older instances. - * @dev We will disable the current registry and call this function once we deploy a new registry - infrastructure system which will require the relayers to have a wallet-like smart contract, because then + * @dev We will disable the current infra and call this function once we deploy a new infrastructure + system which will require the relayers to have a wallet-like smart contract, because then frontends will be able to build proofs for it, and as such there will be no need for this bullshit. */ - function checkRelayerRegistryIsDead() external { - require(router != address(0), "relayer registry already dead"); + function checkInfrastructureIsDead() external { + require(router != address(0), "infrastructure already dead"); + try - IRelayerRegistry(ITornadoRouter(router).relayerRegistry()).burn(address(this), address(0), address(0)) + // Amount of gas forwarded specified so a potential hijacker can't break the system + // by allowing this to not revert, but making it enough gas so router reverts + IRelayerRegistry(ITornadoRouter(router).relayerRegistry()).burn{ gas: 100_000 }( + msg.sender, // Such that it can't be hardcoded for which person + address(0), + address(this) // This will get passed in the withdraw function + ) { - require(false, "registry not dead yet"); + /* Do nothing since registry is ok */ + } catch { + router = address(0); + return; + } + + try + // Amount of gas forwarded specified so a potential hijacker can't break the system + // by allowing this to not revert, but making it enough gas so router reverts + IInstanceRegistry(ITornadoRouter(router).instanceRegistry()).instanceData{ gas: 3_000 }(address(this)) + returns (address _token, uint80, bool _isERC20, bool _isEnabled) { + if (IERC20(_token) != token || !_isERC20 || !_isEnabled) { + router = address(0); + } } catch { router = address(0); } @@ -121,8 +147,8 @@ contract ERC20TornadoCloneable is ERC20Tornado { entire intention of the system was to make it economically (similarly to proof of stake) sybil resistant, but the system has the issue that it can be avoided. So, this is a temporary fix for this until we don't make a full - system upgrade after which will we disable this trash by disabling the registry - and calling `checkRelayerRegistryIsDead()`. + system upgrade after which will we disable this trash by disabling the infra + and calling `checkInfrastructureIsDead()`. * @param _recipient The recipient address of the withdraw. * @param _relayer The relayer address of the withdraw. Must be a registered relayer otherwise router reverts. @@ -135,7 +161,10 @@ contract ERC20TornadoCloneable is ERC20Tornado { uint256 _fee, uint256 _refund ) internal override { - if (router != address(0)) require(msg.sender == router, "if registry not dead, router must be caller"); + if (router != address(0)) { + require(msg.sender == router, "if infrastructure not dead, router must be caller"); + } + super._processWithdraw(_recipient, _relayer, _fee, _refund); } }