Enable over-bridge-messaging in Rococo/Wococo runtime #3377

svyatonik · 2021-06-28T06:57:04Z

This PR adds two pallets (every pallet is 'deployed' twice - on Rococo and on Wococo) to Rococo/Wocoo runtime. The messages pallet configuration isn't trivial, hence the separate bridge_messages.rs file. Normally there'll be to files like that - one that describes bridge with Chain2 and which is a part of Chain1 runtime and another one that describes bridge with Chain1 and which is a part of Chain2 runtime. Here we have a slightly different scenario when there's one runtime => the single file.

Short pallets description (since it is first reference from the polkadot repo):

pallet-bridge-messages: is the 'core' pallet of this PR. It has the only interesting (for end-user/apps) entrypoint - send_message, which enqueues your message (encoded call of the bridged chain) in the internal queue. Eventually, your message will be picked up by the offchain (not to confuse with offchain workers) relayer, which will submit delivery transaction to the bridged chain. Once messages are delivered, relayer will submit confirmation transaction back to the original chain. The side-effect of this tx is the MessagesDelivered event, where you could find info on whether message dispatch has succeeded or not;
pallet-bridge-dispatch: a simple pallet that will try to decode the message from the bridged chain as a call of this chain (in Rococo<>Wococo case it is the same call). If call is decoded and passes all other checks (like spec_version check, weight check, ...), it is dispatched. In any case - for every incoming message, it'll emit event (MessageWeightMismatch, MessageVersionSpecMismatch, MessageDispatched, ...).

Deployment details (mostly for ones, who has some internal knowledge of pallets):

there are no any parameters (like conversion rate) in messages pallet configuration;
the owner of pallets will be empty initially (so only root can halt pallet). Could implement runtime upgrade, but it would be easier to transfer ownership with sudo call (or not to do that at all);
all constants introduced in this PR (which are not a part of bp_rococo/bp_wococo) are a copy of Millau constants.

svyatonik · 2021-06-28T07:40:19Z

runtime/rococo/src/bridge_messages.rs

+	}
+
+	fn transaction_payment(transaction: MessageTransaction<Weight>) -> crate::Balance {
+		// current fee multiplier is used here


~~This will also likely be changed (e.g. x2-ed, not sure yet) in the P<>K deployment, because it describes future transactions, where multiplier may increase.~~ Actually in P<>K we'll need messages parameter for that, because this value must be taken from bridged runtime storage

HCastano

Looks good!

runtime/rococo/src/bridge_messages.rs

HCastano · 2021-06-29T18:25:58Z

runtime/rococo/src/bridge_messages.rs

+	impl MessageBridge for AtRococoWithWococoMessageBridge {
+		const THIS_CHAIN_ID: ChainId = ROCOCO_CHAIN_ID;
+		const BRIDGED_CHAIN_ID: ChainId = WOCOCO_CHAIN_ID;
+		const RELAYER_FEE_PERCENT: u32 = 10;


I think in the future it would be cool to have relayers dictate this rate themselves in order to create more realistic market dynamics

runtime/rococo/src/bridge_messages.rs

HCastano · 2021-06-29T18:31:07Z

runtime/rococo/src/bridge_messages.rs

+		// Since we deal with testnets here, in case of failure + urgency:
+		//
+		// 1) ping bridges team about this failure;
+		// 2) comment/remove the test.


Suggested change

// 2) comment/remove the test.

// 2) comment out or ignore the test.

You can't simply ignore failing test, if it blocks CI

(unless you mean #[ignore], of course)

Yeah I meant #[ignore]. Should we make that more explicit in the comment?

runtime/rococo/src/bridge_messages.rs

HCastano · 2021-06-29T18:35:34Z

runtime/rococo/src/lib.rs

@@ -245,6 +248,13 @@ construct_runtime! {
 		// Validator Manager pallet.
 		ValidatorManager: validator_manager::{Pallet, Call, Storage, Event<T>},

+		// Bridge messages support. The same story as with the bridge grandpa pallet above ^^^ - when we're
+		// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.
+		BridgeRococoMessages: pallet_bridge_messages::{Pallet, Call, Storage, Event<T>, Config<T>} = 43,


It's a little unfortunate that we're being split by ValidatorManager, do you think anything (major) would break if we moved it after the message lane pallets 👀

It requires a transaction spec version update. I personally don't care about this split

I think it would nice to have all the bridge stuff side by side, but I won't push too hard on this

Yeah, I agree Hernando here. @shawntabrizi do you mind if we change the pallet index of ValidatorManager in the next restart/runtime upgrade of Rococo? I'd move it above the bridge stuff.

HCastano · 2021-06-29T18:37:29Z

runtime/rococo/src/lib.rs

+		// Bridge messages support. The same story as with the bridge grandpa pallet above ^^^ - when we're
+		// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.


Suggested change

// Bridge messages support. The same story as with the bridge grandpa pallet above ^^^ - when we're

// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.

// Bridge messages support.

// The same story as with the bridge grandpa pallets: when we're

// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

tomusdrw

Awesome! Looking forward to see that deployed 👨‍🚀

runtime/rococo/src/bridge_messages.rs

tomusdrw · 2021-07-05T12:03:46Z

runtime/rococo/src/lib.rs

@@ -245,6 +248,13 @@ construct_runtime! {
 		// Validator Manager pallet.
 		ValidatorManager: validator_manager::{Pallet, Call, Storage, Event<T>},

+		// Bridge messages support. The same story as with the bridge grandpa pallet above ^^^ - when we're
+		// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.
+		BridgeRococoMessages: pallet_bridge_messages::{Pallet, Call, Storage, Event<T>, Config<T>} = 43,


Yeah, I agree Hernando here. @shawntabrizi do you mind if we change the pallet index of ValidatorManager in the next restart/runtime upgrade of Rococo? I'd move it above the bridge stuff.

runtime/rococo/src/lib.rs

runtime/rococo/src/bridge_messages.rs

tomusdrw · 2021-07-06T10:03:07Z

bot merge

ghost · 2021-07-06T10:03:11Z

Trying merge.

* master: (33 commits) Update all weights, add run_all_benches.sh script (#3400) Enable over-bridge-messaging in Rococo/Wococo runtime (#3377) paras.rs to FRAME V2 (#3403) Add XCM Tracing (#3353) Use MaxEncodedLen trait from new parity-scale-codec v2.2 (#3412) bump a bunch of deps in parity-common (#3402) Warn on low connectivity. (#3408) origin to frame v2 (#3405) Enable logging in the puppet worker (#3411) make it easier to dbg stalls (#3351) XCM `canonicalize` + `prepend_with` fix (#3269) cleanup stream polls (#3397) Staking Miner (#3141) Companion for Substrate#8953 (#3140) Bump version, specs & substrate in prep for v0.9.8 (#3387) Fix busy loops. (#3392) Minor refactor (#3386) add simnet tests (#3381) BEEFY: adjust gossip (#3372) Companion for #9193 (#3376) ...

bridges in W<>R

faa5497

fix node compilation

95d4aa2

svyatonik added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jun 28, 2021

svyatonik commented Jun 28, 2021

View reviewed changes

HCastano approved these changes Jun 29, 2021

View reviewed changes

svyatonik and others added 5 commits June 30, 2021 12:43

Update runtime/rococo/src/bridge_messages.rs

0cde22f

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

Update runtime/rococo/src/bridge_messages.rs

b9727f2

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

Update runtime/rococo/src/bridge_messages.rs

1ad40ca

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

Update runtime/rococo/src/bridge_messages.rs

75383a0

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

long line fix

6a48aa5

HCastano requested a review from tomusdrw June 30, 2021 16:31

comment/remove -> comment/#[ignore]

0fa179e

tomusdrw approved these changes Jul 5, 2021

View reviewed changes

explicit instances

6f9c92a

ghost merged commit ad6e755 into master Jul 6, 2021

ghost deleted the bridges-integration-3 branch July 6, 2021 10:03

tomusdrw mentioned this pull request Jul 6, 2021

Bridge upgrade on Rococo/Wococo polkadot-js/api#3734

Closed

github-actions bot mentioned this pull request Aug 24, 2021

Update substrate/polkadot/cumulus from v0.9.8 to v0.9.9 moonbeam-foundation/moonbeam#729

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable over-bridge-messaging in Rococo/Wococo runtime #3377

Enable over-bridge-messaging in Rococo/Wococo runtime #3377

svyatonik commented Jun 28, 2021

svyatonik Jun 28, 2021 •

edited

HCastano left a comment

HCastano Jun 29, 2021

HCastano Jun 29, 2021

svyatonik Jun 30, 2021

svyatonik Jun 30, 2021

HCastano Jun 30, 2021

HCastano Jun 29, 2021

svyatonik Jun 30, 2021

HCastano Jun 30, 2021

tomusdrw Jul 5, 2021

HCastano Jun 29, 2021

tomusdrw left a comment

tomusdrw Jul 5, 2021

tomusdrw commented Jul 6, 2021

ghost commented Jul 6, 2021

	// 2) comment/remove the test.
	// 2) comment out or ignore the test.

		// Bridge messages support. The same story as with the bridge grandpa pallet above ^^^ - when we're
		// running as Rococo we only use `BridgeWococoMessages`/`BridgeWococoMessagesDispatch`, and vice versa.

Enable over-bridge-messaging in Rococo/Wococo runtime #3377

Enable over-bridge-messaging in Rococo/Wococo runtime #3377

Conversation

svyatonik commented Jun 28, 2021

svyatonik Jun 28, 2021 • edited

Choose a reason for hiding this comment

HCastano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomusdrw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomusdrw commented Jul 6, 2021

ghost commented Jul 6, 2021

svyatonik Jun 28, 2021 •

edited