make challenge manager immutable #498
Conversation
|
I agree around the stakeToken != 0, lets do the check and make sure a stakeToken and amount are always set. If someone wants to deploy without a stake token they can always create a noop erc20 and use that. |
| // upgrade the surrounding contracts eg bridge, outbox, seq inbox, rollup event inbox | ||
| // to set of the new rollup address | ||
| bytes32 rollupSalt = keccak256(abi.encode(config)); | ||
| address expectedRollupAddress = | ||
| Create2Upgradeable.computeAddress(rollupSalt, keccak256(type(RollupProxy).creationCode)); | ||
| upgradeSurroundingContracts(expectedRollupAddress); | ||
|
|
||
| challengeManager.initialize({ | ||
| // deploy the challenge manager | ||
| EdgeChallengeManager challengeManager = CHALLENGE_MANAGER_FACTORY.createChallengeManager({ |
There was a problem hiding this comment.
Why do we use a factory here instead of just deploying the implementation from a template like before?
There was a problem hiding this comment.
because the expectedRollupAddress presumably wouldn't be known before deploying this action contract, and so we wouldn't be able to deploy the challengeManager implementation beforehand.
There was a problem hiding this comment.
it should also deploy this behind a proxy though which it doesn't right now
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #498 +/- ##
==========================================
- Coverage 57.89% 57.86% -0.03%
==========================================
Files 39 39
Lines 6926 6926
==========================================
- Hits 4010 4008 -2
- Misses 2910 2912 +2
Partials 6 6 |
There was a problem hiding this comment.
Just dropping a note that we decided to wait until other PRs have been merged - and then do this one after. It looks like it causes us to have to drop the optimizer runs, and although we estimate that this PR is still a gas improvement we havent been able to definitively confirm that.
the callenge manager is still deployed behind a proxy
there is a new EdgeChallengeManagerFactory contract that replaces the challenge manager template in the rollup creator.
optimizer runs had to be lowered