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

chore(acceptance): upgrade pp's governor #10817

Merged
merged 4 commits into from
Jan 14, 2025
Merged

Conversation

anilhelvaci
Copy link
Collaborator

closes: #10411

Description

According to #10411 we need to showcase we can upgrade contractGovernor for a governed contract. This PR chooses provisionPool as it's example governed contract because;

  • Governance behavior is already tested at governence.test.js
  • Ease of working with an existing core eval

Security Considerations

No security considerations.

Scaling Considerations

As long as CI passes, scaling shouldn't be a problem.

Documentation Considerations

None.

Testing Considerations

CI with integration tests should pass.

Upgrade Considerations

This PR concerns upgrading provisionPool's governor itself.

@anilhelvaci anilhelvaci added the force:integration Force integration tests to run on PR label Jan 8, 2025
@anilhelvaci anilhelvaci self-assigned this Jan 8, 2025
@anilhelvaci anilhelvaci requested a review from a team as a code owner January 8, 2025 11:31
for await (const vatName of Object.keys(vats)) {
actual[vatName] = await getVatDetails(vatName);
}
t.like(actual, vats);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to assume that, at the moment that this test is executed, the incarnation of provisionPool and provisionPool-governor is always 0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it might be worth setting the incarnation values in vats from test.before, and asserting that they have increased by one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved in 5d68e48

Copy link

cloudflare-workers-and-pages bot commented Jan 12, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8c6668f
Status: ✅  Deploy successful!
Preview URL: https://c2761b5c.agoric-sdk.pages.dev
Branch Preview URL: https://anil-10411-gov-upgrade.agoric-sdk.pages.dev

View logs

Copy link
Collaborator

@Jorge-Lopes Jorge-Lopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes introduced on this PR looks good to me.
Particularly, the use of runCommitteeElectionParamChange to reduce duplicated code.

@anilhelvaci anilhelvaci added the automerge:rebase Automatically rebase updates, then merge label Jan 14, 2025
@anilhelvaci anilhelvaci force-pushed the anil/10411-gov-upgrade branch from 5d68e48 to 8c6668f Compare January 14, 2025 12:03
@mergify mergify bot merged commit ad2ef34 into master Jan 14, 2025
81 checks passed
@mergify mergify bot deleted the anil/10411-gov-upgrade branch January 14, 2025 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contract upgrade: governance contracts
3 participants