admin.rs: validation after updating ids/numbers

[xeroc]

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?

[crispheaney]

if any of the validations fail, the tx will fail and the state update will be reverted

[xeroc]

Thanks for clarifying. I came to the same conclusion since i opened the ticket. Still wonder how i ended up in the state i had back then. Need to try reproduce.

in the meantime, closing rhe ticket.