Skip to content
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

AMM trades fail - Error#125: rights were not conserved for brand [object Alleged: RUN brand] #3800

Closed
warner opened this issue Aug 24, 2021 · 7 comments
Assignees
Labels
Zoe package: Zoe

Comments

@warner
Copy link
Member

warner commented Aug 24, 2021

Trades on the AMM were getting rejected with surprising errors.

From the analysis below:

TLDR: The stagedAllocations weren't being cleared after an offer safety error, so future offers were working with a poolSeat that already had a stagedAllocation (one corresponding to a previous offer). The contract code could fix this by clearing the staged allocations in case of an error, but we probably want Zoe to do it instead. We need to spend some time thinking through how we should clean up state after errors (offer safety, rights conservation, staged allocations not included in reallocate, etc).

I'm splitting out this surprising rights-conservation error from the original ticket.

Kate said:

This looks like a "swap" on the AMM was being performed. Do we know what was being traded?

Originally posted by @katelynsills in Agoric/testnet-notes#26 (comment)


Fee details are still available

Note that the AMM Trade Part 1: Make a trade on the AMM and get charged a fee task does not require that the trade goes through. Only that metering fees are charged, which happens even if the trade is refunded.

@warner
Copy link
Member Author

warner commented Aug 24, 2021

I extracted the cranks from block 55576, which appears to include the "rights were not conserved" rejection. Here's a map of vatIDs on this chain:

  v1:   xs-worker        583  bank
  v2:   xs-worker        718  board
  v3:   xs-worker         60  distributeFees
  v4:   xs-worker        395  ibc
  v5:   xs-worker       1416  mints
  v6:   xs-worker        403  network
  v7:   xs-worker        562  priceAuthority
  v8:   xs-worker        283  provisioning
  v9:   xs-worker          1  sharing
 v10:   xs-worker       2072  zoe
 v11:   xs-worker       1230  bootstrap
 v12:   xs-worker         61  vatAdmin
 v13:       local       5879  comms
 v14:   xs-worker       7252  vattp
 v15:   xs-worker         79  timer
 v16:   xs-worker        751  packages/treasury/src/stablecoinMachine.js
 v17:   xs-worker          3  packages/pegasus/src/pegasus.js
 v18:   xs-worker        226  packages/zoe/src/contracts/multipoolAutoswap/multipoolAutoswap.js
 v19:   xs-worker          2  packages/treasury/src/liquidateMinimum.js
 v20:   xs-worker          2  packages/treasury/src/liquidateMinimum.js
 v21:   xs-worker          2  packages/treasury/src/liquidateMinimum.js
 v22:   xs-worker          2  packages/treasury/src/liquidateMinimum.js
 v23:   xs-worker          2  packages/treasury/src/liquidateMinimum.js

and here are the deliveries in block 55576:

["v15",["message","ko38",{"method":"getCurrentTimestamp","args":{"body":"[]","slots":[]},"result":"kp488548"}]]
["v10",["notify",[["kp488543",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD payment\",\"index\":0}","slots":["ko105230"]}}]]]]
["v10",["notify",[["kp488544",{"state":"fulfilled","refCount":1,"data":{"body":"{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD brand\",\"index\":0},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"51053409\"}}","slots":["ko116"]}}]]]]
["v16",["notify",[["kp488545",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"undefined\"}","slots":[]}}]]]]
["v16",["notify",[["kp488546",{"state":"fulfilled","refCount":1,"data":{"body":"{\"updateCount\":{\"@qclass\":\"undefined\"},\"value\":{\"@qclass\":\"undefined\"}}","slots":[]}}]]]]
["v16",["notify",[["kp488547",{"state":"fulfilled","refCount":1,"data":{"body":"{\"updateCount\":{\"@qclass\":\"undefined\"},\"value\":{\"@qclass\":\"undefined\"}}","slots":[]}}]]]]
["v13",["notify",[["kp488516",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD payment\",\"index\":0}","slots":["ko105229"]}}]]]]
["v18",["notify",[["kp488548",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"bigint\",\"digits\":\"1629767527\"}","slots":[]}}]]]]
["v18",["message","ko90",{"method":"handleOffer","args":{"body":"[{\"@qclass\":\"slot\",\"iface\":\"Alleged: InvitationHandle\",\"index\":0},{\"@qclass\":\"slot\",\"iface\":\"Alleged: zoeSeatAdmin\",\"index\":1},{\"initialAllocation\":{\"In\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD brand\",\"index\":2},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"51053409\"}},\"Out\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: RUN brand\",\"index\":3},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}}},\"notifier\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: notifier\",\"index\":4},\"offerArgs\":{\"@qclass\":\"undefined\"},\"proposal\":{\"exit\":{\"onDemand\":null},\"give\":{\"In\":{\"brand\":{\"@qclass\":\"slot\",\"index\":2},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"51053409\"}}},\"want\":{\"Out\":{\"brand\":{\"@qclass\":\"slot\",\"index\":3},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}}}},\"seatHandle\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: SeatHandleHandle\",\"index\":5}}]","slots":["ko105213","ko105231","ko116","ko71","ko105232","ko105233"]},"result":"kp488549"}]]
["v13",["notify",[["kp488520",{"state":"fulfilled","refCount":3,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: userSeat\",\"index\":0}","slots":["ko105234"]}}]]]]
["v10",["message","ko105234",{"method":"getPayout","args":{"body":"[\"In\"]","slots":[]},"result":"kp488521"}]]
["v10",["message","ko105234",{"method":"getPayout","args":{"body":"[\"Out\"]","slots":[]},"result":"kp488522"}]]
["v7",["notify",[["kp488542",{"state":"fulfilled","refCount":1,"data":{"body":"{\"quoteAmount\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: Quote brand\",\"index\":0},\"value\":[{\"amountIn\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD brand\",\"index\":1},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}},\"amountOut\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: RUN brand\",\"index\":2},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}},\"timer\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: timerService\",\"index\":3},\"timestamp\":{\"@qclass\":\"bigint\",\"digits\":\"1629767527\"}}]},\"quotePayment\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: Quote payment\",\"index\":4}}","slots":["ko105235","ko116","ko71","ko38","ko105236"]}}]]]]
["v10",["notify",[["kp488549",{"state":"fulfilled","refCount":1,"data":{"body":"{\"exitObj\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: exitObj\",\"index\":0},\"offerResultP\":{\"@qclass\":\"slot\",\"index\":1}}","slots":["ko105237","kp488550"]}}],["kp488550",{"state":"rejected","refCount":1,"data":{"body":"{\"@qclass\":\"error\",\"errorId\":\"error:liveSlots:v18#70090\",\"message\":\"rights were not conserved for brand (an object)\",\"name\":\"Error\"}","slots":[]}}]]]]
["v10",["message","ko105231",{"method":"fail","args":{"body":"[{\"@qclass\":\"error\",\"errorId\":\"error:liveSlots:v18#70089\",\"message\":\"rights were not conserved for brand (an object)\",\"name\":\"Error\"}]","slots":[]},"result":"kp488551"}]]
["v16",["notify",[["kp488538",{"state":"fulfilled","refCount":1,"data":{"body":"{\"quoteAmount\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: Quote brand\",\"index\":0},\"value\":[{\"amountIn\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD brand\",\"index\":1},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}},\"amountOut\":{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: RUN brand\",\"index\":2},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"0\"}},\"timer\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: timerService\",\"index\":3},\"timestamp\":{\"@qclass\":\"bigint\",\"digits\":\"1629767527\"}}]},\"quotePayment\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: Quote payment\",\"index\":4}}","slots":["ko105235","ko116","ko71","ko38","ko105236"]}}]]]]
["v5",["message","ko131",{"method":"withdraw","args":{"body":"[{\"brand\":{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD brand\",\"index\":0},\"value\":{\"@qclass\":\"bigint\",\"digits\":\"51053409\"}}]","slots":["ko116"]},"result":"kp488552"}]]
["v18",["notify",[["kp488551",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"undefined\"}","slots":[]}}]]]]
["v13",["notify",[["kp488522",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: RUN payment\",\"index\":0}","slots":["ko105238"]}}]]]]
["v10",["notify",[["kp488552",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD payment\",\"index\":0}","slots":["ko105239"]}}]]]]
["v13",["notify",[["kp488521",{"state":"fulfilled","refCount":1,"data":{"body":"{\"@qclass\":\"slot\",\"iface\":\"Alleged: BLD payment\",\"index\":0}","slots":["ko105239"]}}]]]]

(that may be slightly easier to read if you paste it into a file, then pipe it into jq or jq -c, which will colorized the property names)

@dckc
Copy link
Member

dckc commented Aug 24, 2021

Do we know what was being traded?

I tried to trade 30 RUN for 1.56041 Atom, @katelynsills

My full client log is attached:
amm-trade-failed.log

I also took a bunch of screenshots from

https://discord.com/channels/585576150827532298/819073555446759444/879760001051271258

to

https://discord.com/channels/585576150827532298/819073555446759444/879765930165563453

@dckc dckc changed the title Error#125: rights were not conserved for brand [object Alleged: RUN brand] AMM trades fail - Error#125: rights were not conserved for brand [object Alleged: RUN brand] Aug 24, 2021
@katelynsills
Copy link
Contributor

Ok, I found the bug.

TLDR: The stagedAllocations weren't being cleared after an offer safety error, so future offers were working with a poolSeat that already had a stagedAllocation (one corresponding to a previous offer). The contract code could fix this by clearing the staged allocations in case of an error, but we probably want Zoe to do it instead. We need to spend some time thinking through how we should clean up state after errors (offer safety, rights conservation, staged allocations not included in reallocate, etc).

The longer version: After looking manually at the transcript for the AMM vat, I was able to replicate the calls exactly in local, non-swingset tests (see #3759 for the tests).

What happens is:

  1. delivery 50004 (giving RUN and wanting BLD) results in an offer safety error because the allocation didn't satisfy the proposal want. This is a normal occurrence and we should expect many offer safety errors, because users get to chose their own want.
  2. delivery 50008, another swap but this time giving ATOM and wanting RUN, errors in zcf.reallocate with At least one seat has a staged allocation but was not included in the call to reallocate because the swap seat and poolSeat from 50004 have staged allocations but were not included in this reallocate.
  3. delivery 50012, giving BLD and wanting RUN, errors with rights were not conserved for brand "[Alleged: BLD brand]" because we are attempting to add and subtract more from the poolSeat's stagedAllocation, which still hasn't been cleared.

@dckc dckc transferred this issue from Agoric/testnet-notes Sep 7, 2021
@dckc dckc removed their assignment Sep 9, 2021
@katelynsills katelynsills added the Zoe package: Zoe label Sep 16, 2021
@Tartuffo
Copy link
Contributor

@Chris-Hibbert is this fixed?

@Chris-Hibbert
Copy link
Contributor

This is an instance of "Stateful seats and staged allocations" (#3850), which @erights is working on.

@Tartuffo Tartuffo added the MN-1 label Jan 20, 2022
@Tartuffo
Copy link
Contributor

@erights @Chris-Hibbert So is this a dup that should be closed, or should we leave it and #3850 open

@erights
Copy link
Member

erights commented Jan 20, 2022

Closing this while I continue work on #3850

@erights erights closed this as completed Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Zoe package: Zoe
Projects
None yet
Development

No branches or pull requests

7 participants