There should be an existing _agstate directory inside the repo of an Agoric project

We should be able to remove liquidity tokens (locked by the user) from the AMM.

To call a method from the AMM contract from our stopLoss contract we should use offerTo method like here.

We migrate our code to work on https://github.com/Agoric/agoric-sdk/commit/fedf049435d7307311219fbab1b2b342ec6acce8 this commit

Replace 'assertProposalShape' for an appropriated substitute.

SDK Version

We're now based on the beta version, should we migrate to master?

Price Manipulation

Context

The acceptance criteria is that this bounty should be able to work with the existing AMM. A proper demonstration scenario we discussed with @rowgraus is that we deploy our stopLoss contract to the local-chain and interact with it. We're mostly focused on this.

Problem

In order to demonstrate the removal of the LP tokens locked inside the contract, we need to move the price above or below the boundaries specified by the user. How can we achieve that when interacting with the existing AMM where the Central token brand is RUN or IST(depending on if we migrate to master or not)?

Test Case

Context

We make use of mutableQuoteWhenGT and mutableQuoteWhenLT methods to know when a boundary is hit. Like;

  // Get mutable quotes
  /** @type MutableQuote */
  const upperBoundryMutableQuote = E(fromCentralPriceAuthority).mutableQuoteWhenGT(upper.denominator, upper.numerator);
  /** @type MutableQuote */
  const lowerBoundryMutableQuote = E(fromCentralPriceAuthority).mutableQuoteWhenLT(lower.denominator, lower.numerator);

  // Get promises from mutable quotes
  const upperBoundryMutableQuotePromise = E(upperBoundryMutableQuote).getPromise();
  const lowerBoundryMutableQuotePromise = E(lowerBoundryMutableQuote).getPromise();

  const watchBoundries = async () => {
    const quote = await Promise.race([ upperBoundryMutableQuotePromise, lowerBoundryMutableQuotePromise ]);
    boundryPromiseKit.resolve({ code: BOUNDRY_WATCHER_STATUS.SUCCESS, quote });
  };

  watchBoundries().catch(error => boundryPromiseKit.resolve({ code: BOUNDRY_WATCHER_STATUS.FAIL, error }));

And the relationship between the stopLoss contract and boundaryWatcher module is like;

sequenceDiagram
    participant c as Creator
    participant s as stopLoss
    participant b as boundaryWatcher
    c ->>+ s: startInstance
    s ->>+ b: makeBoundaryWatcher
    b -->>-s: { boundaryWatcherPromise, updateBoundaries }
    s -->>-c: { creatorFacet: { updateBoundaries } } 

We want to test the below case

sequenceDiagram
    participant U as User
    participant Ct as Creator
    participant C as Contract
    Ct ->>+ C: Creator starts the contract
    C ->> C: checks the liquidityIssuer provided actually exists in AMM
    Note over C,Ct: Promises from MutableQuote are scheduled
    C -->>- Ct: { creatorFacet, publicFacet }
    U ->> C: Locks LP tokens to the contract
    C->>C: One of the promises reject
    C->>U: Notify user about the error
    U->>+C: Withdraw LP tokens from contract
    C-->>-U: LP tokens received  

We used different actors for User and Creator to separate the roles, in real life those two might be the same person or entity

In order to make the above test case happen, one of the promises from Context section, upperBoundryMutableQuotePromise and lowerBoundryMutableQuotePromise, has to reject. We can make this happen by using the cancel() method of either upperBoundryMutableQuote or lowerBoundryMutableQuote and this would require updating the relationship between stopLoss and boundaryWatcher to this;

sequenceDiagram
    participant c as Creator
    participant s as stopLoss
    participant b as boundaryWatcher
    c ->>+ s: startInstance
    s ->>+ b: makeBoundaryWatcher
    b -->>-s: { boundaryWatcherPromise, updateBoundaries, cancel }
    s -->>-c: { creatorFacet: { updateBoundaries, cancel } } 

Problem

We're hesitant to expose a powerful function like cancel() to the outside world even if it's the creatorFacet. If you agree with us on this, now we have a problem with making the promises we receive from mutableQuote objects reject because the priceAuthority we use is controlled from inside the AMM. We thought about a work around where we inject a manualPriceAuthority thorough the contract terms just for testing this case and do a E(priceAuth).setPrice(undefined) to force mutableTrigger throw an error and therefore reject the promise we are waiting for to resolve in the watchBoundries method. This approach introduced some boilerplate complexity to make the above test case happen. We would love hear engineering team's opinion on that.

Possible edge cases that we may encounter:

• The user decides to update their boundaries in a way that either the lower boundary is greater than the current price or the upper boundary is lower than the current price.
  Agoric/agoric-sdk#5522 (comment)
• The user initiate the contract with the the current price ratio outside of defined boundaries.
• stopLoss contract is not able to remove liquidity from AMM
• User deliberately decide to remove its liquidity from the AMM without the price ratio reached the defined boundaries.
• User increase its liquidity in the same AMM pool, should he be able to lock more LP tokens in the contract.
• User deliberately decide to remove its LP tokens locked in the contract.

We need to import multipoolMarketMaker.js from the @agoric package instead of using an absolute path

In order to test stopLoss contract behaviour when facing a issue while removing liquidity from the amm in exchange for the previously locked LP tokens a test was created.

To force a failure in 'removeLiquidityFromAmm' function, the folowing steps were made:

• an Manual Price Authority was provided in the terms to stopLoss instead of the 'ammPublicFacet';
• a range of boundaries and pool price were established;
• an amount of LP tokens were locked in stopLoss;
• the pool price was updated to trigger the 'removeLiquidityFromAmm'

The 'removeLiquidityFromAmm' requires the 'ammPublicFacet' to get access to the 'removeLiquidityInvitation'. Since this is not provided, it will return an unresolved Promise.

const removeLiquidityFromAmm = async () => {
    const removeLiquidityInvitation =
      E(ammPublicFacet).makeRemoveLiquidityInvitation();

    const lpTokensLockedAmount = stopLossSeat.getAmountAllocated(
      'Liquidity',
      lpTokenBrand,
    );

    const proposal = harden({
      want: {
        Central: AmountMath.makeEmpty(centralBrand),
        Secondary: AmountMath.makeEmpty(secondaryBrand),
      },
      give: {
        Liquidity: lpTokensLockedAmount,
      },
    });

    const { deposited, userSeatPromise: liquiditySeat } = await offerTo(
      zcf,
      removeLiquidityInvitation,
      undefined,
      proposal,
      stopLossSeat,
    );
    
    const [amounts, removeOfferResult] = await Promise.all([deposited, E(liquiditySeat).getOfferResult()]);
    tracer('Amounts from removal', amounts);

    updateAllocationState(ALLOCATION_PHASE.REMOVED);

    return removeOfferResult;
  };

This will return an error, that was handled based on the information gathered on the following Agoric Issue.
test(zoe): offerTo handling of broken invitation #5391
To catch the error, the following statement was included in the 'removeLiquidityFromAmm'

  try {
      await E(liquiditySeat).getOfferResult();
    } catch (error) {
      updateAllocationState(ALLOCATION_PHASE.ERROR);
      tracer('removeLiquidityFromAmm encounted an error: ', error);
      return
};

Which will return the following error when tested:

----- StopLoss.4  3 removeLiquidityFromAmm encounted an error:  (Error#1)
Error#1: A Zoe invitation is required, not Promise {
  <rejected> [TypeError: Cannot deliver "makeRemoveLiquidityInvitation" to target; typeof target is "undefined"]
}
  at onRejected (packages/zoe/src/zoeService/invitationQueries.js:10:26)

The Issue identified on this test is that, when the offerTo promise is rejected, the LP tokens used in the proposal are not returned to the stopLoss seat, neither the liquidity that is trying to be removed from the amm.

 Liquidity: { brand: Object [Alleged: SCRLiquidity brand] {}, value: 0n },
  Central: { brand: Object [Alleged: Central brand] {}, value: 0n },
  Secondary: { brand: Object [Alleged: Secondary brand] {}, value: 0n }
}

The information above is obtained using stopLossSeat.getAmountAllocated(...)

Do the following updates:

• removeLiquidityFromAmm needs to be invoked on "makeWithdrawLiquidityInvitation";
• removeLiquidityFromAmm should be removed from creatorFacet;
• "Test remove Liquidity from AMM" should be removed;
• "Test withdraw Liquidity" should be updated;

We should know when to remove LP tokens. We have a lower and a upper boundary that we should trigger removal if prices exceeds either of those. The initial strategy is to use priceAuthority from the AMM pool but we might switch to the subscriber returned from the pool as well if encounter a problem that blocks us using priceAuthority.

Below are the test cases that are tested or will be tested

• Lock LP tokens
• Lock additional LP tokens
• Withdraw locked LP tokens
• Withdraw liquidity
• Trigger removal by moving price above upper boundary
• Trigger removal by moving price below lower boundary
• Update lower boundary to be lower than it was, move price below first lower boundary, should not trigger removal
• Update upper boundary to be higher than it was, move price above first upper boundary, should not trigger removal
• Update upper boundary to be higher than it was, move price above first upper boundary, should not trigger removal move price above second upper boundary, should trigger removal
• Update lower boundary to be lower than it was, move price below first lower boundary, should not trigger removal move price below second lower boundary, should trigger removal
• Update lower boundary to be lower than it was and upper boundary higher than it was, move price below first lower boundary, should not trigger removal move price above second upper boundary, should trigger removal
• BoundryWatcher fails when there's no lp tokens locked yet then stopLoss state should turn to error
• BoundryWatcher fails when there's lp tokens locked, stopLoss state should turn error then user should be able get their lp tokens back
• Boundaries are updated to a range outside of current amm price ratio, should trigger removal.
• User initiate contract with boundaries range outside of current amm price ratio, should not initiate the contract and return an error
• Remove liquidity from AMM fails when user tries to withdraw his assets, LP tokens should remain in the contract.
• Move amm secondary price down.
• Move amm secondary price up.
• Move amm secondary price down, then up.
• Move amm secondary price up, then down.

Agoric projects need to have _agstate directory in order to make agoric install work

Cannot deploy stopLoss.js in unit tests

In order to demonstrate the functionality requested we should be able to deploy to a local-chain

Challenges

Chain Economy

AMM is considered a part of the chain economy and should be bootstrapped separately when a chain is started.
We're on the beta branch of the agoric-sdk, in this version the cosmic-swingset package does not support the chain economy. Check out the differences of the Makefile between the master and beta branches.

• Makefile beta
• Makefile master

Dan Connolly from Agoric once told on Discord that if copy/paste contents of Makefile from master to the beta it might just work.

Other approaches

• Copy and paste the scenario2-setup part of master's Makefile into our own repository then run the local-chain from our own repo.
• Migrate to master