diff --git a/Cargo.lock b/Cargo.lock index 54b49e0e3f12..352efda75c81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -20917,6 +20917,7 @@ dependencies = [ "parity-scale-codec", "platforms", "polkadot-sdk", + "pretty_assertions", "rand", "regex", "sc-service-test", diff --git a/prdoc/pr_4936.prdoc b/prdoc/pr_4936.prdoc new file mode 100644 index 000000000000..f9b7ee506a7a --- /dev/null +++ b/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 diff --git a/substrate/bin/node/bench/src/import.rs b/substrate/bin/node/bench/src/import.rs index e340869dea02..0b972650ef5a 100644 --- a/substrate/bin/node/bench/src/import.rs +++ b/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 => { diff --git a/substrate/bin/node/cli/Cargo.toml b/substrate/bin/node/cli/Cargo.toml index ab665f0792a4..1856487ac9ca 100644 --- a/substrate/bin/node/cli/Cargo.toml +++ b/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 } diff --git a/substrate/bin/node/cli/tests/basic.rs b/substrate/bin/node/cli/tests/basic.rs index b1f737ce399b..0a2e3fd25047 100644 --- a/substrate/bin/node/cli/tests/basic.rs +++ b/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( diff --git a/substrate/frame/balances/src/impl_currency.rs b/substrate/frame/balances/src/impl_currency.rs index 454aead1773f..049f0cc626c2 100644 --- a/substrate/frame/balances/src/impl_currency.rs +++ b/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, I: 'static> Drop for PositiveImbalance { /// Basic drop handler will just square up the total issuance. fn drop(&mut self) { - >::mutate(|v| *v = v.saturating_add(self.0)); + if !self.0.is_zero() { + >::mutate(|v| *v = v.saturating_add(self.0)); + Pallet::::deposit_event(Event::::Issued { amount: self.0 }); + } } } impl, I: 'static> Drop for NegativeImbalance { /// Basic drop handler will just square up the total issuance. fn drop(&mut self) { - >::mutate(|v| *v = v.saturating_sub(self.0)); + if !self.0.is_zero() { + >::mutate(|v| *v = v.saturating_sub(self.0)); + Pallet::::deposit_event(Event::::Rescinded { amount: self.0 }); + } } } } @@ -263,6 +270,8 @@ where Zero::zero() }); }); + + Pallet::::deposit_event(Event::::Rescinded { amount }); PositiveImbalance::new(amount) } @@ -279,6 +288,8 @@ where Self::Balance::max_value() }) }); + + Pallet::::deposit_event(Event::::Issued { amount }); NegativeImbalance::new(amount) } diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 0aaf618b303f..ddca685aa012 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/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, }; diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index 9ad4aca64406..fb69368a6216 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/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); diff --git a/substrate/frame/balances/src/tests/reentrancy_tests.rs b/substrate/frame/balances/src/tests/reentrancy_tests.rs index 717f04978577..0b66f2c93e3a 100644 --- a/substrate/frame/balances/src/tests/reentrancy_tests.rs +++ b/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 }), ] );