substrate icon indicating copy to clipboard operation
substrate copied to clipboard

Add `with_pays_fee` for conversion to `DispatchErrorWithPostInfo`

Open bragov4ik opened this issue 2 years ago • 5 comments
trafficstars

Description

Extend WithPostDispatchInfo to allow easier specification of pays_fee when converting to DispatchErrorWithPostInfo.

At the time of PR that added with_weight (#5458), pays_fee field was not present, so it makes sense to add add such function. For example, instead of

let mut e = DispatchErrorWithPostInfo::from(Error::<T>::InvalidParameters);
e.pays_fee = Pays::No;
e

one can write

Error::<T>::InvalidParameters.with_pays_fee(Pays::No)

My motivation in making this change is inconvenience when working with errors returned from other functions. In particular, I had the following case:

Self::fallible_function().map_err(|e| {
	let mut e = DispatchErrorWithPostInfo::from(e);
	e.pays_fee = Pays::No;
	e
})?;

and this should be cleaner and easier to read:

Self::fallible_function().map_err(|e| e.with_pays_fee(Pays::No))?;
  • I haven't found an issue about this and I'm not sure if it should be created, as it's mostly convenience PR.
  • Apparently, I don't have permissions to add labels, but I suppose it's something like A0, B0, C1
  • It doesn't change existing functions, only adds an extra one. Shouldn't break API, I guess. Correct me if I'm wrong please :)

bragov4ik avatar Jun 27 '23 13:06 bragov4ik

User @bragov4ik, please sign the CLA here.

cla-bot-2021[bot] avatar Jun 27 '23 13:06 cla-bot-2021[bot]

Also I wanted to implement WithPostDispatchInfo for DispatchErrorWithPostInfo to allow builder pattern for the latter (e.with_pays_fee(Pays::No).with_weight(1234)). However, that impl conflicted with existing <T: Into<DispatchError>> one ("upstream crates may add a new impl of trait" was the error).

One of the ways I see to fix it is to impl for DispatchError instead, but it might break some stuff, so I'm not sure about it :)

bragov4ik avatar Jun 27 '23 13:06 bragov4ik

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 27 '23 20:07 stale[bot]

Not sure why the CI doesn't pass with (lack of feature in sp-keystore) error. I didn't touch any features 🤔

bragov4ik avatar Jul 31 '23 09:07 bragov4ik

Not sure why the CI doesn't pass with (lack of feature in sp-keystore) error. I didn't touch any features 🤔

It seems like the Polkadot companion is off, I will re-run the job.

juangirini avatar Aug 17 '23 09:08 juangirini