search

sidestream-tech / unified-auctions-ui

Unified MakerDAO auctions
https://unified-auctions.makerdao.com
GNU Affero General Public License v3.0
16 stars 13 forks source link

Investigate surplus auctions logic #188

Closed valiafetisov closed 2 years ago

valiafetisov commented 2 years ago

Goal

Step-by-step explanation how it works, implementation proposal

Context

Besides collateral auctions that are already supported by the UI and the keeper, we also want to support Maker surplus auctions (or flap auctions using maker terminology). It is currently supported by the https://auctions.makerdao.com/flap platform (source code), but our UI should eventually support all auction types. There are other projects like flap-auctions-api that may implement desired functionality in other languages that can also be used for reference.

The main goal of the issue is to investigate what smart contracts are responsible for those types of auctions, how existing tools deal with it. This should be done by documenting it in the issue using english terms (instead of maker terms).

For the example of this kind of research issue, you can have a look at the recent Implement wallet-related logic issue https://github.com/sidestream-tech/unified-auctions-ui/issues/124 and its investigation result https://github.com/sidestream-tech/unified-auctions-ui/issues/124#issuecomment-1064277892 as well as the PR https://github.com/sidestream-tech/unified-auctions-ui/pull/143 itself.

Tasks

KirillDogadin-std commented 2 years ago

Here're some (ongoing) investigation records

contract name: MCD_FLAP https://github.com/makerdao/auctions-ui/blob/master/constants.js#L39

Probably will need the following fucntions in the core for the feature:

additional functions that might be related.

https://github.com/makerdao/auctions-ui/blob/master/hooks/useAuctionActions.js#L46-L62 https://github.com/makerdao/auctions-ui/blob/master/hooks/useAllowances.js#L86-L96

So far I did not manage to figure out how the auth should look like in the upcoming implementation but just to keep track, here's some relevant info:
(auth?) https://github.com/makerdao/auctions-ui/blob/master/hooks/useAllowances.js#L190-L204 (auth?) https://github.com/makerdao/auctions-ui/blob/master/hooks/useAllowances.js#L19

misc: provider misc: Accroding to https://github.com/makerdao/flap-auctions-api/blob/04d5250e107d7c2263968e1fec39ee38d57745f6/flap_auctions/events_extractor.py#L79-L111 we might need to react to events:

KirillDogadin-std commented 2 years ago

source

auth required

valiafetisov commented 2 years ago

Main source of information: latest makerdao/dss/src/flap.sol with rate limiting introduced ~3 months ago

Investigation

There is also a possibility to return previous bid and delete auction by calling flap.yank(id) only in case of shutdown

valiafetisov commented 2 years ago

Should we include functionality to start surplus auction within the UI already? I can see few pros/cons:

My proposal is to start without kick functionality, at least not in the UI, to limit the scope, but I would try to keep it in mind for the future.

@LukSteib, what do you think?

LukSteib commented 2 years ago

bids[id].end – final time when auction will end (now + flap.tau() – 2 days atm) reset expiration time to the new value (bids[id].tic = now + flap.ttl(), 3 hours atm)

I think these parameters have been updated in the past and based on https://etherscan.io/address/0xa4f79bc4a5612bdda35904fdf55fc4cb53d1bff6#readContract are currently set to

In case there were no bids (ie bids[id].tic is 0), the auction will expire after bids[id].end time

I don't get the first part of this statement. How does tic (ie. the bid expiry) provide any insights on whether there have been any bids.

MKR collateral authorization is required to bid (tend) but doesn't seem to be required to start auction (flap/kick)

I don't understand the initial state after an auction has been started (flap/ kick), yet. If the bids.[id].bid is 0 does it mean that one could actually make the initial tend with a bid of 0, since beg applied to 0 would result in 0?! 🤯

MRK allowance (TODO: check, since we need MKR to bid, this may be required to move MRK to VAT before bidding)

question is whether MKR needs to be deposited to the VAT first in order to call tend or if this is an atomic operation

DAI allowance to DaiJoin (TODO: check when and why, since no DAI suppose to be withdrawn, but MKR)

the winner of an auction is actually supposed to withdraw DAI from VAT, right? I am wondering whether an allowance needs to be set for DAI since we have learned in our Dai bidding flow that allowance only needs to be set for depositing of DAI but not for withdrawal.

My proposal is to start without kick functionality, at least not in the UI, to limit the scope, but I would try to keep it in mind for the future.

Agree to your proposal. Let's not include this functionality in to the UI right away. However I like the mentioned testing scenario via hardhat (even though we might test without the recently introduced lid parameter for now). However I am still lacking the imagination on how we will be able to manipulate hump on hardhat fork in order to meet requirements to start flap auction.

valiafetisov commented 2 years ago

I think these parameters have been updated

Thanks for checking it out!

In case there were no bids (ie bids[id].tic is 0), the auction will expire after bids[id].end time

I don't get the first part of this statement. How does tic (ie. the bid expiry) provide any insights on whether there have been any bids.

Very good question. According to the code, kick method doesn't set tic whatsoever – so when the auction is started it will not expire quickly after 1800 seconds. Later on, they reuse same variable to check if current bid is a "real bid" or "initial kick".

does it mean that one could actually make the initial tend with a bid of 0

The second bid value supplied to tend should be greater than previous one, so it can't be equal to 0, but can be as little as 1 * 10-18 MKR.

question is whether MKR needs to be deposited to the VAT first in order to call tend or if this is an atomic operation

Yes, it should be deposited. The methods to move DAI and MRK around are vat.move and gem.move – so everything is happening within VAT.

the winner of an auction is actually supposed to withdraw DAI from VAT, right?

That's correct, but initial investigation of the transaction executed by clicking auth buttons in the existing UI lead to this conclusion. That's why there is still a todo. My speculation would be the same, we only need DAI auth, MRK auth and MRK allowance.

I am still lacking the imagination on how we will be able to manipulate hump on hardhat fork

Either directly changing a particular state via hardhat_setStorageAt (see intro), or indirectly, by impersonating an account which have rights to call some auth-protected methods, like grab.

valiafetisov commented 2 years ago

General ideas:

Implementation suggestion

KirillDogadin-std commented 2 years ago

Make sure it's possible to call collateral authorization and collateral allowance functions (and also ones that fetch their statuses) with MKR as a parameter Make sure it's possible to call balance and VAT balance with MRK, deposit and withdrawal

could you elaborate on what you imply by "call with MKR", please?

valiafetisov commented 2 years ago

Currently, we already have functions to interact with gemLike contracts. For example withdrawCollateralFromVat which accepts collateralType as input. But MKR is not a "collateral type" in our internal sense and can't be added to the list of collaterals. So you either need to modify withdrawCollateralFromVat to accept MKR (which would require a lot of refactoring and renaming) or simply copy existing logic into withdrawMRKFromVat and modify it to work with MKR. Looking at the code, I actually think second option would make more sense.

zoey-kaiser commented 2 years ago

As I needed to create the first types for the frontend issues (#306 #305) I currently updated the type mocking to:

export type SurplusAuctionStates =
    | 'just-started'
    | 'have-bids'
    | 'ready-for-collection'
    | 'collected'
    | 'requires-restart';

export type SurplusAuctionEventTypes = 'start' | 'bid' | 'collect';

export declare interface SurplusAuctionData {
    bidAmountMKR?: BigNumber;
    receiveAmountDAI: BigNumber;
    receiverAddress: string;
    auctionEndDate: Date;
    bidEndDate?: Date;
    earliestEndDate: Date;
    state: SurplusAuctionStates;
}

export declare interface SurplusEvent extends SurplusAuctionData {
    type: SurplusAuctionEventTypes;
    address: string;
    transactionHash: string;
    transactionDate: Date;
}

export declare interface InitialSurplusAuction extends SurplusAuctionData {
    id: number;
    network: string;
    events: SurplusEvent[];
}

export declare interface SurplusAuction extends InitialSurplusAuction {
    marketUnitPrice: BigNumber;
    highestBid?: BigNumber;
    auctionPrice?: BigNumber;
}
KirillDogadin-std commented 2 years ago

Regarding the block of the external code that reacts to 3 types of events:

apparently, with ethers it's not straighforward (mb even not really possible) to get the similar result.

As verbally discussed, I've looked into the possibility of getting the transactions list that is relevant to the specific contract. Due to me still not having the complete understanding of the underlying processess, i'd like to get the external input to the picture.

Discoveries

Then, according to https://github.com/makerdao/dss/blob/master/src/dai.sol (and essentially https://github.com/makerdao/dss/blob/master/src/flap.sol)

Does this base idea and concept for implementation?

valiafetisov commented 2 years ago

General context that I think is not mentioned here: contracts actually doesn't emit events on tend and deal, therefore proposed implementation is not directly feasible. But before significantly changing it, I suggested to check if it's still possible to subscribe to transactions instead of events and parse necessary data from them (just like it's done in the python api)

Does this base idea and concept for implementation?

I think there are two types of things: events that are explicitly emitted and transaction receipts, which can contain "events" or may not. Technically, I suppose, there should be a way to subscribe to all transactions on a specific address and parse the transaction data to determine required data (type of the method that was called, )

I would start this investigation by finding previous flap auctions and seeing what actually happening there

KirillDogadin-std commented 2 years ago

examples from prev versions of flapper:

valiafetisov commented 2 years ago

Ethers.js have two different apis for event subscriptions: provider.on and contract.on – we wouldn't want to use the provider, otherwise we would need to parse every single mined block. It would also not allow us to fetch previous events (otherwise we would need to retrieve all blockchain blocks to find particular event). Using pending pool is also problematic, since we only want to fetch confirmed transactions (transactions under a few other blocks), but pending transactions are not even mined yet. Therefore, we need to use contract.on and fetch the events not explicitly emitted, but recorded through the transaction log.

pymaker library first fetches events via getLogs filtered by the contract address, then parses useful events using their signatures.

Thanks for finding previous flap contract address! Can please you try to fetch raw events from the previous flap contract using ethers.js (for now you can use hardhat to roll back to the block where this address is still in chainlog, eg 14073986), post them here and see if we can distinguish those events by the same signatures as in python library?

valiafetisov commented 2 years ago

After a further look: previously flap contract was using note method modifier which was responsible for logging anonymous events after tend calls as seen in the previously deployed contract code. But it's no longer the case. I am not sure if we're actually able to fetch any previous tends or deals

valiafetisov commented 2 years ago

Based on the investigation into the events, the conclusion is that we're not able to fetch information about previous tends and deals. We're also not able to fetch Kick for a particular auction because its id property is not indexed therefore we can only fetch all Kicks or none. Therefore, I suggest to remove the whole possibility to get previous transactions.

I've updated the mock https://github.com/sidestream-tech/unified-auctions-ui/issues/288 based on the findings and also the previous comments left in whimsical.

Implementation proposal should be changed accordingly: we do not fetch events, remove SurplusEvent type:

export type SurplusAuctionState =
    | 'just-started'
    | 'have-bids'
    | 'ready-for-collection'
    | 'collected'
    | 'requires-restart';

export declare interface SurplusAuctionData {
    id: number;
    network: string;
    state: SurplusAuctionState;
    bidAmountMKR: BigNumber; // amount of MRK (bid)
    receiveAmountDAI: BigNumber; // amount of DAI to be received (lot)
    unitPrice: BigNumber; // bidAmountMKR/receiveAmountDAI or the other way around
    receiverAddress: string; // the address of the wallet who receives DAI (guy)
    auctionEndDate: Date; // final date when auction will end (now + `flap.tau()` – 2 days atm) (end)
    bidEndDate?: Date; // (tic)
    earliestEndDate: Date; // earliest of auctionEndDate and auctionEndDate
}

export declare interface SurplusAuction extends SurplusAuctionData {
    marketUnitPrice: BigNumber;
    // ...transaction prices
}
KirillDogadin-std commented 2 years ago

surplus event though contains the initial bid. That means that in order to figure out the difference between just-started and have-bids one has to get the initial bid. That is done via the event.

Then i assume the two states here have to be merged into one state something like live

valiafetisov commented 2 years ago

Initial bid suppose to always be zero since kick is called via vow.flap. But the proper way to distinguish between just-started and have-bids is via bidEndDate (tic) which is not set during kick – this is, at least, how the contract code is doing it.

valiafetisov commented 2 years ago

How to get MKR for testing

According to the UniswapV2 callee code, you can call swapExactTokensForTokens of the uniswap router (deployed at https://etherscan.io/address/0x7a250d5630b4cf539739df2c5dacb4c659f2488d on mainnet) providing:

So you can make a small helper function exchangeEthToMkr(amount: BigNumber = new BigNumber(10)) in our code which executes this function. ETH and MKR addresses can be taken from the chainlog, the rest is known.