Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

pallet-treasury | port to frame v2 #8179

Conversation

shamb0
Copy link
Contributor

@shamb0 shamb0 commented Feb 23, 2021

Plz find the updated PR #8196

& ignore this.


pallet-collective | port to frame v2

related #7882

⚠️ Breaking Change ⚠️

From https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines

storages now use PalletInfo for module_prefix instead of the one given to decl_storage: Thus any use of this pallet in construct_runtime! should be careful to update name in order not to break storage or to upgrade storage (moreover for instantiable pallet). If pallet is published, make sure to warn about this breaking change.

So users of the treasury pallet must be careful about the name they used in construct_runtime!. Hence the runtime-migration label, which might not be needed depending on the configuration of the treasury pallet.

@shamb0 shamb0 marked this pull request as ready for review February 23, 2021 06:55
@bkchr bkchr requested a review from gui1117 February 23, 2021 08:33
@gui1117 gui1117 added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Feb 23, 2021
let pot_account = Treasury::<T, I>::account_id();
let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into());
let _ = T::Currency::make_free_balance_be(&pot_account, value);
}

benchmarks_instance! {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use benchmarks_instance here

Treasury: treasury::{Module, Call, Storage, Config, Event<T>},
#[macro_export]
macro_rules! decl_tests {
($test:ty) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

(same there is no need to wrap in a macro)

Comment on lines 320 to 321
Option<Proposal<T::AccountId, BalanceOf<T, I>>>,
ValueQuery,
Copy link
Contributor

Choose a reason for hiding this comment

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

comparing the metadata should show these kind of error.

Suggested change
Option<Proposal<T::AccountId, BalanceOf<T, I>>>,
ValueQuery,
Proposal<T::AccountId, BalanceOf<T, I>>,
OptionQuery,

@shamb0 shamb0 closed this Feb 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants