# GMX что сделано + пояснения

## HIGH 1. Market prices for underlying tokens can make `USDG` under-collateralized. In this case some of LPs will not be able to withdraw.

**Что изменил:**
- добавил ссылку (но это по сути весь код функций buyUSDG()+sellUSDG())
- еще добавил абзац, где придумал более наглядную математику

**Что думаю:**
- в баунти отправлять не нужно, т.к. скорее всего эта проблема менеджится по факту токеном GLP. USDG как токен не ходит между юзерами, он почти весь держится контрактом GLPManager:
https://arbiscan.io/token/0x45096e7aa921f27590f8f19e457794eb09678141#balances
GLP уже позволяет входить/выходить по классической shares логике.
Но это уже Out of Scope.
- по мне где-то между High и Medium. Ибо это дизайн а не дыра. В лендингах тоже не все ликвидити провайдеры могут вытащить свои токены. До High натягиваю, потому что в нашем упражнении мы представляем что делаем заказной аудит, где есть Scope и не-Scope. Ну и представляем что не Подсматриваем на положение дел в сети.
- кстати связка GLP и USDG у меня вызывает большое любопытство, надеюсь в свободное время гляну как там

## HIGH 2. Irrelevant oracle return selection for stablecoins in case of their depeg.

**Что изменил:**
- было длинное название, сократил

**Что думаю:**
- написал подробное описание, которое можно им заслать, вышло много
https://hackmd.io/@Xbg00EiJQ-a4a46Yn6SZJA/H1xJW1qhi
- я там повысил серьезность, связанный с токеном GLP
При своем минте он в качестве знаминателя берет AUM ваулта - а его считает как раз по getMaxPrice (возвращает 1). Поэтому депегнутый токен будет завышать AUM => занижать выпускаемые GLP.
Причем этот AUM считается на весь пул и для всех токенов сразу, поэтому не важно какой токен приносишь в качестве ликвидности - весь AUM все равно будет завышен засчет этого депегнутого токена на балансе. Поэтому весь GLP станет невыгодно минтить.
- кажется даже, что могу еще что-то более суровое придумать на этой почве, но это надо на GLP побольше посидеть, я его пока поверхностно посмотрел

## HIGH 3. Price feed from AMM V1 is too controllable

**Что изменил:**
- ты просил понизить до Медиума. Но у меня есть контр-тезис, поэтому не исправил, сейчас обосную.

**Что думаю:**
- ты пишешь medium `because admin needs to set useV2Pricing to false`
- но там наоборот при деплое стоит, как дефолтное значение используется корявый V1
`bool public useV2Pricing = false;`
https://github.com/gmx-io/gmx-contracts/blob/96e516bc8d0979c51c7fdffee50622415d5c411d/contracts/core/VaultPriceFeed.sol#L31
а еще
`bool public isAmmEnabled = true;`
https://github.com/gmx-io/gmx-contracts/blob/96e516bc8d0979c51c7fdffee50622415d5c411d/contracts/core/VaultPriceFeed.sol#L29
или действительно ставить priceV2 (сейчас кстати V1 используется)
- поэтому им надо для закрытия этого вектора вызывать отдельную функцию будет
https://github.com/gmx-io/gmx-contracts/blob/96e516bc8d0979c51c7fdffee50622415d5c411d/contracts/core/VaultPriceFeed.sol#L91-L93
- интересный вектор атаки появляется в связке с GLP. Юзер несет ликвидность в токене. Через бандл флэшботов атакующий фронтраном занижает цену, бэкраном возвращет цену обратно. Юзерская транзакция посередине получит на свои токены очень мало USDG. И тогда будет получаться что юзеры кладут много токенов, но получают мало GLP. В этом смысле все другие GLP холдеры в этот момент богатеют. Под таким соусом это даже крит.
- **Важно: в сети стоит isAmmEnabled = false**
https://arbiscan.io/address/0x2d68011bca022ed0e474264145f46cc4de96a002#readContract
**Поэтому на багбаунти отдать нельзя.**
Более того на странице Иммунифая пишут:
`Vault.includeAmmPrice and Vault.useSwapPricing are not reset to default values for certain cases, these variables will not be used`
https://immunefi.com/bounty/gmx/
В транзакциях тоже проверил, что цена АММ не используется.

## HIGH 4. `Vault.usdgAmounts[token]` sets zero in case of decrease below zero.

**Что изменил:**
- там была грубая рекомендация "ревертить если уходит в минус"
- твой комментарий `I think it will stop the whole protocol which is bad.`
- поправил на более абстрактный комментарий "что надо учитывать негативные числа"

**Что думаю:**
- тут я ошибся, что дал слишком директивную рекоммендацию как испралять, и взял ответственность за возможные новые баги. Это нехорошо и опасно.

## MEDIUM 6. Vault.upgradeVault() is a poor way to migrate to a new vault contract

**Что изменил:**
- был High, теперь Medium
- осознал, исправил

## LOW 5. Vault.directPoolDeposit() is a public function but is likely used by the goverance

**Что изменил:**
- комментарий: `it is an informational bug (low)`
- осознал, исправил