paritytech
Add `TryState` hook for `MessageQueue`
The MessageQueue pallet has quite a few storage assumptions which could be checked in a try_state hook.
:point_right: Mentor issues are meant for new-comers. Please ask before picking them up.
Yes @mahmudsudo !
Please familiarize yourself with the try_state hook implementations that we have in other pallets and how to write them.
For example in the bags-list pallet or in nomination-pools.
Then take a look at the message-queue and especially all the debug_assert!+defensive! in it. Ideally we check all these invariants in the try_state function.
So for example check that every page can be decoded into peek_* functions etc.
Hey @mahmudsudo , are you still working on this? If not, I would like to take this over.
Hey @ggwpez I have written the function taking following assumptions:
If serviceHead points to a ready Queue (i.e. has some value), then BookState of that Queue has:
* message_count > 0
* size > 0
* end > begin
* Some(ready_neighbours)
* If ready_neighbours.next == self.origin, then ready_neighbours.prev == self.origin (only queue in ring)
For a particular BookState of Queue present in ReadyRing, all pages from begin to end-1 should have:
* remaining > 0
* remaining_size > 0
* first <= last
* Every page can be decoded into peek_* functions
If all these assumptions are correct then all Queue present in ReadyRing must have atleast 1 message. (Is this correct?) But there are some tests which add Queue with empty book into ReadyRing which causes try_state function to panic. Should I leave those tests or is there something wrong with my assumptions?
If all these assumptions are correct then all Queue present in ReadyRing must have atleast 1 message. (Is this correct?) But there are some tests which add Queue with empty book into ReadyRing which causes try_state function to panic. Should I leave those tests or is there something wrong with my assumptions?
Looks good so far. Please open a Merge request with your changes. I will then go over them in more details.
But AFAIK yes; there should be no empty books in the ready ring.
@ggwpez I have opened the merge request, kindly let me know if you want me to change anything.