Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors referenda pallet to use fungible traits #1785

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Oct 3, 2023

Refactors the referanda pallet to use the new fungible traits.

type pallet_treasury::FunOnUnbalanced: the treasury pallet implements a new type that handles fungible's imbalances to ensure forwards (traits::fungible) and backwards (traits::currency) compatibility. Once all pallets that use the treasury to handle imbalances move to fungibles, the current impl OnUnbalanced<NegativeImbalanceOf<T>> for Pallet can be removed.

Todo:

Related to #226

@gpestana gpestana requested review from a team October 3, 2023 20:44
@gpestana gpestana marked this pull request as draft October 3, 2023 20:44
@gpestana gpestana self-assigned this Oct 3, 2023
@gpestana gpestana added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. T1-FRAME This PR/Issue is related to core FRAME, the framework. D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. and removed C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. labels Oct 3, 2023
@gpestana gpestana requested review from Ank4n and gupnik October 10, 2023 14:43
@gpestana gpestana marked this pull request as ready for review October 10, 2023 14:43
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3959617

Copy link
Contributor

@juangirini juangirini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we need a migration here, so that any deposits currently 'reserved' are moved to be 'held'

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -139,6 +140,7 @@ const ASSEMBLY_ID: LockIdentifier = *b"assembly";

#[frame_support::pallet]
pub mod pallet {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change


Self::deposit_event(Event::<T, I>::DepositSlashed {
who,
amount: amount.saturating_sub(non_slashed),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its possible to first emit the event and use imbalance.peek(), that makes it a bit more understandable IMHO.

{
fn on_nonzero_unbalanced(amount: Credit<<T as frame_system::Config>::AccountId, F>) {
let peeked_amount = amount.peek();
let _ = F::resolve(&<Pallet<T, I>>::account_id(), amount);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this fails then the credit is just dropped correctly, or?

Comment on lines +1119 to 1121
/// Note: replace this implementation for [`FunOnUnbalanced`] once the `Currency` traits are
/// deprecated and not used anymore.
impl<T: Config<I>, I: 'static> OnUnbalanced<NegativeImbalanceOf<T, I>> for Pallet<T, I> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mark it deprecated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants