-
Notifications
You must be signed in to change notification settings - Fork 229
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: virtualize payments, purses, ledger #4618
Conversation
bd23614
to
e4681fd
Compare
a085d12
to
6c6d8f6
Compare
0e37b0a
to
50843dc
Compare
4b71145
to
dbb986b
Compare
b2fa856
to
e40674b
Compare
93db57d
to
99fe828
Compare
99fe828
to
442e16d
Compare
Assuming it passes CI enough, this is now ready for review. Thanks! |
Solving a CI failure led me down a rabbit hold, resulting in 981bc1e which we should definitely discuss before merging. I would like to avoid merging this as part of this PR. However, without it, the generated file ui-components/compiled/test/components/test-NatAmountInput.js fails, apparently because its imports include ERTP, which now depends on storeModule, which depends on the global variable See #4255 |
Yay! CI passes! But only with the 981bc1e kludge. Is there a better way? |
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.
This seems like a good start. I'm not excited about the VatData thing, but I figure we'll find a solution for that sooner or later.
While reviewing this, I realized we have a GC/determinism problem, which I wrote up in #4892. It's not specific to this conversion though, so it shouldn't hold up this PR.
dad4f8d
to
38480a2
Compare
a4fb604
to
a68c837
Compare
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.
Golden!
Virtualize payment, paymentLedger, and purse,depositFacet together as a multi-faceted unit.
This does not virtualize everything that will need to be durable to support upgrade.
This does not even virtualize everything of high arity, needed for scaling, as it does not virtualize the per-purse notifier.