Allow disabling limits after deployment #1492
Conversation
karlb
force-pushed
the
disable-limits
branch
2 times, most recently
from
1040d13
to
7c4e3e9
Compare
Codecov Report
@@ Coverage Diff @@
## master #1492 +/- ##
=======================================
Coverage 80.84% 80.84%
=======================================
Files 22 22
Lines 1566 1566
Branches 189 189
=======================================
Hits 1266 1266
Misses 253 253
Partials 47 47
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks good in general, nice small commits make this easy to review. After seeing the additions of changeController, I think we should add a base class Controllable (or some other name) to bundle the ownership related functionality.
Also in the last commit you should update the gas costs file.
| @@ -200,8 +200,8 @@ contract TokenNetwork is Utils { | |||
| bytes32 participant2_locksroot | |||
| ); | |||
|
|
|||
| modifier onlyDeprecationExecutor() { | |||
| require(msg.sender == deprecation_executor, "Can only be called by deprecation_executor"); | |||
| modifier onlyController() { | |||
There was a problem hiding this comment.
We could split this into a base class like done in https://docs.openzeppelin.com/contracts/4.x/api/access#Ownable
I don't think it will give any improvements in code size, but might make the code more concise.
karlb
force-pushed
the
disable-limits
branch
3 times, most recently
from
928d408
to
1002951
Compare
This is just a convenience function, but I think it will be nice to have once we start removing the limits from the code base.
This user can now also remove limits. It is renamed to make this clear.
We want the limit removal to be completely done once we do it. When new TN are deployed afterwards, they must be deployed without limits. Closes raiden-network#1416
Solidity allows to have constants outside the contract. Unfortunately, solium does seem to understand this and throws a syntax errors. Those can't be silenced, since they break parsing for the whole file.
Less duplication, better separation of concerns.
This requires turning off solium for raiden_contracts/data/source/lib/TokenNetworkUtils.sol, since it does not understand declarations outside of the contract. Also write the number as `2**256 - 1` to make it more clear that this is the right value.
It never belonged into the service dir. I just got confused.
This does not only reduce redundancy in code, but also allows to change the controller of the ServiceRegistry.
|
We need to add a changelog entry, but this can be done in a different PR. |
We want to be able to remove the limits without redeploying the contracts. We do this by setting the limits to MAX_INT.
Closes #1481.
To safely allow 3rd parties to create token networks after the limit removal, creating new TNs must not involve any parametrization. This is done by forcing the limits to be MAX_INT after the limits have been removed from the TNR.
Closes #1416.