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

rotate voting rights for EC committee #10164

Merged
merged 11 commits into from
Oct 8, 2024
Merged

rotate voting rights for EC committee #10164

merged 11 commits into from
Oct 8, 2024

Conversation

rabi-siddique
Copy link
Contributor

@rabi-siddique rabi-siddique commented Sep 27, 2024

closes: #10134
refs: #10134

Description

This PR implements the following:

  • Core eval script to replace the committee.
  • Builder script for setting up the core eval.
  • Bootstrap test to verify the functionality of the implementation.

Security Considerations

The Economic Committee plays a crucial role in managing the Inter-Protocol. The current committee doesn't easily support a smooth transition when members must retire or be replaced. An incomplete transition to a new committee could severely hamper the chain operations.

Accidentally providing access to the wrong parties (or failing to remove parties whose terms have expired) could make it hard to ensure that the right committee members have control. Other chains have been sunk or badly damaged when the stakeholders couldn't restrict some people from making changes.

Testing Considerations

Bootstrap tests have been added in this PR to run in CI for testing.

Documentation Considerations

Specific documentation concerns belong in #10138. It’s important to ensure the proposal is clearly documented to avoid confusion, especially with the old committee still being around.

Upgrade Considerations:

There is minimal risk of regression because the core-eval materials are introduced in new files. The changes introduced particularly the new builder script, can be seamlessly included in any upcoming release.

Scaling Considerations

The core eval script’s performance depends on the number of Economic Committee (EC) members. Since the committee is limited to a maximum of 12 members, the script’s complexity is treated as constant because it won’t have to handle more than 12 people.

Also, the script is executed approximately once a year, and all stakers receive a 3-day notice before it runs, making the scaling impact negligible.

@rabi-siddique rabi-siddique requested a review from a team as a code owner September 27, 2024 06:23
Copy link

cloudflare-workers-and-pages bot commented Sep 27, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: ca27d2f
Status: ✅  Deploy successful!
Preview URL: https://8bf1961b.agoric-sdk.pages.dev
Branch Preview URL: https://rs-rotate-voting-rights.agoric-sdk.pages.dev

View logs

@rabi-siddique rabi-siddique force-pushed the rs-rotate-voting-rights branch 2 times, most recently from 561806e to aa185a5 Compare September 27, 2024 09:10
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

This is fantastic progress, but this part needs to should run in ci too:

To test with a3p, I've been using this bash script. Make sure you've added the gov1 keyring to your local keyring ...

See z:acceptance in a3p-integration.

p.s. I see that an a3p test isn't in the test plan for #10134, so that's negotiable. But you evidently found it important to get confidence in this code. Perhaps it's feasible to get a similar level of confidence by adding to the bootstrap test here?

@Chris-Hibbert
Copy link
Contributor

With an issue as critical as this, I'd prefer that the top sections be more filled out. It's probably the case that the original issue should have indicated that it's not a small deal.

Security Considerations

The Economic Committee plays a crucial role in managing the Agoric chain. The current committee doesn't easily support a smooth transition when members need to retire or be replaced. An incomplete transition to a new committee could severely hamper operations of the chain.

Accidentally providing access to the wrong parties (or failing to remove parties whose terms have expired) could make it hard to ensure that the right committee members have control. Other chains have been sunk or badly damaged when the stakeholders couldn't restrict some people from making changes.

Upgrade considerations

Changing the composition of the committee requires upgrading some of the economic contracts at this point. We need to test and inspect carefully to ensure that a mistake doesn't unintentionally re-use earlier invitations, or reference the wrong version of the committee.

@dckc
Copy link
Member

dckc commented Sep 27, 2024

Changing the composition of the committee requires upgrading some of the economic contracts at this point.

#10134 is carefully scoped to only rotating voting rights, which does not require upgrading any contracts. Calling replaceElectorate() suffices. Upgrade shows up in #10137 etc.

the original issue should have indicated that it's not a small deal.

Yes... I only got as far as TBD in #10134. In #10133 I got as far as "many and subtle". :)

The Economic Committee plays a crucial role in managing the Agoric chain.

I'd say managing The Inter Protocol. I suppose inasmuch as IST is used to pay for contract execution, you could say that applies more widely, though.

addressesToRemove: [],
},
},
BOOTSTRAP_TEST: {
Copy link
Contributor

Choose a reason for hiding this comment

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

updatePriceFeeds.js in #10074 also includes values for main, and devnet. I think it's worth adding those so we can review.

#10146 shows how to add platform-specific build instructions to package.json..

t.context = await makeZoeTestContext(t);
});

test.serial('normal running of committee', async t => {
Copy link
Contributor

Choose a reason for hiding this comment

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

#10163 uses the governanceDriver in bootstrapTests. It vastly simplifies the setup and interactions in order to propose and vote on parameter changes. Calling updateVaultManagerParams() does all the work for param changes.

The driver also has an entrypoint for accepting invitations. I don't know whether you'd have to enhance it to support changing electorates.

Copy link
Member

Choose a reason for hiding this comment

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

I wondered about that driver... I sort of assumed it was limited to puppet governance, but I guess not?

Copy link
Contributor

Choose a reason for hiding this comment

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

didn't appear to be. boot/tools/drivers.ts is definitely enumerating ec members during voting.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Chris-Hibbert @dckc i used the governance driver, but i had to make a lot of updates to its code. lmk if it looks good

Copy link
Contributor

Choose a reason for hiding this comment

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

Those are fine changes. I really like the way it simplified the test.

E.get(permittedPowers.consume.reserveKit).governorCreatorFacet,
E.get(permittedPowers.consume.auctioneerKit).governorCreatorFacet,
E.get(permittedPowers.consume.vaultFactoryKit).governorCreatorFacet,
...[...psmKitMap.values()].map(psmKit => psmKit.psmGovernorCreatorFacet),
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert Sep 27, 2024

Choose a reason for hiding this comment

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

is there a reason to omit provisionPool and the price_feeds? They're all governed by the EC as well.

Unfortunately, with the priceFeeds, once the priceFeed coreEval goes out (see #10074), there will be old priceFeeds and new ones.It would be better if we could just upgrade the governors for the active contracts, but it's not obvious how best to distinguish them.

Copy link
Member

Choose a reason for hiding this comment

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

is there a reason to omit provisionPool and the price_feeds?

not really, but in the test plan for the issue here, I only called for testing one parameter of one governed contract.

Testing that they all work is explicitly in #10133 ... but you're probably right that it should be in scope here.

Copy link
Member

Choose a reason for hiding this comment

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

It would be better if we could just upgrade the governors for the active contracts, but it's not obvious how best to distinguish them.

I see little down-side in doing replaceElectorate for all of them. (again, we're not upgrading anything here... oh... but maybe we'll run into #9982. hm.)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh... but maybe we'll run into #9982. hm.)

That's an issue when the contract is upgraded without upgrading/restarting the governor. It doesn't block upgrading governors themselves. And after #10163 (which ought to be included with #10074) #9982 will be fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. good. let's replace them all.

@Chris-Hibbert
Copy link
Contributor

rabi-siddique requested review from dckc and Chris-Hibbert 13 hours ago

It doesn't appear that all comments have been addressed.

  • notify price feeds of new committee
  • There are several comments that didn't get an ack, even though some have been addressed.
  • replace-electorate-core.js has values for mainNet, but not devNet.
  • My comment "build proposals for PriceFeed coreEvals on multiple platforms. #10146 shows how to add platform-specific build instructions to package.json." got a thumbs up, but I can't tell whether something will be done here or as a separate PR.

@rabi-siddique
Copy link
Contributor Author

  • notify price feeds of new committee

Is this task about notifying price feeds of a new committee different from what was implemented in #10178? If yes, could you provide some pointers?

@rabi-siddique
Copy link
Contributor Author

rabi-siddique commented Oct 2, 2024

  • replace-electorate-core.js has values for mainNet, but not devNet.

Will be adding that after confirmation as I am not aware of addresses associated with devNet.

re: I added values for devNet, but they haven't been confirmed yet, so I included a TODO comment. The same goes for mainNet - the addresses are correct, but it's not confirmed which members will be renewed.

@rabi-siddique
Copy link
Contributor Author

rabi-siddique commented Oct 2, 2024

I'll add that in a separate PR.

rabi-siddique added a commit that referenced this pull request Oct 2, 2024
…er (#10178)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

<!-- Most PRs should close a specific Issue. All PRs should at least
reference one or more Issues. Edit and/or delete the following lines as
appropriate (note: you don't need both `refs` and `closes` for the same
one): -->


refs: #10134  #10133 

## Description
This pull request builds on #10166 and #10164. It updates the core eval
code to utilize the `governorCreatorFacet` for the governed contracts
via the `governedContractKit`. Initially, the aim was to use the
`governedContractKit` to access facets from price feed contracts. But
since it also includes facets for other governed contracts, I expanded
the code to incorporate these as well.

### Security Considerations
Same as #10166 and #10164 

### Scaling Considerations
<!-- Does this change require or encourage significant increase in
consumption of CPU cycles, RAM, on-chain storage, message exchanges, or
other scarce resources? If so, can that be prevented or mitigated? -->

### Documentation Considerations
<!-- Give our docs folks some hints about what needs to be described to
downstream users. Backwards compatibility: what happens to existing data
or deployments when this code is shipped? Do we need to instruct users
to do something to upgrade their saved data? If there is no upgrade path
possible, how bad will that be for users? -->

### Testing Considerations
Same as specified in #10164 

### Upgrade Considerations
<!-- What aspects of this PR are relevant to upgrading live production
systems, and how should they be addressed? -->
@Chris-Hibbert
Copy link
Contributor

Is this task about notifying price feeds of a new committee different from what was implemented in #10178? If yes, could you provide some pointers?

Nope, that's what I'm looking for. I can't tell where those changes were merged, but it doesn't seem to be here.

@Chris-Hibbert
Copy link
Contributor

re: I added values for devNet, but they haven't been confirmed yet, so I included a TODO comment. The same goes for mainNet - the addresses are correct, but it's not confirmed which members will be renewed.

TODOs are too easy to lose track of. I think that's a blocker for merging this PR. A checkbox in the top-comment here would be prominent enough, or else an issue.

@dckc
Copy link
Member

dckc commented Oct 2, 2024

Please take the manual testing stuff out of Testing Considerations.

const configurations = {
MAINNET: {
committeeName: 'Economic Committee',
// TODO: Update the addresses after confirmation
Copy link
Member

Choose a reason for hiding this comment

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

TODO when? before or after merging this PR? If after, please add an issue and cite it here.

Copy link
Member

Choose a reason for hiding this comment

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

#10194 is the issue to cite.

Copy link
Member

Choose a reason for hiding this comment

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

I suppose the 👍 means you agree to add the issue number to this comment, but I don't see any such change yet. I suppose I'll stand by for you to hit the re-review button.

Copy link
Member

Choose a reason for hiding this comment

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

The UNTIL form looked good. Did it go away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reinstated. Just me messing stuff up by going back and forth between the committee and charter branches :D

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

getting close!

I think I should hold off approval until we learn more about the mainnet slate of addresses.

const Usage = `agoric run replace-electorate-core.js ${keys(configurations).join(' | ')}`;
export default async (homeP, endowments) => {
const { scriptArgs } = endowments;
const config = configurations[scriptArgs?.[0]];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const config = configurations[scriptArgs?.[0]];
const variant = scriptArgs?.[0];
const config = configurations[variant];


const { writeCoreEval } = await makeHelpers(homeP, endowments);

await writeCoreEval('replace-committee', (utils, opts) =>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await writeCoreEval('replace-committee', (utils, opts) =>
await writeCoreEval(`replace-committee-${variant}`, (utils, opts) =>

Comment on lines 30 to 67
if (!highPrioritySendersManager) {
throw Error('highPrioritySendersManager is not defined');
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!highPrioritySendersManager) {
throw Error('highPrioritySendersManager is not defined');
}
highPrioritySendersManager || Fail`highPrioritySendersManager is not defined`;

If this isn't in Coding-Style, it should be.

Comment on lines 36 to 42
for (const addr of addressesToAdd) {
trace(`Adding ${addr} to High Priority Senders list`);
await E(highPrioritySendersManager).add(
HIGH_PRIORITY_SENDERS_NAMESPACE,
addr,
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (const addr of addressesToAdd) {
trace(`Adding ${addr} to High Priority Senders list`);
await E(highPrioritySendersManager).add(
HIGH_PRIORITY_SENDERS_NAMESPACE,
addr,
);
}
await Promise.all(addressesToAdd.map(addr => E(highPrioritySendersManager).add(
HIGH_PRIORITY_SENDERS_NAMESPACE,
addr,
)));

If the tracing is important, consider:

const traced = (label, x) => {
  trace(label, x);
  return x;

and then

Suggested change
for (const addr of addressesToAdd) {
trace(`Adding ${addr} to High Priority Senders list`);
await E(highPrioritySendersManager).add(
HIGH_PRIORITY_SENDERS_NAMESPACE,
addr,
);
}
await Promise.all(addressesToAdd.map(addr => E(highPrioritySendersManager).add(
HIGH_PRIORITY_SENDERS_NAMESPACE,
traced('High Priority Senders: adding', addr),
)));

E.get(permittedPowers.consume.reserveKit).governorCreatorFacet,
E.get(permittedPowers.consume.auctioneerKit).governorCreatorFacet,
E.get(permittedPowers.consume.vaultFactoryKit).governorCreatorFacet,
...[...psmKitMap.values()].map(psmKit => psmKit.psmGovernorCreatorFacet),
Copy link
Member

Choose a reason for hiding this comment

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

Ah. good. let's replace them all.

Comment on lines 162 to 164
const psmKitMap = await permittedPowers.consume.psmKit;

const creatorFacets = [
Copy link
Member

Choose a reason for hiding this comment

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

can we get all these from governedContractKits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this in #10178 and after Hibbert's approval merged it in #10166
I didn't merge here because #10166 is based on this PR's branch and would've gotten potential conflicts.

Copy link
Member

Choose a reason for hiding this comment

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

If I were you, I'd push stuff like governedContractKits that's in scope for this PR down into this PR so we can land this one.

I know I recommended stacking #10166 on this, but I was thinking that #10166 would remain in Draft mode until this one is landed.

Merging #10166 into this one is an option, but it would expand the scope considerably. I wouldn't be comfortable approving until I had looked the whole combined thing over.

Copy link
Contributor Author

@rabi-siddique rabi-siddique Oct 3, 2024

Choose a reason for hiding this comment

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

Let me merge #10178 in this PR.
Let me bring relevant changes from #10178 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Made this commit:
5785221

) => ({
manifest: {
[replaceElectorate.name]: {
consume: {
Copy link
Member

Choose a reason for hiding this comment

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

In lists such as this one, I'd like to see the closely held powers listed first, then a comment saying "the rest of these are designed to be widely shared."

The widely shared ones are board and zoe. Hm. short list. Maybe not worthwhile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I would appreciate it a lot if you could share some helpful links where I could expand on my understanding of widely and closely held. Is there some list that widely shared powers and closely shared powers segregated?

Copy link
Member

Choose a reason for hiding this comment

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

I would appreciate it a lot if you could share some helpful links ...

Silly me. I know better than to use jargon like that without providing such links... we lose a round-trip when you have to ask.

In this case, I can't think of what to point you at. @Chris-Hibbert ? Help?

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 207 to 314
reserveKit: true,
auctioneerKit: true,
vaultFactoryKit: true,
psmKit: true,
Copy link
Member

Choose a reason for hiding this comment

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

does governedContractKits subsume all 4 of these for our purposes here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. Made the update.

@dckc
Copy link
Member

dckc commented Oct 3, 2024

I'll add that in a separate PR.

hm. then this PR does not close #10134?

p.s. no... the "separate PR" is about a3p-integration, which is not needed until we're doing contract upgrade. no contract upgrade in #10134.

@dckc
Copy link
Member

dckc commented Oct 3, 2024

I took out "closes #10194". That can't happen until some social governance processes complete.

@Chris-Hibbert
Copy link
Contributor

My comment "#10146 shows how to add platform-specific build instructions to package.json." got a thumbs up, but I can't tell whether something will be done here or as a separate PR.

I'll add that in a separate PR.

hm. then this PR does not close #10134?

p.s. no... the "separate PR" is about a3p-integration, which is not needed until we're doing contract upgrade. no contract upgrade in #10134.

Not for a3p-integration, but for release.

My original "how to add platform-specific build instructions to package.json" was about adding something to the package.json to add a command that builds versions of the proposal for different platforms, when the proposal builder has platform-dependent parameters to account for oracle addresses, or variations in which brands are present on each platform.

I don't expect most proposals to require that dependency, but when it's present, we should add the appropriate commands as part of getting ready for release. It doesn't have to be a blocker for preparing the a3p-integration tests, but that's when the variation in the proposal builder is most visible, so it makes sense to either do it then or add an issue then that can be resolved later.

@dckc
Copy link
Member

dckc commented Oct 3, 2024

...

My original "how to add platform-specific build instructions to package.json" was about adding something to the package.json to add a command that builds versions of the proposal for different platforms,

I suppose I'm mentally tracking that sort of thing as #10138 .

But I've also been saying that #10138 is zero effort on top of features such as this one. Not quite true.

I'm still content to leave it out of scope of #10134 (and hence this PR).

@dckc dckc added the Governance Governance label Oct 7, 2024
@dckc
Copy link
Member

dckc commented Oct 7, 2024

Where did Upgrade Considerations go?

Comparing against the PR template, we have:

  • Security Considerations
  • Scaling Considerations
  • Documentation Considerations
  • Testing Considerations
  • Upgrade Considerations

I suppose Scaling Considerations are unremarkable. The core eval script is probably O(n) in the number of EC members; given a cap of 12, that makes it more like O(1). And the code runs about once a year, with 3 day notice to all stakers.

I suppose Documentation Considerations are mostly a concern for the proposer, so they mostly belong in #10138.

We should note somewhere that the old committee is still hanging around and care should be taken to avoid confusion. I wonder where.

Upgrade Considerations are also unremarkable. The core-eval materials are in new files, so there's ~no risk of regression in existing materials. The new builder script can be included in any upcoming release.

So nothing critical is missing. But in the future, please don't omit these sections. Be explicit that you thought about it and found nothing remarkable.

IIUC, the PR description goes into the git history as the description of the merge commit. So I'm reviewing it too.

@dckc
Copy link
Member

dckc commented Oct 7, 2024

The test failure in ci is a known flake. I'm re-running the job.

not ok 250 - virtual-objects › virtualObjectGC › remotable refcount management

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

Yay! Looks good.

I have a few suggestions for you to consider. No blockers.

The most straightforward one is probably the extra - that's messing up the JSDocs for types.


const { writeCoreEval } = await makeHelpers(homeP, endowments);

await writeCoreEval(`replace-committee-${variant}`, (utils, opts) =>
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any replace-committee-* files where I would expect them:

$ cd ~/projects/agoric-sdk/a3p-integration
$ corepack enable
$ yarn
$ yarn build:submissions
$ find . -name 'replace-committee*'

I suppose this is related to Chris's comment.

And I suppose #10138 ensures that we will get to it eventually. So I supposed it's not a blocker for this PR.

Copy link
Contributor Author

@rabi-siddique rabi-siddique Oct 8, 2024

Choose a reason for hiding this comment

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

I suppose this is related to #10164 (comment).

And I suppose #10138 ensures that we will get to it eventually. So I supposed it's not a blocker for this PR.

Yes

@@ -0,0 +1,326 @@
// @ts-check
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding a /** @file ... */ comment with a line or two of explanation and a reference to replace-electorate-core.js.

@@ -0,0 +1,99 @@
/* global process */
Copy link
Member

Choose a reason for hiding this comment

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

How about a /** @file ... */ comment including usage?

By way of example: init-orca.js

* Starts a new Economic Committee (EC) by creating an instance with the
* provided committee specifications.
*
* - @param {EconomyBootstrapPowers} powers - The resources and capabilities
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* - @param {EconomyBootstrapPowers} powers - The resources and capabilities
* @param {EconomyBootstrapPowers} powers - The resources and capabilities

@frazarshad frazarshad force-pushed the rs-rotate-voting-rights branch from cafe7fd to ca27d2f Compare October 8, 2024 17:01
@rabi-siddique rabi-siddique added the automerge:rebase Automatically rebase updates, then merge label Oct 8, 2024
@mergify mergify bot merged commit fdb7004 into master Oct 8, 2024
91 checks passed
@mergify mergify bot deleted the rs-rotate-voting-rights branch October 8, 2024 17:39
@Chris-Hibbert
Copy link
Contributor

@rabi-siddique, you showed that some PriceFeeds were present in governedContractKits, but I asked

On which platform? Can we tell whether that's also true (for all priceFeeds) on mainNet?

I'm guessing that was in a3p-integration. Do we know how those were set, and whether a similar process took place on MainNet?

@rabi-siddique
Copy link
Contributor Author

@rabi-siddique, you showed that some PriceFeeds were present in governedContractKits, but I asked

On which platform? Can we tell whether that's also true (for all priceFeeds) on mainNet?

I'm guessing that was in a3p-integration. Do we know how those were set, and whether a similar process took place on MainNet?

@Chris-Hibbert Yes, it's a3p-integration. I am not sure how it was set but I will look into.

mergify bot added a commit that referenced this pull request Oct 10, 2024
closes: #10133
refs: #10133

## Description
The PR builds on #10164, introducing code changes to initiate a new charter committee instance and send invitations to the members.

### Security Considerations
The EC Charter is important for deciding who can suggest changes to governed contract parameters. If the wrong people are given access or if expired members aren't removed, it can lead to security risks by letting unauthorized individuals propose changes. 

Since we're starting a new contract, old charter members can still propose questions, even though they can't vote. This still creates a potential security issue, as people who are no longer active can still influence the process. These people might fill the system with too many unnecessary or harmful proposals, which can make it difficult to focus on the important ones and slow down the decision-making process.

### Scaling Considerations


### Documentation Considerations


### Testing Considerations
Same as specified in #10164 

### Upgrade Considerations
mergify bot added a commit that referenced this pull request Oct 24, 2024
closes: #10274

## Description

Half the EC were able to pass a motion, when a majority should have been required.

### Security Considerations

Ordinary vote counting issue.

### Scaling Considerations

N/A

### Documentation Considerations

None

### Testing Considerations

Added tests for committee.

### Upgrade Considerations

The change is in the committee code. This needs to be merged before the proposal in #10164 runs. It [already installs new committee code](https://github.com/Agoric/agoric-sdk/blob/4a33accaeeba27044ab07dd04f64226de1b77759/packages/builders/scripts/inter-protocol/replace-electorate-core.js#L28).
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 Governance Governance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rotate voting rights (revoke, grant)
4 participants