Fix token::burn

.changelog/unreleased/bug-fixes/2408-fix-burn-function.md

@@ -0,0 +1,2 @@
+- Fix the token burn function.
+  ([\#2408](https://github.com/anoma/namada/pull/2408))

crates/apps/src/lib/node/ledger/shell/governance.rs

@@ -201,7 +201,7 @@ where
                 funds,
             )?;
         } else {
-            token::burn(
+            token::burn_tokens(
                 &mut shell.wl_storage,
                 &native_token,
                 &gov_address,

crates/ibc/src/actions.rs

@@ -133,7 +133,7 @@ where
         token: &Address,
         amount: DenominatedAmount,
     ) -> Result<(), StorageError> {
-        token::burn(self.wl_storage, token, target, amount.amount())
+        token::burn_tokens(self.wl_storage, token, target, amount.amount())
     }
 
     fn log_string(&self, message: String) {

crates/trans_token/Cargo.toml

@@ -12,6 +12,12 @@ readme.workspace = true
 repository.workspace = true
 version.workspace = true
 
+[features]
+default = []
+
 [dependencies]
 namada_core = { path = "../core" }
 namada_storage = { path = "../storage" }
+
+[dev-dependencies]
+namada_storage = { path = "../storage", features = ["testing"] }

crates/trans_token/src/storage.rs

@@ -163,8 +163,10 @@ where
     storage.write(&total_supply_key, new_supply)
 }
 
-/// Burn an amount of token for a specific address.
-pub fn burn<S>(
+/// Burn a specified amount of tokens from some address. If the burn amount is
+/// larger than the total balance of the given address, then the remaining
+/// balance is burned. The total supply of the token is properly adjusted.
+pub fn burn_tokens<S>(
     storage: &mut S,
     token: &Address,
     source: &Address,
@@ -173,23 +175,24 @@ pub fn burn<S>(
 where
     S: StorageRead + StorageWrite,
 {
-    let key = balance_key(token, source);
-    let balance = read_balance(storage, token, source)?;
+    let source_balance_key = balance_key(token, source);
+    let source_balance = read_balance(storage, token, source)?;
 
-    let amount_to_burn = match balance.checked_sub(amount) {
-        Some(new_balance) => {
-            storage.write(&key, new_balance)?;
+    let amount_to_burn =
+        if let Some(new_amount) = source_balance.checked_sub(amount) {
+            storage.write(&source_balance_key, new_amount)?;
             amount
-        }
-        None => {
-            storage.write(&key, token::Amount::zero())?;
-            balance
-        }
-    };
+        } else {
+            storage.write(&source_balance_key, token::Amount::zero())?;
+            source_balance
+        };
 
-    let total_supply = read_total_supply(&*storage, source)?;
-    let new_total_supply =
-        total_supply.checked_sub(amount_to_burn).unwrap_or_default();
+    let old_total_supply = read_total_supply(storage, token)?;
+    let new_total_supply = old_total_supply
+        .checked_sub(amount_to_burn)
+        .ok_or_else(|| {
+            storage::Error::new_const("Total token supply underflowed")
+        })?;
 
     let total_supply_key = minted_balance_key(token);
     storage.write(&total_supply_key, new_total_supply)
@@ -224,3 +227,68 @@ pub fn denom_to_amount(
     })?;
     denom_amount.scale(denom).map_err(storage::Error::new)
 }
+
+#[cfg(test)]
+mod testing {
+    use namada_core::types::{address, token};
+    use namada_storage::testing::TestStorage;
+
+    use super::{burn_tokens, credit_tokens, read_balance, read_total_supply};
+
+    #[test]
+    fn test_burn_native_tokens() {
+        let mut storage = TestStorage::default();
+        let native_token = address::nam();
+
+        // Get some addresses
+        let addr1 = address::testing::gen_implicit_address();
+        let addr2 = address::testing::gen_implicit_address();
+        let addr3 = address::testing::gen_implicit_address();
+
+        let balance1 = token::Amount::native_whole(1);
+        let balance2 = token::Amount::native_whole(2);
+        let balance3 = token::Amount::native_whole(3);
+        let tot_init_balance = balance1 + balance2 + balance3;
+
+        credit_tokens(&mut storage, &native_token, &addr1, balance1).unwrap();
+        credit_tokens(&mut storage, &native_token, &addr2, balance2).unwrap();
+        credit_tokens(&mut storage, &native_token, &addr3, balance3).unwrap();
+
+        // Check total initial supply
+        let total_supply = read_total_supply(&storage, &native_token).unwrap();
+        assert_eq!(total_supply, tot_init_balance);
+
+        // Burn some tokens
+        let burn1 = token::Amount::from(547_432);
+        burn_tokens(&mut storage, &native_token, &addr1, burn1).unwrap();
+
+        // Check new balances
+        let addr1_balance =
+            read_balance(&storage, &native_token, &addr1).unwrap();
+        assert_eq!(addr1_balance, balance1 - burn1);
+        let total_supply = read_total_supply(&storage, &native_token).unwrap();
+        assert_eq!(total_supply, tot_init_balance - burn1);
+
+        // Burn more tokens from addr1 than it has remaining
+        let burn2 = token::Amount::from(1_000_000);
+        burn_tokens(&mut storage, &native_token, &addr1, burn2).unwrap();
+
+        // Check new balances
+        let addr1_balance =
+            read_balance(&storage, &native_token, &addr1).unwrap();
+        assert_eq!(addr1_balance, token::Amount::zero());
+        let total_supply = read_total_supply(&storage, &native_token).unwrap();
+        assert_eq!(total_supply, tot_init_balance - balance1);
+
+        // Burn more tokens from addr2 than are in the total supply
+        let burn3 = tot_init_balance + token::Amount::native_whole(1);
+        burn_tokens(&mut storage, &native_token, &addr2, burn3).unwrap();
+
+        // Check balances again
+        let addr2_balance =
+            read_balance(&storage, &native_token, &addr2).unwrap();
+        assert_eq!(addr2_balance, token::Amount::zero());
+        let total_supply = read_total_supply(&storage, &native_token).unwrap();
+        assert_eq!(total_supply, balance3);
+    }
+}