pallet-treasury: Asset Category#10381
Conversation
dhirajs0
added
the
T1-FRAME
|
/fmt |
dhirajs0
left a comment
dhirajs0
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have left some comments.
|
|
||
| LowBalance, | ||
|
|
||
| EmptyAssetCategory, | ||
|
|
||
| InvalidPaymentState, | ||
|
|
||
| ExecutionRateLimit, | ||
|
|
||
| TooManyFailedPayments, | ||
|
|
||
| AssetNotFound, | ||
|
|
||
| ConversionFailed, | ||
|
|
||
| ArithmeticOverflow, | ||
|
|
||
| ConversionRateMismatch, | ||
|
|
||
| InvalidConversionRate, |
There was a problem hiding this comment.
Please add doc comments to each error variant
| } | ||
| } | ||
|
|
||
| Err(Error::<T, I>::LowBalance.into()) |
There was a problem hiding this comment.
I think the balance check at creation time is overly restrictive and contradicts the partial payment design.
| pub enum PaymentState<AssetKind, Balance, Id> { | ||
| /// Pending claim. | ||
| Pending, | ||
| /// Payment attempted with a payment identifier. | ||
| Attempted { id: Id }, | ||
| /// Payment failed. | ||
| Failed, | ||
| /// Payment attempted with payment identifiers for each payment execution | ||
| Attempted { | ||
| executions: BoundedVec<PaymentExecution<AssetKind, Balance, Id>, ConstU32<32>>, | ||
| remaining_amount: Balance, | ||
| }, | ||
| /// Record of all failed payments to retry | ||
| Failed(BoundedVec<PaymentExecution<AssetKind, Balance, Id>, ConstU32<32>>), | ||
| /// Attempt incomplete payment balance from spend | ||
| Partial(Balance), | ||
| } |
There was a problem hiding this comment.
This is breaking storage change and require migration.
| Failed, | ||
| /// Payment attempted with payment identifiers for each payment execution | ||
| Attempted { | ||
| executions: BoundedVec<PaymentExecution<AssetKind, Balance, Id>, ConstU32<32>>, |
There was a problem hiding this comment.
ConstU32<32> please make the limit configurable, so runtimes should be able to choose them.
| BoundedVec<u8, ConstU32<32>>, // Category name | ||
| BoundedVec<T::AssetId, ConstU32<32>>, // Assets in this category |
There was a problem hiding this comment.
Magic numbers: I thing it would be good to make them configurable
| } | ||
|
|
||
| #[test] | ||
| fn category_payout_partial_fulfillment() { |
There was a problem hiding this comment.
Identical with fn category_payout_distributes_across_assets()
|
|
||
| let diff_percent = multiplied_diff.checked_div(max).unwrap_or(0); | ||
|
|
||
| if diff_percent > 5 { |
There was a problem hiding this comment.
Magic number: making it configurable would be better.
|
@Doordashcon are you planing to continue the work on this? we can provide more support. can you please open a separate PR for the ordered payouts. |
|
Yes! @muharem In the process of deprecating this GitHub account after all currently open PRs have been handled, I will push ordered payouts using @jessechejieh. Thanks |
Doordashcon
changed the title
pallet-treasury: Asset Category
|
Closing in favor of #12687 |
5 participants
resolves #7961
Indroduction
This PR enables the treasury request payment from a category of assets (i.e.
USD*stablecoins) instead of just a specificasset_kind, distributing the amount across available assets within that category onpayout. ExtendingSpendAssetto support category based requests, configuringPaymentStateto handle multiple payment executions onpayout, introducingAssetCategoryManagerfor defining category membership and performing balance checks and maintaining backward compatibility for single assetspendrequests andpayout.TODO
AssetCategoryManager