Conversation
…back to privileged fallback
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds LayerZero support: new pallets Changes
Sequence Diagram(s)sequenceDiagram
participant Eth as Ethereum (Snowbridge)
participant Relay as Relay Chain (Dancelight)
participant Proc as LayerZeroProcessor
participant Router as pallet_lz_router
participant XCM as XCM Router
participant Container as Container Chain
participant Receiver as LzReceiverExample
Eth->>Relay: Ethereum inbound -> EthereumInboundQueueV2
Relay->>Proc: submit Message -> try_extract_message
alt extract succeeds
Proc->>Router: process_message(LayerZeroInboundMessage)
Router->>Router: validate routing config & whitelist
Router->>XCM: send Transact XCM to Container
XCM->>Container: deliver Transact
Container->>Receiver: call receive_message (Transact)
Receiver->>Receiver: ensure ParentOrigin & emit MessageReceived
else extract fails
Proc->>Proc: delegate to PrivilegedFallbackProcessor (asset trap / fallback)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…r down to the container chain
WASM runtime size check:Compared to target branchdancebox runtime: 1888 KB (-4 KB) ✅ flashbox runtime: 1120 KB (no changes) ✅ dancelight runtime: 2696 KB (+16 KB) 🚨 starlight runtime: 2588 KB (+4 KB) 🚨 container chain template simple runtime: 1532 KB (+16 KB) container chain template frontier runtime: 1856 KB (-4 KB) ✅ |
Coverage Report@@ Coverage Diff @@
## master tarekkma/lz-processor +/- ##
=========================================================
+ Coverage 72.93% 73.12% +0.19%
+ Files 569 575 +6
+ Lines 81064 81722 +658
=========================================================
+ Hits 59124 59757 +633
+ Misses 21940 21965 +25
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_symbiotic_update.ts (1)
87-87: Assertion appears incorrect - missing equality check.
expect(externalValidatorsList, validators)passesvalidatorsas an optional message parameter toexpect(), not as the expected value. This test will always pass regardless of the actual validators list.🔎 Proposed fix
- expect(externalValidatorsList, validators); + expect(externalValidatorsList).toEqual(validators);
🧹 Nitpick comments (17)
pallets/lz-router/Cargo.toml (1)
31-31: Consider makingframe-benchmarkingoptional.
frame-benchmarkingis included as a regular dependency, meaning it will always be compiled even in non-benchmark builds. This contrasts withpallet-lz-receiver-example/Cargo.tomlwhich correctly marks it as optional.🔎 Proposed fix
-frame-benchmarking = { workspace = true } +frame-benchmarking = { workspace = true, optional = true }And update the
stdfeature:- "frame-benchmarking/std", + "frame-benchmarking?/std",pallets/lz-receiver-example/Cargo.toml (1)
24-27: Dev-dependencies shouldn't be instdfeature list.
sp-coreandsp-ioare declared as dev-dependencies (lines 25-26), but their/stdfeatures are included in thestdfeature array (lines 36-37). Dev-dependencies are only available during tests, so referencing them in the mainstdfeature could cause build issues in certain configurations.🔎 Proposed fix
Remove dev-dependency std features from the main std array:
std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", "parity-scale-codec/std", "scale-info/std", - "sp-core/std", - "sp-io/std", "sp-runtime/std", "xcm/std", ]pallets/lz-router/src/lib.rs (4)
131-132: Placeholder weights need proper benchmarks.
Weight::from_parts(10_000, 0)is a placeholder that doesn't reflect actual execution cost. The PoV (proof-of-value) component is 0, which may undercharge for storage reads.Consider adding a TODO or implementing proper weight benchmarks before production deployment:
// TODO: Add proper weight benchmarks for this call #[pallet::weight(Weight::from_parts(10_000, 0) + T::DbWeight::get().writes(1))]
161-162: Same placeholder weight issue forsend_message_to_ethereum.This extrinsic also uses placeholder weights. Since it's a stub (TODO on line 173), this is acceptable for now but should be addressed when implementation is complete.
173-174: TODO stub for Ethereum outbound queue integration.The
send_message_to_ethereumfunction currently only emits an event without actual message queuing. This is noted as a TODO, which is appropriate for a draft PR.Would you like me to help outline the integration points with the Snowbridge outbound queue when you're ready to implement this?
83-85: Remove#[pallet::without_storage_info]— not needed.
MessageForwardingConfig<T>explicitly derivesMaxEncodedLen, so the pallet can generate storage info without this attribute. This attribute should only be used when storage types don't implementMaxEncodedLen.test/helpers/pallets.ts (1)
1-1: Avoid@ts-nocheck- fix the underlying type issue instead.Disabling all type checking for this file defeats the purpose of using TypeScript. The type issue is likely related to the metadata API types.
🔎 Suggested fix with proper typing
-// @ts-nocheck - import type { DevModeContext } from "@moonwall/cli"; /** * Get the pallet index for a given pallet name */ export const getPalletIndex = async (palletName: string, context: DevModeContext): Promise<number> => { const metadata = await context.polkadotJs().rpc.state.getMetadata(); const pallets = metadata.asLatest.pallets; - const pallet = pallets.find((p) => p.name.toString() === palletName); + const pallet = pallets.find((p: { name: { toString(): string } }) => p.name.toString() === palletName); if (!pallet) { throw new Error(`Pallet ${palletName} not found`); } - return pallet.index.toNumber(); + return (pallet as { index: { toNumber(): number } }).index.toNumber(); };If the types are complex, consider using a targeted
// @ts-expect-errorwith an explanation on the specific problematic line rather than disabling checking for the entire file.chains/orchestrator-relays/runtime/dancelight/src/lib.rs (1)
2071-2076: LzRouter pallet registration is aligned; consider follow-up integration pointsRegistering
LzRouter: pallet_lz_router = 127fits the existing index layout and is sufficient to surface the pallet’s calls. As you evolve this pallet, consider whether it should also:
- Be reachable via specific
ProxyTypevariants / governance origins.- Be allowed or restricted under
MaintenanceFilter.- Get weight/benchmark coverage added to the
benchesmodule.These are follow‑ups rather than blockers.
chains/orchestrator-relays/runtime/dancelight/src/tests/processors/v2/layerzero_message_processor.rs (1)
1-401: LayerZero extraction tests give solid coverage of success and failure pathsThe test module sets up realistic LayerZero ABI payloads and exercises
LayerZeroMessageProcessor::try_extract_messageacross the happy path and key invalid cases (origin, assets/value, magic bytes, ABI decoding, RawPayload decoding, wrong variant). This is a good safety net around the extraction logic.If you touch this again, you might factor out the repeated
Message { .. }construction into a small helper to keep the tests a bit drier, but it’s not necessary for correctness.test/configs/zombie_tanssi_relay_layerzero.json (1)
55-118: Consider using distinct Prometheus ports per collatorAll collators for parachain 2000 share
prometheus_port: 33102(includingFullNode-2000). This can lead to port binding conflicts when nodes run on the same host. If these are intended to be separate processes, assigning unique Prometheus ports per node would be safer.test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message_fails.ts (1)
25-49: Negative-path LZ tests are good; keep error/name and indices in syncBoth failure cases (no config and non‑whitelisted sender) are well covered: you allow the inbound extrinsic to fail, then assert on decoded dispatch error strings, which should catch regressions in the router/processor logic. Two small points to keep in mind:
retrieveDispatchErrorsmatches by substring ("NoForwardingConfig","NotWhitelistedSender"), so any renaming of error variants will require updating these tests.- As in the success test, you configure
notificationDestination: [100, 0]; if the intent is to eventually notify the LZ receiver pallet on the container chain, verify that these indices match the actual(pallet_index, call_index)there.Also applies to: 51-85, 88-135
pallets/lz-receiver-example/src/lib.rs (1)
71-80: Pallet design is minimal and correct; a couple of small optional tweaksThe example pallet cleanly enforces an XCM origin (via
ParentOrigin) and then double‑checks that theLocationis the parent withis_parent, before emitting aMessageReceivedevent. The call’s weight is conservative and appropriate for an event‑only extrinsic.Two optional refinements you might consider later:
- If
ParentOriginis always configured asEnsureXcm<Equals<ParentLocation>>, the extrais_parentcheck becomes redundant; either rely solely on the origin type or relaxParentOriginand keep the explicit check.- If you expect larger payloads, you could switch the event field to a
BoundedVec<u8, MaxMessageSize>tied to the inbound queue’s max size to make the bound explicit.Also applies to: 84-94, 131-143
test/suites/zombie_tanssi_relay_layerzero/test_layerzero_forwarding.ts (2)
1-1: Consider removing@ts-nocheckif possible.Disabling TypeScript checking for the entire file can mask type errors. If specific lines have type issues, consider using
@ts-expect-errorwith explanations on those specific lines instead.
138-139: Hardcoded call index may be fragile.The
callIndex = 0is hardcoded assumingreceive_messageis the first call in the pallet. Consider deriving this from metadata similar to howpalletIndexis derived, which would make the test more resilient to pallet changes.🔎 Suggested approach
You could query the metadata to find the call index programmatically:
// Example: find call index from metadata const lzReceiverCalls = containerMetadata.asLatest.pallets .find(({ name }) => name.toString() === "LzReceiverExample") ?.calls.unwrap().type; // Then lookup the "receive_message" call index from the type registryprimitives/bridge/src/inbound_queue/layerzero_message.rs (1)
56-68: Consider defensive handling instead ofexpect()inFromimplementation.While the comment notes that
lzSourceAddressis always 32 bytes (matchingbytes32in Solidity), usingexpect()in aFromimplementation can cause panics if the invariant is ever violated. Consider usingTryFrominstead, or at minimum, ensure this conversion is only called in contexts where the size is already validated.🔎 Alternative with TryFrom
-impl From<SolMessage> for Message { - fn from(sol_message: SolMessage) -> Self { +impl TryFrom<SolMessage> for Message { + type Error = &'static str; + + fn try_from(sol_message: SolMessage) -> Result<Self, Self::Error> { Self { lz_source_address: sol_message .lzSourceAddress .to_vec() .try_into() - .expect("lzSourceAddress is always 32 bytes; qed"), + .map_err(|_| "lzSourceAddress must be 32 bytes")?, lz_source_endpoint: sol_message.lzSourceEndpoint, destination_chain: sol_message.destinationChain, message: sol_message.message.into(), } } }test/helpers/expect.ts (1)
1-1: Consider reducing@ts-nocheckscope.Similar to the test file,
@ts-nocheckdisables all TypeScript checking. While the complex generic types may require some flexibility, consider using targeted@ts-expect-errorcomments (which are already present in some places) to maintain type safety where possible.test/utils/xcm.ts (1)
13-13: Remove unnecessary console import.The
consoleobject is a Node.js global and doesn't need to be explicitly imported. This import can be safely removed.🔎 Proposed fix
-import * as console from "node:console";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
Cargo.tomlchains/container-chains/runtime-templates/frontier/Cargo.tomlchains/container-chains/runtime-templates/frontier/src/lib.rschains/container-chains/runtime-templates/frontier/src/xcm_config.rschains/container-chains/runtime-templates/simple/Cargo.tomlchains/container-chains/runtime-templates/simple/src/lib.rschains/container-chains/runtime-templates/simple/src/xcm_config.rschains/orchestrator-relays/runtime/dancelight/Cargo.tomlchains/orchestrator-relays/runtime/dancelight/src/bridge_to_ethereum_config.rschains/orchestrator-relays/runtime/dancelight/src/lib.rschains/orchestrator-relays/runtime/dancelight/src/tests/processors/v2/layerzero_message_processor.rschains/orchestrator-relays/runtime/dancelight/src/tests/processors/v2/mod.rschains/orchestrator-relays/runtime/dancelight/src/xcm_config.rschains/runtime-common/Cargo.tomlchains/runtime-common/src/processors/v2/fallback_message_processor.rschains/runtime-common/src/processors/v2/layerzero_message_processor.rschains/runtime-common/src/processors/v2/mod.rschains/runtime-common/src/processors/v2/raw_message_processor.rschains/runtime-common/src/processors/v2/symbiotic_message_processor.rschains/runtime-common/src/relay.rspallets/lz-receiver-example/Cargo.tomlpallets/lz-receiver-example/src/lib.rspallets/lz-router/Cargo.tomlpallets/lz-router/src/lib.rspallets/lz-router/src/types.rsprimitives/bridge/src/inbound_queue/layerzero_message.rsprimitives/bridge/src/inbound_queue/mod.rstest/configs/zombie_tanssi_relay_layerzero.jsontest/helpers/expect.tstest/helpers/index.tstest/helpers/pallets.tstest/moonwall.config.jsontest/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message.tstest/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message_fails.tstest/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_symbiotic_update.tstest/suites/zombie_tanssi_relay_layerzero/test_layerzero_forwarding.tstest/tsconfig.jsontest/utils/light-client.tstest/utils/xcm.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-12T17:11:07.511Z
Learnt from: tmpolaczyk
Repo: moondance-labs/tanssi PR: 1420
File: chains/runtime-common/src/processors/v2/proc_macro/Cargo.toml:1-19
Timestamp: 2025-12-12T17:11:07.511Z
Learning: Ensure that Cargo.toml files target Rust 1.85+ when using edition = "2024". Before bumping edition to 2024, verify the project is built with Rust 1.85.0 or newer and that all dependencies support the new edition. If upgrading, add or update the line: edition = "2024" in the Cargo.toml under [package], and test compilation and feature compatibility across the workspace.
Applied to files:
chains/runtime-common/Cargo.tomlCargo.tomlchains/container-chains/runtime-templates/simple/Cargo.tomlchains/orchestrator-relays/runtime/dancelight/Cargo.tomlpallets/lz-router/Cargo.tomlpallets/lz-receiver-example/Cargo.tomlchains/container-chains/runtime-templates/frontier/Cargo.toml
📚 Learning: 2025-12-16T15:08:22.484Z
Learnt from: tmpolaczyk
Repo: moondance-labs/tanssi PR: 1420
File: Cargo.toml:548-548
Timestamp: 2025-12-16T15:08:22.484Z
Learning: In the moondance-labs/tanssi repo, rely on toml-maid to enforce TOML formatting. Do not raise review issues for whitespace or formatting differences in TOML files that toml-maid does not flag. Focus reviews on substantive content changes; only flag formatting that toml-maid would flag or that indicates a real TOML syntax error.
Applied to files:
chains/runtime-common/Cargo.tomlCargo.tomlchains/container-chains/runtime-templates/simple/Cargo.tomlchains/orchestrator-relays/runtime/dancelight/Cargo.tomlpallets/lz-router/Cargo.tomlpallets/lz-receiver-example/Cargo.tomlchains/container-chains/runtime-templates/frontier/Cargo.toml
🧬 Code graph analysis (7)
test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message_fails.ts (4)
test/helpers/starlightVersions.ts (1)
STARLIGHT_VERSIONS_TO_EXCLUDE_FROM_SNOWBRIDGE_V2(21-21)test/utils/xcm.ts (1)
sendCallAsChildPara(1174-1270)test/utils/light-client.ts (1)
generateLayerZeroOutboundLog(294-309)test/helpers/events.ts (1)
retrieveDispatchErrors(50-71)
test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_symbiotic_update.ts (2)
test/utils/light-client.ts (1)
generateSymbioticOutboundLog(315-326)chains/orchestrator-relays/runtime/dancelight/src/lib.rs (1)
validators(1473-1475)
chains/orchestrator-relays/runtime/dancelight/src/tests/processors/v2/layerzero_message_processor.rs (2)
chains/runtime-common/src/processors/v2/fallback_message_processor.rs (1)
processors(111-111)chains/runtime-common/src/processors/v2/mod.rs (3)
into(93-107)from(60-62)try_extract_message(373-376)
test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message.ts (3)
test/helpers/starlightVersions.ts (1)
STARLIGHT_VERSIONS_TO_EXCLUDE_FROM_SNOWBRIDGE_V2(21-21)test/utils/xcm.ts (2)
getChildParaSovereignAccount(1156-1164)sendCallAsChildPara(1174-1270)test/utils/light-client.ts (2)
generateLayerZeroOutboundLog(294-309)generateUpdate(442-563)
chains/runtime-common/src/processors/v2/fallback_message_processor.rs (1)
chains/runtime-common/src/processors/v2/mod.rs (1)
handle_message(366-366)
primitives/bridge/src/inbound_queue/layerzero_message.rs (1)
chains/runtime-common/src/processors/v2/mod.rs (1)
from(60-62)
test/utils/xcm.ts (1)
test/utils/block.ts (1)
jumpToSession(31-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check-unused-dependencies
- GitHub Check: build
- GitHub Check: cargo-toml-feature-propagation
- GitHub Check: build-and-coverage
🔇 Additional comments (44)
primitives/bridge/src/inbound_queue/mod.rs (1)
17-22: LGTM - New module declaration is appropriate.The
layerzero_messagemodule is correctly declared as public. Note that unlike the other modules, it's not re-exported viapub use. This appears intentional for more explicit imports of LayerZero types.test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_symbiotic_update.ts (2)
10-10: Import refactor looks good.The switch from
generateOutboundMessageAcceptedLogtogenerateSymbioticOutboundLogproperly abstracts the symbiotic-specific defaults.
72-72: Simplified function call is cleaner.The new signature
generateSymbioticOutboundLog(polkadotJs, 1, validators)is more readable and hides the symbiotic-specific defaults internally.chains/container-chains/runtime-templates/frontier/src/xcm_config.rs (2)
107-109: NewParentLocationparameter is well-defined.The parameter correctly defines the parent chain location using
Location::parent().
117-134: Unpaid execution from parent chain is security-sensitive but appropriately scoped.Adding
AllowExplicitUnpaidExecutionFrom<Equals<ParentLocation>>allows the parent/orchestrator chain to send XCM messages that execute without paying fees. This is correctly scoped only to the parent location and positioned beforeWithComputedOriginfor proper evaluation order. Configuration is consistent across both frontier and simple runtime templates.Security consideration: Ensure the parent chain (relay/orchestrator) is fully trusted, as it can now dispatch arbitrary unpaid execution. The
UnpaidExecutioninstruction in the forwarded XCM (frompallet-lz-router) relies on this barrier passing.chains/orchestrator-relays/runtime/dancelight/src/tests/processors/v2/mod.rs (1)
35-37: LGTM - Test module addition follows existing pattern.The new
layerzero_message_processortest module is correctly declared alongside the existing processor test modules.chains/orchestrator-relays/runtime/dancelight/Cargo.toml (1)
145-145: Dependency addition is correct.
pallet-lz-routeris properly added as a workspace dependency.pallets/lz-router/Cargo.toml (1)
33-35: Verify necessity ofpallet-sessiondependency.
pallet-sessionwithhistoricalfeature is included, but reviewing the pallet source (pallets/lz-router/src/lib.rs), there's no apparent usage of session-related functionality. This may be copy-paste from another pallet's Cargo.toml.pallets/lz-router/src/lib.rs (1)
217-227: Remove the double encoding concern; the encoding is intentional and correct for XCM Transact format.The call construction
(pallet_index, call_index, message.encode()).encode()is correct. The XCM Transact instruction extracts the pallet and call indices from the call field, then decodes the remaining bytes as function arguments. Sincereceive_messageexpectspayload: Vec<u8>, the remaining bytes must be decodable as a Vec, which requires a length prefix. Encoding the tuple produces exactly this: pallet_index byte + call_index byte + (length prefix) + message bytes.The Unlimited unpaid execution with
check_origin: Noneis acceptable since the parent chain barrier is trusted, but this assumption should be documented.chains/container-chains/runtime-templates/frontier/Cargo.toml (1)
33-33: LGTM!The
pallet-lz-receiver-exampledependency is properly integrated with all required feature flags (std,runtime-benchmarks,try-runtime), following the established workspace dependency patterns.Also applies to: 194-194, 271-271, 321-321
chains/runtime-common/src/relay.rs (1)
51-56: LGTM!The
LayerZeroMessageProcessoris properly added to the v2 public re-exports, following the existing pattern and correctly gated behind therelayfeature flag.test/helpers/index.ts (1)
6-7: LGTM!New helper modules are properly re-exported, following the existing barrel export pattern.
test/moonwall.config.json (2)
52-52: LGTM!Adding
RUST_LOG=lz=debugenables LayerZero-specific debug logging for the dev environment, which is helpful for development and debugging.
503-565: New zombie test environment looks well-structured.The
zombie_tanssi_relay_layerzeroconfiguration follows existing patterns and includes appropriate setup for LayerZero E2E testing. The referenced config file./configs/zombie_tanssi_relay_layerzero.jsonexists and is included in this PR.chains/runtime-common/src/processors/v2/raw_message_processor.rs (1)
99-103: LGTM!Using a wildcard pattern
_to handle non-XcmRawPayloadvariants is appropriate here, as LayerZero and other message types should be processed by their dedicated processors. This provides forward compatibility for additional payload variants.test/helpers/pallets.ts (1)
8-16: Function logic is correct.The implementation properly retrieves pallet metadata and finds the index by name. The error handling for missing pallets is appropriate.
chains/runtime-common/Cargo.toml (2)
20-20: LGTM!
alloy-corewithsol-typesfeature is appropriate for LayerZero message handling which involves Solidity/Ethereum type encoding.
69-69: LGTM!
pallet-lz-routeris properly integrated as an optional dependency with correct feature flag propagation:
- Optional in dependencies
- Gated behind
relayfeature- Proper
?suffix usage for conditional feature propagation instd,runtime-benchmarks, andtry-runtimeAlso applies to: 95-95, 129-129, 148-148, 186-186
Cargo.toml (1)
90-91: Workspace entries for LayerZero pallets are consistentBoth
pallet-lz-routerandpallet-lz-receiver-examplefollow the existing pattern (path-based,default-features = false) and integrate cleanly into[workspace.dependencies]. No issues from a workspace/layout perspective.chains/runtime-common/src/processors/v2/mod.rs (1)
20-24: LayerZero processor wiring and RawPayload extension look correctExposing
layerzero_message_processorand appendingRawPayload::LayerZero(Vec<u8>)as the last variant keeps SCALE encoding compatible with existing variants and fits the existing processor re-export pattern.Also applies to: 110-115
chains/container-chains/runtime-templates/simple/Cargo.toml (1)
30-30: LZ receiver example pallet is correctly wired into the simple template
pallet-lz-receiver-exampleis added as a workspace dependency and consistently enabled forstd,runtime-benchmarks, andtry-runtime, mirroring other pallets in this runtime template. This looks coherent and ready for runtime integration.Also applies to: 151-152, 225-225, 271-272
chains/container-chains/runtime-templates/simple/src/xcm_config.rs (1)
49-57: XCM barrier change to allow explicit unpaid exec from parent is coherent with LayerZero routingIntroducing
ParentLocationand addingAllowExplicitUnpaidExecutionFrom<Equals<ParentLocation>>intoXcmBarriercleanly allows explicitly‑marked unpaid XCM from the relay/orchestrator while keeping the rest of the barrier unchanged. That’s consistent with using the parent as a trusted entry point for forwarded messages (e.g. LayerZero).Please just re‑confirm this matches your threat model (i.e., the parent may execute unpaid XCM here and cannot be adversarial in your deployment).
Also applies to: 100-102, 109-126
chains/container-chains/runtime-templates/frontier/src/lib.rs (2)
59-61: Trait imports are consistent with later usageNewly imported traits (
Equals,InsideBoth,Contains,InstanceFilter, etc.) are all exercised later in call filters and XCM wiring; no issues spotted.
1111-1113: LzReceiverExample wiring looks correct; verify ParentLocation definitionThe
pallet_lz_receiver_example::Configimplementation andconstruct_runtime!registration (index 79) are consistent and match the pallet’s expectations (Parent-origin via XCM and(pallet_index, call_index)encoding). Just ensure thatxcm_config::ParentLocationis exactly the relay/parent location (parents = 1, interior = Here) so it aligns with the pallet’sis_parentcheck.Also applies to: 1161-1161
chains/orchestrator-relays/runtime/dancelight/src/xcm_config.rs (2)
135-144: XcmPassthrough addition is appropriate for XCM-aware palletsAdding
pallet_xcm::XcmPassthrough<RuntimeOrigin>toLocalOriginConvertercleanly exposespallet_xcm::Origin::Xcmto the runtime, which is required for pallets (like the LZ router) that need to see the raw XCM origin. This fits the existing XCM config and barrier setup.
176-181: LZ router origin and location conversion look sound
OnlyParachainscorrectly constrainsContainerChainOrigintoLocations of the form(parents = 0, [Parachain(_)]), andLocationConverteralready handles these locations. TheMaxWhitelistedSendersbound is reasonable. No issues seen with the newpallet_lz_router::Configwiring.Please double‑check that all expected container parachains present themselves with
parents = 0in XCM origins (and not e.g. via another consensus layer), otherwiseOnlyParachainsmay be too strict.Also applies to: 357-361
chains/orchestrator-relays/runtime/dancelight/src/bridge_to_ethereum_config.rs (1)
46-50: LayerZero inbound processor wiring is consistent with existing processorsThe
LayerZeroInboundMessageProcessorV2alias mirrors the type parameters of the existing Raw and Symbiotic processors, and adding it as the third entry in theMessageProcessortuple forsnowbridge_pallet_inbound_queue_v2::Configlooks structurally correct. As long ascan_process_messagefor Raw/Symbiotic/LayerZero are mutually exclusive or ordered intentionally, this integration should behave as expected.It’s worth confirming in tests that a LayerZero message is actually handled by
LayerZeroInboundMessageProcessorV2and not accidentally claimed by the Raw or Symbiotic processors first.Also applies to: 752-783, 793-797
test/suites/dev-tanssi-relay/ethereum-inbound-queue-v2/test_receive_layerzero_message.ts (1)
25-49: Setup (runtime detection and funding) is reasonableRuntime detection/skip for specific Starlight specVersions and funding the para 2000 sovereign account via
transferAllowDeathalign with the other Snowbridge V2 tests; no issues here.chains/container-chains/runtime-templates/simple/src/lib.rs (2)
53-55: New trait imports align with filters and XCM usageAdding
Contains,Equals,InsideBoth, andInstanceFiltermatches their use in proxy filters, maintenance filters, and XCM execution manager; nothing problematic here.
914-916: LzReceiverExample integration looks consistent with other runtimesThe
pallet_lz_receiver_example::Configimplementation and runtime registration at index 79 mirror the frontier template, withParentOriginconstrained toEnsureXcm<Equals<xcm_config::ParentLocation>>. This is consistent with the example pallet’s expectations.Just confirm
xcm_config::ParentLocationis the same parent location the relay uses when sending LayerZero notifications.Also applies to: 958-958
pallets/lz-router/src/types.rs (1)
17-55: LGTM!The type definitions are well-structured with appropriate documentation. The use of
BoundedVecforwhitelisted_sendersensures bounded storage, and the derive macros are correctly applied for a storage-compatible configuration struct. Theskip_type_params(T)attribute properly handles the generic parameter for scale-info.chains/runtime-common/src/processors/v2/fallback_message_processor.rs (1)
164-250: LGTM!The rename from
SymbioticFallbackProcessortoPrivilegedFallbackProcessoris appropriate as this processor now serves both Symbiotic and LayerZero protocols. The conditional fallback logic correctly differentiates between user errors (trap assets for recovery) and middleware errors (return error to signal the problem).chains/runtime-common/src/processors/v2/symbiotic_message_processor.rs (1)
19-21: LGTM!The updates correctly integrate the renamed
PrivilegedFallbackProcessorand simplify the match arm by using a wildcard pattern for non-Symbiotic payloads. This maintains the same behavior while being more concise and future-proof for additional payload types.Also applies to: 86-89, 210-229
test/suites/zombie_tanssi_relay_layerzero/test_layerzero_forwarding.ts (1)
55-348: Comprehensive E2E test coverage.The test suite thoroughly covers the LayerZero forwarding flow from relay to container chain, including:
- Pallet verification on both relay and container chains
- XCM-based forwarding config setup with proper sovereign account funding
- LayerZero message submission and relay chain processing
- DMP delivery verification on the container chain
chains/runtime-common/src/processors/v2/layerzero_message_processor.rs (2)
40-100: Well-structured LayerZero message processor.The implementation follows the established pattern from
SymbioticMessageProcessorwith appropriate validation:
- Origin verification against gateway proxy
- Asset/value checks to prevent asset-containing messages
- Magic bytes validation for message authenticity
- Clean delegation to
pallet_lz_routerfor forwardingThe use of
PrivilegedFallbackProcessoras the fallback type is consistent with the generalized fallback approach.Also applies to: 102-106, 108-202
70-76: No changes needed; ABI decode mutable slice usage is correct.The
abi_decode_validatemethod requires a mutable reference to track buffer position during deserialization, consistent with how theDecodetrait is used throughout the codebase (e.g.,Decode::decode(&mut signature.as_ref())). The pattern&mut payload.as_slice()is idiomatic for codec operations that consume bytes sequentially.Likely an incorrect or invalid review comment.
primitives/bridge/src/inbound_queue/layerzero_message.rs (1)
24-29: LGTM!The LayerZero message types are well-defined with appropriate MAGIC_BYTES for payload verification. The
sol!macro correctly defines the Solidity-compatible structures, and the RustMessagestruct provides a clean interface with proper derives for encoding/decoding.Also applies to: 31-45, 47-53
test/utils/light-client.ts (2)
118-121: Verify address padding for non-20-byte inputs.The padding logic assumes
lzSourceAddressneeds left-padding to 32 bytes. For a 20-byte Ethereum address, this produces 12 leading zero bytes + 20 address bytes, which is correct. However, if the input is already 32 bytes or has unexpected length, the behavior may be incorrect:
- If
lzSourceAddress.length > 32:slice(0, 32)truncates correctly- If
lzSourceAddress.length < 32:32 - lengthgives positive offset, padding leftThis looks correct for the intended use case (Ethereum addresses to bytes32).
109-129: Well-designed LayerZero test utilities.The additions follow the existing patterns for XCM and Symbiotic payloads. The
LayerZeroMessageParamsinterface provides a clean API, and the convenience functions (generateLayerZeroOutboundLog,generateSymbioticOutboundLog) reduce boilerplate in tests while maintaining flexibility through the underlyinggenerateOutboundMessageAcceptedLog.Also applies to: 131-135, 164-170, 187-204, 290-326
test/helpers/expect.ts (1)
10-39: Useful test assertion helpers.The
expectOk,expectSubstrateEvent,expectSubstrateEvents, andexpectSystemEventhelpers provide good abstractions for common test patterns. The error messages include helpful context about which events were found when assertions fail.Also applies to: 41-97, 99-130, 141-156
test/utils/xcm.ts (4)
178-190: LGTM - but note duplicate function below.The implementation correctly computes the sovereign account using the "para" prefix convention. However, this function is duplicated at lines 1156-1164 as
getChildParaSovereignAccount.
466-479: Verify the omission of maxAssets with customBeneficiary.When
customBeneficiaryis provided, the function creates aDepositAssetinstruction without themaxAssetsfield, while the legacy path (lines 487 and 500) includes it. Themax_assetsparameter is also ignored in this code path.Ensure this behavior is intentional and aligns with the XCM specification for the version being used.
663-675: LGTM!The
as_v4()andas_v5()methods correctly mirror the existingas_v3()pattern and enable versioned XCM message emission for V4 and V5.
1174-1270: LGTM with note on circular dependency pattern.This comprehensive helper function correctly orchestrates child-to-relay XCM message flows for E2E testing. The dynamic import of
jumpToSession(line 1185) is a reasonable workaround for circular dependencies.The validation logic properly searches through blocks to verify message processing, and the hard-coded weight values are appropriate for test utilities.
Consider refactoring the module structure in a future change to eliminate the need for dynamic imports, which can make dependencies less transparent.
test/suites/dev-tanssi-relay/lz-router/test_receive_layerzero_message.ts
Show resolved
Hide resolved
|
/cmd generate-ts-api |
|
/cmd generate-ts-api |
…nd improve error handling. Updated message type to BoundedVec for size constraints and modified conversion logic to return specific errors for oversized messages.
… and OutboundSolMessageEnvelope for better clarity. Updated related payload handling and ensured consistent naming conventions across the codebase.
…cluding magic bytes
| // Expected responses are OK. | ||
| AllowKnownQueryResponses<PolkadotXcm>, | ||
| // Allow unpaid execution from the parent chain (for LayerZero forwarded messages) | ||
| AllowExplicitUnpaidExecutionFrom<Equals<ParentLocation>>, |
There was a problem hiding this comment.
Remember that we still need to discuss if we actually want this barrier in containers or not. Maybe add a TODO for it?
There was a problem hiding this comment.
Okay will leave this comment up, until we decide on what we will do.
...rchestrator-relays/runtime/dancelight/src/tests/processors/v2/layerzero_message_processor.rs
Outdated
Show resolved
Hide resolved
| XcmProcessor: ExecuteXcm<<T as pallet_xcm::Config>::RuntimeCall>, | ||
| XcmWeigher: WeightBounds<<T as pallet_xcm::Config>::RuntimeCall>, | ||
| { | ||
| type Fallback = PrivilegedFallbackProcessor< |
There was a problem hiding this comment.
I'm not sure if this is a good fallback. There is this code in PrivilegedFallbackProcessor:
tanssi/chains/runtime-common/src/processors/v2/fallback_message_processor.rs
Lines 239 to 248 in 7980ced
In LayerZeroMessageProcessor the origin is always the gateway, but it is a user initiated message. So the assumption that gateway origin == privileged message is no longer true, and the fallback processor will return an error instead of trapping the assets.
But, LayerZeroMessageProcessor enforces that the message has no assets and no value in try_extract_message. And the fallback is executed only if try_extract_message returns ok and process_message returns err. So trapping assets is not needed.
There is no "AlwaysErrorFallbackProcessor", but maybe that's what we need here. Or just ignore the error and set a "NoopFallbackProcessor"?
I don't remember what was the problem with returning an error in the fallback processor so I don't know the difference, do you remember @Agusrodri ?
There was a problem hiding this comment.
According to the fallback processor docs:
/// This processor always returns success to prevent reverting the Ethereum transaction,
/// which would leave assets in limbo on the Ethereum side. If XCM execution fails,
/// the error is logged but the processor still returns success, allowing the claimer
/// to recover the trapped assets later.
There was a problem hiding this comment.
I discussed with @ParthDesai about this case. when layerzero message is sent from TanssiLzHub (Eth) to the Gateway contract, we will override the origin to be the gateway. It will be treated as symbiotic update
| if message.origin != gateway_proxy_address { | ||
| return Err(MessageExtractionError::InvalidMessage { | ||
| context: format!( | ||
| "LayerZero message origin is {:?} expected {:?}", | ||
| message.origin, gateway_proxy_address | ||
| ), | ||
| source: None, | ||
| }); |
There was a problem hiding this comment.
So related to @tmpolaczyk comment, we should modify this section as well right?
...integration-tests/xcm/emulated/tests/assets/dancelight/src/tests/unpaid_execution_barrier.rs
Show resolved
Hide resolved
4 participants
Summary by CodeRabbit
New Features
Integration
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.