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

Check lane id of incoming messages #2452

Open
2 tasks
svyatonik opened this issue Jan 11, 2023 · 4 comments
Open
2 tasks

Check lane id of incoming messages #2452

svyatonik opened this issue Jan 11, 2023 · 4 comments
Assignees

Comments

@svyatonik
Copy link
Contributor

slightly related to https://github.com/paritytech/srlabs_findings/issues/142

We shall verify lane of incoming messages - something that we already had before, but lost now. With XCM bridge it means:

  • when we are receiving messages from unsupported lane (e.g. in RBH<>WBH we now only support lane [0, 0, 0, 1] for messages between *mint/*mine chains) - we shall return error from verify_messages_proof if proof contains messages from some other lane. Now this is some static check, but later we'll probably need to maintain map of LaneId => (XcmSource, XcmDestination);
  • @bkontur can we verify that the message is actually sent by the proper source parachain and that it'll be routed to the proper target parachain. For example - when dispatching message that has came by lane [0, 0, 0, 1] to WBH, we want to ensure that it has been actually sent by Rockime2 and is going to Wockmint. Can we do that, or is it handled somewhere already? WDYT about that?

@bkontur @serban300 Can you, guys, please, tackle this?

@bkontur bkontur self-assigned this Jan 11, 2023
@bkontur
Copy link
Contributor

bkontur commented Jan 11, 2023

@svyatonik
I will take it,
because for pallet-bridge-assets-transfer for moving assets, I have PoC for some mapping on-chain for bridge configuration with simple CRUD operations (add_bridge_config, update_bridge_config, remove_bridge_config) with let _ = ensure_root(origin)?; that could be triggered just by governance-like

and next week on CGP retreat, I would like to disccuss this and other stuff with Joe, so after that, I would be able to align all similar configurations and do it in one way

@EmmanuellNorbertTulbure

@bkontur to recheck it

svyatonik referenced this issue Jul 17, 2023
* Add try-runtime feature

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* More feature gates

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Add dummy command

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* update lockfile for {"polkadot", "substrate"}

* Fix code

* Remove unused import

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Imports...

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* fmt

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* fmt

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: parity-processbot <>
@EmmanuellNorbertTulbure EmmanuellNorbertTulbure transferred this issue from paritytech/parity-bridges-common Aug 25, 2023
@the-right-joyce the-right-joyce transferred this issue from paritytech/polkadot-sdk Aug 25, 2023
@EmmanuellNorbertTulbure
Copy link

@bkontur to recheck it

@bkontur
Copy link
Contributor

bkontur commented Sep 22, 2023

@svyatonik
to this discussion: zdave-parity/polkadot-bulletin-chain#28 (comment)
I think for V1 we dont check sender vs lane:

MessagesPallet::<H::Runtime, H::MessagesInstance>::send_message(sender_and_lane.lane, blob)

the only usage of SenderAndLane.location I see in LocalXcmQueueManager,
I am going to add some simple test to BridgeHubRococo runtime,
and I will add simple validation for MessageExporter because impl ExportXcm::validate should have all parameters

bkontur added a commit to paritytech/polkadot-sdk that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants