agoricdev-7: Error when opening vaults

[samsiegart]

Describe the bug

When trying to open a vault via https://treasury.agoric.app on agoricdev-7, the following error occurs: Error: rights were not conserved for brand (an object)

Possibly related to #3850

To Reproduce

Steps to reproduce the behavior:

1. Follow the steps to set up a wallet with docker-compose on the current devnet (agoricdev-7)
2. Go to https://treasury.agoric.app, approve the dapp in your wallet, and try to open a vault
3. Accept the offer in your wallet
4. See error (screenshot below)

Expected behavior

The vault is opened and RUN is received with no error

Platform Environment

agoricdev-7 via docker (https://github.com/Agoric/agoric-sdk/wiki/Setting-up-an-Agoric-Dapp-Client-with-docker-compose)

Additional context

The vaults work when running the sdk locally with the latest changes, just not on the current devnet release. I tried increasing the collateralization percent when opening the vault but still experienced the issue.

Screenshots

[dckc]

Can you get a stack trace out of your ag-solo as well as a gui error message?
We might need to get it from logs of one of the validators.

Or as you noted, try 2b0ce50 outside docker / devnet.

[samsiegart]

Can you get a stack trace out of your ag-solo as well as a gui error message?

The console in the wallet doesn't show an error, and the treasury console just shows an unhelpful CapTP undefined.0 exception message. These are the logs from my docker ag-solo. You can see the "rights not conserved" error as well as "error depositing payment", which always shows up even when things seem to work fine.

[samsiegart]

Or as you noted, try 2b0ce50 outside docker / devnet.

Just tested 2b0ce50 locally and it works fine with https://treasury.agoric.app/, so there's something specific to the devnet causing the error

[samsiegart]

My two ideas for what's wrong are:

• Liquidity is exhausted in devnet
• Some type of memory leak i.e. simple exchange should handle more orders than fit in the heap #4324

[tgrecojs]

Wondering if this is a duplicate of Agoric/dapp-treasury#47.

In that instance i'm pretty sure i'm running the chain locally, but I'm almost certain i've run into this issue on devnet. Can you confirm that you have RUN in your wallet's Zoe Fees Purse?

[samsiegart]

Wondering if this is a duplicate of Agoric/dapp-treasury#47.

In that instance i'm pretty sure i'm running the chain locally, but I'm almost certain i've run into this issue on devnet. Can you confirm that you have RUN in your wallet's Zoe Fees Purse?

Yep I have RUN in the fee purse, and I did try overcollateralizing to be safe to no avail.

[tgrecojs]

I'm going to try and submit a bug report on hackerone when I have a moment, but last night I managed to reproduce this and actually run into some other issues.

Not sure how helpful it is but below is a quick screengrab i took last night.

Screenflick.Movie.15.mp4

In that instance, the offer actually succeeds and I get the USDC requested, but this error still appears in the stack trace. 🤔 Hope it's useful info!

[michaelfig]

@samsiegart, here is the stack trace I found on-chain from when you get the conservation error:

7865]: Logging sent error stack (Error#7)
7865]: Error#7: rights were not conserved for brand "[Alleged: RUN brand]"
7865]: Error: rights were not conserved for brand (an object)
7865]:  at construct ()
7865]:  at Error (/usr/src/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:8833)
7865]:  at makeError (/usr/src/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3103)
7865]:  at fail (/usr/src/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3231)
7865]:  at baseAssert (/usr/src/agoric-sdk/packages/SwingSet/src/kernel/vatManager/lockdown-subprocess-xsnap.js:3249)
7865]:  at (#556:82)
7865]:  at forEach ()
7865]:  at assertEqualPerBrand (#556:78)
7865]:  at assertRightsConserved (#556:100)
7865]:  at reallocate (#542:168)
7865]:  at reallocateReward (#561:105)
7865]:  at openLoan (#967:460)
7865]:  at ()

Looking at reallocateReward as of the agoricdev-7 tag leads to packages/treasury/src/stableCoinMachine.js:

  const reallocateReward = (amount, fromSeat, otherSeat = undefined) => {
    rewardPoolSeat.incrementBy(
      fromSeat.decrementBy(
        harden({
          RUN: amount,
        }),
      ),
    );
    if (otherSeat !== undefined) {
      zcf.reallocate(rewardPoolSeat, fromSeat, otherSeat); // THIS LINE
    } else {
      zcf.reallocate(rewardPoolSeat, fromSeat);
    }
  };

the above-marked THIS LINE is where the error is thrown.

@dtribble can you PTAL at this when you have a chance? I don't know who else is available right now that understands the stablecoin machine.

[Chris-Hibbert]

rights were not conserved wouldn't come up if there were insufficient liquidity. That error means that assets were added to or removed from one seat without a corresponding adjustment to one of the other seats.

The stack trace tells me that this is called from line 462 of vault.js. mintGains has just done a reallocation (line 456). seat shouldn't have any staged allocations at the start of the method. vaultSeat won't have any after mintGains. rewardPoolSeat won't have any stagings, unless a prior transaction failed. All the stagings are created by balanced increments and decrements of the three seats that are passed in.

I don't see what could do this.

[michaelfig]

I don't see what could do this.

Can you take another look at the handling of stagings that are empty/zero values? We'd seen problems in Zoe around this edge case before.

[Chris-Hibbert]

From what I see, empty stagings should be handled fine. the rights conservation check adds up all the amounts aggregated by brand regardless of keyword from both allocations and stagings of all the named seats. Empty amounts should aggregate as zeros.

The error that I remember happening previously with staging had to do with a mintGains() call that failed, poisoning the pool of stagings and making progress impossible. In openLoan(), we've just made a successful call to mintGains(), so we believe that there are no outstanding stagings.

The one helpful thing I can think of to do is to add logging where rights were not conserved for brand is thrown (in assertEqualPerBrand) to show the left and right values. If they're different (as AmountMath.isEqual() is saying), then we should be able to determine whether the total before or after is wrong.

[michaelfig]

The one helpful thing I can think of to do is to add logging where rights were not conserved for brand is thrown (in assertEqualPerBrand) to show the left and right values. If they're different (as AmountMath.isEqual() is saying), then we should be able to determine whether the total before or after is wrong.

Is there a way you could easily add this (I'd be happy to review the PR) so that it gets into the devnet release next week?

[Chris-Hibbert]

When does the release get cut? I should be able to do it tomorrow.

[michaelfig]

When does the release get cut? I should be able to do it tomorrow.

Tomorrow is fine. I'm making motions towards a release on the weekend or Monday.

Oh, and just to confirm, the logging around this exception needs to be arguments to the assert that throws it. console.log and friends will be lost on chain.

[Tartuffo]

@samsiegart Can you please try to reproduce this and get a Zoe stack trace? Then we can determine who should take it from there.

[samsiegart]

Closing because this is from agoricdev-7. If we're not able to open vaults in the latest devnet when expected then we can create another issue (with the relevant stack traces that @Chris-Hibbert improved in the linked PR).