Lite refresh reserve #59
Conversation
| clock: &Clock, | ||
| ) -> ProgramResult { | ||
| let mut reserve = Reserve::unpack(&reserve_info.data.borrow())?; | ||
| if reserve_info.owner != program_id { |
There was a problem hiding this comment.
Should we also check that the reserve belongs to the same lending market?
There was a problem hiding this comment.
I guess probably doesn't need since the original refresh_reserve also doesn't. That one instead checks that Pyth and Switchboard oracles match, which I guess isn't needed here since it doesn't actually use the feeds.
| @@ -1046,6 +1061,11 @@ fn process_deposit_reserve_liquidity_and_obligation_collateral( | |||
| clock, | |||
| token_program_id, | |||
| )?; | |||
| // mark the reserve as stale to make sure no weird bugs happen | |||
There was a problem hiding this comment.
Which weird bugs does this prevent?
There was a problem hiding this comment.
Can think of one, bad liquidations using a very old price but appears to be up-to-date.
There was a problem hiding this comment.
since refresh_reserve_lite doesn't update the actual price, an attacker could exploit the stale price on a refreshed reserve
if we mark it as stale, this is mitigated
basically, an attacker can't do anything with the reserve until the refresh it the legit way
| @@ -419,6 +419,26 @@ fn _refresh_reserve<'a>( | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /// Lite version of refresh_reserve that should be used when the oracle price doesn't need to be updated | |||
| /// BE CAREFUL WHEN USING THIS | |||
| fn _refresh_reserve_lite<'a>( | |||
There was a problem hiding this comment.
This is an internal function, which is good. If it were publicly callable could result in some weird stuff (e.g. attacker might be able to use a really stale price to liquidate someone since this marks the reserve as up to date).
There was a problem hiding this comment.
By internal I mean it's not invokable since there's no process_refresh_reserve_lite matcher in process_instruction.
| switchboard_feed_info, | ||
| clock, | ||
| )?; | ||
| _refresh_reserve_lite(program_id, reserve_info, clock)?; |
There was a problem hiding this comment.
One side effect I can think of is the allowedBorrowValue on the Obligation would be slightly off. Are we still reading this to display borrow power on the frontend? @0xodia
There was a problem hiding this comment.
yea, until the reserve is refreshed the legit way i believe bc the value of the obligation isn't updated with a new price
There was a problem hiding this comment.
allowed borrow value would still be correct when actually borrowing because that requires a full reserve refresh
There was a problem hiding this comment.
yeah this is still heavily used in FE as the borrow limit. The use is only visual though so not the worse thing if it's slightly off from a stale price (if i'm understanding this PR correctly)
remove the need to refresh reserves/read oracle data when depositing liquidity/collateral