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.

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

Add pallet-omni-bridge #3223

Merged
merged 20 commits into from
Jan 8, 2025
Merged

Add pallet-omni-bridge #3223

merged 20 commits into from
Jan 8, 2025

Conversation

Kailai-Wang
Copy link
Collaborator

@Kailai-Wang Kailai-Wang commented Jan 7, 2025

Context

fixes P-1261

It's a rewriting/refactoring of the old bridge pallet for chainsafe, it tries to be as chainsafe-friendly as possible as we are still going to use chainsafe contracts on eth side (due to time pressure), main changes:

  • use a single pallet and decouple it with pallet_assets
  • allows bridging of both native token and local assets (XCM assets are not supported for now)
  • clean up already finalised payouts automatically (*)

(*) please check the code logic - theoretically it might happen that the cleanup takes too much time (as it would go through too many loops), but in practice, there's very low chance that it would happen, it would imply the majority of relayers malfunctions.

Still, we could add some improvements later to bound the execution time.

@Kailai-Wang Kailai-Wang self-assigned this Jan 7, 2025
Copy link

linear bot commented Jan 7, 2025

#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
pub enum ChainType {
Heima, // this chain
Ethereum(u32), // with chain id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If adding new variant here is the only change needed to be made in order to support new chain type I would suggest using something more generic

#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
pub struct ChainAsset<AssetKind> {
pub chain: ChainType,
pub asset: AssetKind,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are currently using different pallets to register/create assets, it would be great to streamline creating assets for bridging via this bridge such that it's easier to create and manage assets accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might require another pallet - something like asset manager/registry.

I intentionally decouple it from this bridge pallet (initially I have something like create_local_asset and then removed it), this pallet handles bridging only but not creation/deletion of assets.

It only cares if an asset exists and can be mutated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we are good with Balances and Assets - I can imagine if we have XCM assets we might need a dedicated pallet to manage them all

@Kailai-Wang Kailai-Wang enabled auto-merge (squash) January 8, 2025 19:38
@Kailai-Wang Kailai-Wang merged commit f82e400 into dev Jan 8, 2025
20 checks passed
@Kailai-Wang Kailai-Wang deleted the add-pallet-omni-bridge branch January 8, 2025 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants