Support LSE collateral auctions #622
Conversation
…play MKR instead of MCD_GOV
|
LSE auction is successfully executed on the mainnet_sep_30_0 testnet using both bot and the frontend. Note that the CI is failing due to the new collateral is not yet initialised on the mainnet. Should pass after 1c executive spell is casted. Logs of the bot |
Functional testing
|
There was a problem hiding this comment.
- Getting an error in the simulation step
create underwater vault(see console error below)
✔ Press Enter to proceed to the next step "Create underwater vault" …
Transaction is preparing to be mined...
Transaction is pending to be mined...
Transaction was mined into block "20863585"
Initial oracle price is 1000 DAI
Duplicate definition of File (File(bytes32,uint256), File(bytes32,bytes32,uint256), File(bytes32,bytes32,address))
Duplicate definition of File (File(bytes32,uint256), File(bytes32,address))
Duplicate definition of File (File(bytes32,uint256), File(bytes32,bytes32,uint256), File(bytes32,bytes32,address))
Duplicate definition of File (File(bytes32,uint256), File(bytes32,address))
Minimum collateral amount to open vault: 33.011
Balance slot is 0x1, language format is solidity, contract address is 0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2
Wallet has 33.011 MCD_GOV
Duplicate definition of File (File(bytes32,address), File(bytes32,uint256))
/Users/lukassteib/Github/unified-auctions-ui/core/node_modules/@ethersproject/logger/src.ts/index.ts:261
const error: any = new Error(message);
^
Error: cannot estimate gas; transaction may fail or may require manual gas limit [ See: https://links.ethers.org/v5-errors-UNPREDICTABLE_GAS_LIMIT ] (error={"reason":"Error: VM Exception while processing transaction: reverted with reason string 'LockstakeEngine/wrong-urn-index'","code":"UNPREDICTABLE_GAS_LIMIT","method":"estimateGas","transaction":{"from":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266","maxPriorityFeePerGas":{"type":"BigNumber","hex":"0x59682f00"},"maxFeePerGas":{"type":"BigNumber","hex":"0x59682f02"},"to":"0x9581c795DBcaf408E477F6f1908a41BE43093122","data":"0x690e7c090000000000000000000000000000000000000000000000000000000000000001","type":2,"accessList":null},"error":{"reason":"processing response error","code":"SERVER_ERROR","body":"{\"jsonrpc\":\"2.0\",\"id\":74,\"error\":{\"code\":-32603,\"message\":\"Error: VM Exception while processing transaction: reverted with reason string 'LockstakeEngine/wrong-urn-index'\",\"data\":{\"message\":\"Error: VM Exception while processing transaction: reverted with reason string 'LockstakeEngine/wrong-urn-index'\",\"data\":\"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001f4c6f636b7374616b65456e67696e652f77726f6e672d75726e2d696e64657800\"}}}","error":{"code":-32603,"data":{"message":"Error: VM Exception while processing transaction: reverted with reason string 'LockstakeEngine/wrong-urn-index'","data":"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001f4c6f636b7374616b65456e67696e652f77726f6e672d75726e2d696e64657800"}},"requestBody":"{\"method\":\"eth_estimateGas\",\"params\":[{\"type\":\"0x2\",\"maxFeePerGas\":\"0x59682f02\",\"maxPriorityFeePerGas\":\"0x59682f00\",\"from\":\"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266\",\"to\":\"0x9581c795dbcaf408e477f6f1908a41be43093122\",\"data\":\"0x690e7c090000000000000000000000000000000000000000000000000000000000000001\"}],\"id\":74,\"jsonrpc\":\"2.0\"}","requestMethod":"POST","url":"http://127.0.0.1:8545"}}, tx={"data":"0x690e7c090000000000000000000000000000000000000000000000000000000000000001","to":{},"from":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266","type":2,"maxFeePerGas":{"type":"BigNumber","hex":"0x59682f02"},"maxPriorityFeePerGas":{"type":"BigNumber","hex":"0x59682f00"},"nonce":{},"gasLimit":{},"chainId":{}}, code=UNPREDICTABLE_GAS_LIMIT, version=abstract-signer/5.6.2)
at Logger.makeError (/Users/lukassteib/Github/unified-auctions-ui/core/node_modules/@ethersproject/logger/src.ts/index.ts:261:28)
at Logger.throwError (/Users/lukassteib/Github/unified-auctions-ui/core/node_modules/@ethersproject/logger/src.ts/index.ts:273:20)
at /Users/lukassteib/Github/unified-auctions-ui/core/node_modules/@ethersproject/abstract-signer/src.ts/index.ts:301:31
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async Promise.all (index 7)
ERROR: "hardhat:simulations" exited with 1.
|
I've updated opening logic, which should solve the problem. Also note: since the 1c spell was already executed you no longer need to use Tenderly Testnet, but can directly test against mainnet simulation. |
There was a problem hiding this comment.
Review notes:
- Able to create and liquidate
LSE-MKR-Avault via hardhat simulation ✅ - Auction is fetched in UI with right amount of
MKRas collateral ✅ - Was able to participate in auction via swap transaction ✅
- Profit indication was correct ✅ (just overestimating fees on fork) (see Dai balance before, estimations and Dai balance after)
- Bid with DAI flow 🔴
Dai balance before
Estimations
Dai balance after:
Review requests:
- NIT: The Bid wit Dai modal shows
MCD_GOVinstead ofMKR(see s1) - In the bid with DAI flow when trying to deposit DAI to VAT getting
Internal JSON RPCerror (see e1 for console screenshot)
s1
e1
Questions:
- I would expect the correct (or at least a realistic
MKRprice from uniswap) (see s2) based on us forking mainnet at a very recent block. Is this assumption wrong? - What's the exchange route the new callee is using?
s2
Most likely you forgot to reset nonce after swapping in your metamask settings. Every time you restart the simulation, you need to go into
The MKR is currently directly exchanged in the MKR/DAI uniswap pool.
Most likely a simulation issue unrelated to this PR. Let's look into it in another issue |
There is no MKR in VAT because this collateral is different from all others and have no |
|
I've replaced MKR/DAI Uniswap v2 market with SKY/USDS. Also improved indication of the market on the frontend: 
|
There was a problem hiding this comment.
- IMO after changing to the new pool we should update the UI as well to denominate values in
USDSinstead of DAI. After swap based execution we will end up with user receivingUSDSand displaying the net profit inDAImight cause confusion in a way the user expects to receive DAI
There was a problem hiding this comment.
- Sorry for being picky here but having the following assumption: There is a difference in the price per MKR on uniswap when comparing
USDSandDAI. The calculations we make in the UI are based on theDAIprice. However, since the actual swap is happening viaUSDSI have a feeling that the calculations are slightly off.
What UI shows:
Uniswap UI for swap of DAI / MKR
Uniswap UI for swap of USDS / MKR
This is not true. For this particular token, the swap is done like so: MKR is auctioned for DAI or USDS by the lockstake clipper, callee takes MKR converts it into SKY using maker converter with fixed rate and then use SKY/USDS univ2 pool to convert it to USDS. So technically it's correct to say that |
|
Makes sense, thanks for the explainer. I think the root of my confusion was the different denominations for the auction price. While on the swap page we now have the denomination s1 s2 |
Since DAI and USDS are 1-to-1 convertible into each other, I don't think this indication is a problem. Since on the "Bid with DAI" page we would still need to display everything in DAI. But ultimately, I don't know a perfect solution for this duality UI-wise, since it's a quite complex issue. In fact the same collateral (LSE-MKR-A) can be traded both with DAI or USDS. So currently (the way it is implemented) indicated profit token depends on the selected market: if we are to add MKR/DAI as a second market, switching to it will result in different profit token being displayed in the swap table. In other words it's already quite complex UI-wise, let's first ensure that this collateral is liquidatable by merging this PR and then fix what is displayed on the frontend. I think your initial concern (of confusing the user with incorrectly specified profit token) is resolved now. |
Adds support for the upcoming new type of auctions based on the new and untypical LockstakeClipper contract.
Note: the addition require multiple code refactorings: