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

Don't use domain when storing routers #1962

Closed cdamian closed 1 month ago

cdamian commented 1 month ago

Main question would be, when to use Routers::<T>::get() and when to use T::RouterId::for_domain().

lemunozm commented 1 month ago

Routers::::get() and when to use T::RouterId::for_domain()

cdamian commented 1 month ago

Routers::::get() and when to use T::RouterId::for_domain()

  • Routers::<T>::get() the list of all configured routers, something like a whitelist for the gateway.
  • T::RouterId::for_domain() the list of all configured routers for a specific domain. Used each time you need to know the routers for a domain.

    • More accurate will be to always filter this list with the Router::<T>::get(). I mean, each router returned by the T::RouterId::for_domain() that is not in Router::<T>::get() must be discarded

    • Maybe an auxiliary method that does that or similar, replacing for_domain() for that auxiliary method

Still not sure what the benefit is from doing all of this, instead of keeping track which router IDs are set for each domain, in storage, like the current approach.

lemunozm commented 1 month ago

Because basically, T::RouterId::for_domain() is representing the static relation between routers and domains, it's something that the gateway doesn't know (and something it can not ensure either). The gateway knows configured routers.

This list: T::RouterId::for_domain(), exists independently of the routers configured in the gateway

T::RouterId::for_domain() is something hardcoded at runtime type level and not configurable for the user

cdamian commented 1 month ago

Because basically, T::RouterId::for_domain() is representing the static relation between routers and domains, it's something that the gateway doesn't know (and something it can not ensure either). The gateway knows configured routers.

This list: T::RouterId::for_domain(), exists independently of the routers configured in the gateway

But what's wrong about having this relation in storage?

If this changes at any time, we'll have to do a RU right? If we store this, we just update the storage with whatever we need.

cdamian commented 1 month ago

Because basically, T::RouterId::for_domain() is representing the static relation between routers and domains, it's something that the gateway doesn't know (and something it can not ensure either). The gateway knows configured routers.

In the current implementation, the gateway knows about this relation. And I would also argue that the gateway should know, going forward.

lemunozm commented 1 month ago

The gateway doesn't know because not all Domain can be mapped to all Routers

lemunozm commented 1 month ago

The relation is not something the operator can choose, is something inherent from how the chain is coded, statically

lemunozm commented 1 month ago

Suppose, you use that storage. The user operator can add an entry Domain::Centrifuge with router AxelarEvm, which could not exists. Also the gateway can not check the relation is wrong

cdamian commented 1 month ago

The gateway doesn't know because not all Domain can be mapped to all Routers Suppose, you use that storage. The user operator can add an entry Domain::Centrifuge with router AxelarEvm, which could not exists. Also the gateway can not check the relation is wrong

OK, this is the case in the current implementation and one can definitely screw up the domain to router relationship.

The relation is not something the operator can choose, is something inherent from how the chain is coded, statically

^ fixes that.

Thank you for clarifying @lemunozm!

cdamian commented 1 month ago

Will adjust some things here then merge so I can continue working on the initial branch for multi-routers.