Skip to content
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

vat-zoe is holding a lot of 0.0 IST Payments in the recovery set #8404

Open
warner opened this issue Sep 28, 2023 · 7 comments
Open

vat-zoe is holding a lot of 0.0 IST Payments in the recovery set #8404

warner opened this issue Sep 28, 2023 · 7 comments
Labels
bug Something isn't working Zoe package: Zoe

Comments

@warner
Copy link
Member

warner commented Sep 28, 2023

While investigating #8401, I noticed that v9-zoe also has 1,718 zero-balance IST Payment objects outstanding, kept alive by the recovery set. It looks like they appear in a payouts.Fee property, in case that helps track them down.

I vaguely remember us investigating this a while ago, and concluding that something about the wallet (perhaps the front end?) didn't bother to claim/deposit empty Payments. From an economic point of view, this makes sense (why waste transaction fees on an empty box), but to save storage space, we should find a way to deposit or burn these Payments. They currently consume 10 vatstore rows each (plus 10 IAVL rows, since vatstore lives in kvStore, and kvStore is mirrored into IAVL), and I think this amount grows over time. I don't yet know how they got here, so I can't estimate how fast we're accumulating them.

Here's an example, v9-zoe o+d11/981:

% grep 'o+d11/981\b' v9.vs
v9.vs.vc.13.r0000000531:o+d11/981|{"body":"#null","slots":[]}
v9.vs.vc.13.|o+d11/981|531
v9.vs.vc.6.r0000000981:o+d11/981|{"body":"#{\"brand\":\"$0.Alleged: IST brand\",\"value\":\"+0\"}","slots":["o+d10/1"]}
v9.vs.vc.6.|o+d11/981|981
v9.vs.vc.7.r0000000875:o+d11/981|{"body":"#\"$0.Alleged: setStore\"","slots":["o+d8/13"]}
v9.vs.vc.7.|o+d11/981|875
v9.vs.vom.ir.o+d11/981|6|1
v9.vs.vom.ir.o+d11/981|7|1
v9.vs.vom.o+d11/981|{}
v9.vs.vom.o+d31/7744|{"currentAllocation":{"body":"#{\"Fee\":{\"brand\":\"$0.Alleged: IST brand\",\"value\":\"+0\"}}","slots":["o+d10/1"]},"proposal":{"body":"#{\"exit\":{\"onDemand\":null},\"give\":{},\"want\":{}}","slots":[]},"exitObj":{"body":"#\"$0.Alleged: ExitObject\"","slots":["o-16203"]},"offerResult":{"body":"#\"paid out 0\"","slots":[]},"offerResultStored":{"body":"#true","slots":[]},"instanceAdminHelper":{"body":"#\"$0.Alleged: instanceAdmin helper\"","slots":["o+d33/36:0"]},"withdrawFacet":{"body":"#\"$0.Alleged: InstanceStorageManager withdrawFacet\"","slots":["o+d26/36:2"]},"publisher":{"body":"#\"$0.Alleged: zoe Seat publisher publisher\"","slots":["o+d30/7744:0"]},"subscriber":{"body":"#\"$0.Alleged: zoe Seat publisher subscriber\"","slots":["o+d30/7744:1"]},"payouts":{"body":"#{\"Fee\":\"$0.Alleged: IST payment\"}","slots":["o+d11/981"]},"exiting":{"body":"#true","slots":[]}}
v9.vs.vom.rc.o+d11/981|2

The third row (v9.vs.vc.6.r000..) is the paymentLedger table entry, mapping this Payment object to a balance of 0n uIST. The next-to-last line (v9.vs.vom.o+D31/7744 is a zoeSeatAdmin record, which points to this Payment in state.payouts.Fee.

cc @Chris-Hibbert @erights

@warner warner added bug Something isn't working Zoe package: Zoe labels Sep 28, 2023
@Chris-Hibbert
Copy link
Contributor

@warner wrote:

The #8404 0-IST zoe Payments issue can probably be cleaned up one-fell-swoop, as of 01-dec we have 4k of them, and I think those should GC in about 13 seconds. So one long block and then it should be done.

@Chris-Hibbert
Copy link
Contributor

It's not obvious to me how we can release these Payments. The only Purses that Zoe holds onto are the escrow Purses, and those (even the one for Fee) can't be marked as noRecoverySet. Without that, neither the incremental nor monolithic clean-ups can be triggered.

If this is an artifact of people never depositing zero-value payouts from contracts, we're going to have a gradual accumulation of them from many issuers.

I keep trying to resolve this by having a single global instance of a zero value payment for every issuer. It wouldn't need to be added to the recovery set, and we'd have to be careful to not burn it, either when deposited or explicitly burned. Is there a reason that won't work? As long as we never burn it, nobody can learn anything from it about payments held by others.

@erights
Copy link
Member

erights commented Dec 19, 2023

Are you suggesting that users would have access to this payment, and therefore be able to ask that it be deposited or burned, but that doing so does not use it up? This would be a surprising irregularity.

@Chris-Hibbert
Copy link
Contributor

Are you suggesting that users would have access to this payment, and therefore be able to ask that it be deposited or burned, but that doing so does not use it up? This would be a surprising irregularity.

Yeah, that's what I'm suggesting. I don't know if it can be made to work, but having unique objects for common values (like 0 or 1) is a common way to prevent the need to GC objects with extremely high cardinality.

How about if every burned or deposited payment turned into the global zero for that issuer? (I'm brainstorming here). I think we could do that, since all the user has is a key to a weakMap we control. As long as they hold onto it, we have to keep an entry, but the value could be the shared canonical zero.

@erights
Copy link
Member

erights commented Dec 19, 2023

Yeah, that's what I'm suggesting. I don't know if it can be made to work, but having unique objects for common values (like 0 or 1) is a common way to prevent the need to GC objects with extremely high cardinality.

For objects like amounts that represent immutable values, yes. But payments have an inherently mutable lifecycle, where the only mutation event is when it gets used up.

How about if every burned or deposited payment turned into the global zero for that issuer? (I'm brainstorming here). I think we could do that, since all the user has is a key to a weakMap we control. As long as they hold onto it, we have to keep an entry, but the value could be the shared canonical zero.

The zero amount associated with the payment could be shared. But each payment itself still needs its own unique identity and lifecycle.

@erights
Copy link
Member

erights commented Dec 19, 2023

@Chris-Hibbert
Copy link
Contributor

RecoverySets in vat-zoe are probably limited to the escrowPurses, which can't use noRecoverSets from #8498. The fix to #8498 in 21f1aab at least stops Zoe from adding new zero-value payments to the recoverySet.

I suppose we could add code to Zoe to (incrementally?) iterate the escrowPurses' recoverSets and remove zero-value payments from them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Zoe package: Zoe
Projects
None yet
Development

No branches or pull requests

3 participants