-
Notifications
You must be signed in to change notification settings - Fork 215
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
Can we consolidate trade
and swap
into one helper?
#1531
Comments
Let's look into checking whether the seats have exited and otherwise making the two functions more uniform but let's make sure to keep swap for the specialized use cases. Open issue in documentation to clarify use cases for each. |
Just as a study of how we use agoric-sdk/packages/zoe/src/contracts/atomicSwap.js Lines 31 to 34 in da2b92d
agoric-sdk/packages/zoe/src/contracts/coveredCall.js Lines 49 to 54 in da2b92d
agoric-sdk/packages/zoe/src/contracts/simpleExchange.js Lines 68 to 79 in da2b92d
|
I wonder if we really need
trade
andswap
as distinct abstractions. Swap begins by checking if keepSeat has exited. However, it should also error if trySeat has exited.trade
should also error if either has exited, so both checks could be moved intotrade
.Looking at the callers of
trade
, a major reason they can't useswap
is that thekeepSeat
remains open. They all do exit the trySeat.Looking at the callers of
swap
, sinceswap
throws on failure and only closes thekeepSeat
on success, it would not be an undue burden for swap's callers to exit the keepSeat after the call to swap. Since we did presciently call it thekeepSeat
, it would also not be surprising forswap
itself not to close it._Originally posted by @erights in #1414 (comment)
The text was updated successfully, but these errors were encountered: