Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dmp: Check that the para exist before delivering a message #6604

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Nov 21, 2024

No description provided.

@bkchr bkchr added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Nov 21, 2024
@bkchr
Copy link
Member Author

bkchr commented Nov 29, 2024

/cmd prdoc --audience runtime_dev --bump patch

prdoc/pr_6604.prdoc Outdated Show resolved Hide resolved
@bkontur
Copy link
Contributor

bkontur commented Nov 29, 2024

for those failed rococo/westend benchmarks, for ToParachainDeliveryHelper e.g. here adding some EnsureForParachain implementation which does that paras::Heads::<Runtime>::insert(p, HeadData(p.encode().into())); should fix it (this is needed just for relaychains, which does dmp)

@bkontur
Copy link
Contributor

bkontur commented Nov 29, 2024

there is also one place: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/parachains/src/hrmp.rs#L1886-L1898, I am not sure if this new Unroutable could happen in this place, maybe we could change that e.g.:

                // try to enqueue
		if let Err(error) =
			dmp::Pallet::<T>::queue_downward_message(&config, dest, notification_bytes)
		{
			// this should never happen unless the max downward message size is configured to a
			// jokingly small number or para is not registered/active.
			log::error!(
				target: "runtime::hrmp",
				"sending '{log_label}::notification_bytes' failed with error: {error:?}."
			);
			debug_assert!(false);
		}

@bkontur
Copy link
Contributor

bkontur commented Nov 30, 2024

for those failed rococo/westend benchmarks, for ToParachainDeliveryHelper e.g. here adding some EnsureForParachain implementation which does that paras::Heads::<Runtime>::insert(p, HeadData(p.encode().into())); should fix it (this is needed just for relaychains, which does dmp)

let me fix this for you

@bkchr
Copy link
Member Author

bkchr commented Nov 30, 2024

let me fix this for you

I already have it fixed locally :) Just need to fix more :D

@bkchr bkchr requested a review from a team as a code owner November 30, 2024 21:47
@bkontur
Copy link
Contributor

bkontur commented Nov 30, 2024

let me fix this for you

I already have it fixed locally :) Just need to fix more :D

:D :D nice, so at least please consider one small clean-up of my older code: #6714

@paritytech-review-bot paritytech-review-bot bot requested a review from a team December 2, 2024 16:54
polkadot/runtime/common/src/xcm_sender.rs Outdated Show resolved Hide resolved
polkadot/runtime/common/src/xcm_sender.rs Outdated Show resolved Hide resolved
polkadot/runtime/parachains/src/dmp/tests.rs Outdated Show resolved Hide resolved
Comment on lines +142 to +147
#[cfg(feature = "runtime-benchmarks")]
fn ensure_successful_delivery(location: Option<Location>) {
if let Some((0, [Parachain(id)])) = location.as_ref().map(|l| l.unpack()) {
dmp::Pallet::<T>::make_parachain_reachable(*id);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍, I think we could now add this to ParentAsUmp, XcmpQueue and maybe it will simplify and deduplicate other stuff - I can take a look as a follow-up

@bkchr
Copy link
Member Author

bkchr commented Dec 9, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented Dec 9, 2024

@bkchr https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7888261 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 18-1ef39872-7e53-4cf7-9e5a-9751f4825d5a to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 9, 2024

@bkchr Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7888261 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7888261/artifacts/download.

@bkchr bkchr enabled auto-merge December 9, 2024 21:49
@bkchr
Copy link
Member Author

bkchr commented Dec 10, 2024

/cmd prdoc --audience runtime_dev --bump major

Copy link

Command "prdoc --audience runtime_dev --bump major" has failed ❌! See logs here

@bkchr
Copy link
Member Author

bkchr commented Dec 10, 2024

/cmd prdoc --audience runtime_dev --bump major --force

prdoc/pr_6604.prdoc Outdated Show resolved Hide resolved
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12265614412
Failed job name: test-linux-stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants