paritytech / trappist

Apache License 2.0
82 stars 17 forks source link

Configure Origins for Transact in a secure way #118

Open hbulgarini opened 1 year ago

hbulgarini commented 1 year ago

For the sub0 presentation i did a hack for converting the XCM caller into the proper chain Origin:

pub struct ParachainAccountId32Aliases<Network, AccountId>(PhantomData<(Network, AccountId)>);
impl<Network: Get<Option<NetworkId>>, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone>
    Convert<MultiLocation, AccountId> for ParachainAccountId32Aliases<Network, AccountId>
{
    fn convert(location: MultiLocation) -> Result<AccountId, MultiLocation> {
        let id = match location {
            MultiLocation { parents: 1, interior: X2(Parachain(_), AccountId32 { id, network: None }) } => id,
            MultiLocation { parents: 1, interior: X2(Parachain(_), AccountId32 { id, network }) }
                if network == Network::get() =>
                id,
            _ => return Err(location),
        };
        Ok(id.into())
    }

    fn reverse(who: AccountId) -> Result<MultiLocation, AccountId> {
        Ok(AccountId32 { id: who.into(), network: Network::get() }.into())
    }
}

https://github.com/paritytech/trappist/blob/sub0_2022/runtime/trappist/src/xcm_config.rs#L73:L91

While i was discussing this implementation with the moonbeam guys, they addressed this configuration might bring some security issues (like sending the call from other parachain than the one expected and pass the Origin convertion anyway).

The point here would be to properly extract which account and which chain the XCM message was sent from so we can properly address the security around this.

hbulgarini commented 1 year ago

@stiiifff can i take this one?

stiiifff commented 1 year ago

@hbulgarini I'd rather complete M1 first.

hbulgarini commented 1 year ago

@hbulgarini I'd rather complete M1 first.

Yes, the idea is once M1 is completed not now.

stiiifff commented 1 year ago

@hbulgarini can you review this one and amend if needed ? Ideally, the design need to be fully specified before we can move it to the Ready status.

hbulgarini commented 1 year ago

@hbulgarini can you review this one and amend if needed ? Ideally, the design need to be fully specified before we can move it to the Ready status.

It seems that XCM code base has considered this need already with this new PR: https://github.com/paritytech/polkadot/pull/6662

Potentially i will review that feature in order to implement this one.