Add `try_state` check to Pallet `MessageQueue` #13502

gitofdeepanshu · 2023-03-01T12:02:06Z

Fix #13115

Description

Add try_state hook implementation for pallet message-queue.

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

Test Status

5 tests are failing, if assumptions are correct then these tests are not a part of valid state.

failures:
tests::bump_service_head_bails
tests::bump_service_head_trivial_works
tests::bump_service_head_works
tests::execute_overweight_works
tests::reap_page_permanent_overweight_works

Polkadot address: 12y2pdexQRzCvR7rj6HhMYkLbL3atjikGptSit78eeGWn6ma

ggwpez

I will take a look at the failing tests. Probably not all of them keep the invariants because of mocked storage.

frame/message-queue/src/integration_test.rs

frame/message-queue/src/lib.rs

frame/message-queue/src/tests.rs

frame/message-queue/src/lib.rs

frame/message-queue/src/tests.rs

change if let to while let and modify bump_service_head function Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez

Looks pretty good, thanks.
Please put your address for a tip like described here: https://github.com/paritytech/substrate-tip-bot#pull-request-body

The CI will also need a format, then we can fix the remaining tests.

ggwpez · 2023-03-16T14:32:55Z

frame/message-queue/src/lib.rs

+
+		// No state to check
+		if ServiceHead::<T>::get().is_none() {
+			return Ok(())


I think there are still a few assumptions about books even if they are not in the ready ring, but it is okay for a first version.

frame/message-queue/src/mock.rs

stale · 2023-04-15T18:17:17Z

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

frame/message-queue/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

juangirini · 2023-05-22T09:08:50Z

/tip small

substrate-tip-bot · 2023-05-22T09:08:53Z

@juangirini A small tip was successfully submitted for gitofdeepanshu (12y2pdexQRzCvR7rj6HhMYkLbL3atjikGptSit78eeGWn6ma on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

juangirini · 2023-05-22T09:10:26Z

Hey @gitofdeepanshu thanks for your contribution! Would you apply those changes requested by @ggwpez so we can give a final review before merging it?

juangirini · 2023-06-09T14:41:34Z

Hey @gitofdeepanshu would you be able to take this over again?

gitofdeepanshu · 2023-06-12T03:27:23Z

I will do it this week, apologies for delay!

kianenigma · 2023-06-12T09:09:03Z

frame/message-queue/src/mock.rs

+{
+	new_test_ext::<T>().execute_with(|| {
+		test();
+		MessageQueue::do_try_state().unwrap();


perfecto 👍

stale · 2023-07-12T11:15:09Z

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

frame/message-queue/src/mock.rs

juangirini · 2023-07-12T13:43:45Z

@gitofdeepanshu I am very sorry this took ages to get merged and had a few rebase conflicts now resolved but still it is falling due to the fact that the try_state signature has changed. Would you be able to give it another look and fix that?

stale · 2023-08-11T13:50:08Z

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

ggwpez · 2023-08-14T15:22:11Z

bot rebase

paritytech-processbot · 2023-08-14T15:22:20Z

Error: Command 'Command { std: cd "/storage/repositories/substrate" && "git" "push" "--porcelain" "gitofdeepanshu" "feat/try-state_message-queue", kill_on_drop: false }' failed with status Some(1); output: error: failed to push some refs to 'https://github.com/gitofdeepanshu/substrate.git'

…age-queue

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez · 2023-08-14T17:18:59Z

bot merge

* feat: Add try_state for message_queue * Update frame/message-queue/src/lib.rs change if let to while let and modify bump_service_head function Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * feat: update try_state, add checks for storage * fix: add try_state builder for remaining tests * Update frame/message-queue/src/mock.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * chore: assert statement to ensure * Fix tests Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use ensure instead of assert Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix function signature and feature gate Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cleanup code Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Juan <juangirini@gmail.com>

ggwpez self-requested a review March 1, 2023 16:43

ggwpez reviewed Mar 1, 2023

View reviewed changes

frame/message-queue/src/integration_test.rs Show resolved Hide resolved

frame/message-queue/src/lib.rs Show resolved Hide resolved

frame/message-queue/src/lib.rs Outdated Show resolved Hide resolved

ggwpez reviewed Mar 1, 2023

View reviewed changes

frame/message-queue/src/tests.rs Outdated Show resolved Hide resolved

frame/message-queue/src/lib.rs Outdated Show resolved Hide resolved

frame/message-queue/src/tests.rs Outdated Show resolved Hide resolved

frame/message-queue/src/tests.rs Show resolved Hide resolved

ggwpez mentioned this pull request Mar 3, 2023

MessageQueue: unknit permanently overweight books #13528

Merged

2 tasks

gitofdeepanshu and others added 4 commits March 13, 2023 14:11

feat: Add try_state for message_queue

a0c94a4

Update frame/message-queue/src/lib.rs

6a4a26e

change if let to while let and modify bump_service_head function Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

feat: update try_state, add checks for storage

73b8e3d

fix: add try_state builder for remaining tests

a9624a3

gitofdeepanshu force-pushed the feat/try-state_message-queue branch from a8f922f to a9624a3 Compare March 13, 2023 09:54

ggwpez reviewed Mar 16, 2023

View reviewed changes

stale bot added the A3-stale label Apr 15, 2023

ggwpez approved these changes Apr 15, 2023

View reviewed changes

frame/message-queue/src/lib.rs Outdated Show resolved Hide resolved

stale bot removed the A3-stale label Apr 15, 2023

gitofdeepanshu and others added 2 commits May 2, 2023 09:40

Update frame/message-queue/src/mock.rs

d7f3f33

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

chore: assert statement to ensure

a67f631

juangirini assigned gitofdeepanshu May 22, 2023

juangirini added the T1-runtime This PR/Issue is related to the topic “runtime”. label May 22, 2023

juangirini assigned ggwpez Jun 9, 2023

juangirini requested a review from a team June 9, 2023 14:42

sam0x17 approved these changes Jun 12, 2023

View reviewed changes

kianenigma approved these changes Jun 12, 2023

View reviewed changes

stale bot added the A3-stale label Jul 12, 2023

Merge branch 'master' into feat/try-state_message-queue

057f6ea

juangirini requested a review from a team July 12, 2023 13:25

juangirini reviewed Jul 12, 2023

View reviewed changes

frame/message-queue/src/mock.rs Show resolved Hide resolved

juangirini requested a review from ggwpez July 12, 2023 13:28

stale bot removed A3-stale labels Jul 12, 2023

stale bot added the A3-stale label Aug 11, 2023

ggwpez unassigned gitofdeepanshu Aug 14, 2023

Merge remote-tracking branch 'origin/master' into feat/try-state_mess…

70ea918

…age-queue

ggwpez removed the A3-stale label Aug 14, 2023

ggwpez added 4 commits August 14, 2023 17:49

Fix tests

3684bc0

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Use ensure instead of assert

1c8da37

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix function signature and feature gate

980249f

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup code

0b0567e

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez approved these changes Aug 14, 2023

View reviewed changes

ggwpez changed the title ~~feat: Add try_state for message_queue~~ Add try_state check to Pallet MessageQueue Aug 14, 2023

paritytech-processbot bot merged commit 34f45c6 into paritytech:master Aug 14, 2023
5 checks passed

crystalin mentioned this pull request Oct 20, 2023

Update substrate/polkadot/cumulus from v0.9.43 to v1.1.0 moonbeam-foundation/moonbeam#2535

Closed

JesseAbram mentioned this pull request Oct 26, 2023

Update substrate and subxt entropyxyz/entropy-core#435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `try_state` check to Pallet `MessageQueue` #13502

Add `try_state` check to Pallet `MessageQueue` #13502

gitofdeepanshu commented Mar 1, 2023 •

edited

Loading

ggwpez left a comment

ggwpez left a comment

ggwpez Mar 16, 2023

stale bot commented Apr 15, 2023

juangirini commented May 22, 2023

substrate-tip-bot bot commented May 22, 2023

juangirini commented May 22, 2023

juangirini commented Jun 9, 2023

gitofdeepanshu commented Jun 12, 2023

kianenigma Jun 12, 2023

stale bot commented Jul 12, 2023

juangirini commented Jul 12, 2023

stale bot commented Aug 11, 2023

ggwpez commented Aug 14, 2023

paritytech-processbot bot commented Aug 14, 2023

ggwpez commented Aug 14, 2023

Add try_state check to Pallet MessageQueue #13502

Add try_state check to Pallet MessageQueue #13502

Conversation

gitofdeepanshu commented Mar 1, 2023 • edited Loading

Description

Assumptions:

Test Status

ggwpez left a comment

Choose a reason for hiding this comment

ggwpez left a comment

Choose a reason for hiding this comment

ggwpez Mar 16, 2023

Choose a reason for hiding this comment

stale bot commented Apr 15, 2023

juangirini commented May 22, 2023

substrate-tip-bot bot commented May 22, 2023

juangirini commented May 22, 2023

juangirini commented Jun 9, 2023

gitofdeepanshu commented Jun 12, 2023

kianenigma Jun 12, 2023

Choose a reason for hiding this comment

stale bot commented Jul 12, 2023

juangirini commented Jul 12, 2023

stale bot commented Aug 11, 2023

ggwpez commented Aug 14, 2023

paritytech-processbot bot commented Aug 14, 2023

ggwpez commented Aug 14, 2023

Add `try_state` check to Pallet `MessageQueue` #13502

Add `try_state` check to Pallet `MessageQueue` #13502

gitofdeepanshu commented Mar 1, 2023 •

edited

Loading