Pallet assets-vesting

[pandres95]

Fixes #4090.

Description

The pallet-assets-vesting is a pallet that supports handling vesting of an instances of pallet-assets through freezing fungibles.

The behaviour is analog to what already exists with pallet-vesting, with small differences (like adding an asset parameter where the AssetId goes on every call). Also, WithdrawalReasons are no longer used, since they're related to Currency.

Integration

This pallet can be integrated into any runtime that already has an instance of pallet-assets and pallet-assets-freezer.

The instance must be the same as the one that pallets mentioned above use.

type AssetsInstance = pallet_assets::Instance1;

#[derive_impl(pallet_assets::config_preludes::ParachainDefaultConfig)]
impl pallet_assets::Config<AssetsInstance> for Test {
	type Currency = Balances;
	type ForceOrigin = EnsureRoot<AccountId>;
	type CreateOrigin = EnsureSigned<AccountId>;
	type Freezer = AssetsFreezer;
}

impl pallet_assets_freezer::Config<AssetsInstance> for Runtime {
	type RuntimeFreezeReason = RuntimeFreezeReason;
	type RuntimeEvent = RuntimeEvent;
}

impl pallet_assets_vesting::Config<AssetsInstance> for Test {
	type RuntimeEvent = RuntimeEvent;
	type ForceOrigin = EnsureRoot<AccountId>;
	type Assets = Assets;
	type Freezer = AssetsFreezer;
	type BlockNumberToBalance = Identity;
	type RuntimeFreezeReason = RuntimeFreezeReason;
	type WeightInfo = ();
	type MinVestedTransfer = MinVestedTransfer;
	type BlockNumberProvider = System;
	const MAX_VESTING_SCHEDULES: u32 = 3;
}

Checklist

• My PR includes a detailed description as outlined in the "Description" and its two subsections above.
• My PR follows the labeling requirements of this project (at minimum one label for T required)
  • External contributors: ask maintainers to put the right label on your PR.
• I have made corresponding changes to the documentation (if applicable)
• I have added tests that prove my fix is effective or that my feature works (if applicable)

[gui1117]

Suggested change

	title: Bridges small nits/improvements
	title: "Introduce `pallet-assets-vesting`"

[gui1117]

I don't see anything wrong in the trait itself, but it is very different from how other fungibles features are implemented.

fungibles is defined with Inspect and Mutate, then fungibles freeze and holds have their own Inspect and Mutate.

Here we have VestingSchedule and VestingTransfer.

I think we can merge both schedule and transfer, and then split it again but into Inspect and Mutate.

Inspect trait will bound fungibles::Inspect so that AssetId, Balance are defined their and not duplicated
Mutate will bound Inspect probably fungibles::Mutate.

Then the trait can have many method already implemented similarly to other traits like in hold. But I don't mind also no default implementation.

WDYT?

[pandres95]

So, it'd be something like fungible/fungibles::vesting::Inspect and fungible/fungibles::vesting::Mutate? Then, include the methods from VestingSchedule and VestingTransfer inside.

[gui1117]

Yes I think so

[gui1117]

Maybe we can have a more precise type rather than a tuple?

[gui1117]

If per_block is zero here it will trigger vesting info invalid. Maybe

Suggested change

	let per_block = frozen / length_as_balance.max(One::one());
	let per_block = (frozen / length_as_balance.max(One::one())).max(One::one());

[gui1117]

out of curiosity, would add_versing_schedule work? it would be simpler than manually adding the schedule.

[gui1117]

isn't it disadvantageous to merge if 1 or 2 schedule are future?
all the unlock from the earlier to the later is postponed to after later?

Hmm something seems wrong, if I have 2 identical vesting schedule, and I merge them I get a new vesting schedule that double the frozen amount.

AFAICT it is never advantageous to merge schedules, in any circumstances no? Do I miss something, why do have this feature to merge schedules?

EDIT: maybe we don't need this merge, we can just have vesting without merging vesting schedules.

[pandres95]

Hmm… good question! It was already in the Currency version (pallet-vesting), so I just moved it here to keep compatibility. Maybe we don't really need it, but there might be use cases that already take advantage from it. Not sure whether those assumptions would hold.

[gui1117]

Ok, let's keep it as it is, but let's add in the doc of the call that the resulting vesting is always disadvantageous to the caller, and the only advantage is to reduce the number of vesting schedule in storage.