feat: Implement validation for cross-chain output oracles #307
Open
nahimterrazas wants to merge 3 commits intomainfrom
Open
feat: Implement validation for cross-chain output oracles #307nahimterrazas wants to merge 3 commits intomainfrom
nahimterrazas wants to merge 3 commits intomainfrom
Conversation
nahimterrazas
requested review from
NicoMolinaOZ,
pepebndc and
shahnami
as code owners
📝 WalkthroughWalkthroughAdded defense-in-depth oracle validations for EIP-7683 cross-chain outputs: reject all-zero oracles, reject non-zero upper 12 bytes, and require exact lower-20-byte oracle address matches supported destination routes across fill, claim, and validate flows; tests and fixtures updated accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/solver-order/src/implementations/standards/_7683.rs (1)
655-659: Useoutput_indexin all output-route error branches for consistent diagnostics.You already enumerate outputs; including index in unsupported-route/incompatible-oracle errors will make multi-output debugging much easier.
Also applies to: 686-690
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-order/src/implementations/standards/_7683.rs` around lines 655 - 659, The error messages for unsupported routes and incompatible-oracle branches currently reference origin_chain, dest_chain and input_oracle but omit the output index; update the two error branches that return OrderError::ValidationFailed (the branch checking supported_destinations.contains(&dest_chain) and the similar branch around lines 686-690) to include the output_index variable in the formatted message (e.g. "output {output_index}: route from chain {origin_chain} to chain {dest_chain} ..." and similarly for the incompatible-oracle message) so diagnostics consistently show which output failed; keep the existing context (input_oracle, origin_chain, dest_chain) and insert output_index into the format strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/solver-order/src/implementations/standards/_7683.rs`:
- Around line 317-329: The current validation only checks that cross-chain
output oracles are non-zero and have zero upper bytes but does not verify the
oracle is actually supported for the destination chain; add a route-membership
check after the existing canonical checks to ensure the oracle corresponds to a
known/registered route for output.chain_id (e.g. call a routing lookup like
is_oracle_valid_for_chain(output.oracle, output.chain_id) or consult the
RouteRegistry/AllowedOracles for that chain), and return
OrderError::ValidationFailed with a clear message ("unsupported oracle for
destination chain {chain_id}") if the oracle is not a member; apply the same
additional check in the other analogous block (the fill/claim validation at the
other location) so both places enforce route membership.
---
Nitpick comments:
In `@crates/solver-order/src/implementations/standards/_7683.rs`:
- Around line 655-659: The error messages for unsupported routes and
incompatible-oracle branches currently reference origin_chain, dest_chain and
input_oracle but omit the output index; update the two error branches that
return OrderError::ValidationFailed (the branch checking
supported_destinations.contains(&dest_chain) and the similar branch around lines
686-690) to include the output_index variable in the formatted message (e.g.
"output {output_index}: route from chain {origin_chain} to chain {dest_chain}
..." and similarly for the incompatible-oracle message) so diagnostics
consistently show which output failed; keep the existing context (input_oracle,
origin_chain, dest_chain) and insert output_index into the format strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0136949f-09f7-4eff-9696-7f23488c105c
📒 Files selected for processing (1)
crates/solver-order/src/implementations/standards/_7683.rs
Comment on lines
+317
to
+329
| // Defense in depth: cross-chain outputs must have a canonical non-zero oracle. | ||
| if output.oracle == [0u8; 32] { | ||
| return Err(OrderError::ValidationFailed(format!( | ||
| "Zero oracle not allowed for cross-chain output on chain {}", | ||
| output.chain_id.to::<u64>() | ||
| ))); | ||
| } | ||
| if output.oracle[..12].iter().any(|&byte| byte != 0) { | ||
| return Err(OrderError::ValidationFailed(format!( | ||
| "Output oracle has dirty upper bytes for cross-chain output on chain {}", | ||
| output.chain_id.to::<u64>() | ||
| ))); | ||
| } |
There was a problem hiding this comment.
Add route-membership validation in fill/claim (not just zero/dirty-byte checks).
These blocks validate canonical encoding, but they still accept a canonical unsupported output oracle for a destination chain. That leaves a defense-in-depth gap in transaction generation paths.
Proposed patch direction
@@ async fn generate_fill_transaction(...)
- if output.oracle[..12].iter().any(|&byte| byte != 0) {
+ if output.oracle[..12].iter().any(|&byte| byte != 0) {
return Err(OrderError::ValidationFailed(format!(
"Output oracle has dirty upper bytes for cross-chain output on chain {}",
output.chain_id.to::<u64>()
)));
}
+ let dest_chain_id = output.chain_id.to::<u64>();
+ let output_oracle_address = &output.oracle[12..32];
+ let has_supported_output_oracle = self.oracle_routes.supported_routes.values().any(|outs| {
+ outs.iter().any(|supported| {
+ supported.chain_id == dest_chain_id
+ && supported.oracle.0.as_slice() == output_oracle_address
+ })
+ });
+ if !has_supported_output_oracle {
+ return Err(OrderError::ValidationFailed(format!(
+ "Output oracle is not supported for destination chain {}",
+ dest_chain_id
+ )));
+ }
@@ async fn generate_claim_transaction(...)
if output.oracle[..12].iter().any(|&byte| byte != 0) {
return Err(OrderError::ValidationFailed(format!(
"Output oracle has dirty upper bytes for cross-chain output on chain {}",
output.chain_id.to::<u64>()
)));
}
+ let dest_chain_id = output.chain_id.to::<u64>();
+ let output_oracle_address = &output.oracle[12..32];
+ let has_supported_output_oracle = self.oracle_routes.supported_routes.values().any(|outs| {
+ outs.iter().any(|supported| {
+ supported.chain_id == dest_chain_id
+ && supported.oracle.0.as_slice() == output_oracle_address
+ })
+ });
+ if !has_supported_output_oracle {
+ return Err(OrderError::ValidationFailed(format!(
+ "Output oracle is not supported for destination chain {}",
+ dest_chain_id
+ )));
+ }
}Also applies to: 444-458
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-order/src/implementations/standards/_7683.rs` around lines 317
- 329, The current validation only checks that cross-chain output oracles are
non-zero and have zero upper bytes but does not verify the oracle is actually
supported for the destination chain; add a route-membership check after the
existing canonical checks to ensure the oracle corresponds to a known/registered
route for output.chain_id (e.g. call a routing lookup like
is_oracle_valid_for_chain(output.oracle, output.chain_id) or consult the
RouteRegistry/AllowedOracles for that chain), and return
OrderError::ValidationFailed with a clear message ("unsupported oracle for
destination chain {chain_id}") if the oracle is not a member; apply the same
additional check in the other analogous block (the fill/claim validation at the
other location) so both places enforce route membership.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ation in Eip7683OrderImpl
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/solver-order/src/implementations/standards/_7683.rs (1)
317-329:⚠️ Potential issue | 🟠 MajorEnforce supported-route membership in fill/claim, not only canonical encoding.
Line 317 and Line 444 only validate canonical bytes32 shape (non-zero + clean upper 12 bytes). A clean but unsupported oracle for the destination chain still passes these tx-generation paths, so the defense-in-depth gap remains.
🔧 Proposed patch direction
@@ async fn generate_fill_transaction( - // Defense in depth: cross-chain outputs must have a canonical non-zero oracle. + // Defense in depth: cross-chain outputs must have a canonical non-zero oracle. + let dest_chain_id = output.chain_id.to::<u64>(); if output.oracle == [0u8; 32] { return Err(OrderError::ValidationFailed(format!( "Zero oracle not allowed for cross-chain output on chain {}", - output.chain_id.to::<u64>() + dest_chain_id ))); } if output.oracle[..12].iter().any(|&byte| byte != 0) { return Err(OrderError::ValidationFailed(format!( "Output oracle has dirty upper bytes for cross-chain output on chain {}", - output.chain_id.to::<u64>() + dest_chain_id ))); } + let output_oracle_address = &output.oracle[12..32]; + let has_supported_output_oracle = + self.oracle_routes.supported_routes.values().any(|outs| { + outs.iter().any(|supported| { + supported.chain_id == dest_chain_id + && supported.oracle.0.as_slice() == output_oracle_address + }) + }); + if !has_supported_output_oracle { + return Err(OrderError::ValidationFailed(format!( + "Output oracle is not supported for destination chain {}", + dest_chain_id + ))); + } @@ - let dest_chain_id = output.chain_id.to::<u64>(); + // dest_chain_id already derived above @@ async fn generate_claim_transaction( if output.oracle[..12].iter().any(|&byte| byte != 0) { return Err(OrderError::ValidationFailed(format!( "Output oracle has dirty upper bytes for cross-chain output on chain {}", output.chain_id.to::<u64>() ))); } + let dest_chain_id = output.chain_id.to::<u64>(); + let output_oracle_address = &output.oracle[12..32]; + let has_supported_output_oracle = + self.oracle_routes.supported_routes.values().any(|outs| { + outs.iter().any(|supported| { + supported.chain_id == dest_chain_id + && supported.oracle.0.as_slice() == output_oracle_address + }) + }); + if !has_supported_output_oracle { + return Err(OrderError::ValidationFailed(format!( + "Output oracle is not supported for destination chain {}", + dest_chain_id + ))); + } }Also applies to: 444-458
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-order/src/implementations/standards/_7683.rs` around lines 317 - 329, The current checks only assert canonical oracle shape (output.oracle not all-zero and upper 12 bytes zero) but do not ensure the oracle is a supported destination route; update both validation sites that inspect output.oracle (the blocks returning OrderError::ValidationFailed for zero oracle and dirty upper bytes) to also verify membership in the supported-route registry (e.g., call the route table or SupportedRoutes::contains/get for the destination chain_id and/or oracle bytes). If the oracle is not present in the supported routes, return OrderError::ValidationFailed with a clear message including output.chain_id and the oracle identifier; apply this same membership check at the second validation site handling cross-chain outputs so unsupported-but-canonical oracles are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/solver-order/src/implementations/standards/_7683.rs`:
- Around line 317-329: The current checks only assert canonical oracle shape
(output.oracle not all-zero and upper 12 bytes zero) but do not ensure the
oracle is a supported destination route; update both validation sites that
inspect output.oracle (the blocks returning OrderError::ValidationFailed for
zero oracle and dirty upper bytes) to also verify membership in the
supported-route registry (e.g., call the route table or
SupportedRoutes::contains/get for the destination chain_id and/or oracle bytes).
If the oracle is not present in the supported routes, return
OrderError::ValidationFailed with a clear message including output.chain_id and
the oracle identifier; apply this same membership check at the second validation
site handling cross-chain outputs so unsupported-but-canonical oracles are
rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9876b0c-2d7e-4ad9-8e12-b77b00c890ae
📒 Files selected for processing (1)
crates/solver-order/src/implementations/standards/_7683.rs
pepebndc
approved these changes
Mar 6, 2026
shahnami
approved these changes
Mar 9, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/solver-order/src/implementations/standards/_7683.rs (1)
321-333:⚠️ Potential issue | 🟠 MajorStill missing supported-route validation in fill/claim.
These checks harden the bytes32 encoding, but they still allow a clean oracle whose lower 20 bytes are not registered for
output.chain_id.validate_order()already rejects that at Lines 679-686, and it matters here becausevalidate_and_create_order()can preserve parsableintent_datainstead of rebuildingorder.data.outputsfrom the validatedStandardOrder. Please reuse the same route-membership check here, ideally through a shared helper so these paths stay in sync.Patch direction
- if output.oracle == [0u8; 32] { - return Err(...); - } - if output.oracle[..12].iter().any(|&byte| byte != 0) { - return Err(...); - } + self.validate_cross_chain_output_oracle(output.chain_id.to::<u64>(), &output.oracle)?;Have
validate_cross_chain_output_oracle(...)include the same zero-oracle, dirty-upper-byte, and supported-route checks used byvalidate_order().Also applies to: 448-462
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-order/src/implementations/standards/_7683.rs` around lines 321 - 333, The cross-chain oracle check must also verify that the oracle (lower 20 bytes) is a supported route for output.chain_id as validate_order() does; modify validate_cross_chain_output_oracle(...) to perform the same three checks (non-zero oracle, clean upper 12 bytes, and membership of the lower-20-byte address in the supported routes for the given chain_id) so validate_and_create_order() and validate_order() share identical validation logic; reuse or extract the route-membership logic into a helper (called from validate_cross_chain_output_oracle and validate_order) so the behavior for StandardOrder route validation stays in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/solver-order/src/implementations/standards/_7683.rs`:
- Around line 321-333: The cross-chain oracle check must also verify that the
oracle (lower 20 bytes) is a supported route for output.chain_id as
validate_order() does; modify validate_cross_chain_output_oracle(...) to perform
the same three checks (non-zero oracle, clean upper 12 bytes, and membership of
the lower-20-byte address in the supported routes for the given chain_id) so
validate_and_create_order() and validate_order() share identical validation
logic; reuse or extract the route-membership logic into a helper (called from
validate_cross_chain_output_oracle and validate_order) so the behavior for
StandardOrder route validation stays in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e1acb734-9b23-4c84-b352-eb10d3f43fd8
📒 Files selected for processing (1)
crates/solver-order/src/implementations/standards/_7683.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit