protocol-v2 icon indicating copy to clipboard operation
protocol-v2 copied to clipboard

admin.rs: validation after updating ids/numbers

Open xeroc opened this issue 9 months ago • 0 comments

Hey there,

while experimenting a bit with rift on devnet, I noticed and issue that i believe is part of the code. Excuse me if I understand something wrong, but it appears that you are validating values after you changed state.

In particular, I worry about calls of get_then_update_id over here:

https://github.com/drift-labs/protocol-v2/blob/9b6f3e968ce9e94027d06efad349ed3af86875e0/programs/drift/src/instructions/admin.rs#L133

This changes the value of number_of_spot_markets in your state.

However, if any of the validations fail, the number seems to still be incremented. At least that's what it looks like to me.

The issue with this is that you derive the spot market account that is required using the spot_market seed and a value from the state (the one that is incremented!!) here:

https://github.com/drift-labs/protocol-v2/blob/9b6f3e968ce9e94027d06efad349ed3af86875e0/programs/drift/src/instructions/admin.rs#L2675

Example

I failed to setup the correct oracle for spot market 0 (the quote spot market) and used Pyth instead of QUOTE_ASSET. That lead to the validations fail, but when calling with correct values, the new spot market I created would carry id 1, instead of 0

Is that intended?

xeroc avatar Apr 30 '24 11:04 xeroc