Skip to content

Commit

Permalink
Add MaxTipAmount for pallet-tips (paritytech#1709)
Browse files Browse the repository at this point in the history
Last week we experienced a governance attack.
Surprisingly, there was no upper limit on the tip amount.

Due to the mechanism of pallet-fragment-election, the council members
will be refreshed immediately. Attacker is easy to control the council
and give a large tip amount.
  • Loading branch information
aurexav authored Sep 28, 2023
1 parent 0b040ff commit 917eafa
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 1 deletion.
1 change: 1 addition & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ impl pallet_tips::Config for Runtime {
type TipCountdown = TipCountdown;
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = TipReportDepositBase;
type MaxTipAmount = ConstU128<{ 500 * DOLLARS }>;
type WeightInfo = pallet_tips::weights::SubstrateWeight<Runtime>;
}

Expand Down
1 change: 1 addition & 0 deletions substrate/docs/Upgrading-2.0-to-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ impl pallet_tips::Config for Runtime {
type TipCountdown = TipCountdown;
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = TipReportDepositBase;
type MaxTipAmount = MaxTipAmount;
type WeightInfo = pallet_tips::weights::SubstrateWeight<Runtime>;
}
```
Expand Down
14 changes: 13 additions & 1 deletion substrate/frame/tips/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ pub mod pallet {
#[pallet::constant]
type TipReportDepositBase: Get<BalanceOf<Self, I>>;

/// The maximum amount for a single tip.
#[pallet::constant]
type MaxTipAmount: Get<BalanceOf<Self, I>>;

/// Origin from which tippers must come.
///
/// `ContainsLengthBound::max_len` must be cost free (i.e. no storage read or heavy
Expand Down Expand Up @@ -208,6 +212,8 @@ pub mod pallet {
AlreadyKnown,
/// The tip hash is unknown.
UnknownTip,
/// The tip given was too generous.
MaxTipAmountExceeded,
/// The account attempting to retract the tip is not the finder of the tip.
NotFinder,
/// The tip cannot be claimed/closed because there are not enough tippers yet.
Expand Down Expand Up @@ -336,10 +342,13 @@ pub mod pallet {
let tipper = ensure_signed(origin)?;
let who = T::Lookup::lookup(who)?;
ensure!(T::Tippers::contains(&tipper), BadOrigin);

ensure!(T::MaxTipAmount::get() >= tip_value, Error::<T, I>::MaxTipAmountExceeded);

let reason_hash = T::Hashing::hash(&reason[..]);
ensure!(!Reasons::<T, I>::contains_key(&reason_hash), Error::<T, I>::AlreadyKnown);
let hash = T::Hashing::hash_of(&(&reason_hash, &who));

let hash = T::Hashing::hash_of(&(&reason_hash, &who));
Reasons::<T, I>::insert(&reason_hash, &reason);
Self::deposit_event(Event::NewTip { tip_hash: hash });
let tips = vec![(tipper.clone(), tip_value)];
Expand Down Expand Up @@ -387,7 +396,10 @@ pub mod pallet {
let tipper = ensure_signed(origin)?;
ensure!(T::Tippers::contains(&tipper), BadOrigin);

ensure!(T::MaxTipAmount::get() >= tip_value, Error::<T, I>::MaxTipAmountExceeded);

let mut tip = Tips::<T, I>::get(hash).ok_or(Error::<T, I>::UnknownTip)?;

if Self::insert_tip_and_check_closing(&mut tip, tipper, tip_value) {
Self::deposit_event(Event::TipClosing { tip_hash: hash });
}
Expand Down
19 changes: 19 additions & 0 deletions substrate/frame/tips/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ impl Config for Test {
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = ConstU64<1>;
type DataDepositPerByte = ConstU64<1>;
type MaxTipAmount = ConstU64<10_000_000>;
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();
}
Expand All @@ -183,6 +184,7 @@ impl Config<Instance1> for Test {
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = ConstU64<1>;
type DataDepositPerByte = ConstU64<1>;
type MaxTipAmount = ConstU64<10_000_000>;
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();
}
Expand Down Expand Up @@ -396,6 +398,23 @@ fn tip_median_calculation_works() {
});
}

#[test]
fn tip_large_should_fail() {
new_test_ext().execute_with(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Tips::tip_new(RuntimeOrigin::signed(10), b"awesome.dot".to_vec(), 3, 0));
let h = tip_hash();
assert_noop!(
Tips::tip(
RuntimeOrigin::signed(12),
h,
<<Test as Config>::MaxTipAmount as Get<u64>>::get() + 1
),
Error::<Test>::MaxTipAmountExceeded
);
});
}

#[test]
fn tip_changing_works() {
new_test_ext().execute_with(|| {
Expand Down

0 comments on commit 917eafa

Please sign in to comment.