Merge pull request oasisprotocol#1380 from oasisprotocol/ptrus/featur…

…e/fee-transfer-events

Emit Transfer events for fee payments/disbursements

client-sdk/go/modules/accounts/accounts.go

@@ -10,6 +10,13 @@ import (
 	"github.com/oasisprotocol/oasis-sdk/client-sdk/go/types"
 )
 
+var (
+	// CommonPoolAddress is the address of the internal common pool account in the accounts module.
+	CommonPoolAddress = types.NewAddressForModule("accounts", []byte("common-pool"))
+	// FeeAccumulatorAddress is the address of the internal fee accumulator account in the accounts module.
+	FeeAccumulatorAddress = types.NewAddressForModule("accounts", []byte("fee-accumulator"))
+)
+
 var (
 	// Callable methods.
 	methodTransfer = types.NewMethodName("accounts.Transfer", Transfer{})

runtime-sdk/modules/evm/src/lib.rs

@@ -591,7 +591,7 @@ impl<Cfg: Config> Module<Cfg> {
 
         // Move the difference from the fee accumulator back to the caller.
         let caller_address = Cfg::map_address(source.into());
-        Cfg::Accounts::move_from_fee_accumulator(
+        Cfg::Accounts::return_tx_fee(
             ctx,
             caller_address,
             &token::BaseUnits::new(return_fee.as_u128(), fee_denomination),

runtime-sdk/src/dispatcher.rs

@@ -187,6 +187,39 @@ impl<R: Runtime> Dispatcher<R> {
         call: types::transaction::Call,
         opts: &DispatchOptions<'_>,
     ) -> (module::CallResult, callformat::Metadata)
+    where
+        C::Store: NestedStore,
+    {
+        let read_only = call.read_only;
+
+        // Dispatch the call.
+        let (result, metadata) = Self::_dispatch_tx_call(ctx, call, opts);
+
+        // Unconditionally call after handle call hook.
+        let result = match R::Modules::after_handle_call(ctx, result) {
+            Ok(result) => result,
+            Err(e) => {
+                // If the call failed, return the error.
+                return (e.into_call_result(), metadata);
+            }
+        };
+
+        // Make sure that a read-only call did not result in any modifications.
+        if read_only && ctx.runtime_state().has_pending_updates() {
+            return (
+                modules::core::Error::ReadOnlyTransaction.into_call_result(),
+                metadata,
+            );
+        }
+
+        (result, metadata)
+    }
+
+    fn _dispatch_tx_call<C: TxContext>(
+        ctx: &mut C,
+        call: types::transaction::Call,
+        opts: &DispatchOptions<'_>,
+    ) -> (module::CallResult, callformat::Metadata)
     where
         C::Store: NestedStore,
     {
@@ -224,19 +257,6 @@ impl<R: Runtime> Dispatcher<R> {
             }
         };
 
-        // Call after hook.
-        if let Err(e) = R::Modules::after_handle_call(ctx) {
-            return (e.into_call_result(), call_format_metadata);
-        }
-
-        // Make sure that a read-only call did not result in any modifications.
-        if call.read_only && ctx.runtime_state().has_pending_updates() {
-            return (
-                modules::core::Error::ReadOnlyTransaction.into_call_result(),
-                call_format_metadata,
-            );
-        }
-
         (result, call_format_metadata)
     }
 

runtime-sdk/src/module.rs

@@ -382,9 +382,12 @@ pub trait TransactionHandler {
     /// Perform any action after call, within the transaction context.
     ///
     /// If an error is returned the transaction call fails and updates are rolled back.
-    fn after_handle_call<C: TxContext>(_ctx: &mut C) -> Result<(), modules::core::Error> {
+    fn after_handle_call<C: TxContext>(
+        _ctx: &mut C,
+        result: CallResult,
+    ) -> Result<CallResult, modules::core::Error> {
         // Default implementation doesn't do anything.
-        Ok(())
+        Ok(result)
     }
 
     /// Perform any action after dispatching the transaction, in batch context.
@@ -438,9 +441,14 @@ impl TransactionHandler for Tuple {
         Ok(())
     }
 
-    fn after_handle_call<C: TxContext>(ctx: &mut C) -> Result<(), modules::core::Error> {
-        for_tuples!( #( Tuple::after_handle_call(ctx)?; )* );
-        Ok(())
+    fn after_handle_call<C: TxContext>(
+        ctx: &mut C,
+        mut result: CallResult,
+    ) -> Result<CallResult, modules::core::Error> {
+        for_tuples!( #(
+            result = Tuple::after_handle_call(ctx, result)?;
+        )* );
+        Ok(result)
     }
 
     fn after_dispatch_tx<C: Context>(ctx: &mut C, tx_auth_info: &AuthInfo, result: &CallResult) {

runtime-sdk/src/modules/accounts/mod.rs

@@ -218,15 +218,15 @@ pub trait API {
         denomination: &token::Denomination,
     ) -> Result<types::DenominationInfo, Error>;
 
-    /// Move amount from address into fee accumulator.
-    fn move_into_fee_accumulator<C: Context>(
+    /// Moves the amount into the per-transaction fee accumulator.
+    fn charge_tx_fee<C: Context>(
         ctx: &mut C,
         from: Address,
         amount: &token::BaseUnits,
     ) -> Result<(), modules::core::Error>;
 
-    /// Move amount from fee accumulator into address.
-    fn move_from_fee_accumulator<C: Context>(
+    /// Returns the amount from the per-transaction fee accumulator to the account.
+    fn return_tx_fee<C: Context>(
         ctx: &mut C,
         to: Address,
         amount: &token::BaseUnits,
@@ -429,7 +429,9 @@ impl FeeAccumulator {
     }
 }
 
-/// Context key for the fee accumulator.
+/// Context key for the per-transaction fee accumulator.
+const CONTEXT_KEY_TX_FEE_ACCUMULATOR: &str = "accounts.TxFeeAccumulator";
+/// Context key for the per block fee accumulator.
 const CONTEXT_KEY_FEE_ACCUMULATOR: &str = "accounts.FeeAccumulator";
 
 impl API for Module {
@@ -596,7 +598,7 @@ impl API for Module {
             .ok_or(Error::NotFound)
     }
 
-    fn move_into_fee_accumulator<C: Context>(
+    fn charge_tx_fee<C: Context>(
         ctx: &mut C,
         from: Address,
         amount: &token::BaseUnits,
@@ -608,14 +610,14 @@ impl API for Module {
         Self::sub_amount(ctx.runtime_state(), from, amount)
             .map_err(|_| modules::core::Error::InsufficientFeeBalance)?;
 
-        ctx.value::<FeeAccumulator>(CONTEXT_KEY_FEE_ACCUMULATOR)
+        ctx.value::<FeeAccumulator>(CONTEXT_KEY_TX_FEE_ACCUMULATOR)
             .or_default()
             .add(amount);
 
         Ok(())
     }
 
-    fn move_from_fee_accumulator<C: Context>(
+    fn return_tx_fee<C: Context>(
         ctx: &mut C,
         to: Address,
         amount: &token::BaseUnits,
@@ -624,7 +626,7 @@ impl API for Module {
             return Ok(());
         }
 
-        ctx.value::<FeeAccumulator>(CONTEXT_KEY_FEE_ACCUMULATOR)
+        ctx.value::<FeeAccumulator>(CONTEXT_KEY_TX_FEE_ACCUMULATOR)
             .or_default()
             .sub(amount)
             .map_err(|_| modules::core::Error::InsufficientFeeBalance)?;
@@ -929,11 +931,9 @@ impl module::TransactionHandler for Module {
                     .map_err(|_| modules::core::Error::InsufficientFeeBalance)?;
             } else {
                 // Actually perform the move.
-                Self::move_into_fee_accumulator(ctx, payer, &tx.auth_info.fee.amount)?;
+                Self::charge_tx_fee(ctx, payer, &tx.auth_info.fee.amount)?;
             }
 
-            // TODO: Emit event that fee has been paid.
-
             let gas_price = tx.auth_info.fee.gas_price();
             // Set transaction priority.
             <C::Runtime as Runtime>::Core::set_priority(
@@ -952,6 +952,35 @@ impl module::TransactionHandler for Module {
         Ok(())
     }
 
+    fn after_handle_call<C: TxContext>(
+        ctx: &mut C,
+        result: module::CallResult,
+    ) -> Result<module::CallResult, modules::core::Error> {
+        let acc = ctx
+            .value::<FeeAccumulator>(CONTEXT_KEY_TX_FEE_ACCUMULATOR)
+            .take()
+            .unwrap_or_default();
+
+        // Emit events for paid fees.
+        for (denom, amount) in acc.total_fees.iter() {
+            ctx.emit_unconditional_event(Event::Transfer {
+                from: ctx.tx_caller_address(),
+                to: *ADDRESS_FEE_ACCUMULATOR,
+                amount: token::BaseUnits::new(*amount, denom.clone()),
+            });
+        }
+
+        // Move transaction fees into the block fee accumulator.
+        let fee_acc = ctx
+            .value::<FeeAccumulator>(CONTEXT_KEY_FEE_ACCUMULATOR)
+            .or_default();
+        for (denom, amount) in acc.total_fees.into_iter() {
+            fee_acc.add(&token::BaseUnits(amount, denom));
+        }
+
+        Ok(result)
+    }
+
     fn after_dispatch_tx<C: Context>(
         ctx: &mut C,
         tx_auth_info: &AuthInfo,
@@ -1029,18 +1058,29 @@ impl module::BlockHandler for Module {
 
                     Self::add_amount(ctx.runtime_state(), address, amount)
                         .expect("add_amount must succeed for fee disbursement");
+
+                    // Emit transfer event for fee disbursement.
+                    ctx.emit_event(Event::Transfer {
+                        from: *ADDRESS_FEE_ACCUMULATOR,
+                        to: address,
+                        amount: amount.clone(),
+                    });
                 }
             }
         }
 
         // Transfer remainder to a common pool account.
         for (denom, remainder) in previous_fees.into_iter() {
-            Self::add_amount(
-                ctx.runtime_state(),
-                *ADDRESS_COMMON_POOL,
-                &token::BaseUnits::new(remainder, denom),
-            )
-            .expect("add_amount must succeed for transfer to common pool")
+            let amount = token::BaseUnits::new(remainder, denom);
+            Self::add_amount(ctx.runtime_state(), *ADDRESS_COMMON_POOL, &amount)
+                .expect("add_amount must succeed for transfer to common pool");
+
+            // Emit transfer event for fee disbursement.
+            ctx.emit_event(Event::Transfer {
+                from: *ADDRESS_FEE_ACCUMULATOR,
+                to: *ADDRESS_COMMON_POOL,
+                amount,
+            })
         }
 
         // Fees for the active block should be transferred to the fee accumulator address.

runtime-sdk/src/modules/accounts/test.rs

@@ -8,7 +8,7 @@ use anyhow::anyhow;
 
 use crate::{
     context::{BatchContext, Context},
-    module::{BlockHandler, InvariantHandler, MethodHandler, TransactionHandler},
+    module::{self, BlockHandler, InvariantHandler, MethodHandler, TransactionHandler},
     modules::{core, core::API as _},
     testing::{keys, mock},
     types::{
@@ -571,6 +571,16 @@ fn test_fee_disbursement() {
 
     // Authenticate transaction, fees should be moved to accumulator.
     Accounts::authenticate_tx(&mut ctx, &tx).expect("transaction authentication should succeed");
+    ctx.with_tx(0, 0, tx, |mut tx_ctx, _call| {
+        // Run after call tx handler.
+        Accounts::after_handle_call(
+            &mut tx_ctx,
+            module::CallResult::Ok(cbor::Value::Simple(cbor::SimpleValue::NullValue)),
+        )
+        .expect("after_handle_call should succeed");
+        tx_ctx.commit()
+    });
+
     // Run end block handler.
     Accounts::end_block(&mut ctx);
 
@@ -1019,14 +1029,14 @@ fn test_fee_acc() {
 
     init_accounts(&mut ctx);
 
-    // Check that Accounts::move_{into,from}_fee_accumulator work.
+    // Check that Accounts::{charge,return}_tx_fee work.
     ctx.with_tx(0, 0, mock::transaction(), |mut tx_ctx, _call| {
-        Accounts::move_into_fee_accumulator(
+        Accounts::charge_tx_fee(
             &mut tx_ctx,
             keys::alice::address(),
             &BaseUnits::new(1_000, Denomination::NATIVE),
         )
-        .expect("move into should succeed");
+        .expect("charge tx fee should succeed");
 
         let ab = Accounts::get_balance(
             tx_ctx.runtime_state(),
@@ -1036,12 +1046,12 @@ fn test_fee_acc() {
         .expect("get_balance should succeed");
         assert_eq!(ab, 999_000, "balance in source account should be correct");
 
-        Accounts::move_from_fee_accumulator(
+        Accounts::return_tx_fee(
             &mut tx_ctx,
             keys::alice::address(),
             &BaseUnits::new(1_000, Denomination::NATIVE),
         )
-        .expect("move from should succeed");
+        .expect("return tx fee should succeed");
 
         let ab = Accounts::get_balance(
             tx_ctx.runtime_state(),
@@ -1060,16 +1070,16 @@ fn test_fee_acc_sim() {
 
     init_accounts(&mut ctx);
 
-    // Check that Accounts::move_{into,from}_fee_accumulator don't do
+    // Check that Accounts::{charge,return}_tx_fee don't do
     // anything in simulation mode.
     ctx.with_simulation(|mut sctx| {
         sctx.with_tx(0, 0, mock::transaction(), |mut tx_ctx, _call| {
-            Accounts::move_into_fee_accumulator(
+            Accounts::charge_tx_fee(
                 &mut tx_ctx,
                 keys::alice::address(),
                 &BaseUnits::new(1_000, Denomination::NATIVE),
             )
-            .expect("move into should succeed");
+            .expect("charge tx fee should succeed");
 
             let ab = Accounts::get_balance(
                 tx_ctx.runtime_state(),
@@ -1079,12 +1089,12 @@ fn test_fee_acc_sim() {
             .expect("get_balance should succeed");
             assert_eq!(ab, 1_000_000, "balance in source account should be correct");
 
-            Accounts::move_from_fee_accumulator(
+            Accounts::return_tx_fee(
                 &mut tx_ctx,
                 keys::alice::address(),
                 &BaseUnits::new(1_000, Denomination::NATIVE),
             )
-            .expect("move from should succeed");
+            .expect("return tx fee should succeed");
 
             let ab = Accounts::get_balance(
                 tx_ctx.runtime_state(),

runtime-sdk/src/modules/core/mod.rs

@@ -999,14 +999,17 @@ impl<Cfg: Config> module::TransactionHandler for Module<Cfg> {
         Ok(())
     }
 
-    fn after_handle_call<C: TxContext>(ctx: &mut C) -> Result<(), Error> {
+    fn after_handle_call<C: TxContext>(
+        ctx: &mut C,
+        result: module::CallResult,
+    ) -> Result<module::CallResult, Error> {
         // Emit gas used event.
         if Cfg::EMIT_GAS_USED_EVENTS {
             let used_gas = Self::used_tx_gas(ctx);
             ctx.emit_unconditional_event(Event::GasUsed { amount: used_gas });
         }
 
-        Ok(())
+        Ok(result)
     }
 }
 

runtime-sdk/src/modules/core/test.rs

@@ -1086,7 +1086,11 @@ fn test_gas_used_events() {
     let etags = ctx.with_tx(0, 0, tx, |mut tx_ctx, _call| {
         Core::use_tx_gas(&mut tx_ctx, 10).expect("using gas under limit should succeed");
         assert_eq!(Core::used_tx_gas(&mut tx_ctx), 10);
-        Core::after_handle_call(&mut tx_ctx).unwrap();
+        Core::after_handle_call(
+            &mut tx_ctx,
+            module::CallResult::Ok(cbor::Value::Simple(cbor::SimpleValue::NullValue)),
+        )
+        .expect("after_handle_call should succeed");
 
         let (etags, _) = tx_ctx.commit();
         let tags = etags.clone().into_tags();