Implement on-chain liquidation

[prestwich]

Introduction

Liquidation is important to maintain the supply peg of TBTC. We currently have this contained in DepositLiquidation, where there are two mechanisms: auction and onchain liquidation. The onchain route is currently unimplemented in attemptToLiquidateOnchain, intended for instant liquidation of the signer bonds for TBTC.

Uniswap is an on-chain decentralized exchange which provides such 'instant liquidity' (for a price). It uses a constant product invariant as a pricing formula - we can preflight the purchase with this check to see if the ETH can buy enough tokens.

Implementation

This should be a fairly straightforward function calling Uniswap. It will require Uniswap's address and interface. https://docs.uniswap.io/smart-contract-integration/interface - note we have Uniswap deployed in #263.

--

Implementor: @liamzebedee
Feature buddy: @mhluongo, @gakonst , @nkuba, @pdyraga @Shadowfiend

[prestwich]

Use this to make sure a TBTC exchange exists:
tokenToExchange

[mhluongo]

Open question - how are we going to handle external dependencies like this locally and on our internal testnet? cc @NicholasDotSol @ngrinkevich @sthompson22

[ngrinkevich]

I think with npm like we do with open-zeppelin contracts? Looks like there's a package created already https://www.npmjs.com/package/@gnosis.pm/dx-uniswap-arbitrage

[ngrinkevich]

ah sorry no that package is something else, having a closer look now ...

[ngrinkevich]

so it's just two smart contract interface files that we would import into our contracts, I guess we can wrap it into a npm package

[mhluongo]

But we won't have Uniswap running on our testnet, so I'm not sure how we'll test on-chain liquidation? It seems like we need to deploy versions or stubs of external contract we plan to rely on

[NicholasDotSol]

I agree, not sure how we get around deploying dependencies like this locally

[NicholasDotSol]

There an older issue on deploying uniswap for testing
Uniswap/v1-contracts#12

[prestwich]

I would recommend a stub that implements the interface and has configurable behavior. I already wrote a few of these for other contracts for existing tests

[NicholasDotSol]

@prestwich Don't both share the issue of not very realistic tests? I'm assuming that with a deployed version there is the option of creating TXs to mimic market activity

[mhluongo]

Definitely the way we work outside tbtc is to get to closer testnet / mainnet parity and I'd like to continue that here. I'm down to start with a stub, but want to see a better solution on testnet before mainnet if we can. Splitting this issue into pieces scoped to milestones

[prestwich]

Don't both share the issue of not very realistic tests

the goal of the tests is to unittest the contract's behavior, not to integration test or e2e test. When it comes to outside interactions, realism is a downside. We need predictability so that we can test each code path

I'm not sure what we actually gain by using real code for any outside contract in tests

[mhluongo]

the goal of the tests is to unittest the contract's behavior

👍

not to integration test or e2e test.

We are interested in full integration tests, just not right now (#129)

[NicholasDotSol]

We need predictability so that we can test each code path

makes sense. can work with an interface stub for now for easier testing, and thank you @prestwich :)

As a side note for e2e testing:
When the time comes, non-deterministic tests should not be an issue with local development and can be avoided in testnet if needed

[liamzebedee]

We're assuming immediate liquidation through the uniswap exchange - is the auction out-of-scope for this issue?

[mhluongo]

You've got it, the auction is a separate piece of work. If Uniswap doesn't allow full liquidation, revert. We'll replace that case with the in-system auction.

[prestwich]

don't revert if uniswap doesn't allow it. I'm pretty sure uniswap has a pre-flight function, so just return false if uniswap doesn't have liquidity at the right price

…

On Sun, Jun 9, 2019, 13:21 Matt Luongo ***@***.***> wrote: You've got it, the auction is a separate piece of work. If Uniswap doesn't allow full liquidation, revert. We'll replace that case with the in-system auction. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#105?email_source=notifications&email_token=ACNN4MLJBCEYWBR4HPSBSPDPZVQ4DA5CNFSM4HIZOUO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXIRNHI#issuecomment-500242077>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNN4MMNHJV3WZGBN4VUGT3PZVQ4DANCNFSM4HIZOUOQ> .

[mhluongo]

It's a TODO 🤷‍♂️

[liamzebedee]

Uniswap is up-and-running, just did a trade TBTC<->ETH. There's an e2e Truffle test, as well as a whole lot of notes on the fine details of Uniswap (not a lot of docs there mates 😉!).

To clarify, do we want to just liquidate the entire lot of TBTC? Just thinking about the invariant and how the price will scale for different size liquidation events.

[mhluongo]

We want to buy up the full amount of TBTC to be burnt - so we're liquidating as much of the signer bond as is necessary for that to happen

[prestwich]

for more context, check out the way `startSignerLiquidation` is implemented (and `startSignerFraudLiquidation`). They call `attemptToLiquidateOnchain`, and then take action based on the result. If uniswap has sufficient liquidity, everything closes out, if not, we start the auction process happy to walk through this on a call sometime if it's helpful

…

On Tue, Jun 11, 2019, 12:40 Matt Luongo ***@***.***> wrote: We want to buy up the full amount of TBTC to be burnt - so we're liquidating as much of the signer bond as is necessary for that to happen — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#105?email_source=notifications&email_token=ACNN4MOUDLIPAD2ALHWRTQTPZ75TJA5CNFSM4HIZOUO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXOI2JY#issuecomment-500993319>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNN4MP4WVL7JJ7KLMLKRP3PZ75TJANCNFSM4HIZOUOQ> .

[mhluongo]

happy to walk through this on a call sometime if it's helpful

But we have such beautiful docs!

The primary goal of the liquidation process is to bring the TBTC supply in line with the BTC custodied by Deposits. The most valuable asset held by the system is the signers' bonds. Therefore, the liquidation process seizes the signers bonds and attempts to use the bonded value to purchase and burn TBTC.

First, the contract attempts to use on-chain liquidity sources, such as Uniswap. If the bond is sufficient to cover the outstanding TBTC value on these markets, it is immediately exchanged for TBTC.

Second, the contract starts a falling-price auction. It offers 75% of the signer bond for sale for the outstanding TBTC amount. The amount of bond on sale increases over time until someone chooses to purchase it, or the auction reaches 100% of the bond. The auction will remain open until a buyer is found.

[mhluongo]

@liamzebedee remember that some of the TBTC minted goes back to the Deposit in escrow for the signers!

[liamzebedee]

attemptToLiquidateOnchain now fully functional, liquidating using a privately deployed Uniswap instance.
Now I'm on to testing the abort/fraud liquidation flows, and I was looking to clarify. startSignerFraudLiquidation and startSignerAbortLiquidation are pretty much the same, if not for this block in the former:

if (_d.auctionTBTCAmount() == 0) {
        // we came from the redemption flow
        _d.setLiquidated();
        _d.requesterAddress.transfer(_seized);
        _d.logLiquidated();
        return;
}

Is there a reason why this isn't in the abort flow? ie. in the scenario when a requestor calls requestRedemption, it burns their TBTC, but if the signers don't produce a sig, that's considered an abort rather than fraud? And if so, then this should belong in the abort flow (otherwise the requestor loses out ).

You seem to be right here, good catch.

Actually maybe I spoke to soon here. Our docs say:

For example, should the signers fail to produce a redemption signature in a timely manner, their bonds are liquidated to protect the supply peg, but any remainder is returned to them.

What's our approach?

[prestwich]

Actually maybe I spoke to soon here. Our docs say:

For example, should the signers fail to produce a redemption signature in a timely manner, their bonds are liquidated to protect the supply peg, but any remainder is returned to them.

What's our approach?

that section of the docs is describing what to do with excess ETH after liquidating. It's handled in the if(_liquidated) block in both abort and fraud.

Whether fraud or abort, if we came from redemption the redeemer has already burnt 1 TBTC. So there should be code that ensures they receive 1 TBTC (or >1 TBTC in some mix of TBTC and ETH).

Come to think of it, we should make attemptToLiquidateOnChain try to liquidate for 1 TBTC, and then send that to the redeemer.

[liamzebedee]

TBTC received during this process is burned to maintain the supply peg. If any bond value is left after liquidation, a small fee is distributed to the account which trigger liquidation. After that, any remaining value is either distributed to the signers (in case of liquidation due to undercollateralization) or burned (in case of liquidation due to fraud).

Can't see anything in DepositLiquidation that transfers such a fee yet 😉 shall I add it?

Whether fraud or abort, if we came from redemption the redeemer has already burnt 1 TBTC. So there should be code that ensures they receive 1 TBTC (or >1 TBTC in some mix of TBTC and ETH).

Sounds about right. Is this our preferred UX @mhluongo ? We'll need to retry in the dapp (can't remember, who is on this?)

Come to think of it, we should make attemptToLiquidateOnChain try to liquidate for 1 TBTC, and then send that to the redeemer.

💯

Currently have something like so:

        uint ethSold = address(this).balance;
        uint ONE_TBTC = (10 ** 8);

        uint MIN_TBTC_BOUGHT = ONE_TBTC * 500 / 1000;

        uint tbtcBought = exchange.getEthToTokenInputPrice(ethSold);
        if(tbtcBought < MIN_TBTC_BOUGHT) {
            return false; // TODO go to auction
        }

Defensive programming but we shouldn't always assume the Uniswap pool is the best option for money?

[prestwich]

TBTC received during this process is burned to maintain the supply peg. If any bond value is left after liquidation, a small fee is distributed to the account which trigger liquidation. After that, any remaining value is either distributed to the signers (in case of liquidation due to undercollateralization) or burned (in case of liquidation due to fraud).

Can't see anything in DepositLiquidation that transfers such a fee yet shall I add it?

yeah, I think it got left out of implementation as it hasn't been parameterized at all. @mhluongo did we ever decide whether this should be in ETH or just in governance token?

Whether fraud or abort, if we came from redemption the redeemer has already burnt 1 TBTC. So there should be code that ensures they receive 1 TBTC (or >1 TBTC in some mix of TBTC and ETH).

Sounds about right. Is this our preferred UX @mhluongo ? We'll need to retry in the dapp (can't remember, who is on this?)

The more I think about it, the more I think we should return TBTC. Which is to say, as implemented now, the requestor will get either BTC or ETH in the failure case. It might be better if the requestor got either BTC or TBTC back. Getting TBTC back seems better for the vast majority of users

[liamzebedee]

I agree with returning tBTC. Fraud/abort we are striving against, but in the case it does happen, we are feeding this signal back into the TBTC price through having to liquidate and buyback TBTC to refund. Escrowing it would complicate, and since the user is specifically requesting for BTC, why give them ETH.

[liamzebedee]

There's a full test passing for startSignerFraudLiquidation, based on the design above. @prestwich would love to get your eyes on this, mostly all in DepositLiquidation. To summarise:

• attemptToLiquidateOnchain now trys to buy up TBTCConstants.getLotSize().add(DepositUtils.beneficiaryReward()); or fails. I'm making an assumption that our low boundary for collateralisation (125%) will be enough.
• refund the beneficiary and requestor in TBTC
• send remaining ETH to requestor (for now, could also burn it)

Haven't implemented the abort flow yet.

[mhluongo]

send remaining ETH to requestor (for now, could also burn it)

FWIW I'm in the "never burn what you don't need to" design camp- that could be profit for a maintainer, or hell it could go to a dev fee.

[liamzebedee]

Haha definitely, here to build bridges not burn 'em. We're liquidating a bond of ~125% - 3% uniswap fee - 0.05% beneficiary reward - 0.x% re-redemption costs, so that leaves ~22% the value left. Since we're going for peer review, maybe we should iron out this detail this week? Can allocate it to requestor in the meantime, but 22% is a bit much IMO (even with the bond value falling further + the cost of redoing redemption).

[mhluongo]

but 22% is a bit much IMO

Is it? If the price falls fast enough it'll be 0% 😃

After buying back TBTC a fixed portion should go to the maintainer. After that, return the rest to the signers. We don't give signers a straightforward way to "maintain their account" so punishing them by yanking excess bond is unreasonable.

[liamzebedee]

Truuee. 🤠

But let's refund the beneficiary first and foremost? Then the requestor/redeemer, then maintainer, then signer if it's not fraud.

[mhluongo]

Heh sorry, didn't mean to forgot the beneficiary

We need to get this piece heavily documented as well- I want to make sure all participants know the failure case behavior.

[pdyraga]

Remaining estimate: 3 days

[Shadowfiend]

@liamzebedee regarding testing, it seems like the higher level testing flows are best managed as a separate issue; how do we feel about testing around on-chain liquidation in particular? Is #265 meant to sufficiently cover that piece?

[liamzebedee]

Closed with #320. Other work that happened here around testing liquidation flows (#319) and some protocol discussion (#325) has been broken out