Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Fix xcmp message processing condition.#510

Merged
bkchr merged 6 commits intoparitytech:masterfrom
shaunxw:sw/fix-xcmp-queue-condition
Jul 6, 2021
Merged

Fix xcmp message processing condition.#510
bkchr merged 6 commits intoparitytech:masterfrom
shaunxw:sw/fix-xcmp-queue-condition

Conversation

@shaunxw
Copy link
Copy Markdown
Contributor

@shaunxw shaunxw commented Jun 25, 2021

The messages processing loop shouldn't be entered again if there is no message left. Before this fix, calling handle_xcmp_messages with one message would panic.

fn handle_xcmp_messages<'a, I: Iterator<Item = (ParaId, RelayBlockNumber, &'a [u8])>>(
let weight_processed = if status[index].2.is_empty() {
debug_assert!(
false,
"channel exists in status; there must be messages; qed"
);
0
} else {

Fixes: #473

@apopiak
Copy link
Copy Markdown
Contributor

apopiak commented Jun 25, 2021

add a smoke test?

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Jun 28, 2021

add a smoke test?

#400

@apopiak can you review this?

@bkchr bkchr requested a review from apopiak June 28, 2021 13:20
@gavofyork
Copy link
Copy Markdown
Member

A test would be nice.

@apopiak
Copy link
Copy Markdown
Contributor

apopiak commented Jul 5, 2021

add a smoke test?

#400

@apopiak can you review this?

@bkchr Do you mean that it's fine to merge as-is without a test because we have a tracking issue?

@shawntabrizi
Copy link
Copy Markdown
Member

@apopiak no we should add a test here. If you can, add one, if not, I will do it later today

@shawntabrizi
Copy link
Copy Markdown
Member

@apopiak please take a look and merge if you think it is okay

@bkchr bkchr merged commit f119e39 into paritytech:master Jul 6, 2021
@shaunxw shaunxw deleted the sw/fix-xcmp-queue-condition branch July 7, 2021 03:04
slumber pushed a commit that referenced this pull request Sep 1, 2021
* Fix xcmp message processing condition.

* add a very simple test

* Update Cargo.lock

* remove comment

* remove comment

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XCM teleport asset panic

5 participants