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

Loadgen vault close causes amountGiven or amountWanted must be greater than 0 #4114

Closed
mhofman opened this issue Nov 23, 2021 · 8 comments · Fixed by #4522
Closed

Loadgen vault close causes amountGiven or amountWanted must be greater than 0 #4114

mhofman opened this issue Nov 23, 2021 · 8 comments · Fixed by #4522
Assignees
Labels
AMM bug Something isn't working needs-design
Milestone

Comments

@mhofman
Copy link
Member

mhofman commented Nov 23, 2021

Describe the bug

After #4026 landed, the chain started to print errors in the stderr (in verbose mode) when the loadgen's create-vault task performs a vault close.

To Reproduce

Steps to reproduce the behavior:

  1. Checkout agoric-sdk on revision b605a6a or more recent,
  2. yarn && yarn build && make -C packages/cosmic-swingset
  3. make sure agoric is in the $PATH (yarn link-cli)
  4. Checkout the mhofman/report-stage-results branch of https://github.com/Agoric/testnet-load-generator/tree/mhofman/report-stage-results. The following all takes place in the testnet-load-generator directory.
  5. agoric install
  6. In first terminal: agoric start local-chain --verbose
  7. In second terminal, after chain reaches a few blocks in height: agoric start local-solo 8000 --verbose
  8. In a third terminal, after the client shows Deployed Wallet! in the output: yarn loadgen
  9. In a fourth terminal, after the loadgen is ready: curl -X PUT --data '{"vault":{"interval":600}}' http://127.0.0.1:3352/config
  10. During the vault task, the following error is thrown and printed in the chain output: amountGiven or amountWanted must be greater than 0: {"brand":"[Alleged: RUN brand]","value":"[0n]"} {"brand":"[Alleged: BLD brand]","value":"[0n]"}

Expected behavior

No error in output

Platform Environment

  • Debian based Docker image under WSL2
  • Not an usual environment it's not relevant to this bug
  • agoric-sdk revision b605a6a

Additional context

Strangely the close offer succeeds, and the error is never surfaced to the client making requests.

This is the relevant create-vault code: https://github.com/Agoric/testnet-load-generator/blob/834421d023adb036f09eeb5500326b6fe9ff2499/loadgen/contract/agent-create-vault.js#L91-L105

Screenshots

The relevant output of 2 cycles run before and after the PR landed: https://gist.github.com/mhofman/a07bc58efccd7cb42f9fb31814df8fe4

This is the deliveries resulting from the relevant loadgen code above:

[
  [ 211, '211:194:deliver:ro+85:makeCloseInvitation:rp-229;[]' ],
  [
    212,
    '212:194:deliver:ro+51:withdraw:rp-230:ro+48;[{"brand":{"@qclass":"slot","iface":"Alleged: RUN brand","index":0},"value":{"@qclass":"bigint","digits":"76708106"}}]'
  ],
  [
    213,
    '213:194:deliver:ro+32:offer:rp-231:rp-229:ro+48:ro+46:rp-230;[{"@qclass":"slot","index":0},{"give":{"RUN":{"brand":{"@qclass":"slot","iface":"Alleged: RUN brand","index":1},"value":{"@qclass":"bigint","digits":"76708106"}}},"want":{"Collateral":{"brand":{"@qclass":"slot","iface":"Alleged: BLD brand","index":2},"value":{"@qclass":"bigint","digits":"0"}}}},{"RUN":{"@qclass":"slot","index":3}}]'
  ],
  [ 214, '214:194:deliver:rp-231:getPayout:rp-232;["RUN"]' ],
  [ 215, '215:194:deliver:rp-231:getPayout:rp-233;["Collateral"]' ]
]
@mhofman mhofman added the bug Something isn't working label Nov 23, 2021
@Chris-Hibbert
Copy link
Contributor

I was unable to reproduce, and there was a suggestion that loadgen may not be useable on a Mac.

@mhofman
Copy link
Member Author

mhofman commented Nov 23, 2021

I've updated instructions to avoid the loadgen runner which relies on Linux procfs

@mhofman
Copy link
Member Author

mhofman commented Jan 5, 2022

FYI, I am still running into this error on master.

@Chris-Hibbert
Copy link
Contributor

That error message is generated in the AMM when it receives a swap request whose want and give are both zero. IIRC, the previous code silently did an empty trade instead of complaining.

I don't see why this would be prompted by a request to close the vault. (Closing the vault shouldn't cause any interaction with the AMM.)

Perhaps the loadgen task is checking the result of the previous transaction at that time?

@Chris-Hibbert
Copy link
Contributor

Ah, I see. swap is also called for price quotes. Lemme see if I can figure out who's asking for prices.

chain: Error#1: amountGiven or amountWanted must be greater than 0: {"brand":"[Alleged: RUN brand]","value":"[0n]"} {"brand":"[Alleged: BLD brand]","value":"[0n]"}
chain: Error: amountGiven or amountWanted must be greater than 0: (an object) (an object)
chain:  at construct ()
chain:  at Error (/workspaces/agoric/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:8833)
chain:  at makeError (/workspaces/agoric/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3103)
chain:  at fail (/workspaces/agoric/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3231)
chain:  at baseAssert (/workspaces/agoric/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3249)
chain:  at swap (#875:194)
chain:  at (#862:33)
chain:  at getInputPrice (#915:88)
chain:  at calcAmountOut (#913:31)
chain:  at (#611:280)
chain:  at createQuote (#913:40)
chain:  at quoteGiven (#611:278)
chain:  at ()
chain:  at ()
chain: 

@Chris-Hibbert
Copy link
Contributor

closeHook() calls updateUiState(). updateUiState() calls getCollateralizationRatio(), which asks the priceAuthority for the current value of the collateral (even when the collateral has been reduced to zero.) As I mentioned above, swap no longer gives a helpful answer when asked for the price of zero of the collateral. grumble

@mhofman mhofman removed their assignment Jan 20, 2022
@Tartuffo Tartuffo added the MN-1 label Feb 2, 2022
@Tartuffo
Copy link
Contributor

Tartuffo commented Feb 5, 2022

@Chris-Hibbert For proper project planning and tracking, this needs an area label covered by one of our weekly planning meetings. Please pick the appropriate one from: agd, agoric-cli, agoric-cosmos, amm, core economy, cosmic-swingset, endo, ertp, getrun, governance, installation-bundling, metering, oracle, pegasus, run-protocol, ses, staking, swingset, swingset-runner, tc39, token economy, tooling, ui, wallet, xsnap, zoe, zoe contract

@Chris-Hibbert Chris-Hibbert added Core Economy OBSOLETE in favor of INTER-protocol AMM and removed Core Economy OBSOLETE in favor of INTER-protocol labels Feb 7, 2022
@Chris-Hibbert
Copy link
Contributor

This is a bug in the AMM that is affecting loadgen.

@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
Chris-Hibbert added a commit that referenced this issue Feb 10, 2022
I thought no one needed to request zero prices, and so I threw in that
case.  #4114 reveals a
situation where it will happen in ordinary circumstances.

Change the AMM code to respond with an empty amount when an empty
amount is offered, and vice versa.
@mergify mergify bot closed this as completed in #4522 Feb 10, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMM bug Something isn't working needs-design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants