Token-swap-over-bridge pallet

[svyatonik]

closes #543 (second option is implemented)
on top of #935 => draft

It won't probably be reviewed (or I'll be unable to respond to review) until at least mid May - I'm opening it early, because I have some questions + some changes for messages pallet that may be illustrated with this PR.

So the actual set of questions:

1. I'm not sure why we need this unique random_nonce mentioned in Token swap pallet #543 @tomusdrw . I guess it is somewhat linked to the replay protection, but I don't see how it can be exploited. Could you elaborate? Without reviewing the code - just the explanation on why we need it;
2. I'm not yet sure how to deal with weights of OnMessagesDelivered callbacks (see Relay basic single-bit message dispatch results back to the source chain #935 (comment)). Most probably it'll be pre-hardcoded to 1 db read + 1 write per delivered message. Then all callbacks must be implemented as something like: for message in &messages { if check_if_we_are_waiting_this_message(message) { remember_to_do_something_in_next_on_initialize(); } } Update delivery confirmation transaction weights to account for delivery callbacks #1012.

TODOs for this PR:

1. deal with this comment - so iiuc transaction_id must be used to protect against Alice starting the claim again. I've missed that part when implementing cda7e8f

TODOs for parallel PRs:

1. add runtime-internal function to the messages pallet;
2. add enum MessageOrigin { Runtime, Root, User } and pass it to the LaneMessageVerifier::verify_message() instead of the current sender. This would allow to implement lanes that are dedicated to some applications or general usage lanes This probably can be solved by using custom

	#[pallet::origin]
	#[derive(RuntimeDebug, Clone, Eq, PartialEq, Encode, Decode)]
	pub struct Origin<T: Config<I>, I: 'static = ()> {
		/// Sender account.
		sending_on_behalf_of: T::AccountId,
		/// Phantom marker.
		_phantom: sp_std::marker::PhantomData<I>,
	}

and then passing this origin to the send_message function. So this is most probably for the runtime-integration PR;

TODOs for future PRs:

1. integrate into runtime;
2. generate token swap calls.

[tomusdrw]

I'm not sure why we need this unique random_nonce mentioned in #543 @tomusdrw . I guess it is somewhat linked to the replay protection, but I don't see how it can be exploited. Could you elaborate? Without reviewing the code - just the explanation on why we need it;

Since it's used for transaction_id I think the idea was to prevent these things clashing (i.e. you can have two exact same orders in place). Also notice that transaction_id affects lock_account_id and that's what bob_signature contains - so we are preventing re-use of the signature here.

[svyatonik]

Just my current thoughts (no need to comment now).

So from what I understand nonce should prevent Alice from resubmitting the same swap again after it has been claimed/cancelled. With current pallet it is possible. So if Bob is using some permanent account, Alice may swap until Bob's account depletes. I don't yet see how nonce may solve this problem (given current pallet implementation) - Alice may reuse the same signature in all create_swap calls. So probably pallet needs to remember the nonce. OTOH the pallet isn't verifying bob_signature now, so we can't check if nonce matches the nonce from signature. Then the pallet needs to remember used signatures. but it'll require a storage entry for every completed swap => may be used as a way of bloating runtime storage. Will need to think how to do that properly.

[tomusdrw]

Looks good! One critical thing (crate_swap enforcing LockClaim block number) and some smaller suggestions.

[tomusdrw]

Couldn't we group all Bridged* types into one larger interface? Ideally why we can't extract that from any already existing chain representations? Wouldn't extending pallet-bridge-messages::Config trait help here a lot in removing the duplicates?

[svyatonik]

I can move Balance, AccountId, Public and Signature fields from relay_substrate_client::Chain to bp_runtime::Chain. But that would also mean that even if you only deploying GRANPA pallet you'll need to specify those types as well. Other fields are too specific for messaging or this pallet needs.

Let's do that in a separate PR, though - it'll touch (I assume) too many files. So list of PRs to get this pallet ready:

1. this ^^ refactoring;
2. integrating into Millau or Rialto runtimes;
3. Support dedicated lanes for pallets #962
4. weights

[tomusdrw]

Makes sense!

[tomusdrw]

Hmm, AFAICT if the swap is in Failed state you can still cancel it, no? So no funds should be lost.

[svyatonik]

If you've signed any call that has succeeded at the target chain (e.g. system::remark), you can't cancel it on this chain.

[tomusdrw]

Oh, yeah, you're right. Actually given we allow arbitrary calls here, I'd suggest having the user provide weight as well. This will enable calling into proxy accounts, mutlisigs, etc. Also one less thing the pallet needs to know about the remote chain.

[svyatonik]

Right. I've also moved bridged_chain_spec_version to arguments