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.

Sign up for GitHub

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

Jump to bottom

EthereumBlobExporter should not consume dest/msg when returning NotApplicable #5788

Closed
1 task
bkontur opened this issue Sep 20, 2024 · 0 comments · Fixed by #5789
Closed
1 task

EthereumBlobExporter should not consume dest/msg when returning NotApplicable #5788

bkontur opened this issue Sep 20, 2024 · 0 comments · Fixed by #5789
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T15-bridges This PR/Issue is related to bridges.

Comments

@bkontur
Copy link
Contributor

bkontur commented Sep 20, 2024

For example, here, a .take clears dest:

let dest = destination.take().ok_or(SendError::MissingArgument)?;
if dest != Here {
    log::trace!(target: "xcm::ethereum_blob_exporter", "skipped due to unmatched remote destination {dest:?}.");
    return Err(SendError::NotApplicable);
}

According to this, when returning NotApplicable, the exporter should not modify the input values, as this would prevent appending more exporters after this router in the tuple for xcm_executor::Config::MessageExporter.

I fixed something similar for other exporters here: #1519.

Note: This is not a crucial issue unless we are aware of it. However, if we don't fix it, we won't be able to append more exporters after EthereumBlobExporter.

TODO

  • fix all places when returning NotApplicable in the EthereumBlobExporter
@bkontur bkontur added T15-bridges This PR/Issue is related to bridges. C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Sep 20, 2024
@claravanstaden claravanstaden mentioned this issue Sep 20, 2024
Merged
@acatangiu acatangiu moved this to Todo in Bridges + XCM Sep 20, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2024
@claravanstaden @acatangiu
Prevents EthereumBlobExporter from consuming dest when returning NotA…
111b244 
…pplicable (#5789)

# Description

The EthereumBlobExporter consumes the `dest` parameter when the
destination is not `Here`. Subsequent exporters will receive a `None`
value for the destination instead of the original destination value,
which is incorrect.
 
Closes #5788

## Integration

Minor fix related to the exporter behaviour.

## Review Notes

Verified that tests
`exporter_validate_with_invalid_dest_does_not_alter_destination` and
`exporter_validate_with_invalid_universal_source_does_not_alter_universal_source`
fail without the fix in the exporter.

---------

Co-authored-by: Adrian Catangiu <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in Bridges + XCM Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T15-bridges This PR/Issue is related to bridges.
Projects
Bridges + XCM
Status: Done
Milestone
No milestone
1 participant
@bkontur