feat: save the outgoing EC Charter instance and kit #10749

Chris-Hibbert · 2024-12-19T21:54:10Z

refs: #10680

Description

Add the EconomicCommittee Charter to the instances and kits being saved.

Security Considerations

Don't throw away stuff you might need.

Scaling Considerations

Negligible

Documentation Considerations

None

Testing Considerations

Added a test to see that the charterID was saved.

Upgrade Considerations

If U18 and U19 get combined, this will impact the contracts being updated in U18. If we only pull some changes from U19 into U18, this should be inclduded.

cloudflare-workers-and-pages · 2024-12-19T22:15:32Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`355a2e1`
Status:	✅ Deploy successful!
Preview URL:	https://8b81bc41.agoric-sdk.pages.dev
Branch Preview URL:	https://cth-retain-charter.agoric-sdk.pages.dev

View logs

dckc

I'm not positive I understand the motivation / context; you may want more eyes.

a few suggestions to consider...

dckc · 2024-12-20T18:01:15Z

packages/inter-protocol/src/proposals/replaceElectorate.js

-  });
+
+  const label = 'econCommitteeCharter';
+  const previousCharterKit = { label, ...econCharterKit };


This reads as if any label in econCharterKit overrides the one on line 335.

Suggested change

const previousCharterKit = { label, ...econCharterKit };

const previousCharterKit = { ...econCharterKit, label };

In some ways I prefer the other order. We need to add this because the label wasn't added initially, not because we want to change it. If this proposal is re-used, there's no reason to change an existing label.

But I'll change it.

dckc · 2024-12-20T18:03:13Z

packages/inter-protocol/src/proposals/replaceElectorate.js

+ *   PromiseSpaceOf<{ retiredContractInstances: MapStore<string, Instance> }>} powers
+ *   - The resources and capabilities required to start the committee.


I wonder if/when retiredContractInstances should be added in vats/src
/core/types-ambient.d.ts

dckc · 2024-12-20T18:05:10Z

packages/inter-protocol/src/proposals/replaceElectorate.js


  trace('Starting new EC Charter Instance');

+  const terms = await harden({


Spurious await?

Suggested change

const terms = await harden({

const terms = harden({

dckc · 2024-12-20T18:07:18Z

packages/inter-protocol/src/proposals/replaceElectorate.js

+  const previousCharterKit = { label, ...econCharterKit };
+
+  const boardID = await E(board).getId(previousCharterKit.instance);
+  const identifier = `${label}-${boardID}`;


are we relying on this identifier syntax somewhere? It seems like there should be helper functions to construct and parse it. perhaps near provideRetiredInstances?

So far there's no plan for code to rely on it. People may construct scripts to make use of it (like Richard's vat killer), but the boardID has to be looked up manually at this point to know what name to make.

dckc · 2024-12-20T18:12:20Z

packages/builders/scripts/testing/recorded-retired-instances.js

+  const charterIDs = Array.from(retiredContractInstances.keys()).filter(k =>
+    k.startsWith('econCommitteeCharter'),
+  );
+  assert(charterIDs);


log it/them? I'd feel better seeing positive evidence in a ci log.

dckc · 2024-12-20T18:13:41Z

packages/builders/scripts/testing/recorded-retired-instances.js

@@ -36,6 +36,12 @@ export const testRecordedRetiredInstances = async ({
  const committeeInstance = retiredContractInstances.get(committeeIDs[0]);
  assert(await E(contractKits).get(committeeInstance));

+  const charterIDs = Array.from(retiredContractInstances.keys()).filter(k =>


I'm used to seeing...

Suggested change

const charterIDs = Array.from(retiredContractInstances.keys()).filter(k =>

const charterIDs = [...retiredContractInstances.keys()].filter(k =>

warner

yeah, looks good to me

warner · 2024-12-20T22:22:29Z

packages/inter-protocol/src/proposals/replaceElectorate.js

+    previousInstanceP,
+    contractKitsP,
+    econCharterKitP,
+    provideRetiredInstances(retiredContractInstancesP, produceRetiredInstances),


still slightly weirded out by our approach for this, but it looks sound

in case it makes you feel any better, the approach goes back to the vaults release.

agoric-sdk/packages/inter-protocol/src/proposals/econ-behaviors.js

Lines 471 to 475 in ba3b776

// Initialize the periodic collectors list if we don't have one.

periodicFeeCollectorsP.resolve(

makeScalarBigMapStore('periodicCollectors', { durable: true }),

);

const periodicCollectors = await periodicFeeCollectors;

or even further (mid 2022) using []:

agoric-sdk/packages/run-protocol/src/proposals/econ-behaviors.js

Lines 681 to 683 in b5d9869

// Initialize the periodic collectors list if we don't have one.

periodicFeeCollectorsP.resolve([]);

const periodicCollectors = await periodicFeeCollectors;

Chris-Hibbert added code-style defensive correctness patterns; readability thru consistency Governance Governance contract-upgrade force:integration Force integration tests to run on PR labels Dec 19, 2024

Chris-Hibbert self-assigned this Dec 19, 2024

Chris-Hibbert requested a review from a team as a code owner December 19, 2024 21:54

Chris-Hibbert requested a review from AgoricTriage December 19, 2024 21:54

Chris-Hibbert force-pushed the cth-retain-charter branch from 41ebc93 to 1bccc25 Compare December 19, 2024 22:15

Chris-Hibbert force-pushed the cth-retain-charter branch from 1bccc25 to 07869f8 Compare December 20, 2024 01:38

Chris-Hibbert requested review from warner and dckc December 20, 2024 17:54

dckc approved these changes Dec 20, 2024

View reviewed changes

warner approved these changes Dec 20, 2024

View reviewed changes

Chris-Hibbert added the automerge:rebase Automatically rebase updates, then merge label Dec 20, 2024

Chris-Hibbert added 2 commits December 20, 2024 22:25

feat: save the outgoing EC Charter instance and kit

c2c9be3

chore: from review: add trace, minor cleanups

355a2e1

Chris-Hibbert force-pushed the cth-retain-charter branch from 7a093c6 to 355a2e1 Compare December 20, 2024 22:25

mergify bot merged commit e5813b9 into master Dec 20, 2024
81 checks passed

mergify bot deleted the cth-retain-charter branch December 20, 2024 23:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: save the outgoing EC Charter instance and kit #10749

feat: save the outgoing EC Charter instance and kit #10749

Chris-Hibbert commented Dec 19, 2024

cloudflare-workers-and-pages bot commented Dec 19, 2024 •

edited

Loading

dckc left a comment

dckc Dec 20, 2024

Chris-Hibbert Dec 20, 2024

dckc Dec 20, 2024

dckc Dec 20, 2024

Chris-Hibbert Dec 20, 2024

dckc Dec 20, 2024

Chris-Hibbert Dec 20, 2024

dckc Dec 20, 2024

Chris-Hibbert Dec 20, 2024

dckc Dec 20, 2024

Chris-Hibbert Dec 20, 2024

warner left a comment

warner Dec 20, 2024

dckc Dec 20, 2024

	const previousCharterKit = { label, ...econCharterKit };
	const previousCharterKit = { ...econCharterKit, label };

		* PromiseSpaceOf<{ retiredContractInstances: MapStore<string, Instance> }>} powers
		* - The resources and capabilities required to start the committee.


		trace('Starting new EC Charter Instance');

		const terms = await harden({

	const charterIDs = Array.from(retiredContractInstances.keys()).filter(k =>
	const charterIDs = [...retiredContractInstances.keys()].filter(k =>

	// Initialize the periodic collectors list if we don't have one.
	periodicFeeCollectorsP.resolve(
	makeScalarBigMapStore('periodicCollectors', { durable: true }),
	);
	const periodicCollectors = await periodicFeeCollectors;

	// Initialize the periodic collectors list if we don't have one.
	periodicFeeCollectorsP.resolve([]);
	const periodicCollectors = await periodicFeeCollectors;

feat: save the outgoing EC Charter instance and kit #10749

feat: save the outgoing EC Charter instance and kit #10749

Conversation

Chris-Hibbert commented Dec 19, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Dec 19, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Dec 19, 2024 •

edited

Loading