Match notifier semantics to async iterables

[erights]

This new semantics and representation derive from conversations with @dtribble and @Chris-Hibbert . It is also influenced by @kriskowal 's GTOR -- General Theory of Reactivity. It should be a better basis for reconstructing #962 soundly, and for eventually forming working remote presences of notifiers and async iterables.

There is a key difference on starting, and another key difference on terminating.

On starting, if no initialState argument is provided, then the notifier starts with no initial state. It's initial state will be set by the first update. I also removed the getCurrentState synchronous access, as that conflicts with a delayed initial state. No one could explain why it was added.

On ending, two kinds of termination are supported, successfully with a last state value, and erroneously with a rejection reason.

With both of these changes, we can adapt both ways between notifiers and async iterators, while losing only the fidelity that is the point of notifiers --- the dropping of less recent states.

[michaelfig]

I also removed the getCurrentState synchronous access, as that conflicts with a delayed initial state. No one could explain why it was added.

It was added because the getStateSince function used to have a synchronous mode that I removed for consistency but I wasn't sure if it was still needed.

Good riddance.

[erights]

Ready for review

[michaelfig]

TypeError: updater.fulfill is not a function

Test failed in cosmic-swingset.

[erights]

TypeError: updater.fulfill is not a function

Test failed in cosmic-swingset.

This would seem to be a very deterministic error. Why would it work for me under yarn test but fail under CI? FWIW, updater.fulfill is the one new method I added.

[michaelfig]

TypeError: updater.fulfill is not a function

Test failed in cosmic-swingset.

This would seem to be a very deterministic error. Why would it work for me under yarn test but fail under CI? FWIW, updater.fulfill is the one new method I added.

Usually because updater is a presence and we forgot to add E().

[michaelfig]

This would seem to be a very deterministic error. Why would it work for me under yarn test but fail under CI? FWIW, updater.fulfill is the one new method I added.

Did you run yarn test in cosmic-swingset as well? I don't see any other reason it wouldn't fail for you too.

[erights]

This would seem to be a very deterministic error. Why would it work for me under yarn test but fail under CI? FWIW, updater.fulfill is the one new method I added.

Did you run yarn test in cosmic-swingset as well? I don't see any other reason it wouldn't fail for you too.

I did. It passed.

[michaelfig]

I did. It passed.

So, now zoe has a necessary build step basically every time you change branches. You need to (cd packages/zoe && yarn build), then your cosmic-swingset test will fail in the same way.

The error is on

agoric-sdk/packages/zoe/src/contractFacet.js

Line 373 in 05eaa73

resolve: () => {},

Rename resolve: to fulfill:, redo yarn build in Zoe, and the problem goes away!

The next thing to do is to create a compatible branch of dapp-encouragement and use @katelynsills trick of being able to specify on the current PR which branch of the dapp to use for integration tests.

[michaelfig]

LGTM, modulo the single contractFacet change I mentioned.

[erights]

The next thing to do is to create a compatible branch of dapp-encouragement and use @katelynsills trick of being able to specify on the current PR which branch of the dapp to use for integration tests.

I made this change temporarily legacy supporting enough, such that the current dapp-encouragement works under CI as is. Once all clients are upgraded to the new API, I would then like to remove the transient legacy support in notifier.js.

@katelynsills what is this trick? What other notifier clients would we need to upgrade before I can remove the legacy support in notifier.js? Since removing this support is a compat break, should I bump some version number at that time?

[michaelfig]

I made this change temporarily legacy supporting enough, such that the current dapp-encouragement works under CI as is. Once all clients are upgraded to the new API, I would then like to remove the transient legacy support in notifier.js.

Great plan. I always appreciate compatibility with a deprecation plan.

@katelynsills what is this trick?

It's adding # dapp-encouragement-branch: XXX to the description text of your PR to tell the CI to build against branch XXX of the Agoric/dapp-encouragement repository.

What other notifier clients would we need to upgrade before I can remove the legacy support in notifier.js?

Many of the dapps (search for dapp-* repositories in the Agoric organization) use notifier support. They all need to be upgraded.

Since removing this support is a compat break, should I bump some version number at that time?

The version number will be automatically bumped upon next release if you add an exclamation point before the colon of your Conventional Commit message, like fix!: ... or feat!: ... or fix(notifier)!: remove legacy support.