Release empty escrowed payments #9401
Conversation
Upgraded issuers would keep the recoverySet without removing anything.
Chris-Hibbert
added
Zoe
packages/zoe/src/zoeService/zoe.js
Outdated
| // recoverySet claims to be a copySet, but it's a record with a payload | ||
| // field containing an array of payment promises | ||
| for (const p of recoverySet.payload) { |
There was a problem hiding this comment.
| // recoverySet claims to be a copySet, but it's a record with a payload | |
| // field containing an array of payment promises | |
| for (const p of recoverySet.payload) { | |
| for (const p of getCopySetKeys(recoverySet)) { |
The comment is correct, but for the "but". recoverySet is a CopySet. A CopySet is a Tagged, which is a record-like structure with a tag (accessed by getTag) and a payload field, as you discovered. However, as a user of a copySet, you should generally avoid assuming too much about its internal implementation. Rather, use the getCopySetKeys function for obtaining those keys. Its implementation at https://github.com/endojs/endo/blob/0a91fbe7a066ef3d754b350d5da7ccf55638c12d/packages/patterns/src/keys/checkKey.js#L200-L209 is
export const getCopySetKeys = s => {
assertCopySet(s);
return s.payload;
};There was a problem hiding this comment.
I looked and didn't find useful declarations or documentation. The wonderful endojs docs do link from CopySet to CopyTagged, but I didn't find anything to suggest that getCopySetKeys() was the appropriate way to access it. If I wasn't just looking in the wrong place, where can we add it so others will know?
There was a problem hiding this comment.
I updated the code and dropped the comment.
There was a problem hiding this comment.
I looked and didn't find useful declarations or documentation. The wonderful endojs docs do link from CopySet to CopyTagged, but I didn't find anything to suggest that getCopySetKeys() was the appropriate way to access it. If I wasn't just looking in the wrong place, where can we add it so others will know?
Please file a bug on this?
There was a problem hiding this comment.
Deploying agoric-sdk with
|
| Latest commit: |
7159bbf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e9c0a509.agoric-sdk.pages.dev |
| Branch Preview URL: | https://8686-releaseemptyescrowedpay.agoric-sdk.pages.dev |
Chris-Hibbert
force-pushed
the
8400-upgradeScaledPAs
branch
from
f9e3b65 to
04ba2b5
Compare
closes: #8686
Description
Release empty IST payments from Zoe's recoverySet for escrowed payments.
#8404 described the general problem.
#8498#9013 stopped adding empty payments to recovery sets. The largest known collection of retained empty payments was in Zoe. This removes a limited number. The presumption is that nothing else refers to them, so the kernel wouldn't have to propagate news of their release. If that's not true, then we shouldd release them gradually.Security Considerations
None. The action is benign and idempotent. We should remove the code after exercising it once, but it's inaccessible to users.
Scaling Considerations
Cleans up some leftover unneeded objects.
Documentation Considerations
None
Testing Considerations
Tested in A3P, where it successfully deleted 4 vestigial empty payments. On MainNet, we counted 1718 a few months ago.
Upgrade Considerations
This requires an upgrade to Zoe, with its downstream consequences. The release is triggered by a proposal, which could be run once with a large parameter, or multiple times with a smaller parameter to clean up these excess objects.