create new @agoric/time package, move timeMath.js and types #6867

warner · 2023-01-27T09:06:17Z

This creates a new package named @agoric/time, to house timerMath.js, and types for TimerBrand, Timestamp, and RelativeTime.

It also holds types for a TimerService, although the service implementation remains in SwingSet (inside vat-timer).

Clients who use time must now get their types and math functions with e.g. import { TimeMath } from '@agoric/time'.

refs #6003

But does not close it yet, because the Timestamps are not yet universally branded.

warner · 2023-01-27T09:07:24Z

@erights I deleted timed-iteration.js because it was unused, but if that was too harsh of me, say the word and I'll bring it back.

Chris-Hibbert

NEWS.md and CHANGELOG.md were copied in toto, and should be cut down.

packages/time/NEWS.md

packages/time/CHANGELOG.md

warner · 2023-01-27T19:48:51Z

I'm iterating on types, there will be some CI back and forth as I figure out what I'm doing (thanks @turadg !)

erights · 2023-01-27T23:04:15Z

@erights I deleted timed-iteration.js because it was unused, but if that was too harsh of me, say the word and I'll bring it back.

Deletion is fine. Since this is the PR deleting it, I'll note here that it is a great example of how to do a cold notifier --- by simply doing a cold async iterable. Attn @michaelfig

warner · 2023-01-28T01:50:34Z

packages/governance/test/swingsetTests/committeeBinary/bootstrap.js

@@ -22,8 +22,8 @@ const makeVoterVat = async (log, vats, zoe) => {
 /**
 *
 * @param {Pick<QuestionDetails, 'issue' | 'positions' | 'electionType'>} qDetails
- * @param {TimestampValue} closingTime
- * @param {{ electorateFacet: import('../../../src/committee.js').CommitteeElectorateCreatorFacet, installations: Record<string, Installation>, timer: TimerService }} tools
+ * @param {import('@agoric/time/src/types').Timestamp} closingTime


note: this replaces TimestampValue (which is just an integer) with the preferred TimeStamp (which is currently either an integer or a branded record, but the next step of #6003 is to require them all to be branded)

warner · 2023-01-28T01:55:00Z

Ok, ready for review. My use of types is probably pretty bad, please recommend fixes.

I'll do some squashing/rewriting of this after review, before I land it, to make the last few "fighting with CI" commits more sensible.

@erights please see if this matches your vision for time management (knowing that I'm working on the next step, to require brands on all Timestamp and RelativeTime objects).

@turadg please be critical about types.

@Chris-Hibbert please see if this matches what you need/expect on the client side.

turadg · 2023-01-28T02:00:35Z

packages/SwingSet/src/vats/timer/vat-timer.js


 // This consumes O(N) RAM only for outstanding promises, via wakeAt(),
 // delay(), and Notifiers/Iterators (for each actively-waiting
 // client). Everything else should remain in the DB.

 /**
+ * @typedef {import('@agoric/time/src/types').Timestamp} Timestamp


as discussed, this faux import copies the structure but not the documentation.

I think that's acceptable for the scope of this change.

turadg · 2023-01-28T02:02:11Z

packages/governance/src/contractGovernance/governApi.js

@@ -32,7 +32,7 @@ const makeApiInvocationPositions = (apiMethodName, methodArgs) => {
 * @param {Instance} governedInstance
 * @param {ERef<{ [methodName: string]: (...args: any) => unknown }>} governedApis
 * @param {Array<string | symbol>} governedNames names of the governed API methods
- * @param {ERef<TimerService>} timer
+ * @param {ERef<import('@agoric/time/src/types').TimerService>} timer


I think it would be worth the investment to get these on the package export:

Suggested change

* @param {ERef<import('@agoric/time/src/types').TimerService>} timer

* @param {ERef<import('@agoric/time').TimerService>} timer

But I don't think that need be a blocker. It stands out like a sore thumb so it'll get addressed.

b7987bf should do it

turadg · 2023-01-28T02:04:01Z

packages/time/src/types.d.ts

+// These aren't in the global runtime environment. They are just types that are
+// meant to be globally accessible as a side-effect of importing this module.


this was for the declare global so no longer applicable.

turadg · 2023-01-28T02:04:37Z

packages/vats/src/core/startWalletFactory.js

@@ -7,9 +7,6 @@ import { CONTRACT_ELECTORATE, ParamTypes } from '@agoric/governance';
 import { makeStorageNodeChild } from '@agoric/internal/src/lib-chainStorage.js';
 import { Stable } from '../tokens.js';

-// Ambient types (globals)


michaelfig · 2023-01-28T22:39:22Z

I deleted timed-iteration.js because it was unused,
I'll note here that it is a great example of how to do a cold notifier --- by simply doing a cold async iterable.

Noted. As per our discussions around the future of @agoric/notifier, I expect the unified API will help encourage people to write cold (pull-based) producers when possible.

This will house timerMath.js, and types for TimerBrand, Timestamp, and RelativeTime. It also holds types for a TimerService, although the service implementation is in SwingSet (inside `vat-timer`). refs #6003

This is a straight move, with no code changes, for the sake of review. Of course this means that both old and new packages won't work until the next few commits, which will fix up the necessary imports.

* zoe * governamce * inter-protocol * swingset This also removes the old type "<reference>" clause from swingset's `exported.js`, since the timer types are no longer found there.

This updates a lot of downstream packages to get their Time-related types from the right place, and adds package.json dependencies so they can import those types.

warner requested review from erights and Chris-Hibbert January 27, 2023 09:06

warner self-assigned this Jan 27, 2023

warner added the SwingSet package: SwingSet label Jan 27, 2023

Chris-Hibbert reviewed Jan 27, 2023

View reviewed changes

packages/time/NEWS.md Outdated Show resolved Hide resolved

packages/time/CHANGELOG.md Show resolved Hide resolved

warner force-pushed the 6003-time-math-package branch from 5e7f150 to 1e8e4c6 Compare January 27, 2023 22:08

warner force-pushed the 6003-time-math-package branch from 1e8e4c6 to a09b47a Compare January 28, 2023 01:21

warner commented Jan 28, 2023

View reviewed changes

warner marked this pull request as ready for review January 28, 2023 01:55

warner requested review from Chris-Hibbert and turadg January 28, 2023 01:55

turadg reviewed Jan 28, 2023

View reviewed changes

Chris-Hibbert approved these changes Jan 30, 2023

View reviewed changes

warner force-pushed the 6003-time-math-package branch 3 times, most recently from 5cb6fff to f897df4 Compare January 31, 2023 21:45

warner added 7 commits January 31, 2023 23:43

fix(swingset): delete unused timed-iteration.js

d35669a

feat: create new @agoric/time package

a61a3fb

This will house timerMath.js, and types for TimerBrand, Timestamp, and RelativeTime. It also holds types for a TimerService, although the service implementation is in SwingSet (inside `vat-timer`). refs #6003

fix: move timer files to new package

c105bde

This is a straight move, with no code changes, for the sake of review. Of course this means that both old and new packages won't work until the next few commits, which will fix up the necessary imports.

fix(time): fix the code/test to work in its new home

504d333

fix: update all clients of @agoric/time to handle the new home

5c4fb24

* zoe * governamce * inter-protocol * swingset This also removes the old type "<reference>" clause from swingset's `exported.js`, since the timer types are no longer found there.

fix: clean up types

6f53f19

fix: update types/dependencies for new @agoric/time

418545a

This updates a lot of downstream packages to get their Time-related types from the right place, and adds package.json dependencies so they can import those types.

warner force-pushed the 6003-time-math-package branch from f897df4 to 418545a Compare February 1, 2023 07:43

warner added the automerge:rebase Automatically rebase updates, then merge label Feb 1, 2023

mergify bot merged commit a7a2f9a into master Feb 1, 2023

mergify bot deleted the 6003-time-math-package branch February 1, 2023 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create new @agoric/time package, move timeMath.js and types #6867

create new @agoric/time package, move timeMath.js and types #6867

warner commented Jan 27, 2023

warner commented Jan 27, 2023

Chris-Hibbert left a comment

warner commented Jan 27, 2023

erights commented Jan 27, 2023 •

edited

Loading

warner Jan 28, 2023

warner commented Jan 28, 2023

turadg Jan 28, 2023

turadg Jan 28, 2023

turadg Jan 28, 2023

turadg Jan 28, 2023

turadg Jan 28, 2023

michaelfig commented Jan 28, 2023

	* @param {ERef<import('@agoric/time/src/types').TimerService>} timer
	* @param {ERef<import('@agoric/time').TimerService>} timer

		// These aren't in the global runtime environment. They are just types that are
		// meant to be globally accessible as a side-effect of importing this module.

create new @agoric/time package, move timeMath.js and types #6867

create new @agoric/time package, move timeMath.js and types #6867

Conversation

warner commented Jan 27, 2023

warner commented Jan 27, 2023

Chris-Hibbert left a comment

Choose a reason for hiding this comment

warner commented Jan 27, 2023

erights commented Jan 27, 2023 • edited Loading

warner Jan 28, 2023

Choose a reason for hiding this comment

warner commented Jan 28, 2023

turadg Jan 28, 2023

Choose a reason for hiding this comment

turadg Jan 28, 2023

Choose a reason for hiding this comment

turadg Jan 28, 2023

Choose a reason for hiding this comment

turadg Jan 28, 2023

Choose a reason for hiding this comment

turadg Jan 28, 2023

Choose a reason for hiding this comment

michaelfig commented Jan 28, 2023

erights commented Jan 27, 2023 •

edited

Loading