centrifuge / centrifuge-chain

Centrifuge Chain: the layer-1 blockchain for real-world assets, built on Substrate.

https://centrifuge.io

GNU Lesser General Public License v3.0

182 stars 78 forks source link

lp-forwarder: Always use Centrifuge domain when wrapping outbound mes… #1980

Open cdamian opened 3 weeks ago

cdamian commented 3 weeks ago

Description

The current implementation is using the stored domain when wrapping forward messages, however, this is incorrect since we should always use Domain::Centrifuge.

Checklist

[x] I have added Rust doc comments to structs, enums, traits and functions
[x] I have made corresponding changes to the documentation
[x] I have performed a self-review of my code
[x] I have added tests that prove my fix is effective or that my feature works

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 49.00%. Comparing base (c55e74f) to head (fa6c253). Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1980 +/- ## ========================================== + Coverage 48.98% 49.00% +0.01% ========================================== Files 183 183 Lines 13202 13202 ========================================== + Hits 6467 6469 +2 + Misses 6735 6733 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cdamian commented 3 weeks ago

LGTM! Thanks @cdamian.

A question, the check of line: https://github.com/centrifuge/centrifuge-chain/pull/1980/files#diff-d763817d4b90117d6295fc32e8fd8a66d2ee4093bfb40d90941a3368e72aa099R244-R247 makes sense? I mean, should we treat the forward info domain as a whitelist for domains?

I would leave it like that for now and get back to it after we discuss the rest of the LP concerns. Furthermore, setting forwarding info can only be done by admins so we can trust it for the time being anyway IMO. TBD if we need extra/different validation.