Fixes during GDA mainnet rollout #1780
Conversation
Changelog ReminderReminder to update the CHANGELOG.md for any of the modified packages in this PR.
|
…ript improvements
…ript improvements
…ng package building
| * @title used on first deployment (upgrade case) of GDA | ||
| * in order to solve the circular dependency between GDA and SuperfluidPool | ||
| */ | ||
| contract SuperfluidPoolPlaceholder is BeaconProxiable { |
There was a problem hiding this comment.
You sure we have to use it instead of just using ZERO_ADDRESS?
nix-shell$ git grep SuperfluidPoolPlaceholder
packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPoolPlaceholder.sol:contract SuperfluidPoolPlaceholder is BeaconProxiable {
packages/ethereum-contracts/ops-scripts/deploy-framework.js: "SuperfluidPoolPlaceholder",
packages/ethereum-contracts/ops-scripts/deploy-framework.js: SuperfluidPoolPlaceholder,
packages/ethereum-contracts/ops-scripts/deploy-framework.js: SuperfluidPoolPlaceholder.new,
packages/ethereum-contracts/ops-scripts/deploy-framework.js: "SuperfluidPoolPlaceholder.new"
There was a problem hiding this comment.
we need it in order to have a compatible logic:
contract SuperfluidUpgradeableBeacon is UpgradeableBeacon {
...
function upgradeTo(address newImplementation) public override onlyOwner {
...
if (BeaconProxiable(newImplementation).proxiableUUID() != BeaconProxiable(implementation()).proxiableUUID()) {
revert INCOMPATIBLE_LOGIC();
}
There was a problem hiding this comment.
interesting, checkout the SuperfluidDeployer code, it uses zero address, what's the difference?
There was a problem hiding this comment.
there were 2 reasons:
- don't waste gas for deploying a logic which is then immediately replaced
and more importantly:
- have all code paths blocked during the intermediate phase with the new agreement registered, but not yet properly set up (if
SuperfluidPool.initialize()reverts, thenGeneralDistributionAgreementV1.createPool()reverts too, thus we know for sure there's no way to use the agreement to manipulate SuperToken state as long as we're not done with the setup).
There was a problem hiding this comment.
we can remove this after upgrading all mainnets, not needed for new deployments
packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts
Outdated
Show resolved
Hide resolved
hellwolf
requested review from
kasparkallas,
0xdavinchee,
ngmachado,
vmichalik,
philipandersson and
a team
as code owners
|
Please let's keep these up to date and merge:
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #1780 +/- ##
==========================================
- Coverage 74.30% 74.26% -0.05%
==========================================
Files 109 110 +1
Lines 5884 5886 +2
Branches 216 216
==========================================
- Hits 4372 4371 -1
- Misses 1460 1463 +3
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
XKCD Comic RelifLink: https://xkcd.com/1780 |
No description provided.