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

feat(provisionPool): allow overriding PerAccountInitialAmount during contract upgrade #10594

Closed
wants to merge 2 commits into from

Conversation

Jorge-Lopes
Copy link
Collaborator

@Jorge-Lopes Jorge-Lopes commented Dec 2, 2024

closes: #10562

Description

This PR updates the provisionPool contract to allow overriding the PerAccountInitialAmount parameter, which is governed by the Economic Committee during contract upgrades.

To achieve the above functionality, this PR introduces the following changes:

  • The PerAccountInitialAmount is now injected to the contract through privateArgs instead of terms.
  • Use the makeParamManagerFromTerms method from the governance package to manage the PerAccountInitialAmount values during upgrades.
  • Updated the public and creator facets to handle the replace handleParamGovernance with makeParamManagerFromTerms.

The core-eval proposal used to upgrade provisionPool was also updated to fetch the current value of PerAccountInitialAmount set on Vstorage and include it on the new privateArgs

Security Considerations

The changes introduced in this PR modifies the provisionPool publicFacet, which no longer exposes all method declared in the publicMixinAPI methodGuard, more specifically, those declared in typedAccessors.
My analysis indicates that these methods are not utilized by any contract consuming the provisionPool publicFacet, which is why they were omitted.

However, if this analysis overlooked any instances where the removed methods are in use, their absence could introduce vulnerabilities.

Scaling Considerations

Documentation Considerations

Testing Considerations

The existing unit tests for the inter-protocol and vats packages, as well as the a3p-integration tests have been verified to continue passing with these changes.

Additionally, these changes were tested against the governance acceptance tests in the a3p-integration which ensures that governed parameter values remain intact after a contract upgrade.
To maintain the scope of this PR, that specific test is not included here. However, it will be introduced in PR #10555, which is planned to be opened once this change is merged.

Upgrade Considerations

@Jorge-Lopes Jorge-Lopes force-pushed the jorge/10562-provisionPool branch from 7dbc32b to d818720 Compare December 2, 2024 15:32
Copy link

cloudflare-workers-and-pages bot commented Dec 2, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 197aa4b
Status:🚫  Build failed.

View logs

@Jorge-Lopes Jorge-Lopes added the force:integration Force integration tests to run on PR label Dec 2, 2024
@Jorge-Lopes Jorge-Lopes changed the title Provide ability to override the ProvisionPool governedParam PerAccountInitialAmount feat(provisionPool): allow overriding PerAccountInitialAmount during contract upgrade Dec 2, 2024
@Jorge-Lopes Jorge-Lopes closed this Dec 4, 2024
@Jorge-Lopes Jorge-Lopes force-pushed the jorge/10562-provisionPool branch from d818720 to 963ebe1 Compare December 4, 2024 16:37
@Jorge-Lopes Jorge-Lopes reopened this Dec 5, 2024
@Jorge-Lopes Jorge-Lopes marked this pull request as ready for review December 5, 2024 22:26
@Jorge-Lopes Jorge-Lopes requested a review from a team as a code owner December 5, 2024 22:26
@Jorge-Lopes Jorge-Lopes requested review from AgoricTriage, iomekam and Chris-Hibbert and removed request for AgoricTriage December 5, 2024 22:26
@Jorge-Lopes
Copy link
Collaborator Author

Jorge-Lopes commented Dec 5, 2024

@Chris-Hibbert, after I had already assigned you as the reviewer, I noticed some unexpected CI failures in this PR. I'll investigate the root cause of these issues first thing tomorrow morning.

@Jorge-Lopes Jorge-Lopes force-pushed the jorge/10562-provisionPool branch from d2eba40 to 3b38951 Compare December 6, 2024 10:35
@Jorge-Lopes Jorge-Lopes force-pushed the jorge/10562-provisionPool branch from 3b38951 to 197aa4b Compare December 6, 2024 10:46
@Jorge-Lopes Jorge-Lopes closed this Dec 6, 2024
@Jorge-Lopes Jorge-Lopes deleted the jorge/10562-provisionPool branch December 6, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to override the ProvisionPool governedParam PerAccountInitialAmount
1 participant