-
Notifications
You must be signed in to change notification settings - Fork 208
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
feat(zoe): revoke used-up payments #8982
base: master
Are you sure you want to change the base?
Conversation
8bcd3b0
to
f9fc35b
Compare
26d1ad4
to
db99ac8
Compare
f9fc35b
to
7a3e2c5
Compare
db99ac8
to
4f749e3
Compare
7a3e2c5
to
ade6ec9
Compare
4f749e3
to
782cac8
Compare
ade6ec9
to
774a52e
Compare
782cac8
to
e70e068
Compare
774a52e
to
fba5993
Compare
e70e068
to
dd87509
Compare
99fce04
to
4bf8835
Compare
dd87509
to
5ce33fb
Compare
@@ -901,7 +901,7 @@ test('lib-wallet offer methods', async t => { | |||
description: 'getRefund', | |||
handle: { | |||
kind: 'unnamed', | |||
petname: 'unnamed-7', | |||
petname: 'unnamed-6', |
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 change made blindly. Reviewers, how should I figure out whether I should worry about this change?
@@ -914,7 +914,7 @@ test('lib-wallet offer methods', async t => { | |||
}, | |||
inviteHandleBoardId: 'board0257', | |||
meta: { | |||
id: 9, | |||
id: 8, |
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 change made blindly. Reviewers, how should I figure out whether I should worry about this change?
0a7af31
to
9ec6de5
Compare
9ec6de5
to
a70707f
Compare
a70707f
to
721cc3e
Compare
Deploying agoric-sdk with Cloudflare Pages
|
721cc3e
to
81a7370
Compare
5bbeb1e
to
f883dcc
Compare
49d7ec1
to
9d259f8
Compare
9d259f8
to
2d616e0
Compare
2d616e0
to
121d928
Compare
121d928
to
e53274d
Compare
e53274d
to
8723afa
Compare
8fcdbbc
to
3da98ca
Compare
3da98ca
to
f2f2f3b
Compare
closes: #XXXX
refs: #XXXX
Description
Prior to this PR, a used up payment was still a live exo object, even with one method that continued to work --
getAllegedBrand()
. This PR wraps it with a revocable caretaker, and only exposes the caretaker as the payment. When the payment is used up, it is also revoked.This PR is in theory not compat with code prior to this PR because
getAllegedBrand()
on a used up payment will no longer work. However, we have not seen that failure in practice.Security Considerations
Since used up payments were already reliably powerless, the additional protection of a general revocation mechanism probably has no effect on actual security.
Scaling Considerations
This PR has a bit of a cost -- twice as many payment-ish objects, the underlying original payment and the new caretaker. However, by marking the exposed object as revoked, we hope eventually to make use of this known dead state to drop bookkeeping elsewhere. But we do not yet have a plausible story for doing so, in which case this PR imposes this code with no compensating performance benefit.
Documentation Considerations
Only that
getAllegedBrand()
stops working. Otherwise this PR should have no externally visible effects.Testing Considerations
Because the revocable payment has a different "alleged" label, we had to fix some golden tests.
Upgrade Considerations
Payments already created prior to this PR will remain the underlying non-revocable payment. Only payments created after this PR will be revocable. However, the only observable anomaly is that
getAllegedBrand()
on those old payments will continue to work after they have been used up.