[move] fix lifetime burn info (#85)

Co-authored-by: 0o-de-lally <[email protected]>

framework/libra-framework/sources/modified_source/coin.move

@@ -18,6 +18,7 @@ module diem_framework::coin {
     // use diem_std::debug::print;
 
     friend ol_framework::gas_coin;
+    friend ol_framework::burn;
     friend ol_framework::ol_account;
     friend diem_framework::genesis;
     friend diem_framework::genesis_migration;
@@ -341,7 +342,10 @@ module diem_framework::coin {
 
     // Public functions
     /// Burn `coin` with capability.
-    /// The capability `_cap` should be passed as a reference to `BurnCapability<CoinType>`.
+    /// The capability `_cap` should be passed as a reference to
+    /// `BurnCapability<CoinType>`.
+
+    // 0L: TODO: isn't this deprecated?
     public(friend) fun burn<CoinType>(
         coin: Coin<CoinType>,
         _cap: &BurnCapability<CoinType>,
@@ -358,8 +362,9 @@ module diem_framework::coin {
 
     //////// 0L ////////
     /// user can burn own coin they are holding.
-    /// TODO: evaluate if this should be a `friend` method
-    public fun user_burn<CoinType>(
+    /// must be a Friend function, because the relevant tracking
+    /// happens in Burn.move
+    public(friend) fun user_burn<CoinType>(
         coin: Coin<CoinType>,
     ) acquires CoinInfo { // cannot abort
         if (value(&coin) == 0) {
@@ -380,7 +385,10 @@ module diem_framework::coin {
     /// The capability `burn_cap` should be passed as a reference to `BurnCapability<CoinType>`.
     /// This function shouldn't fail as it's called as part of transaction fee burning.
     ///
-    /// Note: This bypasses CoinStore::frozen -- coins within a frozen CoinStore can be burned.
+    /// Note: This bypasses CoinStore::frozen -- coins within a frozen CoinStore
+    /// can be burned.
+
+    // 0L: is this deprecated?
     public(friend) fun burn_from<CoinType>(
         account_addr: address,
         amount: u64,

framework/libra-framework/sources/ol_sources/burn.move

@@ -95,12 +95,12 @@ module ol_framework::burn {
           i = i + 1;
       };
 
-    // // Transaction fee account should be empty at the end of the epoch
-    // // Superman 3 decimal errors. https://www.youtube.com/watch?v=N7JBXGkBoFc
-    // // anything that is remaining should be burned
+    // Transaction fee account should be empty at the end of the epoch
+    // Superman 3 decimal errors. https://www.youtube.com/watch?v=N7JBXGkBoFc
+    // anything that is remaining should be burned
     let remainder = coin::value(coins);
     let leftover = coin::extract(coins, remainder);
-    coin::user_burn(leftover);
+    burn_and_track(leftover);
     // Note: we are still retruning an empty coin to be destroyed by the caller
   }
 
@@ -122,6 +122,18 @@ module ol_framework::burn {
   }
 
 
+  /// performs a burn and increments the tracker
+  /// NOTE: this is unchecked, any user can perform this.
+  /// the user should call this function and not burn methods on coin.move
+  /// since those methods do not track the lifetime_burned
+  public fun burn_and_track(coin: Coin<GasCoin>) acquires BurnCounter {
+    let value_sent = coin::value(&coin);
+    let state = borrow_global_mut<BurnCounter>(@ol_framework);
+    coin::user_burn(coin);
+    state.lifetime_burned = state.lifetime_burned + value_sent;
+  }
+
+  /// performs a burn or recycle according to the attributed user's preference
   public fun burn_with_user_preference(
     vm: &signer, payer: address, user_share: Coin<GasCoin>
   ) acquires BurnCounter, UserBurnPreference {
@@ -135,16 +147,13 @@ module ol_framework::burn {
          // update the root state tracker
         let state = borrow_global_mut<BurnCounter>(@ol_framework);
         state.lifetime_recycled = state.lifetime_recycled + value_sent;
-
-        coin::user_burn(user_share);
-        return
       }
     };
 
-    // Superman 3
-    let state = borrow_global_mut<BurnCounter>(@ol_framework);
-    state.lifetime_burned = state.lifetime_burned + value_sent;
-    coin::user_burn(user_share);
+    // do a default burn which is not attributed
+    // also checks for Superman 3
+    burn_and_track(user_share);
+
   }
 
 

framework/libra-framework/sources/ol_sources/epoch_boundary.move

@@ -230,8 +230,10 @@ module diem_framework::epoch_boundary {
 
           // remainder gets burnt according to fee maker preferences
           burn::epoch_burn_fees(root, &mut all_fees);
-          // there might be some dust, that should get burned
-          coin::user_burn(all_fees);
+
+          // coin can finally be destroyed. Up to here we have been extracting from a mutable.
+          // It's possible there might be some dust, that should get burned
+          burn::burn_and_track(all_fees);
         };
 
         compliant_vals

framework/libra-framework/sources/ol_sources/pledge_accounts.move

@@ -47,6 +47,7 @@
         use ol_framework::gas_coin::LibraCoin as GasCoin;
         use ol_framework::ol_account;
         use ol_framework::epoch_helper;
+        use ol_framework::burn;
         use diem_framework::system_addresses;
         use diem_framework::coin::{Self, Coin};
 
@@ -466,7 +467,7 @@
 
                 if (is_burn && option::is_some(&c)) {
                   let burn_this = option::extract(&mut c);
-                  coin::user_burn(burn_this);
+                  burn::burn_and_track(burn_this);
                   option::destroy_none(c);
                 } else if (option::is_some(&c)) {
                   let refund_coin = option::extract(&mut c);

framework/libra-framework/sources/ol_sources/proof_of_fee.move

@@ -90,7 +90,7 @@ module ol_framework::proof_of_fee {
   // from supply data.
   public fun genesis_migrate_reward(vm: &signer, nominal_reward: u64) acquires
   ConsensusReward {
-    if (signer::address_of(vm) != @ol_framework) return;
+    system_addresses::assert_ol(vm); // either 0x1 or 0x0
 
     let state = borrow_global_mut<ConsensusReward>(@ol_framework);
     state.nominal_reward = nominal_reward;

framework/libra-framework/sources/ol_sources/tests/burn.test.move

@@ -2,7 +2,7 @@
 #[test_only]
 module ol_framework::test_burn {
   use ol_framework::mock;
-  use ol_framework::gas_coin::{Self, LibraCoin as GasCoin};
+  use ol_framework::gas_coin;
   use ol_framework::ol_account;
   use ol_framework::match_index;
   use ol_framework::burn;
@@ -27,7 +27,7 @@ module ol_framework::test_burn {
 
     let alice_burn = 5;
     let c = ol_account::withdraw(alice, alice_burn);
-    coin::user_burn<GasCoin>(c);
+    burn::burn_and_track(c);
     let supply = gas_coin::supply();
     assert!(supply == (supply_pre - alice_burn), 7357001);
 

framework/libra-framework/sources/ol_sources/tests/donor_directed.test.move

@@ -304,8 +304,11 @@ module ol_framework::test_donor_directed {
 
       let vals = mock::genesis_n_vals(root, 5); // need to include eve to init funds
       mock::ol_initialize_coin_and_fund_vals(root, 100000, true);
-      // start at epoch 1, since turnout tally needs epoch info, and 0 may cause issues
+      // start at epoch 1, since turnout tally needs epoch info, and 0 may cause
+      // issues
       mock::trigger_epoch(root);
+      // a burn happend in the epoch above, so let's compare it to end of epoch
+      let (lifetime_burn_pre, _) = burn::get_lifetime_tracker();
 
       let (resource_sig, _cap) = ol_account::ol_create_resource_account(alice, b"0x1");
       let donor_directed_address = signer::address_of(&resource_sig);
@@ -322,8 +325,9 @@ module ol_framework::test_donor_directed {
       let is_donor = donor_directed_governance::check_is_donor(donor_directed_address, signer::address_of(eve));
       assert!(is_donor, 7357002);
 
-      // Dave will also be a donor, with half the amount of what Eve sends
-      ol_account::transfer(dave, donor_directed_address, 21);
+      // Dave will also be a donor, with half the amount of what Eve
+      let dave_donation = 21;
+      ol_account::transfer(dave, donor_directed_address, dave_donation);
       let is_donor = donor_directed_governance::check_is_donor(donor_directed_address, signer::address_of(dave));
       assert!(is_donor, 7357003);
 
@@ -352,9 +356,9 @@ module ol_framework::test_donor_directed {
       let superman_3 = 1; // rounding from fixed_point32
       assert!((*eve_donation_pro_rata + superman_3) == eve_donation, 7357008);
 
-      let eve_donation_pro_rata = vector::borrow(&refunds, 1);
+      let dave_donation_pro_rata = vector::borrow(&refunds, 1);
       let superman_3 = 1; // rounding from fixed_point32
-      assert!((*eve_donation_pro_rata + superman_3) == 21, 7357009);
+      assert!((*dave_donation_pro_rata + superman_3) == dave_donation, 7357009);
 
 
       let (_, program_balance_pre) = ol_account::balance(donor_directed_address);
@@ -368,16 +372,17 @@ module ol_framework::test_donor_directed {
       assert!(program_balance < program_balance_pre, 7357010);
       assert!(program_balance == 0, 7357011);
 
+      // eve shoul have received funds back
       assert!(eve_balance > eve_balance_pre, 7357010);
 
-      let (lifetime_burn, _lifetime_match) = burn::get_lifetime_tracker();
+      let (lifetime_burn_now, _) = burn::get_lifetime_tracker();
       // nothing should have been burned, it was a refund
-      assert!(lifetime_burn == 0, 7357011);
+      assert!(lifetime_burn_now == lifetime_burn_pre, 7357011);
 
     }
 
     #[test(root = @ol_framework, alice = @0x1000a, dave = @0x1000d, eve = @0x1000e)]
-    fun dd_liquidate_to_community(root: &signer, alice: &signer, dave: &signer, eve: &signer) {
+    fun dd_liquidate_to_match_index(root: &signer, alice: &signer, dave: &signer, eve: &signer) {
       // Scenario:
       // Alice creates a donor directed account where Alice, Bob and Carol, are admins.
       // Dave and Eve make a donation and so are able to have some voting on that account.
@@ -387,6 +392,8 @@ module ol_framework::test_donor_directed {
       mock::ol_initialize_coin_and_fund_vals(root, 100000, true);
       // start at epoch 1, since turnout tally needs epoch info, and 0 may cause issues
       mock::trigger_epoch(root);
+      // a burn happend in the epoch above, so let's compare it to end of epoch
+      let (lifetime_burn_pre, _) = burn::get_lifetime_tracker();
 
       let (resource_sig, _cap) = ol_account::ol_create_resource_account(alice, b"0x1");
       let donor_directed_address = signer::address_of(&resource_sig);
@@ -404,7 +411,8 @@ module ol_framework::test_donor_directed {
       assert!(is_donor, 7357002);
 
       // Dave will also be a donor, with half the amount of what Eve sends
-      ol_account::transfer(dave, donor_directed_address, 21);
+      let dave_donation = 21;
+      ol_account::transfer(dave, donor_directed_address, dave_donation);
       let is_donor = donor_directed_governance::check_is_donor(donor_directed_address, signer::address_of(dave));
       assert!(is_donor, 7357003);
 
@@ -423,18 +431,21 @@ module ol_framework::test_donor_directed {
       let list = donor_directed::get_liquidation_queue();
       assert!(vector::length(&list) > 0, 7357005);
 
+      // check the table of refunds
+      // dave and eve should be receiving
       let (addrs, refunds) = donor_directed::get_pro_rata(donor_directed_address);
 
       assert!(*vector::borrow(&addrs, 0) == @0x1000e, 7357006);
       assert!(*vector::borrow(&addrs, 1) == @0x1000d, 7357007);
 
+      // for the first refund,
       let eve_donation_pro_rata = vector::borrow(&refunds, 0);
       let superman_3 = 1; // rounding from fixed_point32
       assert!((*eve_donation_pro_rata + superman_3) == eve_donation, 7357008);
 
-      let eve_donation_pro_rata = vector::borrow(&refunds, 1);
+      let dave_donation_pro_rata = vector::borrow(&refunds, 1);
       let superman_3 = 1; // rounding from fixed_point32
-      assert!((*eve_donation_pro_rata + superman_3) == 21, 7357009);
+      assert!((*dave_donation_pro_rata + superman_3) == dave_donation, 7357009);
 
       let (_, program_balance_pre) = ol_account::balance(donor_directed_address);
       let (_, eve_balance_pre) = ol_account::balance(@0x1000e);
@@ -447,11 +458,15 @@ module ol_framework::test_donor_directed {
       assert!(program_balance < program_balance_pre, 7357010);
       assert!(program_balance == 0, 7357011);
 
+      // eve's account should not have changed.
+      // she does not get a refund, and the policy of the
+      // program says it goes to the matching index.
       assert!(eve_balance == eve_balance_pre, 7357010);
 
-      let (lifetime_burn, _lifetime_match) = burn::get_lifetime_tracker();
+      let (lifetime_burn_now, _lifetime_match) = burn::get_lifetime_tracker();
+
       // nothing should have been burned, it was a refund
-      assert!(lifetime_burn == 0, 7357011);
+      assert!(lifetime_burn_now == lifetime_burn_pre, 7357011);
     }
 }
 

framework/libra-framework/sources/ol_sources/tests/rewards.test.move

@@ -1,15 +1,11 @@
 #[test_only]
 module ol_framework::test_rewards {
-  #[test_only]
   use ol_framework::rewards;
-  #[test_only]
   use ol_framework::gas_coin::{Self, LibraCoin as GasCoin};
-  #[test_only]
   use diem_framework::coin;
-  #[test_only]
   use ol_framework::mock;
-  #[test_only]
   use diem_framework::stake;
+  use ol_framework::burn;
 
   #[test(root = @ol_framework)]
   fun test_pay_reward_single(root: signer) {
@@ -58,6 +54,6 @@ module ol_framework::test_rewards {
     let b = coin::balance<GasCoin>(dave);
     assert!(b == 1000, 7357002);
 
-    coin::user_burn(new_coin);
+    burn::burn_and_track(new_coin);
   }
 }