Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contractGovernor holds contract's paramManager; on upgrade, reference is broken #9982

Closed
Chris-Hibbert opened this issue Aug 27, 2024 · 0 comments · Fixed by #10163
Closed
Assignees
Labels
bug Something isn't working Governance Governance

Comments

@Chris-Hibbert
Copy link
Contributor

Describe the bug

The otherwise reachably durable contract governor holds onto the governed contract's ephemeral paramManager. If the governed contract upgrades, the ephemeral object turns into a pumpkin.

To Reproduce

This bit the auctions and vaults coreEval in RC2, necessitating RC3.

Expected behavior

Upgrading the contract shouldn't break its governor.

Additional context

In contractGovernorKit.js, we check to see if we have a saved version of the paramManager, and use it if we do. If the contract has been upgraded, sending a message to this object will fail. We can catch that error, and call provideParamGovernance() again when that happens.

        provideParamGovernance() {
          if (!paramGovernance) {
            const { timer } = powers;
            const { creatorFacet, instance } = this.state;
            paramGovernance = setupParamGovernance(
              E(creatorFacet).getParamMgrRetriever(),
              instance,
              timer,
              () => this.facets.helper.getUpdatedPoserFacet(),
            );
          }
          return paramGovernance;
        },
@Chris-Hibbert Chris-Hibbert added bug Something isn't working Governance Governance labels Aug 27, 2024
@Chris-Hibbert Chris-Hibbert self-assigned this Aug 27, 2024
@mergify mergify bot closed this as completed in #10163 Oct 9, 2024
mergify bot added a commit that referenced this issue Oct 9, 2024
…10163)

closes: #9982
closes: #10172

## Description

When a governed contract is upgraded, its paramManager (which was ephemeral) is replaced, and the contractGovernor needs to get a fresh copy.

### Security Considerations

This bug caused us to need to cut RC2 when preparing the vaultFactory release. That time, the fix was to upgrade the contractGovernor when upgrading a governed contract.  

### Scaling Considerations

No scaling impacts.

### Testing Considerations

added a new test, which fails without the fix.

### Upgrade Considerations

This is staged on top of #10074, and the goal is to include it with the priceFeed coreEval. Recent commits have updated the coreEval to include upgrading and installing the contractGovernor code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Governance Governance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant