Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

How many uses of makeNotifierKit should use makeSubscriptionKit instead? #3784

Open erights opened 3 years ago

erights commented 3 years ago

Outside of the notifier package, and test and demo directories, it looks like we have 34 calls to makeNotifierKit in agoric-sdk. With those same filters, we currently have 1 call to makeSubscriptionKit. This may or may not be a good split --- it depends on the semantics of the notifications being pushed. Fortunately, 35 is small enough to inspect. From

I suspect that some of these notifier uses really should be subscription uses. The example which made me aware of this is represented well by #3783 . The code it corrects happened to work with a notifier by accident. #3783 demonstrates how painless it is to convert code from using a notifier to using a subscription. Please take a look.

zarutian commented 3 years ago

Defenitely Notifiers on ERTP purses shoulde Subscriptions instead, specially on Zoe fee purses of running contract vats. The latter for fuel gauge alarm reasons alone.

erights commented 3 years ago

https://github.com/Agoric/agoric-sdk/issues/3756#issuecomment-911121024

Tartuffo commented 2 years ago

@Chris-Hibbert This needs an area label to get picked up the right Wed planning meeting. This looked like Core Economy, please change if that is not right.

dtribble commented 2 years ago

A general note is that notifiers should be used only to report states, not transitions. Thus dropping one update doesn't prevent getting the latest state update. To use notifiers to communicate events, the update for a getUpdateSince(x) must include all the events since the specified time x or the "state" should be the entire list of events.

erights commented 2 years ago

To use notifiers to communicate events,

or, if you are communicating events, use subscriptions rather than notifiers. If subscriptions are not better at events than notifiers, then subscriptions should either be fixed or retired.

erights commented 1 year ago

Hi @gibson042 , I just added you