Skip to content

Commit

Permalink
Fix not emitting fungible_asset::Withdraw event when paying gas (#15835)
Browse files Browse the repository at this point in the history
Co-authored-by: Igor <[email protected]>
(cherry picked from commit b4c4e96)
  • Loading branch information
igor-aptos committed Jan 29, 2025
1 parent 55909aa commit d99c74c
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 54 deletions.
67 changes: 58 additions & 9 deletions aptos-move/framework/aptos-framework/doc/coin.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ This module provides the foundation for typesafe Coins.
- [Function `coin_supply`](#0x1_coin_coin_supply)
- [Function `burn`](#0x1_coin_burn)
- [Function `burn_from`](#0x1_coin_burn_from)
- [Function `burn_from_for_gas`](#0x1_coin_burn_from_for_gas)
- [Function `deposit`](#0x1_coin_deposit)
- [Function `deposit_with_signer`](#0x1_coin_deposit_with_signer)
- [Function `can_receive_paired_fungible_asset`](#0x1_coin_can_receive_paired_fungible_asset)
- [Function `force_deposit`](#0x1_coin_force_deposit)
- [Function `deposit_for_gas_fee`](#0x1_coin_deposit_for_gas_fee)
- [Function `destroy_zero`](#0x1_coin_destroy_zero)
- [Function `extract`](#0x1_coin_extract)
- [Function `extract_all`](#0x1_coin_extract_all)
Expand Down Expand Up @@ -119,7 +120,7 @@ This module provides the foundation for typesafe Coins.
- [Function `burn`](#@Specification_1_burn)
- [Function `burn_from`](#@Specification_1_burn_from)
- [Function `deposit`](#@Specification_1_deposit)
- [Function `force_deposit`](#@Specification_1_force_deposit)
- [Function `deposit_for_gas_fee`](#@Specification_1_deposit_for_gas_fee)
- [Function `destroy_zero`](#@Specification_1_destroy_zero)
- [Function `extract`](#@Specification_1_extract)
- [Function `extract_all`](#@Specification_1_extract_all)
Expand Down Expand Up @@ -2672,6 +2673,54 @@ Note: This bypasses CoinStore::frozen -- coins within a frozen CoinStore can be



</details>

<a id="0x1_coin_burn_from_for_gas"></a>

## Function `burn_from_for_gas`



<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_burn_from_for_gas">burn_from_for_gas</a>&lt;CoinType&gt;(account_addr: <b>address</b>, amount: u64, burn_cap: &<a href="coin.md#0x1_coin_BurnCapability">coin::BurnCapability</a>&lt;CoinType&gt;)
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_burn_from_for_gas">burn_from_for_gas</a>&lt;CoinType&gt;(
account_addr: <b>address</b>,
amount: u64,
burn_cap: &<a href="coin.md#0x1_coin_BurnCapability">BurnCapability</a>&lt;CoinType&gt;,
) <b>acquires</b> <a href="coin.md#0x1_coin_CoinInfo">CoinInfo</a>, <a href="coin.md#0x1_coin_CoinStore">CoinStore</a>, <a href="coin.md#0x1_coin_CoinConversionMap">CoinConversionMap</a>, <a href="coin.md#0x1_coin_PairedFungibleAssetRefs">PairedFungibleAssetRefs</a> {
// Skip burning <b>if</b> amount is zero. This shouldn't <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error">error</a> out <b>as</b> it's called <b>as</b> part of transaction fee burning.
<b>if</b> (amount == 0) {
<b>return</b>
};

<b>let</b> (coin_amount_to_burn, fa_amount_to_burn) = <a href="coin.md#0x1_coin_calculate_amount_to_withdraw">calculate_amount_to_withdraw</a>&lt;CoinType&gt;(
account_addr,
amount
);
<b>if</b> (coin_amount_to_burn &gt; 0) {
<b>let</b> coin_store = <b>borrow_global_mut</b>&lt;<a href="coin.md#0x1_coin_CoinStore">CoinStore</a>&lt;CoinType&gt;&gt;(account_addr);
<b>let</b> coin_to_burn = <a href="coin.md#0x1_coin_extract">extract</a>(&<b>mut</b> coin_store.<a href="coin.md#0x1_coin">coin</a>, coin_amount_to_burn);
<a href="coin.md#0x1_coin_burn">burn</a>(coin_to_burn, burn_cap);
};
<b>if</b> (fa_amount_to_burn &gt; 0) {
<a href="fungible_asset.md#0x1_fungible_asset_address_burn_from_for_gas">fungible_asset::address_burn_from_for_gas</a>(
<a href="coin.md#0x1_coin_borrow_paired_burn_ref">borrow_paired_burn_ref</a>(burn_cap),
<a href="primary_fungible_store.md#0x1_primary_fungible_store_primary_store_address">primary_fungible_store::primary_store_address</a>(account_addr, <a href="../../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option_destroy_some">option::destroy_some</a>(<a href="coin.md#0x1_coin_paired_metadata">paired_metadata</a>&lt;CoinType&gt;())),
fa_amount_to_burn
);
};
}
</code></pre>



</details>

<a id="0x1_coin_deposit"></a>
Expand Down Expand Up @@ -2797,15 +2846,15 @@ Deposit the coin balance into the recipient's account and emit an event.

</details>

<a id="0x1_coin_force_deposit"></a>
<a id="0x1_coin_deposit_for_gas_fee"></a>

## Function `force_deposit`
## Function `deposit_for_gas_fee`

Deposit the coin balance into the recipient's account without checking if the account is frozen.
This is for internal use only and doesn't emit an DepositEvent.


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_force_deposit">force_deposit</a>&lt;CoinType&gt;(account_addr: <b>address</b>, <a href="coin.md#0x1_coin">coin</a>: <a href="coin.md#0x1_coin_Coin">coin::Coin</a>&lt;CoinType&gt;)
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_deposit_for_gas_fee">deposit_for_gas_fee</a>&lt;CoinType&gt;(account_addr: <b>address</b>, <a href="coin.md#0x1_coin">coin</a>: <a href="coin.md#0x1_coin_Coin">coin::Coin</a>&lt;CoinType&gt;)
</code></pre>


Expand All @@ -2814,7 +2863,7 @@ This is for internal use only and doesn't emit an DepositEvent.
<summary>Implementation</summary>


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_force_deposit">force_deposit</a>&lt;CoinType&gt;(
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_deposit_for_gas_fee">deposit_for_gas_fee</a>&lt;CoinType&gt;(
account_addr: <b>address</b>,
<a href="coin.md#0x1_coin">coin</a>: <a href="coin.md#0x1_coin_Coin">Coin</a>&lt;CoinType&gt;
) <b>acquires</b> <a href="coin.md#0x1_coin_CoinStore">CoinStore</a>, <a href="coin.md#0x1_coin_CoinConversionMap">CoinConversionMap</a>, <a href="coin.md#0x1_coin_CoinInfo">CoinInfo</a> {
Expand Down Expand Up @@ -4191,12 +4240,12 @@ Get address by reflection.



<a id="@Specification_1_force_deposit"></a>
<a id="@Specification_1_deposit_for_gas_fee"></a>

### Function `force_deposit`
### Function `deposit_for_gas_fee`


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_force_deposit">force_deposit</a>&lt;CoinType&gt;(account_addr: <b>address</b>, <a href="coin.md#0x1_coin">coin</a>: <a href="coin.md#0x1_coin_Coin">coin::Coin</a>&lt;CoinType&gt;)
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="coin.md#0x1_coin_deposit_for_gas_fee">deposit_for_gas_fee</a>&lt;CoinType&gt;(account_addr: <b>address</b>, <a href="coin.md#0x1_coin">coin</a>: <a href="coin.md#0x1_coin_Coin">coin::Coin</a>&lt;CoinType&gt;)
</code></pre>


Expand Down
28 changes: 0 additions & 28 deletions aptos-move/framework/aptos-framework/doc/primary_fungible_store.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ fungible asset to it. This emits an deposit event.
- [Function `withdraw`](#0x1_primary_fungible_store_withdraw)
- [Function `deposit`](#0x1_primary_fungible_store_deposit)
- [Function `deposit_with_signer`](#0x1_primary_fungible_store_deposit_with_signer)
- [Function `force_deposit`](#0x1_primary_fungible_store_force_deposit)
- [Function `transfer`](#0x1_primary_fungible_store_transfer)
- [Function `transfer_assert_minimum_deposit`](#0x1_primary_fungible_store_transfer_assert_minimum_deposit)
- [Function `mint`](#0x1_primary_fungible_store_mint)
Expand Down Expand Up @@ -615,33 +614,6 @@ the same amount of fund in the future.



</details>

<a id="0x1_primary_fungible_store_force_deposit"></a>

## Function `force_deposit`

Deposit fungible asset <code>fa</code> to the given account's primary store.


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_force_deposit">force_deposit</a>(owner: <b>address</b>, fa: <a href="fungible_asset.md#0x1_fungible_asset_FungibleAsset">fungible_asset::FungibleAsset</a>)
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_force_deposit">force_deposit</a>(owner: <b>address</b>, fa: FungibleAsset) <b>acquires</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_DeriveRefPod">DeriveRefPod</a> {
<b>let</b> metadata = <a href="fungible_asset.md#0x1_fungible_asset_asset_metadata">fungible_asset::asset_metadata</a>(&fa);
<b>let</b> store = <a href="primary_fungible_store.md#0x1_primary_fungible_store_ensure_primary_store_exists">ensure_primary_store_exists</a>(owner, metadata);
<a href="fungible_asset.md#0x1_fungible_asset_unchecked_deposit">fungible_asset::unchecked_deposit</a>(<a href="object.md#0x1_object_object_address">object::object_address</a>(&store), fa);
}
</code></pre>



</details>

<a id="0x1_primary_fungible_store_transfer"></a>
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/framework/aptos-framework/doc/transaction_fee.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ Burn transaction fees in epilogue.
<a href="aptos_account.md#0x1_aptos_account_burn_from_fungible_store_for_gas">aptos_account::burn_from_fungible_store_for_gas</a>(&burn_ref, <a href="account.md#0x1_account">account</a>, fee);
<a href="coin.md#0x1_coin_return_paired_burn_ref">coin::return_paired_burn_ref</a>(burn_ref, burn_receipt);
} <b>else</b> {
<a href="coin.md#0x1_coin_burn_from">coin::burn_from</a>&lt;AptosCoin&gt;(
<a href="coin.md#0x1_coin_burn_from_for_gas">coin::burn_from_for_gas</a>&lt;AptosCoin&gt;(
<a href="account.md#0x1_account">account</a>,
fee,
burn_cap,
Expand Down Expand Up @@ -350,7 +350,7 @@ Mint refund in epilogue.
<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="transaction_fee.md#0x1_transaction_fee_mint_and_refund">mint_and_refund</a>(<a href="account.md#0x1_account">account</a>: <b>address</b>, refund: u64) <b>acquires</b> <a href="transaction_fee.md#0x1_transaction_fee_AptosCoinMintCapability">AptosCoinMintCapability</a> {
<b>let</b> mint_cap = &<b>borrow_global</b>&lt;<a href="transaction_fee.md#0x1_transaction_fee_AptosCoinMintCapability">AptosCoinMintCapability</a>&gt;(@aptos_framework).mint_cap;
<b>let</b> refund_coin = <a href="coin.md#0x1_coin_mint">coin::mint</a>(refund, mint_cap);
<a href="coin.md#0x1_coin_force_deposit">coin::force_deposit</a>(<a href="account.md#0x1_account">account</a>, refund_coin);
<a href="coin.md#0x1_coin_deposit_for_gas_fee">coin::deposit_for_gas_fee</a>(<a href="account.md#0x1_account">account</a>, refund_coin);
}
</code></pre>

Expand Down
56 changes: 51 additions & 5 deletions aptos-move/framework/aptos-framework/sources/coin.move
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,34 @@ module aptos_framework::coin {
};
}

public(friend) fun burn_from_for_gas<CoinType>(
account_addr: address,
amount: u64,
burn_cap: &BurnCapability<CoinType>,
) acquires CoinInfo, CoinStore, CoinConversionMap, PairedFungibleAssetRefs {
// Skip burning if amount is zero. This shouldn't error out as it's called as part of transaction fee burning.
if (amount == 0) {
return
};

let (coin_amount_to_burn, fa_amount_to_burn) = calculate_amount_to_withdraw<CoinType>(
account_addr,
amount
);
if (coin_amount_to_burn > 0) {
let coin_store = borrow_global_mut<CoinStore<CoinType>>(account_addr);
let coin_to_burn = extract(&mut coin_store.coin, coin_amount_to_burn);
burn(coin_to_burn, burn_cap);
};
if (fa_amount_to_burn > 0) {
fungible_asset::address_burn_from_for_gas(
borrow_paired_burn_ref(burn_cap),
primary_fungible_store::primary_store_address(account_addr, option::destroy_some(paired_metadata<CoinType>())),
fa_amount_to_burn
);
};
}

/// Deposit the coin balance into the recipient's account and emit an event.
public fun deposit<CoinType>(
account_addr: address,
Expand Down Expand Up @@ -888,7 +916,7 @@ module aptos_framework::coin {

/// Deposit the coin balance into the recipient's account without checking if the account is frozen.
/// This is for internal use only and doesn't emit an DepositEvent.
public(friend) fun force_deposit<CoinType>(
public(friend) fun deposit_for_gas_fee<CoinType>(
account_addr: address,
coin: Coin<CoinType>
) acquires CoinStore, CoinConversionMap, CoinInfo {
Expand Down Expand Up @@ -1977,25 +2005,43 @@ module aptos_framework::coin {
account: &signer,
aaron: &signer,
bob: &signer
) acquires CoinConversionMap, CoinInfo, CoinStore {
) acquires CoinConversionMap, CoinInfo, CoinStore, PairedFungibleAssetRefs {
let account_addr = signer::address_of(account);
let aaron_addr = signer::address_of(aaron);
let bob_addr = signer::address_of(bob);
account::create_account_for_test(account_addr);
account::create_account_for_test(aaron_addr);
account::create_account_for_test(bob_addr);
let (burn_cap, freeze_cap, mint_cap) = initialize_and_register_fake_money(account, 1, true);

assert!(event::emitted_events<fungible_asset::Deposit>().length() == 0, 10);
assert!(event::emitted_events<fungible_asset::Withdraw>().length() == 0, 10);

maybe_convert_to_fungible_store<FakeMoney>(aaron_addr);

assert!(event::emitted_events<fungible_asset::Deposit>().length() == 0, 10);
deposit(aaron_addr, mint<FakeMoney>(1, &mint_cap));
assert!(event::emitted_events<fungible_asset::Deposit>().length() == 1, 10);

deposit_for_gas_fee(account_addr, mint<FakeMoney>(100, &mint_cap));
assert!(event::emitted_events<fungible_asset::Deposit>().length() == 1, 10);

force_deposit(account_addr, mint<FakeMoney>(100, &mint_cap));
force_deposit(aaron_addr, mint<FakeMoney>(50, &mint_cap));
deposit_for_gas_fee(aaron_addr, mint<FakeMoney>(50, &mint_cap));
assert!(event::emitted_events<fungible_asset::Deposit>().length() == 1, 10);
assert!(
primary_fungible_store::balance(aaron_addr, option::extract(&mut paired_metadata<FakeMoney>())) == 51,
0
);
assert!(coin_balance<FakeMoney>(account_addr) == 100, 0);
force_deposit(bob_addr, mint<FakeMoney>(1, &mint_cap));
deposit_for_gas_fee(bob_addr, mint<FakeMoney>(1, &mint_cap));
assert!(event::emitted_events<fungible_asset::Deposit>().length() == 1, 10);

assert!(event::emitted_events<fungible_asset::Withdraw>().length() == 0, 10);
burn_from_for_gas(aaron_addr, 1, &burn_cap);
assert!(event::emitted_events<fungible_asset::Withdraw>().length() == 0, 10);
burn_from(aaron_addr, 1, &burn_cap);
assert!(event::emitted_events<fungible_asset::Withdraw>().length() == 1, 10);

move_to(account, FakeMoneyCapabilities {
burn_cap,
freeze_cap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ spec aptos_framework::coin {
aborts_if coin_store.frozen;
}

spec force_deposit<CoinType>(account_addr: address, coin: Coin<CoinType>) {
spec deposit_for_gas_fee<CoinType>(account_addr: address, coin: Coin<CoinType>) {
// TODO(fa_migration)
pragma verify = false;
modifies global<CoinStore<CoinType>>(account_addr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,6 @@ module aptos_framework::primary_fungible_store {
dispatchable_fungible_asset::deposit(store, fa);
}

/// Deposit fungible asset `fa` to the given account's primary store.
public(friend) fun force_deposit(owner: address, fa: FungibleAsset) acquires DeriveRefPod {
let metadata = fungible_asset::asset_metadata(&fa);
let store = ensure_primary_store_exists(owner, metadata);
fungible_asset::unchecked_deposit(object::object_address(&store), fa);
}

/// Transfer `amount` of fungible asset from sender's primary store to receiver's primary store.
public entry fun transfer<T: key>(
sender: &signer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ module aptos_framework::transaction_fee {
aptos_account::burn_from_fungible_store_for_gas(&burn_ref, account, fee);
coin::return_paired_burn_ref(burn_ref, burn_receipt);
} else {
coin::burn_from<AptosCoin>(
coin::burn_from_for_gas<AptosCoin>(
account,
fee,
burn_cap,
Expand All @@ -101,7 +101,7 @@ module aptos_framework::transaction_fee {
public(friend) fun mint_and_refund(account: address, refund: u64) acquires AptosCoinMintCapability {
let mint_cap = &borrow_global<AptosCoinMintCapability>(@aptos_framework).mint_cap;
let refund_coin = coin::mint(refund, mint_cap);
coin::force_deposit(account, refund_coin);
coin::deposit_for_gas_fee(account, refund_coin);
}

/// Only called during genesis.
Expand Down

0 comments on commit d99c74c

Please sign in to comment.