Add Deposit and Withdraw Events to Balances Pallet

bin/node/bench/src/import.rs

@@ -135,18 +135,20 @@ impl core::Benchmark for ImportBenchmark {
 			.inspect_state(|| {
 				match self.block_type {
 					BlockType::RandomTransfersKeepAlive => {
-						// should be 5 per signed extrinsic + 1 per unsigned
+						// should be 7 per signed extrinsic + 1 per unsigned
 						// we have 1 unsigned and the rest are signed in the block
-						// those 5 events per signed are:
-						//    - new account (RawEvent::NewAccount) as we always transfer fund to
-						//      non-existant account
-						//    - endowed (RawEvent::Endowed) for this new account
-						//    - successful transfer (RawEvent::Transfer) for this transfer operation
-						//    - deposit event for charging transaction fee
+						// those 7 events per signed are:
+						//    - 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
+						//    - 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!(
 							node_runtime::System::events().len(),
-							(self.block.extrinsics.len() - 1) * 5 + 1,
+							(self.block.extrinsics.len() - 1) * 7 + 1,
 						);
 					},
 					BlockType::Noop => {

bin/node/executor/tests/basic.rs

@@ -385,6 +385,11 @@ fn full_native_block_import_works() {
 				})),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: Event::Balances(pallet_balances::Event::Withdraw(alice().into(), fees)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: Event::Balances(pallet_balances::Event::Transfer(
@@ -394,6 +399,14 @@ fn full_native_block_import_works() {
 				)),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: Event::Balances(pallet_balances::Event::Deposit(
+					pallet_treasury::Pallet::<Runtime>::account_id(),
+					fees * 8 / 10,
+				)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: Event::Treasury(pallet_treasury::Event::Deposit(fees * 8 / 10)),
@@ -439,6 +452,11 @@ fn full_native_block_import_works() {
 				})),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: Event::Balances(pallet_balances::Event::Withdraw(bob().into(), fees)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: Event::Balances(pallet_balances::Event::Transfer(
@@ -448,6 +466,14 @@ fn full_native_block_import_works() {
 				)),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(1),
+				event: Event::Balances(pallet_balances::Event::Deposit(
+					pallet_treasury::Pallet::<Runtime>::account_id(),
+					fees * 8 / 10,
+				)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
 				event: Event::Treasury(pallet_treasury::Event::Deposit(fees * 8 / 10)),
@@ -461,6 +487,11 @@ fn full_native_block_import_works() {
 				})),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(2),
+				event: Event::Balances(pallet_balances::Event::Withdraw(alice().into(), fees)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(2),
 				event: Event::Balances(pallet_balances::Event::Transfer(
@@ -470,6 +501,14 @@ fn full_native_block_import_works() {
 				)),
 				topics: vec![],
 			},
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(2),
+				event: Event::Balances(pallet_balances::Event::Deposit(
+					pallet_treasury::Pallet::<Runtime>::account_id(),
+					fees * 8 / 10,
+				)),
+				topics: vec![],
+			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(2),
 				event: Event::Treasury(pallet_treasury::Event::Deposit(fees * 8 / 10)),

bin/node/runtime/src/lib.rs

@@ -121,8 +121,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 267,
-	impl_version: 1,
+	spec_version: 268,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 2,
 };

frame/balances/src/lib.rs

@@ -463,8 +463,6 @@ pub mod pallet {
 		Transfer(T::AccountId, T::AccountId, T::Balance),
 		/// A balance was set by root. \[who, free, reserved\]
 		BalanceSet(T::AccountId, T::Balance, T::Balance),
-		/// Some amount was deposited (e.g. for transaction fees). \[who, deposit\]
-		Deposit(T::AccountId, T::Balance),
 		/// Some balance was reserved (moved from free to reserved). \[who, value\]
 		Reserved(T::AccountId, T::Balance),
 		/// Some balance was unreserved (moved from reserved to free). \[who, value\]
@@ -473,6 +471,14 @@ pub mod pallet {
 		/// Final argument indicates the destination balance type.
 		/// \[from, to, balance, destination_status\]
 		ReserveRepatriated(T::AccountId, T::AccountId, T::Balance, Status),
+		/// Some amount was deposited into the account (e.g. for transaction fees). \[who,
+		/// deposit\]
+		Deposit(T::AccountId, T::Balance),
+		/// Some amount was withdrawn from the account (e.g. for transaction fees). \[who, value\]
+		Withdraw(T::AccountId, T::Balance),
+		/// Some amount was removed from the account (e.g. for misbehavior). \[who,
+		/// amount_slashed\]
+		Slashed(T::AccountId, T::Balance),
 	}
 
 	/// Old name generated by `decl_event`.
@@ -1103,6 +1109,7 @@ impl<T: Config<I>, I: 'static> fungible::Mutate<T::AccountId> for Pallet<T, I> {
 			Ok(())
 		})?;
 		TotalIssuance::<T, I>::mutate(|t| *t += amount);
+		Self::deposit_event(Event::Deposit(who.clone(), amount));
 		Ok(())
 	}
 
@@ -1123,6 +1130,7 @@ impl<T: Config<I>, I: 'static> fungible::Mutate<T::AccountId> for Pallet<T, I> {
 			},
 		)?;
 		TotalIssuance::<T, I>::mutate(|t| *t -= actual);
+		Self::deposit_event(Event::Withdraw(who.clone(), amount));
 		Ok(actual)
 	}
 }
@@ -1141,7 +1149,10 @@ impl<T: Config<I>, I: 'static> fungible::Transfer<T::AccountId> for Pallet<T, I>
 
 impl<T: Config<I>, I: 'static> fungible::Unbalanced<T::AccountId> for Pallet<T, I> {
 	fn set_balance(who: &T::AccountId, amount: Self::Balance) -> DispatchResult {
-		Self::mutate_account(who, |account| account.free = amount)?;
+		Self::mutate_account(who, |account| {
+			account.free = amount;
+			Self::deposit_event(Event::BalanceSet(who.clone(), account.free, account.reserved));
+		})?;
 		Ok(())
 	}
 
@@ -1583,7 +1594,13 @@ where
 					}
 				},
 			) {
-				Ok(r) => return r,
+				Ok((imbalance, not_slashed)) => {
+					Self::deposit_event(Event::Slashed(
+						who.clone(),
+						value.saturating_sub(not_slashed),
+					));
+					return (imbalance, not_slashed)
+				},
 				Err(_) => (),
 			}
 		}
@@ -1608,6 +1625,7 @@ where
 			|account, is_new| -> Result<Self::PositiveImbalance, DispatchError> {
 				ensure!(!is_new, Error::<T, I>::DeadAccount);
 				account.free = account.free.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
+				Self::deposit_event(Event::Deposit(who.clone(), value));
 				Ok(PositiveImbalance::new(value))
 			},
 		)
@@ -1640,6 +1658,7 @@ where
 					None => return Ok(Self::PositiveImbalance::zero()),
 				};
 
+				Self::deposit_event(Event::Deposit(who.clone(), value));
 				Ok(PositiveImbalance::new(value))
 			},
 		)
@@ -1677,6 +1696,7 @@ where
 
 				account.free = new_free_account;
 
+				Self::deposit_event(Event::Withdraw(who.clone(), value));
 				Ok(NegativeImbalance::new(value))
 			},
 		)
@@ -1709,6 +1729,7 @@ where
 					SignedImbalance::Negative(NegativeImbalance::new(account.free - value))
 				};
 				account.free = value;
+				Self::deposit_event(Event::BalanceSet(who.clone(), account.free, account.reserved));
 				Ok(imbalance)
 			},
 		)
@@ -1824,7 +1845,13 @@ where
 				// underflow should never happen, but it if does, there's nothing to be done here.
 				(NegativeImbalance::new(actual), value - actual)
 			}) {
-				Ok(r) => return r,
+				Ok((imbalance, not_slashed)) => {
+					Self::deposit_event(Event::Slashed(
+						who.clone(),
+						value.saturating_sub(not_slashed),
+					));
+					return (imbalance, not_slashed)
+				},
 				Err(_) => (),
 			}
 		}
@@ -1965,6 +1992,7 @@ where
 					// `actual <= to_change` and `to_change <= amount`; qed;
 					reserves[index].amount -= actual;
 
+					Self::deposit_event(Event::Slashed(who.clone(), actual));
 					(imb, value - actual)
 				},
 				Err(_) => (NegativeImbalance::zero(), value),

frame/balances/src/tests.rs

@@ -314,6 +314,7 @@ macro_rules! decl_tests {
 			<$ext_builder>::default().monied(true).build().execute_with(|| {
 				assert_eq!(Balances::total_balance(&1), 10);
 				assert_ok!(Balances::deposit_into_existing(&1, 10).map(drop));
+				System::assert_last_event(Event::Balances(crate::Event::Deposit(1, 10)));
 				assert_eq!(Balances::total_balance(&1), 20);
 				assert_eq!(<TotalIssuance<$test>>::get(), 120);
 			});
@@ -341,6 +342,7 @@ macro_rules! decl_tests {
 		fn balance_works() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = Balances::deposit_creating(&1, 42);
+				System::assert_has_event(Event::Balances(crate::Event::Deposit(1, 42)));
 				assert_eq!(Balances::free_balance(1), 42);
 				assert_eq!(Balances::reserved_balance(1), 0);
 				assert_eq!(Balances::total_balance(&1), 42);
@@ -435,6 +437,19 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn withdrawing_balance_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = Balances::deposit_creating(&2, 111);
+				let _ = Balances::withdraw(
+					&2, 11, WithdrawReasons::TRANSFER, ExistenceRequirement::KeepAlive
+				);
+				System::assert_last_event(Event::Balances(crate::Event::Withdraw(2, 11)));
+				assert_eq!(Balances::free_balance(2), 100);
+				assert_eq!(<TotalIssuance<$test>>::get(), 100);
+			});
+		}
+
 		#[test]
 		fn slashing_incomplete_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -749,6 +764,7 @@ macro_rules! decl_tests {
 						[
 							Event::System(system::Event::KilledAccount(1)),
 							Event::Balances(crate::Event::DustLost(1, 99)),
+							Event::Balances(crate::Event::Slashed(1, 1)),
 						]
 					);
 				});
@@ -777,7 +793,8 @@ macro_rules! decl_tests {
 					assert_eq!(
 						events(),
 						[
-							Event::System(system::Event::KilledAccount(1))
+							Event::System(system::Event::KilledAccount(1)),
+							Event::Balances(crate::Event::Slashed(1, 100)),
 						]
 					);
 				});
@@ -797,6 +814,7 @@ macro_rules! decl_tests {
 					assert_eq!(Balances::slash(&1, 900), (NegativeImbalance::new(900), 0));
 					// Account is still alive
 					assert!(System::account_exists(&1));
+					System::assert_last_event(Event::Balances(crate::Event::Slashed(1, 900)));
 
 					// SCENARIO: Slash will kill account because not enough balance left.
 					assert_ok!(Balances::set_balance(Origin::root(), 1, 1_000, 0));

frame/balances/src/tests_local.rs

@@ -173,7 +173,7 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 		assert_eq!(res, (NegativeImbalance::new(98), 0));
 
 		// no events
-		assert_eq!(events(), []);
+		assert_eq!(events(), [Event::Balances(crate::Event::Slashed(1, 98))]);
 
 		let res = Balances::slash(&1, 1);
 		assert_eq!(res, (NegativeImbalance::new(1), 0));
@@ -183,6 +183,7 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 			[
 				Event::System(system::Event::KilledAccount(1)),
 				Event::Balances(crate::Event::DustLost(1, 1)),
+				Event::Balances(crate::Event::Slashed(1, 1)),
 			]
 		);
 	});

frame/balances/src/tests_reentrancy.rs

@@ -167,11 +167,11 @@ fn transfer_dust_removal_tst1_should_work() {
 		assert_eq!(Balances::free_balance(&1), 1050);
 
 		// Verify the events
-		// Number of events expected is 8
-		assert_eq!(System::events().len(), 11);
+		assert_eq!(System::events().len(), 12);
 
 		System::assert_has_event(Event::Balances(crate::Event::Transfer(2, 3, 450)));
 		System::assert_has_event(Event::Balances(crate::Event::DustLost(2, 50)));
+		System::assert_has_event(Event::Balances(crate::Event::Deposit(1, 50)));
 	});
 }
 
@@ -195,11 +195,11 @@ fn transfer_dust_removal_tst2_should_work() {
 		assert_eq!(Balances::free_balance(&1), 1500);
 
 		// Verify the events
-		// Number of events expected is 8
-		assert_eq!(System::events().len(), 9);
+		assert_eq!(System::events().len(), 10);
 
 		System::assert_has_event(Event::Balances(crate::Event::Transfer(2, 1, 450)));
 		System::assert_has_event(Event::Balances(crate::Event::DustLost(2, 50)));
+		System::assert_has_event(Event::Balances(crate::Event::Deposit(1, 50)));
 	});
 }
 
@@ -232,16 +232,15 @@ fn repatriating_reserved_balance_dust_removal_should_work() {
 		assert_eq!(Balances::free_balance(1), 1500);
 
 		// Verify the events
-		// Number of events expected is 10
-		assert_eq!(System::events().len(), 10);
+		assert_eq!(System::events().len(), 11);
 
 		System::assert_has_event(Event::Balances(crate::Event::ReserveRepatriated(
 			2,
 			1,
 			450,
 			Status::Free,
 		)));
-
-		System::assert_last_event(Event::Balances(crate::Event::DustLost(2, 50)));
+		System::assert_has_event(Event::Balances(crate::Event::DustLost(2, 50)));
+		System::assert_last_event(Event::Balances(crate::Event::Deposit(1, 50)));
 	});
 }

frame/contracts/src/tests.rs

@@ -444,6 +444,11 @@ fn instantiate_and_call_and_deposit_event() {
 		assert_eq!(
 			System::events(),
 			vec![
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Balances(pallet_balances::Event::Deposit(ALICE, 1_000_000)),
+					topics: vec![],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::System(frame_system::Event::NewAccount(ALICE.clone())),