Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

A bad close offer fails in the wrong place #17

Closed
dtribble opened this issue Apr 7, 2021 · 14 comments
Closed

A bad close offer fails in the wrong place #17

dtribble opened this issue Apr 7, 2021 · 14 comments
Assignees
Labels

Comments

@dtribble
Copy link
Member

dtribble commented Apr 7, 2021

Repro

  1. In manage vault, Issue a close-vault offer (but don't approve it)
  2. In manage vault, withdraw some collateral
  3. In wallet, approve the withdrawal offer
  4. In wallet, approve the close offer

The close offer will fail, but it does so trying to get a bogus price quote.

Stack in chain

UnhandledPromiseRejectionWarning: (Error#16)
Error#16: priceQuery must return a quote

  at createQuote (packages/zoe/src/contracts/multipoolAutoswap/priceAuthority.js:42:20)
  at mutableTrigger (packages/zoe/src/contractSupport/priceAuthority.js:169:40)
  at Alleged: PriceAuthority.mutableQuoteWhenOutTrigger [as mutableQuoteWhenLT] (packages/zoe/src/contractSupport/priceAuthority.js:202:25)

Error#16 ERROR_NOTE: Rejection from: (Error#17) : 22662 . 0
Nested error under Error#16
  Error#17: Event: 22661.1
  
    at Function.applyMethod (packages/tame-metering/src/tame.js:184:20)
    at meteredConstructor.deliver (packages/SwingSet/src/kernel/liveSlots.js:525:28)
    at eval (packages/SwingSet/src/kernel/vatManager/deliver.js:51:48)
  
Error#16 ERROR_NOTE: Sent as error:liveSlots:v17#70004
(node:11672) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 7)
(Use `node --trace-warnings ...` to show where the warning was created)
UnhandledPromiseRejectionWarning: (RemoteError(error:liveSlots:v17#70004)#18)
RemoteError(error:liveSlots:v17#70004)#18: priceQuery must return a quote

  at fullRevive (packages/marshal/src/marshal.js:874:43)
  at meteredConstructor.unserialize (packages/marshal/src/marshal.js:954:19)
  at notifyOnePromise (packages/SwingSet/src/kernel/liveSlots.js:597:19)
  at meteredConstructor.notify (packages/SwingSet/src/kernel/liveSlots.js:610:7)
  at eval (packages/SwingSet/src/kernel/vatManager/deliver.js:51:48)

RemoteError(error:liveSlots:v17#70004)#18 ERROR_NOTE: Rejection from: (Error#19) : 22660 . 0
Nested error under RemoteError(error:liveSlots:v17#70004)#18
  Error#19: Event: 22659.1
  
    at Function.applyMethod (packages/tame-metering/src/tame.js:202:19)
    at Proxy.eval (packages/eventual-send/src/E.js:37:126)
    at reschedulePriceCheck (packages/treasury/src/vaultManager.js:123:60)
  
UnhandledPromiseRejectionWarning: (RemoteError(error:liveSlots:v10#70001)#22)
RemoteError(error:liveSlots:v10#70001)#22: Check failed

  at fullRevive (packages/marshal/src/marshal.js:874:43)
  at meteredConstructor.unserialize (packages/marshal/src/marshal.js:954:19)
  at notifyOnePromise (packages/SwingSet/src/kernel/liveSlots.js:597:19)
  at meteredConstructor.notify (packages/SwingSet/src/kernel/liveSlots.js:610:7)
  at eval (packages/SwingSet/src/kernel/vatManager/deliver.js:51:48)

RemoteError(error:liveSlots:v10#70001)#22 ERROR_NOTE: Rejection from: (Error#23) : 24682 . 0
Nested error under RemoteError(error:liveSlots:v10#70001)#22
  Error#23: Event: 24680.2
  
    at Function.applyMethod (packages/tame-metering/src/tame.js:202:19)
    at Proxy.eval (packages/eventual-send/src/E.js:37:126)
    at meteredConstructor.acceptOffer (packages/dapp-svelte-wallet/api/src/lib-wallet.js:998:51)
  
UnhandledPromiseRejectionWarning: (RemoteError(error:liveSlots:v10#70002)#24)
RemoteError(error:liveSlots:v10#70002)#24: Check failed

  at fullRevive (packages/marshal/src/marshal.js:874:43)
  at meteredConstructor.unserialize (packages/marshal/src/marshal.js:954:19)
  at notifyOnePromise (packages/SwingSet/src/kernel/liveSlots.js:597:19)
  at meteredConstructor.notify (packages/SwingSet/src/kernel/liveSlots.js:610:7)
  at eval (packages/SwingSet/src/kernel/vatManager/deliver.js:51:48)

RemoteError(error:liveSlots:v10#70002)#24 ERROR_NOTE: Rejection from: (Error#25) : 24776 . 0
Nested error under RemoteError(error:liveSlots:v10#70002)#24
  Error#25: Event: 24775.1
  
    at Function.applyMethod (packages/tame-metering/src/tame.js:202:19)
    at Proxy.eval (packages/eventual-send/src/E.js:37:126)
    at eval (packages/dapp-svelte-wallet/api/src/lib-wallet.js:467:17)
  
@Tartuffo
Copy link

Sam to talk to @dckc and/or @Chris-Hibbert about this.

@dckc
Copy link
Member

dckc commented Feb 1, 2022

@samsiegart are you able to reproduce it?

@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@samsiegart
Copy link
Contributor

@samsiegart are you able to reproduce it?

It seems to fail in the expected way now:

2022-02-09T01:11:40.966Z SwingSet: ls: v22: Error#1: Offer safety was violated by the proposed allocation: { Collateral: { brand: Object [Alleged: LINK brand] {}, value: 4900000000000000000n }, RUN: { brand: Object [Alleged: RUN brand] {}, value: 0n } } . Proposal was { exit: { onDemand: null }, give: { RUN: { brand: Object [Alleged: RUN brand] {}, value: 89406516n } }, want: { Collateral: { brand: Object [Alleged: LINK brand] {}, value: 5000000000000000000n } } }
2022-02-09T01:11:40.967Z SwingSet: ls: v22: Error: Offer safety was violated by the proposed allocation: (an object). Proposal was (an object)

Makes sense since I wanted 5 LINK to close the vault, but I only had 4.9 left after withdrawing 0.1. However, the offer in the wallet API showed a status of "accept", which seems incorrect. I would think it would be "rejected" https://github.com/Agoric/agoric-sdk/blob/master/packages/wallet/api/src/lib-wallet.js#L1118

@samsiegart
Copy link
Contributor

samsiegart commented Feb 9, 2022

@dckc @michaelfig I tried logging here and looks like the contract is still returning payouts (maybe 0-payments?) https://github.com/Agoric/agoric-sdk/blob/master/packages/wallet/api/src/lib-wallet.js#L630

The wallet uses whether or not there's payouts to mark the offer as accepted or rejected. Should we be checking in the wallet if those payouts are 0, or change the contract to throw instead of returning a payout in this case?

@Chris-Hibbert
Copy link
Contributor

From Zoe's and the contract's point of view, the request shouldn't throw. Satisfying the user's give condition is a normal outcome. The wallet should be prepared for this. If the payments are returned, the wallet should show the user that the offer was refunded or some such. Does the wallet figure out that the user's payment was returned and deposit it to a reasonable choice of purses?

@samsiegart
Copy link
Contributor

Does the wallet figure out that the user's payment was returned and deposit it to a reasonable choice of purses?

Yes, the purse balances are the same before and after the offer. I don't think accepted is a good label when you didn't get what you wanted, and figured rejected would be appropriate. Maybe related to #4360

@michaelfig
Copy link
Member

I don't think accepted is a good label when you didn't get what you wanted, and figured rejected would be appropriate.

rejected sounds too bad. I would say refunded.

@samsiegart
Copy link
Contributor

samsiegart commented Feb 10, 2022

@Chris-Hibbert @michaelfig Does it seem fair to close this in favor of Agoric/agoric-sdk#4360?

@Chris-Hibbert
Copy link
Contributor

Does it seem fair to close this in favor of Agoric/agoric-sdk#4360?

I'm fine with that.

@erights
Copy link
Member

erights commented Feb 11, 2022

Keep in mind that one possible outcome is that the user got >= what they wanted and >= what they gave.

@erights
Copy link
Member

erights commented Feb 11, 2022

@Chris-Hibbert @michaelfig Does it seem fair to close this in favor of Agoric/agoric-sdk#4360?

Me too.

@samsiegart
Copy link
Contributor

Closing to defer to Agoric/agoric-sdk#4360 as discussed

@samsiegart
Copy link
Contributor

@erights You mean ">= what they wanted wanted and <= (as opposed to >=) what they gave" right?

@erights
Copy link
Member

erights commented Feb 11, 2022 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants