Add ConsiderationFromLegacy traits

[NachoPal]

This PR addresses a potential issue that may arise when migrating pallets from using Currency to Consideration. It specifically addresses scenarios where a pallet's migrations involve a Deposit that hasn't been constant over time (e.g. increased by a runtime upgrade).

During the migration, the new HoldConsideration results in a fixed Balance based on a constant Footprint and a Convert function. For instance, if the new HoldConsideration is expected to have a Balance of 10, what happens when, for a particular account, the stored Deposit is 5? After calling Currency::unreserve(account, 5), attempting to create a new ticket to hold a Balance of 10 might fail due to insufficient balance. Currently, HoldConsideration::new() returns either Ok(()) or Err. What should be done if it's not possible to hold the balance?

Following a similar approach to #1813 (comment), I introduced two new traits:

• FreezeConsiderationFromLegacy
• HoldConsiderationFromLegacy

These traits extend Consideration with a new method new_from_exact() which proves useful when a new ticket with a precise balance is needed (as illustrated in the migration example) rather than deriving it from a footprint.

[ggwpez]

Now it looks clean, thanks!

(test could still be good though)

[liamaharon]

This default impl feels like a footgun to me

Suggested change

	Self: Sized,
	{
	Ok(None)
	}
	Self: Sized;

[NachoPal]

Replied why of the default impl in the next comment

[liamaharon]

same, not sure why anyone would want to use this default impl

Suggested change

	Self: Sized,
	{
	Ok(None)
	}
	Self: Sized;

[NachoPal]

The FRAME pallet that makes use of it on its Config would look something like this:

  type Consideration: Consideration<Self::AccountId> + HoldConsiderationFromLegacy<Self::AccountId, BalanceOf<T>>

The default impl is for 3rd parties that make use of the pallet, so then they do not have to manually implement it.

[liamaharon]

Why do these return Result<Option<Self>, DispatchError> instead of Result<Self, DispatchError>?

[NachoPal]

I believe I implemented that to return None as the default impl. As mentioned earlier, ConsiderationFromLegacy will be required for those pallets that need it during the migration.

[liamaharon]

Hmm I'm still not convinced

1. this feels like a hack to pass a type indirectly to a migration.. As discussed in dm, I'd like to check if it's possible to pass this type directly to the migration if indeed it is only needed in the migration, not the pallet
2. there are genuine use cases for when you do want to use this directly in the pallet, e.g. @gupnik is working on a lazy migration right now where we're discussing potentially using it

[ggwpez]

I guess the default impl would never be used and is only meant to make it compile once all currency related crates were removed from the runtime config.
But then it should indeed return an Error and not None.

[liamaharon]

Yeah would feel much better returning an error to me instead of the None 👍

[liamaharon]

I still would like to see if possible to avoid the default impl (and polluting the pallet config) by passing this type directly to the migration it is used in tho.

[NachoPal]

@liamaharon @ggwpez I replaced None with Err. It should be now good to go.

For reference, here an example of the new method being used: https://github.com/paritytech/polkadot-sdk/pull/1789/files#diff-489bbb1c7c1b1b8803ab344f63079bdc1b7d67db3d2d8809d6a3a755443619f0R77-R107