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

Make relayer loops generic #2491

Closed
tomusdrw opened this issue Oct 23, 2020 · 7 comments
Closed

Make relayer loops generic #2491

tomusdrw opened this issue Oct 23, 2020 · 7 comments
Assignees
Milestone

Comments

@tomusdrw
Copy link
Contributor

Currently for each combination of chains we need to prepare a separate headers & messages sync loop.
There are two main reasons for that:

  1. We need to encode a Call to a specific runtime when creating the transaction and add the right SignedExtensions data.
  2. RuntimeApis exposed by the pallets are specific to chains.

For (1) the solution would be to create transactions based on Metadata somehow or via RPC. (1) has also a very unwanted consequence of being dependent on the entire runtime code - that could be alleviated by extracting some of the runtime code which allows transaction creation.

For (2) we should probably just add some identifier to the pallet instances and have them share the RuntimeApi, but add an extra parameter with instance id.

@tomusdrw
Copy link
Contributor Author

@tomusdrw
Copy link
Contributor Author

Partially done via recent CLI refactoring (#849), we might keep this around to make the relayer based on metadata only (scale-info stuff).

@EmmanuellNorbertTulbure
Copy link

@svyatonik is this done? Is still something needed to be done?

@svyatonik
Copy link
Contributor

@svyatonik is this done? Is still something needed to be done?

Yes - it needs to be done

@serban300
Copy link
Collaborator

  1. RuntimeApis exposed by the pallets are specific to chains.

For (2) we should probably just add some identifier to the pallet instances and have them share the RuntimeApi, but add an extra parameter with instance id.

I couldn't find a way to implement this. I hope I didn't miss anything. The problem is that the function parameters/return types are different depending on the chain and in order to handle this we need some way of specifying generic params. I tried 2 options:

	impl bp_runtime::chain::BridgeFinalityApi<Block, bp_rialto::Rialto> for Runtime {
		fn best_finalized() -> Option<bp_runtime::HeaderIdOf<bp_rialto::Rialto>> {
			pallet_bridge_grandpa::Pallet::<Runtime, RialtoGrandpaInstance>::best_finalized()
		}
	}
	
	impl bp_runtime::chain::BridgeFinalityApi<Block, bp_westend::Westend> for Runtime {
		fn best_finalized() -> Option<bp_runtime::HeaderIdOf<bp_westend::Westend>> {
			pallet_bridge_grandpa::Pallet::<Runtime, WestendGrandpaInstance>::best_finalized()
		}
	}

This doesn't work because impl_runtime_apis doesn't allow to implement BridgeFinalityApi twice, even if the generic params are different:
Two traits with the same name detected! The trait name is used to generate its ID. Please rename one trait at the declaration!

  1. Providing generic params to the functions in the trait:
	pub trait BridgeFinalityApi {
		/// Returns number and hash of the best finalized header known to the bridge module.
		fn best_finalized<C: Chain>() -> Option<crate::HeaderIdOf<C>>;
	}

But decl_runtime_apis doesn't allow this:

384 | |     pub trait BridgeFinalityApi {
    | |               ----------------- this trait cannot be made into an object...
385 | |         /// Returns number and hash of the best finalized header known to the bridge module.
386 | |         fn best_finalized<C: Chain>() -> Option<crate::HeaderIdOf<C>>;
    | |            ^^^^^^^^^^^^^^ ...because method `best_finalized` has generic type parameters
387 | |     }

Still thinking if there are other solutions.

@serban300
Copy link
Collaborator

For (1) the solution would be to create transactions based on Metadata somehow or via RPC

Looking on subxt-codegen and subxt to see how the generated code based on metadata would look like and if there's anything that would help us.

@serban300
Copy link
Collaborator

Building calls based on metadata done in #1812

@EmmanuellNorbertTulbure EmmanuellNorbertTulbure transferred this issue from paritytech/parity-bridges-common Aug 25, 2023
@the-right-joyce the-right-joyce transferred this issue from paritytech/polkadot-sdk Aug 25, 2023
@EmmanuellNorbertTulbure EmmanuellNorbertTulbure added this to the Nice To Have milestone Sep 1, 2023
@acatangiu acatangiu closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
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

No branches or pull requests

5 participants