Balances Pallet: Emit events when TI is updated in currency impl (#4936)

# Description

Previously, in the `Currency` impl, the implementation of
`pallet_balances` was not emitting any instances of `Issued` and
`Rescinded` events, even though the `Fungible` equivalent was.

This PR adds the `Issued` and `Rescinded` events in appropriate places
in `impl_currency` along with tests.

Closes #4028 

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <info@kchr.de>

prdoc/pr_4936.prdoc

@@ -0,0 +1,13 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "Balances Pallet: Emit events when TI is updated in currency impl"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Previously, in the Currency impl, the implementation of pallet_balances was not emitting any instances of Issued and Rescinded events, even though the Fungible equivalent was. This PR adds the Issued and Rescinded events in appropriate places in impl_currency along with tests.
+
+crates:
+- name: pallet-balances
+  bump: patch

substrate/bin/node/bench/src/import.rs

@@ -121,22 +121,23 @@ impl core::Benchmark for ImportBenchmark {
 			.inspect_state(|| {
 				match self.block_type {
 					BlockType::RandomTransfersKeepAlive => {
-						// should be 8 per signed extrinsic + 1 per unsigned
+						// should be 9 per signed extrinsic + 1 per unsigned
 						// we have 2 unsigned (timestamp and glutton bloat) while the rest are
 						// signed in the block.
-						// those 8 events per signed are:
+						// those 9 events per signed are:
 						//    - transaction paid for the transaction payment
 						//    - withdraw (Balances::Withdraw) for charging the transaction fee
 						//    - new account (System::NewAccount) as we always transfer fund to
 						//      non-existent account
 						//    - endowed (Balances::Endowed) for this new account
+						//    - issued (Balances::Issued) as total issuance is increased
 						//    - successful transfer (Event::Transfer) for this transfer operation
 						//    - 2x deposit (Balances::Deposit and Treasury::Deposit) for depositing
 						//      the transaction fee into the treasury
 						//    - extrinsic success
 						assert_eq!(
 							kitchensink_runtime::System::events().len(),
-							(self.block.extrinsics.len() - 2) * 8 + 2,
+							(self.block.extrinsics.len() - 2) * 9 + 2,
 						);
 					},
 					BlockType::Noop => {

substrate/bin/node/cli/Cargo.toml

@@ -71,6 +71,7 @@ wait-timeout = { workspace = true }
 wat = { workspace = true }
 serde_json = { workspace = true, default-features = true }
 scale-info = { features = ["derive", "serde"], workspace = true, default-features = true }
+pretty_assertions.workspace = true
 
 # These testing-only dependencies are not exported by the Polkadot-SDK crate:
 node-testing = { workspace = true }

substrate/bin/node/cli/tests/basic.rs

@@ -35,6 +35,7 @@ use kitchensink_runtime::{
 };
 use node_primitives::{Balance, Hash};
 use node_testing::keyring::*;
+use pretty_assertions::assert_eq;
 use wat;
 
 pub mod common;
@@ -380,6 +381,13 @@ fn full_native_block_import_works() {
 				}),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded {
+					amount: fees * 2 / 10,
+				}),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: RuntimeEvent::TransactionPayment(
@@ -465,6 +473,13 @@ fn full_native_block_import_works() {
 				}),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded {
+					amount: fees - fees * 8 / 10,
+				}),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: RuntimeEvent::TransactionPayment(
@@ -515,6 +530,13 @@ fn full_native_block_import_works() {
 				}),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(2),
+				event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded {
+					amount: fees - fees * 8 / 10,
+				}),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(2),
 				event: RuntimeEvent::TransactionPayment(

substrate/frame/balances/src/impl_currency.rs

@@ -34,11 +34,12 @@ use frame_support::{
 };
 use frame_system::pallet_prelude::BlockNumberFor;
 pub use imbalances::{NegativeImbalance, PositiveImbalance};
+use sp_runtime::traits::Bounded;
 
 // wrapping these imbalances in a private module is necessary to ensure absolute privacy
 // of the inner member.
 mod imbalances {
-	use super::{result, Config, Imbalance, RuntimeDebug, Saturating, TryDrop, Zero};
+	use super::*;
 	use core::mem;
 	use frame_support::traits::SameOrOther;
 
@@ -199,14 +200,20 @@ mod imbalances {
 	impl<T: Config<I>, I: 'static> Drop for PositiveImbalance<T, I> {
 		/// Basic drop handler will just square up the total issuance.
 		fn drop(&mut self) {
-			<super::TotalIssuance<T, I>>::mutate(|v| *v = v.saturating_add(self.0));
+			if !self.0.is_zero() {
+				<super::TotalIssuance<T, I>>::mutate(|v| *v = v.saturating_add(self.0));
+				Pallet::<T, I>::deposit_event(Event::<T, I>::Issued { amount: self.0 });
+			}
 		}
 	}
 
 	impl<T: Config<I>, I: 'static> Drop for NegativeImbalance<T, I> {
 		/// Basic drop handler will just square up the total issuance.
 		fn drop(&mut self) {
-			<super::TotalIssuance<T, I>>::mutate(|v| *v = v.saturating_sub(self.0));
+			if !self.0.is_zero() {
+				<super::TotalIssuance<T, I>>::mutate(|v| *v = v.saturating_sub(self.0));
+				Pallet::<T, I>::deposit_event(Event::<T, I>::Rescinded { amount: self.0 });
+			}
 		}
 	}
 }
@@ -263,6 +270,8 @@ where
 				Zero::zero()
 			});
 		});
+
+		Pallet::<T, I>::deposit_event(Event::<T, I>::Rescinded { amount });
 		PositiveImbalance::new(amount)
 	}
 
@@ -279,6 +288,8 @@ where
 				Self::Balance::max_value()
 			})
 		});
+
+		Pallet::<T, I>::deposit_event(Event::<T, I>::Issued { amount });
 		NegativeImbalance::new(amount)
 	}
 

substrate/frame/balances/src/lib.rs

@@ -175,8 +175,8 @@ pub use impl_currency::{NegativeImbalance, PositiveImbalance};
 use scale_info::TypeInfo;
 use sp_runtime::{
 	traits::{
-		AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedSub, MaybeSerializeDeserialize,
-		Saturating, StaticLookup, Zero,
+		AtLeast32BitUnsigned, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Saturating,
+		StaticLookup, Zero,
 	},
 	ArithmeticError, DispatchError, FixedPointOperand, Perbill, RuntimeDebug, TokenError,
 };

substrate/frame/balances/src/tests/currency_tests.rs

@@ -399,10 +399,13 @@ fn reward_should_work() {
 	ExtBuilder::default().monied(true).build_and_execute_with(|| {
 		assert_eq!(Balances::total_balance(&1), 10);
 		assert_ok!(Balances::deposit_into_existing(&1, 10).map(drop));
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Deposit {
-			who: 1,
-			amount: 10,
-		}));
+		assert_eq!(
+			events(),
+			[
+				RuntimeEvent::Balances(crate::Event::Deposit { who: 1, amount: 10 }),
+				RuntimeEvent::Balances(crate::Event::Issued { amount: 10 }),
+			]
+		);
 		assert_eq!(Balances::total_balance(&1), 20);
 		assert_eq!(Balances::total_issuance(), 120);
 	});
@@ -480,7 +483,7 @@ fn withdrawing_balance_should_work() {
 		let _ = Balances::deposit_creating(&2, 111);
 		let _ =
 			Balances::withdraw(&2, 11, WithdrawReasons::TRANSFER, ExistenceRequirement::KeepAlive);
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Withdraw {
+		System::assert_has_event(RuntimeEvent::Balances(crate::Event::Withdraw {
 			who: 2,
 			amount: 11,
 		}));
@@ -889,6 +892,7 @@ fn emit_events_with_existential_deposit() {
 			[
 				RuntimeEvent::System(system::Event::NewAccount { account: 1 }),
 				RuntimeEvent::Balances(crate::Event::Endowed { account: 1, free_balance: 100 }),
+				RuntimeEvent::Balances(crate::Event::Issued { amount: 100 }),
 				RuntimeEvent::Balances(crate::Event::BalanceSet { who: 1, free: 100 }),
 			]
 		);
@@ -902,6 +906,7 @@ fn emit_events_with_existential_deposit() {
 				RuntimeEvent::System(system::Event::KilledAccount { account: 1 }),
 				RuntimeEvent::Balances(crate::Event::DustLost { account: 1, amount: 99 }),
 				RuntimeEvent::Balances(crate::Event::Slashed { who: 1, amount: 1 }),
+				RuntimeEvent::Balances(crate::Event::Rescinded { amount: 1 }),
 			]
 		);
 	});
@@ -918,6 +923,7 @@ fn emit_events_with_no_existential_deposit_suicide() {
 				RuntimeEvent::Balances(crate::Event::BalanceSet { who: 1, free: 100 }),
 				RuntimeEvent::System(system::Event::NewAccount { account: 1 }),
 				RuntimeEvent::Balances(crate::Event::Endowed { account: 1, free_balance: 100 }),
+				RuntimeEvent::Balances(crate::Event::Issued { amount: 100 }),
 			]
 		);
 
@@ -929,6 +935,7 @@ fn emit_events_with_no_existential_deposit_suicide() {
 			[
 				RuntimeEvent::System(system::Event::KilledAccount { account: 1 }),
 				RuntimeEvent::Balances(crate::Event::Slashed { who: 1, amount: 100 }),
+				RuntimeEvent::Balances(crate::Event::Rescinded { amount: 100 }),
 			]
 		);
 	});
@@ -954,7 +961,7 @@ fn slash_full_works() {
 		assert_eq!(Balances::slash(&1, 1_000), (NegativeImbalance::new(1000), 0));
 		// Account is still alive
 		assert!(!System::account_exists(&1));
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Slashed {
+		System::assert_has_event(RuntimeEvent::Balances(crate::Event::Slashed {
 			who: 1,
 			amount: 1000,
 		}));
@@ -969,7 +976,7 @@ fn slash_partial_works() {
 		assert_eq!(Balances::slash(&1, 900), (NegativeImbalance::new(900), 0));
 		// Account is still alive
 		assert!(System::account_exists(&1));
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Slashed {
+		System::assert_has_event(RuntimeEvent::Balances(crate::Event::Slashed {
 			who: 1,
 			amount: 900,
 		}));
@@ -983,7 +990,7 @@ fn slash_dusting_works() {
 		// Slashed completed in full
 		assert_eq!(Balances::slash(&1, 950), (NegativeImbalance::new(950), 0));
 		assert!(!System::account_exists(&1));
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Slashed {
+		System::assert_has_event(RuntimeEvent::Balances(crate::Event::Slashed {
 			who: 1,
 			amount: 950,
 		}));
@@ -998,7 +1005,7 @@ fn slash_does_not_take_from_reserve() {
 		// Slashed completed in full
 		assert_eq!(Balances::slash(&1, 900), (NegativeImbalance::new(800), 100));
 		assert_eq!(Balances::reserved_balance(&1), 100);
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Slashed {
+		System::assert_has_event(RuntimeEvent::Balances(crate::Event::Slashed {
 			who: 1,
 			amount: 800,
 		}));
@@ -1399,6 +1406,7 @@ fn self_transfer_noop() {
 				Event::Deposit { who: 1, amount: 100 }.into(),
 				SysEvent::NewAccount { account: 1 }.into(),
 				Event::Endowed { account: 1, free_balance: 100 }.into(),
+				Event::Issued { amount: 100 }.into(),
 			]
 		);
 		assert_eq!(Balances::free_balance(1), 100);

substrate/frame/balances/src/tests/reentrancy_tests.rs

@@ -52,7 +52,7 @@ fn transfer_dust_removal_tst1_should_work() {
 			assert_eq!(Balances::free_balance(&1), 1050);
 
 			// Verify the events
-			assert_eq!(System::events().len(), 12);
+			assert_eq!(System::events().len(), 14);
 
 			System::assert_has_event(RuntimeEvent::Balances(crate::Event::Transfer {
 				from: 2,
@@ -93,7 +93,7 @@ fn transfer_dust_removal_tst2_should_work() {
 			assert_eq!(Balances::free_balance(&1), 1500);
 
 			// Verify the events
-			assert_eq!(System::events().len(), 10);
+			assert_eq!(System::events().len(), 12);
 
 			System::assert_has_event(RuntimeEvent::Balances(crate::Event::Transfer {
 				from: 2,
@@ -139,7 +139,7 @@ fn repatriating_reserved_balance_dust_removal_should_work() {
 			assert_eq!(Balances::free_balance(1), 1500);
 
 			// Verify the events
-			assert_eq!(System::events().len(), 10);
+			assert_eq!(System::events().len(), 12);
 
 			System::assert_has_event(RuntimeEvent::Balances(crate::Event::Transfer {
 				from: 2,
@@ -167,6 +167,7 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 			[
 				RuntimeEvent::System(system::Event::NewAccount { account: 1 }),
 				RuntimeEvent::Balances(crate::Event::Endowed { account: 1, free_balance: 100 }),
+				RuntimeEvent::Balances(crate::Event::Issued { amount: 100 }),
 				RuntimeEvent::Balances(crate::Event::BalanceSet { who: 1, free: 100 }),
 			]
 		);