Add inbound queue #750

ParthDesai · 2024-11-18T09:34:12Z

Description

This PR adds inbound queue to starlight and include tests for the same.

It adds message processor called SymbioticMessageProcessor which upon receiving new validators, calls ExternalValidators pallet to set the validators.

What is remaining?

Benchmarking

Note

~~The base of the PR is at tomasz-generic-aggregate-message-origin since this PR is dependent upon code introduced there.~~
Base changed to master after PR merge.

github-actions · 2024-11-18T09:44:27Z

WASM runtime size check:

Compared to target branch

dancebox runtime: 1424 KB (no changes) ✅

flashbox runtime: 820 KB (no changes) ✅

dancelight runtime: 2104 KB (+2104 KB) ⚠️

container chain template simple runtime: 1124 KB (-4296 KB) ✅

container chain template frontier runtime: 1400 KB (-5152 KB) ✅

solo-chains/runtime/dancelight/src/bridge_to_ethereum_config.rs

solo-chains/runtime/dancelight/src/symbiotic_message_processor.rs

solo-chains/runtime/dancelight/src/tests/inbound_queue_tests/mock.rs

github-actions · 2024-11-20T09:11:02Z

Coverage Report

(master)

@@                 Coverage Diff                  @@
##           master   add-inbound-queue     +/-   ##
====================================================
  Coverage   64.57%              64.57%   0.00%     
+ Files         317                 320      +3     
+ Lines       55357               55407     +50     
====================================================
+ Hits        35745               35777     +32     
+ Misses      19612               19630     +18

Files Changed	Coverage
/solo-chains/runtime/dancelight/src/lib.rs	66.21% (+0.02%)	✅

Coverage generated Tue Nov 26 13:23:17 UTC 2024

tmpolaczyk · 2024-11-26T13:37:54Z

solo-chains/runtime/dancelight/src/bridge_to_ethereum_config.rs

 };

+// Ethereum Bridge
+parameter_types! {
+    pub storage EthereumGatewayAddress: H160 = H160(hex_literal::hex!("EDa338E4dC46038493b885327842fD3E301CaB39"));


What is this storage modifier? It means that this is stored in storage and can be modified by root?

it's the first time that I see it too

The storage modifier means following:

From name of the variable a key is derived. Formula for derivation is: twox_128(":" ++ NAME ++ ":")

If there is a value under the key, it is returned in response to get()

If not, value specified in the assignment H160(hex_literal::hex!("EDa338E4dC46038493b885327842fD3E301CaB39")) is returned.

Also, yes since only root can write to arbitrary key, this is modified by root.

tmpolaczyk · 2024-11-26T13:38:48Z

primitives/bridge/src/symbiotic_message_processor.rs

+}
+
+#[derive(Encode, Decode)]
+pub enum Message<T>


Add docs that this is the message that comes from ethereum

tmpolaczyk · 2024-11-26T13:40:11Z

Cargo.toml

@@ -259,12 +259,18 @@ xcm-runtime-apis = { git = "https://github.com/moondance-labs/polkadot-sdk", bra


 # Bridges (wasm)
+alloy-primitives = { version = "0.4.2", default-features = false }


Suggested change

alloy-primitives = { version = "0.4.2", default-features = false }

I believe this is unused

girazoki · 2024-11-26T13:46:54Z

primitives/bridge/src/symbiotic_message_processor.rs

+        let message = if let Ok(payload) = decode_result {
+            payload.message
+        } else {
+            return Err(DispatchError::Other("unable to parse the payload"));


Suggested change

return Err(DispatchError::Other("unable to parse the payload"));

return Err(DispatchError::Other("unable to parse the envelope payload"));

girazoki · 2024-11-26T13:47:46Z

primitives/bridge/src/symbiotic_message_processor.rs

+}
+
+#[derive(Encode, Decode)]
+pub enum Command<T>


shal we call this InboundCommand? reason is we have a different est of outbound commands right?

girazoki · 2024-11-26T13:48:50Z

solo-chains/runtime/dancelight/src/bridge_to_ethereum_config.rs

@@ -140,16 +153,23 @@ impl snowbridge_pallet_system::Config for Runtime {
    #[cfg(feature = "runtime-benchmarks")]
    type Helper = benchmark_helper::EthSystemBenchHelper;
    type DefaultPricingParameters = Parameters;
-    type InboundDeliveryCost = ();
-    //type InboundDeliveryCost = EthereumInboundQueue;
+    type InboundDeliveryCost = EthereumInboundQueue;


what are the implicatrions of this? are we going to charge for the validators payload?

from what I can see it is the price that we can communicate to the SC, so I think its correct, but plz confirm

Yes. And we only communicate in case root calls the extrinsic on system pallet. So, I think it is okay.

girazoki

Approved but I have an important question and minor comments. Specially in the inbound delivery cost

ParthDesai changed the title ~~[WIP] Add inbound queue + tests~~ [WIP] Add inbound queue Nov 18, 2024

ParthDesai force-pushed the add-inbound-queue branch from 529100a to 77d47fb Compare November 19, 2024 15:48

girazoki reviewed Nov 19, 2024

View reviewed changes

solo-chains/runtime/dancelight/src/bridge_to_ethereum_config.rs Outdated Show resolved Hide resolved

girazoki reviewed Nov 19, 2024

View reviewed changes

solo-chains/runtime/dancelight/src/symbiotic_message_processor.rs Outdated Show resolved Hide resolved

girazoki reviewed Nov 19, 2024

View reviewed changes

solo-chains/runtime/dancelight/src/tests/inbound_queue_tests/mock.rs Outdated Show resolved Hide resolved

ParthDesai force-pushed the add-inbound-queue branch 4 times, most recently from fff9fdc to 3eba4a5 Compare November 25, 2024 15:14

ParthDesai changed the title ~~[WIP] Add inbound queue~~ Add inbound queue Nov 25, 2024

ParthDesai force-pushed the add-inbound-queue branch 2 times, most recently from d2b0917 to 33b0d32 Compare November 26, 2024 11:28

Base automatically changed from tomasz-generic-aggregate-message-origin to master November 26, 2024 12:20

ParthDesai added 7 commits November 26, 2024 16:26

Initial commit

4c7f8ed

fix runtime benchmark

4c3ff69

remove mock runtime

0842b9d

move dummy data structures to primitives

7bf964c

remove un-needed implementation

804b9dc

move symbiotic_message_processor to primitives

6637ea7

fix CI

cb35524

ParthDesai force-pushed the add-inbound-queue branch from 33b0d32 to cb35524 Compare November 26, 2024 12:26

ParthDesai marked this pull request as ready for review November 26, 2024 12:27

tmpolaczyk reviewed Nov 26, 2024

View reviewed changes

girazoki reviewed Nov 26, 2024

View reviewed changes

girazoki approved these changes Nov 26, 2024

View reviewed changes

girazoki added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Nov 26, 2024

address review feedback

a10d028

ParthDesai added the breaking Needs to be mentioned in breaking changes label Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inbound queue #750

Add inbound queue #750

ParthDesai commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

tmpolaczyk Nov 26, 2024

girazoki Nov 26, 2024

ParthDesai Nov 26, 2024

ParthDesai Nov 26, 2024

tmpolaczyk Nov 26, 2024

tmpolaczyk Nov 26, 2024

girazoki Nov 26, 2024

girazoki Nov 26, 2024

girazoki Nov 26, 2024

girazoki Nov 26, 2024

ParthDesai Nov 27, 2024

girazoki left a comment

		@@ -259,12 +259,18 @@ xcm-runtime-apis = { git = "https://github.com/moondance-labs/polkadot-sdk", bra


		# Bridges (wasm)
		alloy-primitives = { version = "0.4.2", default-features = false }

	return Err(DispatchError::Other("unable to parse the payload"));
	return Err(DispatchError::Other("unable to parse the envelope payload"));

Add inbound queue #750

Are you sure you want to change the base?

Add inbound queue #750

Conversation

ParthDesai commented Nov 18, 2024 • edited Loading

Description

What is remaining?

Note

github-actions bot commented Nov 18, 2024 • edited Loading

WASM runtime size check:

Compared to target branch

github-actions bot commented Nov 20, 2024 • edited Loading

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

girazoki left a comment

Choose a reason for hiding this comment

ParthDesai commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading