Deprecation switch for Bug Bounty release #275
Conversation
loredanacirstea
force-pushed
the
deprecation-switch
branch
6 times, most recently
from
cf0a840 to
82fc5b0
Compare
loredanacirstea
force-pushed
the
deprecation-switch
branch
from
82fc5b0 to
7506e55
Compare
|
reviewing |
loredanacirstea
force-pushed
the
deprecation-switch
branch
from
7506e55 to
9b1e82c
Compare
| administrator = _administrator; | ||
| } | ||
|
|
||
| function setSafetyDeprecationSwitch(bool status) onlyAdmin public { |
There was a problem hiding this comment.
This looks more like a toggleSafetyDeprecationSwitch() function. Seems that it can toggle between true and false values for the switch. If we keep with this behaviour use toggle in the name.
But also from what I understood from our conversation we want this to be set only once and to not be settable again. Once it's deprecated that's a final action which should not be undoable.
Let's discuss this also offline.
| } | ||
|
|
||
| /// @notice Deploy a new TokenNetwork contract for the Token deployed at | ||
| /// `_token_address`. | ||
| /// @param _token_address Ethereum address of an already deployed token, to | ||
| /// be used in the new TokenNetwork contract. | ||
| function createERC20TokenNetwork(address _token_address) | ||
| onlyAdmin |
There was a problem hiding this comment.
I had a different idea about how this should be done.
The idea would be to allow only one token network to exist instead of allowing only the administrator to deploy one.
The end result would indeed be the same but would at least agree with our limitations and give less power to the administrator.
| }) | ||
| assert token_network.functions.safety_deprecation_switch().call() is True | ||
|
|
||
| # Now we cannot deposit in existent channels |
There was a problem hiding this comment.
nitpick: /s/existent/existing both here and in the rest of the tests below
loredanacirstea
force-pushed
the
deprecation-switch
branch
from
9b1e82c to
84662ab
Compare
This is intended as a temporary safety measure for the Bug Bounty release: - added `deprecation_executor` role to `TokenNetworkRegistry` - only the `deprecation_executor` can create new `TokenNetwork` contracts - the `deprecation_executor` is inherited by the `TokenNetwork` contract - the `deprecation_executor` can switch off the `TokenNetwork`, stopping the creation of new channels and stopping new channel deposits - added tests - setting the deprecation switch + edge cases - making sure channels cannot be opened and deposits cannot be made after the deprecation switch is set to `True`
loredanacirstea
force-pushed
the
deprecation-switch
branch
from
84662ab to
71218d7
Compare
loredanacirstea
force-pushed
the
deprecation-switch
branch
from
7f8d8cf to
25dd104
Compare
| @@ -16,11 +16,20 @@ contract TokenNetworkRegistry is Utils { | |||
| uint256 public settlement_timeout_min; | |||
| uint256 public settlement_timeout_max; | |||
|
|
|||
| // Only for the limited Bug Bounty release | |||
| address public deprecation_executor; | |||
| bool public bug_bounty_token_network_created = false; | |||
There was a problem hiding this comment.
please change to token_network_created or if you want to include the release name use red_eyes.
| @@ -16,11 +16,20 @@ contract TokenNetworkRegistry is Utils { | |||
| uint256 public settlement_timeout_min; | |||
| uint256 public settlement_timeout_max; | |||
|
|
|||
| // Only for the limited Bug Bounty release | |||
There was a problem hiding this comment.
Add red eyes here to make it clear for which release it is.
fixes #263
To be merged after #270