Transfer to beneficiary after transfer_on_hold

frame/contracts/src/exec.rs

@@ -22,7 +22,7 @@ use crate::{
  storage::{self, WriteOutcome},
  BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf,
  DebugBufferVec, Determinism, Error, Event, Nonce, Origin, Pallet as Contracts, Schedule,
- System, WasmBlob, LOG_TARGET,
+ WasmBlob, LOG_TARGET,
 };
 use frame_support::{
  crypto::ecdsa::ECDSAExt,
@@ -33,7 +33,7 @@ use frame_support::{
  storage::{with_transaction, TransactionOutcome},
  traits::{
  fungible::{Inspect, Mutate},
- tokens::{Fortitude::Polite, Preservation},
+ tokens::Preservation,
  Contains, OriginTrait, Randomness, Time,
  },
  weights::Weight,
@@ -1283,14 +1283,8 @@ where
  }
  let frame = self.top_frame_mut();
  let info = frame.terminate();
- frame.nested_storage.terminate(&info);
- System::<T>::dec_consumers(&frame.account_id);
- Self::transfer(
- Preservation::Expendable,
- &frame.account_id,
- beneficiary,
- T::Currency::reducible_balance(&frame.account_id, Preservation::Expendable, Polite),
- )?;
+ frame.nested_storage.terminate(&info, beneficiary.clone());
+
  info.queue_trie_for_deletion();
  ContractInfoOf::<T>::remove(&frame.account_id);
  E::decrement_refcount(info.code_hash);

frame/contracts/src/storage/meter.rs

@@ -18,8 +18,8 @@
 //! This module contains functions to meter the storage deposit.
 
 use crate::{
- storage::ContractInfo, BalanceOf, CodeInfo, Config, Error, Event, HoldReason, Inspect, Origin,
- Pallet, StorageDeposit as Deposit, System, LOG_TARGET,
+ storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error, Event, HoldReason,
+ Inspect, Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET,
 };
 
 use frame_support::{
@@ -88,7 +88,7 @@ pub trait Ext<T: Config> {
  origin: &T::AccountId,
  contract: &T::AccountId,
  amount: &DepositOf<T>,
- terminated: bool,
+ status: &ContractStatus<T>,
  ) -> Result<(), DispatchError>;
 }
 
@@ -224,6 +224,15 @@ impl Diff {
  }
 }
 
+/// The status of a contract.
+///
+/// In case of termination the beneficiary is indicated.
+#[derive(RuntimeDebugNoBound, Clone, PartialEq, Eq)]
+pub enum ContractStatus<T: Config> {
+ Alive,
+ Terminated { beneficiary: AccountIdOf<T> },
+}
+
 /// Records information to charge or refund a plain account.
 ///
 /// All the charges are deferred to the end of a whole call stack. Reason is that by doing
@@ -237,7 +246,7 @@ impl Diff {
 struct Charge<T: Config> {
  contract: T::AccountId,
  amount: DepositOf<T>,
- terminated: bool,
+ status: ContractStatus<T>,
 }
 
 /// Records the storage changes of a storage meter.
@@ -249,16 +258,18 @@ enum Contribution<T: Config> {
  /// its execution. In this process the [`Diff`] was converted into a [`Deposit`].
  Checked(DepositOf<T>),
  /// The contract was terminated. In this process the [`Diff`] was converted into a [`Deposit`]
- /// in order to calculate the refund.
- Terminated(DepositOf<T>),
+ /// in order to calculate the refund. Upon termination the `reducible_balance` in the
+ /// contract's account is transferred to the [`beneficiary`].
+ Terminated { deposit: DepositOf<T>, beneficiary: AccountIdOf<T> },
 }
 
 impl<T: Config> Contribution<T> {
  /// See [`Diff::update_contract`].
  fn update_contract(&self, info: Option<&mut ContractInfo<T>>) -> DepositOf<T> {
  match self {
  Self::Alive(diff) => diff.update_contract::<T>(info),
- Self::Terminated(deposit) | Self::Checked(deposit) => deposit.clone(),
+ Self::Terminated { deposit, beneficiary: _ } | Self::Checked(deposit) =>
+ deposit.clone(),
  }
  }
 }
@@ -325,7 +336,7 @@ where
  self.charges.push(Charge {
  contract: contract.clone(),
  amount: own_deposit,
- terminated: absorbed.is_terminated(),
+ status: absorbed.is_terminated(),
  });
  }
  }
@@ -341,8 +352,12 @@ where
  }
 
  /// True if the contract is terminated.
- fn is_terminated(&self) -> bool {
- matches!(self.own_contribution, Contribution::Terminated(_))
+ fn is_terminated(&self) -> ContractStatus<T> {
+ match &self.own_contribution {
+ Contribution::Terminated { deposit: _, beneficiary } =>
+ ContractStatus::Terminated { beneficiary: beneficiary.clone() },
+ _ => ContractStatus::Alive,
+ }
  }
 }
 
@@ -386,10 +401,10 @@ where
  Origin::Signed(o) => o,
  };
  for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Refund(_))) {
- E::charge(origin, &charge.contract, &charge.amount, charge.terminated)?;
+ E::charge(origin, &charge.contract, &charge.amount, &charge.status)?;
  }
  for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Charge(_))) {
- E::charge(origin, &charge.contract, &charge.amount, charge.terminated)?;
+ E::charge(origin, &charge.contract, &charge.amount, &charge.status)?;
  }
  Ok(self.total_deposit)
  }
@@ -417,7 +432,7 @@ where
  /// deposit charge separately from the storage charge.
  pub fn charge_deposit(&mut self, contract: T::AccountId, amount: DepositOf<T>) {
  self.total_deposit = self.total_deposit.saturating_add(&amount);
- self.charges.push(Charge { contract, amount, terminated: false });
+ self.charges.push(Charge { contract, amount, status: ContractStatus::Alive });
  }
 
  /// Charge from `origin` a storage deposit plus the ed for contract instantiation.
@@ -447,23 +462,35 @@ where
  // We need to make sure that the contract's account exists.
  T::Currency::transfer(origin, contract, ed, Preservation::Preserve)?;
 
+ // A consumer is added at account creation and removed it on termination, otherwise the
+ // runtime could remove the account. As long as a contract exists its account must exist.
+ // With the consumer, a correct runtime cannot remove the account.
  System::<T>::inc_consumers(contract)?;
 
  // Normally, deposit charges are deferred to be able to coalesce them with refunds.
  // However, we need to charge immediately so that the account is created before
  // charges possibly below the ed are collected and fail.
- E::charge(origin, contract, &deposit.saturating_sub(&Deposit::Charge(ed)), false)?;
+ E::charge(
+ origin,
+ contract,
+ &deposit.saturating_sub(&Deposit::Charge(ed)),
+ &ContractStatus::Alive,
+ )?;
 
  Ok(deposit)
  }
 
  /// Call to tell the meter that the currently executing contract was executed.
  ///
  /// This will manipulate the meter so that all storage deposit accumulated in
- /// `contract_info` will be refunded to the `origin` of the meter.
- pub fn terminate(&mut self, info: &ContractInfo<T>) {
+ /// `contract_info` will be refunded to the `origin` of the meter. And the free
+ /// (`reducible_balance`) will be sent to the `beneficiary`.
+ pub fn terminate(&mut self, info: &ContractInfo<T>, beneficiary: T::AccountId) {
  debug_assert!(self.is_alive());
- self.own_contribution = Contribution::Terminated(Deposit::Refund(info.total_deposit()));
+ self.own_contribution = Contribution::Terminated {
+ deposit: Deposit::Refund(info.total_deposit()),
+ beneficiary,
+ };
  }
 
  /// [`Self::charge`] does not enforce the storage limit since we want to do this check as late
@@ -532,7 +559,7 @@ impl<T: Config> Ext<T> for ReservingExt {
  origin: &T::AccountId,
  contract: &T::AccountId,
  amount: &DepositOf<T>,
- terminated: bool,
+ status: &ContractStatus<T>,
  ) -> Result<(), DispatchError> {
  match amount {
  Deposit::Charge(amount) | Deposit::Refund(amount) if amount.is_zero() => return Ok(()),
@@ -590,8 +617,15 @@ impl<T: Config> Ext<T> for ReservingExt {
  }
  },
  }
- if terminated {
+ if let ContractStatus::<T>::Terminated { beneficiary } = status {
  System::<T>::dec_consumers(&contract);
+ // Whatever is left in the contract is sent to the termination beneficiary.
+ T::Currency::transfer(
+ &contract,
+ &beneficiary,
+ T::Currency::reducible_balance(&contract, Preservation::Expendable, Polite),
+ Preservation::Expendable,
+ )?;
  }
  Ok(())
  }
@@ -631,7 +665,7 @@ mod tests {
  origin: AccountIdOf<Test>,
  contract: AccountIdOf<Test>,
  amount: DepositOf<Test>,
- terminated: bool,
+ status: ContractStatus<Test>,
  }
 
  #[derive(Default, Debug, PartialEq, Eq, Clone)]
@@ -665,14 +699,14 @@ mod tests {
  origin: &AccountIdOf<Test>,
  contract: &AccountIdOf<Test>,
  amount: &DepositOf<Test>,
- terminated: bool,
+ status: &ContractStatus<Test>,
  ) -> Result<(), DispatchError> {
  TestExtTestValue::mutate(|ext| {
  ext.charges.push(Charge {
  origin: origin.clone(),
  contract: contract.clone(),
  amount: amount.clone(),
- terminated,
+ status: status.clone(),
  })
  });
  Ok(())
@@ -759,19 +793,19 @@ mod tests {
  origin: ALICE,
  contract: CHARLIE,
  amount: Deposit::Refund(10),
- terminated: false,
+ status: ContractStatus::Alive,
  },
  Charge {
  origin: ALICE,
  contract: CHARLIE,
  amount: Deposit::Refund(20),
- terminated: false,
+ status: ContractStatus::Alive,
  },
  Charge {
  origin: ALICE,
  contract: BOB,
  amount: Deposit::Charge(2),
- terminated: false,
+ status: ContractStatus::Alive,
  },
  ],
  },
@@ -850,13 +884,13 @@ mod tests {
  origin: ALICE,
  contract: CHARLIE,
  amount: Deposit::Refund(119),
- terminated: true,
+ status: ContractStatus::Terminated { beneficiary: CHARLIE },
  },
  Charge {
  origin: ALICE,
  contract: BOB,
  amount: Deposit::Charge(12),
- terminated: false,
+ status: ContractStatus::Alive,
  },
  ],
  },
@@ -892,7 +926,7 @@ mod tests {
  let mut nested1 = nested0.nested(BalanceOf::<Test>::zero());
  nested1.charge(&Diff { items_removed: 5, ..Default::default() });
  nested1.charge(&Diff { bytes_added: 20, ..Default::default() });
- nested1.terminate(&nested1_info);
+ nested1.terminate(&nested1_info, CHARLIE);
  nested0.enforce_limit(Some(&mut nested1_info)).unwrap();
  nested0.absorb(nested1, &CHARLIE, None);
 

frame/contracts/src/tests.rs

@@ -1598,15 +1598,6 @@ fn self_destruct_works() {
  pretty_assertions::assert_eq!(
  System::events(),
  vec![
- EventRecord {
- phase: Phase::Initialization,
- event: RuntimeEvent::Balances(pallet_balances::Event::Transfer {
- from: addr.clone(),
- to: DJANGO,
- amount: 100_000 + min_balance,
- }),
- topics: vec![],
- },
  EventRecord {
  phase: Phase::Initialization,
  event: RuntimeEvent::Contracts(crate::Event::Terminated {
@@ -1641,6 +1632,15 @@ fn self_destruct_works() {
  }),
  topics: vec![],
  },
+ EventRecord {
+ phase: Phase::Initialization,
+ event: RuntimeEvent::Balances(pallet_balances::Event::Transfer {
+ from: addr.clone(),
+ to: DJANGO,
+ amount: 100_000 + min_balance,
+ }),
+ topics: vec![],
+ },
  ],
  );
  });