-
Notifications
You must be signed in to change notification settings - Fork 223
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(localOrchAccount): Deposit, Withdraw invitationMakers #10117
Conversation
99f89dc
to
ea73ef7
Compare
08488af
to
81943fd
Compare
Deploying agoric-sdk with
|
Latest commit: |
c92ef27
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2dd11342.agoric-sdk.pages.dev |
Branch Preview URL: | https://9193-deposit-withdraw-invita.agoric-sdk.pages.dev |
return watch( | ||
zoeTools.localTransfer( | ||
seat, | ||
// @ts-expect-error LocalAccount vs LocalAccountMethods | ||
this.state.account, | ||
give, | ||
), | ||
this.facets.seatExiterHandler, | ||
seat, | ||
); | ||
}, | ||
'Deposit', | ||
undefined, | ||
M.splitRecord({ give: AnyNatAmountsRecord, want: {} }), | ||
); |
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.
Given zoeTools.localTransfer
is expected to resolve promptly, we technically do not need to use watch
here and instead could use asVow
.
This approach seems harmless, and appeases the linter. It also allows a developer to call Deposit()
inside an async-flow if they wished.
81943fd
to
b2932ee
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.
LGTM. Tests cover happy and error cases. Looking forward to E2E. (noting this PR doesn't close the issue)
@@ -180,3 +180,13 @@ export const TxBodyOptsShape = M.splitRecord( | |||
nonCriticalExtensionOptions: M.arrayOf(M.any()), | |||
}, | |||
); | |||
|
|||
/** | |||
* Ensures at least one {@link AmountKeywordRecord} entry is present and only |
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.
clever
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.
h/t SingleNatAmountRecord
fka SingleAmountRecord
from send-anywhere
Base branch is changed to master. Please re-run the integration tests by adding 'force:integration' label. |
- narrows typeguard as only NatAmounts are supported in send-anywhere
b2932ee
to
c92ef27
Compare
refs: #9193
Description
Adds
Deposit
andWithdraw
invitationMakers toLocalOrchestrationAccount
, leveragingZoeTools.localTransfer
andZoeTools.withdrawToSeat
Security Considerations
Involves withdrawing payments to a temporary seat, which can be risky. Leverages
ZoeTools
which rolls back allocations in failure and partial failure scenarios.Scaling Considerations
n/a
Documentation Considerations
I'm not sure how well we document the platform-provided
invitationMakers
and.asContinuingOffer()
helper. We might consider doing so in the future.Testing Considerations
Includes new unit tests. Wallet Driver tests in an e2e environment are coming in a future PR after #9966 lands
Upgrade Considerations
n/a, library code