From 383a813b225efa9bb81c9273322390dd725d1e95 Mon Sep 17 00:00:00 2001 From: Gianmarco Fraccaroli <> Date: Tue, 16 Jan 2024 22:17:37 +0100 Subject: [PATCH 1/4] fix: token::burn storage api --- crates/trans_token/src/storage.rs | 35 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 429ec94154..141e315f6e 100644 --- a/crates/trans_token/src/storage.rs +++ b/crates/trans_token/src/storage.rs @@ -173,23 +173,28 @@ pub fn burn( where S: StorageRead + StorageWrite, { - let key = balance_key(token, source); - let balance = read_balance(storage, token, source)?; + let source_balance_key = token::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(amount) = source_balance.checked_sub(amount) { + storage.write(&source_balance_key, amount)?; amount - } - None => { - storage.write(&key, token::Amount::zero())?; - balance - } - }; - - let total_supply = read_total_supply(&*storage, source)?; - let new_total_supply = - total_supply.checked_sub(amount_to_burn).unwrap_or_default(); + } else { + storage.write(&source_balance_key, token::Amount::zero())?; + source_balance + }; + + let old_total_supply = read_total_supply(storage, token)?; + let new_total_supply = old_total_supply + .checked_sub(amount_to_burn) + .ok_or_else(|| { + tracing::error!( + "Burning more token than the total supply of {}", + token + ); + storage_api::Error::new_const("Token total supply underflowed") + })?; let total_supply_key = minted_balance_key(token); storage.write(&total_supply_key, new_total_supply) From 29361fad2f95dfc3998217d19bf432a055db9839 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 16 Jan 2024 20:28:13 -0500 Subject: [PATCH 2/4] another fix and a test --- Cargo.lock | 1 + .../src/lib/node/ledger/shell/governance.rs | 2 +- crates/ibc/src/actions.rs | 2 +- crates/trans_token/Cargo.toml | 6 ++ crates/trans_token/src/storage.rs | 83 ++++++++++++++++--- 5 files changed, 82 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 71515d9ae8..f971e87c38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4748,6 +4748,7 @@ name = "namada_trans_token" version = "0.30.1" dependencies = [ "namada_core", + "namada_state", "namada_storage", ] diff --git a/crates/apps/src/lib/node/ledger/shell/governance.rs b/crates/apps/src/lib/node/ledger/shell/governance.rs index 540019a565..f66c514c33 100644 --- a/crates/apps/src/lib/node/ledger/shell/governance.rs +++ b/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, diff --git a/crates/ibc/src/actions.rs b/crates/ibc/src/actions.rs index f56192802b..eed8f284ae 100644 --- a/crates/ibc/src/actions.rs +++ b/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) { diff --git a/crates/trans_token/Cargo.toml b/crates/trans_token/Cargo.toml index 6a1fa96a9f..ff77aa643f 100644 --- a/crates/trans_token/Cargo.toml +++ b/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_state = { path = "../state", features = ["testing"] } \ No newline at end of file diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 141e315f6e..550ed826d7 100644 --- a/crates/trans_token/src/storage.rs +++ b/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( +/// 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( storage: &mut S, token: &Address, source: &Address, @@ -173,12 +175,12 @@ pub fn burn( where S: StorageRead + StorageWrite, { - let source_balance_key = token::balance_key(token, source); + let source_balance_key = balance_key(token, source); let source_balance = read_balance(storage, token, source)?; let amount_to_burn = - if let Some(amount) = source_balance.checked_sub(amount) { - storage.write(&source_balance_key, amount)?; + if let Some(new_amount) = source_balance.checked_sub(amount) { + storage.write(&source_balance_key, new_amount)?; amount } else { storage.write(&source_balance_key, token::Amount::zero())?; @@ -189,11 +191,7 @@ where let new_total_supply = old_total_supply .checked_sub(amount_to_burn) .ok_or_else(|| { - tracing::error!( - "Burning more token than the total supply of {}", - token - ); - storage_api::Error::new_const("Token total supply underflowed") + storage::Error::new_const("Total token supply underflowed") })?; let total_supply_key = minted_balance_key(token); @@ -229,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_state::testing::TestWlStorage; + + use super::{burn_tokens, credit_tokens, read_balance, read_total_supply}; + + #[test] + fn test_burn_native_tokens() { + let mut storage = TestWlStorage::default(); + let native_token = storage.storage.native_token.clone(); + + // 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); + } +} From 5b12fa4c6862c82f9c8219377b2a65b3058709fc Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 16 Jan 2024 20:30:44 -0500 Subject: [PATCH 3/4] changelog: add #2408 --- .changelog/unreleased/bug-fixes/2408-fix-burn-function.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/2408-fix-burn-function.md diff --git a/.changelog/unreleased/bug-fixes/2408-fix-burn-function.md b/.changelog/unreleased/bug-fixes/2408-fix-burn-function.md new file mode 100644 index 0000000000..0c95626035 --- /dev/null +++ b/.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)) \ No newline at end of file From 8951b73445d38a02881e535b71a7e4c165c2a783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 26 Jan 2024 09:35:05 +0000 Subject: [PATCH 4/4] trans_token: avoid cyclic dependency --- Cargo.lock | 1 - crates/trans_token/Cargo.toml | 2 +- crates/trans_token/src/storage.rs | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f971e87c38..71515d9ae8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4748,7 +4748,6 @@ name = "namada_trans_token" version = "0.30.1" dependencies = [ "namada_core", - "namada_state", "namada_storage", ] diff --git a/crates/trans_token/Cargo.toml b/crates/trans_token/Cargo.toml index ff77aa643f..deed56fac0 100644 --- a/crates/trans_token/Cargo.toml +++ b/crates/trans_token/Cargo.toml @@ -20,4 +20,4 @@ namada_core = { path = "../core" } namada_storage = { path = "../storage" } [dev-dependencies] -namada_state = { path = "../state", features = ["testing"] } \ No newline at end of file +namada_storage = { path = "../storage", features = ["testing"] } diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 550ed826d7..7104c708a1 100644 --- a/crates/trans_token/src/storage.rs +++ b/crates/trans_token/src/storage.rs @@ -231,14 +231,14 @@ pub fn denom_to_amount( #[cfg(test)] mod testing { use namada_core::types::{address, token}; - use namada_state::testing::TestWlStorage; + 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 = TestWlStorage::default(); - let native_token = storage.storage.native_token.clone(); + let mut storage = TestStorage::default(); + let native_token = address::nam(); // Get some addresses let addr1 = address::testing::gen_implicit_address();