Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
326 stars 205 forks source link

Is it time to update TimerService? #3612

Open zarutian opened 3 years ago

zarutian commented 3 years ago

What is the Problem Being Solved?

This issue sprung forth from a comment on a diffrent issue.

The problem is that the current interface design of TimerService(s) was always meant to be temporary per @warner @Chris-Hibbert and possibly @erights and frankly it feels cobbled together and inelegant.

Plus, the documentation for TimerService(s) is curiously absent from the agoric.com documentation which I take as a sign for possible deprication of the current design of the interface.

Description of the Design

See this file in a gist

Security Considerations

Same as for original TimerService though I am not quite sure about the subscription part of repeaters would be a denial of service oppertunity through excessive slow consumption of stateUpdate data-objects forcing the publicating vat to hold on to them.

Test Plan

Probably every test of components or contracts that use TimerService(s)

zarutian commented 3 years ago

paging @katelynsills as she showed intrest per the linked comment

Chris-Hibbert commented 3 years ago

Plus, the documentation for TimerService(s) is curiously absent from the agoric.com documentation which I take as a sign for possible deprication of the current design of the interface.

see https://agoric.com/documentation/repl/timerServices.html

zarutian commented 3 years ago

I stand corrected. But the rest stands for itself.

On 06/08/2021, Chris Hibbert @.***> wrote:

Plus, the documentation for TimerService(s) is curiously absent from the agoric.com documentation which I take as a sign for possible deprication of the current design of the interface.

see https://agoric.com/documentation/repl/timerServices.html

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/Agoric/agoric-sdk/issues/3612#issuecomment-894370004

warner commented 3 years ago

What I'd really like to do is rewrite devices so they can handle Promises (#1346) so the timer device could have a simple Promise-based interface (#1930) instead of needing to register a handler just for one-shot wakeups. And then maybe repeaters could be implemented with a Notifier.

zarutian commented 3 years ago

I have not dived into how devices work but if there are ?upcalls? available, that is callback from a device to a vat then I do think that TimerService as presented in the design can live in a vat and tick device upcalls into it at every tick interval.

Does that obviate your requirement that devices handle promises, @warner ?

zarutian commented 3 years ago

I updated that gists example file to add example E that demonstrates a TimerService implementation that relies on an external agent to call a .tick() method on a privateTickFacet. That external agent could be the current timer service vat that got endowed with the timer device. That way neither that timer device or the vat endowed with it needs to deal with promises.

(This basically means what I meant by that obviate comment above)

katelynsills commented 3 years ago

Thanks @zarutian! I'll dive into this in the next few days.

zarutian commented 3 years ago

@katelynsills how deep did you dive into this?

You think this is revelant to #3748 ?

dckc commented 3 years ago

@Chris-Hibbert do you suppose recent work on TimerService addressed this issue?

Chris-Hibbert commented 3 years ago

We have fixed some bugs and made the implementation consistent with the documentation, but we haven't changed anything about the API that would make it more elegant.

There's been discussion about making the notification API more performant (relying on subscriptions rather than notifiers), but I don't know of proposals for a redesigned interface.

Chris-Hibbert commented 3 years ago

Here's a thought on a different way to approach the timer service, sparked by a conversation between @dckc and @dtribble.

Our current timer service is remote everywhere, even though nearly all of our code runs on the same chain. There's a single timerService vat wrapping a timer device, and all the code that wants to deal with time makes eventual calls to the remote device, even though they're running on the same chain.

I started out thinking that what we wanted was some kind of federated time service. Isn't that how timeD works to keep clocks synced across the network? Then I realized that what we want isn't some system to keep different timers in sync so that we can have a local representive of some distant time authority, what we want is a local representative of the consensus time for the chain that we're on when that's the time that we're reliant on. We still want the ability to represent a particular time authority when we're communicating off the chain, or sending time quotes among vats that might someday be able to migrate. But once we have the identity of the timer we want to use, we ought to be able to turn that into a local timer if we're running on the same chain.

Few of our calls on the timer service actaully want the current time (other than as a refernce mark to schedule future events against.) Most want to get a wakeup() or callback or promise resolution at somem future time or sequence of future times.

What I'm looking for (and haven't quite figured out how to do is to have a unique identifier for the chain timer. It would be passed around to represent the clock that times are relative to. Any vat on the same chain would be able to hand that token to SwingSet, and get a local synchronous timer that would accept scheduling calls. When passed to code running on other chains or ag-solos, the timer would be wrapped as a remote object with the same API, but would only be useable with E() calls.

One weakness of this idea is that code that might be mobile, or wouldn't always be used in proximity to the timer it was passed would want to adapt smoothly to the timer being remote. Since the main ppint of a scheduling call is an eventual callback to a supplied function, maybe the remoteness of the timer doesn't matter much?

An approach like this also addresses the desire we have to make Timers legible/recognizable when they aappear in terms or invitations.

zarutian commented 2 years ago

Here's a thought on a different way to approach the timer service, sparked by a conversation between @dckc and @dtribble.

Our current timer service is remote everywhere, even though nearly all of our code runs on the same chain. There's a single timerService vat wrapping a timer device, and all the code that wants to deal with time makes eventual calls to the remote device, even though they're running on the same chain.

I had the assumption that there might be diffrent TimerServices, some not even running on chain. Example of one such is a TimerService run on a solo vat hosted by say the British Royal Horological Society and is based of UTC. Another example would be a TimerService based of religious malleable calendar.

I started out thinking that what we wanted was some kind of federated time service. Isn't that how timeD works to keep clocks synced across the network? Then I realized that what we want isn't some system to keep different timers in sync so that we can have a local representive of some distant time authority, what we want is a local representative of the consensus time for the chain that we're on when that's the time that we're reliant on. We still want the ability to represent a particular time authority when we're communicating off the chain, or sending time quotes among vats that might someday be able to migrate. But once we have the identity of the timer we want to use, we ought to be able to turn that into a local timer if we're running on the same chain.

Few of our calls on the timer service actaully want the current time (other than as a refernce mark to schedule future events against.) Most want to get a wakeup() or callback or promise resolution at somem future time or sequence of future times.

What I'm looking for (and haven't quite figured out how to do is to have a unique identifier for the chain timer. It would be passed around to represent the clock that times are relative to. Any vat on the same chain would be able to hand that token to SwingSet, and get a local synchronous timer that would accept scheduling calls. When passed to code running on other chains or ag-solos, the timer would be wrapped as a remote object with the same API, but would only be useable with E() calls.

Only seen record-objects of the form { timer, timestamp } if that is what you are talking about.

One weakness of this idea is that code that might be mobile, or wouldn't always be used in proximity to the timer it was passed would want to adapt smoothly to the timer being remote. Since the main point of a scheduling call is an eventual callback to a supplied function, maybe the remoteness of the timer doesn't matter much?

Not only mobile code but also mobile vats that might migrate from say qourum vat to a chain vat.

An approach like this also addresses the desire we have to make Timers legible/recognizable when they appear in terms or invitations.

Same kind of brand mechanism like Issuers have might be in order?