-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: cheap auxdata #6355
base: master
Are you sure you want to change the base?
fix: cheap auxdata #6355
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ERTP/Zoe changes look fine. Someone else should approve the SwingSet changes.
There's a lint (import ordering) failure in CI.
@@ -13,6 +13,7 @@ import buildManualTimer from '../../../tools/manualTimer.js'; | |||
|
|||
import { setupZCFTest } from './setupZcfTest.js'; | |||
import { assertAmountsEqual } from '../../zoeTestHelpers.js'; | |||
import { M } from '@agoric/store'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failure from lint. Move this up above line 9.
Converting to draft because of unintuitive interaction with durable state and upgrade :( (Will file bugs explaining the problems) |
0664fa8
to
c48a881
Compare
1cfa395
to
8321dd7
Compare
8321dd7
to
e9c7254
Compare
e9c7254
to
56d7dcb
Compare
56d7dcb
to
03b53d8
Compare
This will require the program to understand, and have a way to test, whether an object is local or remote. Do we have such a predicate today?
Sounds like releasing Zalgo in the worst possible shape. I'd much prefer have a this method return a pseudo promise which is guaranteed to have a stable identity, and have an external promise result memoization helper. Even then I still don't understand how you'll be able to escape our rules that we never await conditionally. Seems like releasing Zalgo is a design choice here? Edit: this is based on reading the PR description. I did not read the implementation. |
03b53d8
to
2784454
Compare
5df58dc
to
dc67a20
Compare
54ab831
to
85225cc
Compare
689c5b1
to
d63b7c9
Compare
d63b7c9
to
d58b94b
Compare
d5101b8
to
a994ef7
Compare
90bd935
to
e7cd52a
Compare
Datadog ReportBranch report: ✅ |
e7cd52a
to
9c90ec5
Compare
Datadog ReportBranch report: ✅ |
9c90ec5
to
8118747
Compare
Closing in favor of an actual solution to this problem, which I need to explain. |
Reopening, as my "better idea" does not work at all. See #2069 (comment) |
Liveslots adds to all remote presences an
aux()
method.A remote presence has an
aux()
method that either returns a promise or a non-promise. The first time it is called, it does an eventual-send of anaux()
message to the far object it designates, remembers that promise as its auxdata, and returns that. It also watches that promise to react to its fulfillment. Until that reaction, every time itsaux()
is called, it will return that same promise. Once it reacts to the promise being fulfilled, if that ever happens, then the fulfillment becomes its new auxdata which gets returned from then on.Initially only liveslots does this. But the mechanism is in a reusable
makeRemotePresence
function that should probably move to@endo/marshal
so that other things that make remote presences can use it.A good first test will be for ERTP brands to provide an auxdata object with the things that you can currently ask a brand for that are stable, and that zoe needs to cache before it can register the brand. Zoe's caching logic should then become much simpler as it need only delay registration until a local call to the brand's
aux()
returns a non-promise. After that, the remote presence'saux()
method must always immediately return the same non-promise.(Some brands are co-located with Zoe, in which case their
aux()
method may not be similarly stable. But that would be a misbehaving brand. The only brands co-located with the Zoe service are made by Zoe and relied on not to misbehave.)Back in draft pending needed redesign to play well with virtual and durable storage.
If we believe this approach is adequate for our auxdata needs, then
Fixes: #2069
@Chris-Hibbert @warner @FUDCo @michaelfig @mhofman when this is ready, I'll ask you to review. Until then, please ignore. @Chris-Hibbert I'm leaving you listed since you already commented.