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

OpenGov improvements for Kusama #6372

Merged
merged 12 commits into from
Dec 5, 2022
Merged

OpenGov improvements for Kusama #6372

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6f18e65
Tweaks to optimise gov2
gavofyork Nov 30, 2022
5980e4f
Use new inactive funds
gavofyork Nov 30, 2022
18429a6
Merge branch 'master' into gav-tweak-gov2
gavofyork Dec 1, 2022
b294ffe
Merge remote-tracking branch 'origin/master' into gav-tweak-gov2
gavofyork Dec 3, 2022
0ebf85c
Introduce migrations
gavofyork Dec 3, 2022
a5b47d1
Fixes
gavofyork Dec 3, 2022
58d1360
Fixes
gavofyork Dec 3, 2022
e30ede4
Fixes
gavofyork Dec 3, 2022
e3b6d65
Some constant updates for Fellowship
gavofyork Dec 3, 2022
49c0169
Further tweaks
gavofyork Dec 4, 2022
3197402
Lower floor for whitelisted
gavofyork Dec 5, 2022
b4e12b9
Merge remote-tracking branch 'origin/master' into gav-tweak-gov2
gavofyork Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion runtime/common/src/crowdloan/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,47 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use super::*;
use frame_support::{storage_alias, Twox64Concat};
use frame_support::{storage_alias, Twox64Concat, traits::OnRuntimeUpgrade};

pub struct MigrateToTrackInactive<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToTrackInactive<T> {
fn on_runtime_upgrade() -> Weight {
let current_version = Pallet::<T>::current_storage_version();
let onchain_version = Pallet::<T>::on_chain_storage_version();

if onchain_version == 0 && current_version == 1 {
let mut translated = 0u64;
for index in Funds::<T>::iter_keys() {
let b = CurrencyOf::<T>::total_balance(&Pallet::<T>::fund_account_id(index));
CurrencyOf::<T>::deactivate(b);
translated.saturating_inc();
}
current_version.put::<Pallet<T>>();
log::info!(target: "runtime::crowdloan", "Summed {} funds, storage to version {:?}", translated, current_version);
T::DbWeight::get().reads_writes(translated * 2 + 1, translated * 2 + 1)
} else {
log::info!(target: "runtime::crowdloan", "Migration did not execute. This probably should be removed");
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let total = Funds::<T>::iter_keys()
.map(|index| CurrencyOf::<T>::total_balance(&Pallet::<T>::fund_account_id(index)))
.sum();
Ok((total.encode(), CurrencyOf::<T>::active_issuance()))
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(total: Vec<u8>) -> Result<(), &'static str> {
let (total, active) = <(BalanceOf::<T>, BalanceOf::<T>)>::decode(&mut total.as_slice()).expect(
"the state parameter should be something that was generated by pre_upgrade",
);
assert_eq!(active - total, CurrencyOf::<T>::active_issuance(), "the total be correct");
Ok(())
}
}

/// Migrations for using fund index to create fund accounts instead of para ID.
pub mod crowdloan_index_migration {
Expand Down
5 changes: 5 additions & 0 deletions runtime/common/src/crowdloan/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@

pub mod migration;

// TODO: Expose the total amount held.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't ReservedAmount from the Auctions pallet capture everything in crowdloans and self-funded bids?

Copy link
Member Author

Choose a reason for hiding this comment

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

Self-funded bids can still be used in governance.

Copy link
Contributor

@joepetrowski joepetrowski Dec 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Really? How?


use crate::{
slot_range::SlotRange,
traits::{Auctioneer, Registrar},
Expand Down Expand Up @@ -488,6 +490,7 @@ pub mod pallet {
ensure!(balance > Zero::zero(), Error::<T>::NoContributions);

CurrencyOf::<T>::transfer(&fund_account, &who, balance, AllowDeath)?;
CurrencyOf::<T>::reactivate(balance);

Self::contribution_kill(fund.fund_index, &who);
fund.raised = fund.raised.saturating_sub(balance);
Expand Down Expand Up @@ -527,6 +530,7 @@ pub mod pallet {
break
}
CurrencyOf::<T>::transfer(&fund_account, &who, balance, AllowDeath)?;
CurrencyOf::<T>::reactivate(balance);
Self::contribution_kill(fund.fund_index, &who);
fund.raised = fund.raised.saturating_sub(balance);
refund_count += 1;
Expand Down Expand Up @@ -777,6 +781,7 @@ impl<T: Config> Pallet<T> {
}

CurrencyOf::<T>::transfer(&who, &fund_account, value, existence)?;
CurrencyOf::<T>::deactivate(balance);

let balance = old_balance.saturating_add(value);
Self::contribution_put(fund.fund_index, &who, &balance, &memo);
Expand Down
3 changes: 2 additions & 1 deletion runtime/kusama/src/governance/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ impl pallet_conviction_voting::Config for Runtime {
type Currency = Balances;
type VoteLockingPeriod = VoteLockingPeriod;
type MaxVotes = ConstU32<512>;
type MaxTurnout = frame_support::traits::TotalIssuanceOf<Balances, Self::AccountId>;
// TODO: Should be `total_issuance` minus funds trapped in system pots/checked
type MaxTurnout = frame_support::traits::ActiveIssuanceOf<Balances, Self::AccountId>;
type Polls = Referenda;
}

Expand Down
2 changes: 2 additions & 0 deletions runtime/kusama/src/governance/origins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ pub mod pallet_custom_origins {
}
decl_unit_ensures!(
StakingAdmin,
Treasurer,
FellowshipAdmin,
GeneralAdmin,
AuctionAdmin,
Expand Down Expand Up @@ -173,6 +174,7 @@ pub mod pallet_custom_origins {
SmallSpender = 10 * GRAND,
MediumSpender = 100 * GRAND,
BigSpender = 1_000 * GRAND,
Treasurer = 10_000 * GRAND,
}
}

Expand Down
44 changes: 22 additions & 22 deletions runtime/kusama/src/governance/tracks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
pallet_referenda::TrackInfo {
name: "root",
max_deciding: 1,
decision_deposit: 1_000 * GRAND,
prepare_period: 3 * HOURS,
decision_deposit: 100 * GRAND,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 3 * HOURS,
Expand All @@ -85,8 +85,8 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
pallet_referenda::TrackInfo {
name: "whitelisted_caller",
max_deciding: 10,
decision_deposit: 10_000 * GRAND,
prepare_period: 3 * HOURS,
decision_deposit: 100 * GRAND,
Copy link
Contributor

@brenzi brenzi Dec 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

thank you for this. But doesn't this setup now mean that one can maliciously jam this track with just 10*100 GRAND, which is only 3.33k KSM. Or is there a "decision gate" requiring the whitlelisting to be done to enter deciding?
Why not set max_deciding to 1000? Thanks to the fellowship we can always filter referenda on this track by whitelisting status

Copy link
Contributor

Choose a reason for hiding this comment

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

The root track would have the same issue then. I wonder if the better option is to set the Root track to a high decision deposit and also allow the WhitelistOrigin to remove proposals from the WhitelistedCaller track, essentially saying "we're not going to Whitelist this call so there's no need for a Whitelist track referendum".

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated QUID and increased the max_deciding so this is now 100 kKSM to jam it. With ReferendumKiller origin, we can remove spam and slash the spammer's deposits.

prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 10 * MINUTES,
min_enactment_period: 30 * MINUTES,
Expand All @@ -100,7 +100,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "staking_admin",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder why min_enactment_period of the staking admin track would be less than of the root?

Copy link
Member Author

Choose a reason for hiding this comment

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

Root is hugely dangerous, so we're being a bit more conservative on the enactment time.

Copy link
Contributor

Choose a reason for hiding this comment

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

I expressed it wrong, the root min_enactment_period less than staking_admin now.
root 3 hours vs staking_admin 2 days

Expand All @@ -114,7 +114,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "treasurer",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Expand All @@ -128,7 +128,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "lease_admin",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Expand All @@ -142,7 +142,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "fellowship_admin",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Expand All @@ -156,7 +156,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "general_admin",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Expand All @@ -170,7 +170,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "auction_admin",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 2 * DAYS,
Expand All @@ -184,7 +184,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "referendum_canceller",
max_deciding: 1_000,
decision_deposit: 50 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 10 * MINUTES,
Expand All @@ -198,7 +198,7 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "referendum_killer",
max_deciding: 1_000,
decision_deposit: 50 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 10 * MINUTES,
Expand All @@ -212,10 +212,10 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "small_tipper",
max_deciding: 200,
decision_deposit: 5 * QUID,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 3 * HOURS,
min_enactment_period: 28 * DAYS,
min_enactment_period: 24 * HOURS,
min_approval: APP_SMALL_TIPPER,
min_support: SUP_SMALL_TIPPER,
},
Expand All @@ -226,10 +226,10 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "big_tipper",
max_deciding: 100,
decision_deposit: 50 * QUID,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 6 * HOURS,
min_enactment_period: 28 * DAYS,
min_enactment_period: 24 * HOURS,
min_approval: APP_BIG_TIPPER,
min_support: SUP_BIG_TIPPER,
},
Expand All @@ -240,10 +240,10 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "small_spender",
max_deciding: 50,
decision_deposit: 500 * QUID,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 12 * HOURS,
min_enactment_period: 28 * DAYS,
min_enactment_period: 24 * HOURS,
min_approval: APP_SMALL_SPENDER,
min_support: SUP_SMALL_SPENDER,
},
Expand All @@ -254,10 +254,10 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "medium_spender",
max_deciding: 20,
decision_deposit: 1_500 * QUID,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
confirm_period: 24 * HOURS,
min_enactment_period: 28 * DAYS,
min_enactment_period: 24 * HOURS,
min_approval: APP_MEDIUM_SPENDER,
min_support: SUP_MEDIUM_SPENDER,
},
Expand All @@ -268,10 +268,10 @@ const TRACKS_DATA: [(u16, pallet_referenda::TrackInfo<Balance, BlockNumber>); 15
name: "big_spender",
max_deciding: 10,
decision_deposit: 5 * GRAND,
prepare_period: 4,
prepare_period: 4 * HOURS,
decision_period: 28 * DAYS,
Copy link
Contributor

@brenzi brenzi Dec 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

any chance we can set decision_period to 7 DAYS for all spenders and tippers? The curves are quite demanding on the support threshold. 28 days means a lot of planning and exchange rate uncertainty, which is harmful for ecosystem dev teams

Copy link
Member Author

Choose a reason for hiding this comment

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

Reduced to 14 days throughout, with tippers at 7 days.

The curves are designed to be safe. If a faster conclusion is desired by teams, then I think they really ought to be helping increase turnout for their proposal from token holders/voters.

confirm_period: 48 * HOURS,
min_enactment_period: 28 * DAYS,
min_enactment_period: 24 * HOURS,
min_approval: APP_BIG_SPENDER,
min_support: SUP_BIG_SPENDER,
},
Expand Down
4 changes: 2 additions & 2 deletions runtime/kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,8 @@ parameter_types! {
impl pallet_treasury::Config for Runtime {
type PalletId = TreasuryPalletId;
type Currency = Balances;
type ApproveOrigin = EnsureRoot<AccountId>;
type RejectOrigin = EnsureRoot<AccountId>;
type ApproveOrigin = Treasurer;
type RejectOrigin = Treasurer;
type RuntimeEvent = RuntimeEvent;
type OnSlash = Treasury;
type ProposalBond = ProposalBond;
Expand Down
4 changes: 4 additions & 0 deletions xcm/xcm-builder/src/currency_adapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ impl<
AllowDeath,
)
.is_ok();
if ok {
Currency::reactivate(amount);
}
debug_assert!(
ok,
"`can_check_in` must have returned `true` immediately prior; qed"
Expand All @@ -138,6 +141,7 @@ impl<
if let Some(amount) = Matcher::matches_fungible(what) {
if let Some(checked_account) = CheckedAccount::get() {
Currency::deposit_creating(&checked_account, amount);
Currency::deactivate(amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

both currency calls, withdraw and deposit_creating return imbalance, can be used to determine the reactivate/deactivate amount

Copy link
Member Author

Choose a reason for hiding this comment

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

it should be equivalent.

}
}
}
Expand Down