From 3ae7d206180dd72009726168953deb2fdae2cf07 Mon Sep 17 00:00:00 2001 From: Boyan Barakov <9572072+brozorec@users.noreply.github.com> Date: Thu, 30 Jan 2025 18:02:30 +0100 Subject: [PATCH] Error Numbers (#25) * fix param types in burnable and mintable traits * fungible example with our our traits * fungible example with TokenInterface * typo * improve docs * typo * assign meaningful numbers in error enums * typo * remove redundant over- and underflow checks --- .../fungible/src/extensions/burnable/test.rs | 6 +- contracts/token/fungible/src/fungible.rs | 12 +- contracts/token/fungible/src/storage.rs | 21 ++- contracts/token/fungible/src/test.rs | 38 ++++- ...set_allowance_with_neg_amount_fails.1.json | 75 +++++++++ .../test/update_overflow_panics.1.json | 150 ++++++++++++++++++ contracts/utils/pausable/src/pausable.rs | 4 +- contracts/utils/pausable/src/test.rs | 4 +- examples/fungible-pausable/src/test.rs | 8 +- examples/pausable/src/contract.rs | 1 - examples/pausable/src/test.rs | 4 +- 11 files changed, 293 insertions(+), 30 deletions(-) create mode 100644 contracts/token/fungible/test_snapshots/test/set_allowance_with_neg_amount_fails.1.json create mode 100644 contracts/token/fungible/test_snapshots/test/update_overflow_panics.1.json diff --git a/contracts/token/fungible/src/extensions/burnable/test.rs b/contracts/token/fungible/src/extensions/burnable/test.rs index 47d3dd3..18d417b 100644 --- a/contracts/token/fungible/src/extensions/burnable/test.rs +++ b/contracts/token/fungible/src/extensions/burnable/test.rs @@ -47,7 +47,7 @@ fn burn_with_allowance_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #200)")] fn burn_with_insufficient_balance_panics() { let e = Env::default(); e.mock_all_auths(); @@ -62,7 +62,7 @@ fn burn_with_insufficient_balance_panics() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #201)")] fn burn_with_no_allowance_panics() { let e = Env::default(); e.mock_all_auths(); @@ -78,7 +78,7 @@ fn burn_with_no_allowance_panics() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #201)")] fn burn_with_insufficient_allowance_panics() { let e = Env::default(); e.mock_all_auths(); diff --git a/contracts/token/fungible/src/fungible.rs b/contracts/token/fungible/src/fungible.rs index d4e48ea..82c23fe 100644 --- a/contracts/token/fungible/src/fungible.rs +++ b/contracts/token/fungible/src/fungible.rs @@ -182,13 +182,19 @@ pub trait FungibleToken { pub enum FungibleTokenError { /// Indicates an error related to the current balance of account from which /// tokens are expected to be transferred. - InsufficientBalance = 1, + InsufficientBalance = 200, /// Indicates a failure with the allowance mechanism when a given spender /// doesn't have enough allowance. - InsufficientAllowance = 2, + InsufficientAllowance = 201, /// Indicates an invalid value for `live_until_ledger` when setting an /// allowance. - InvalidLiveUntilLedger = 3, + InvalidLiveUntilLedger = 202, + /// Indicates an error when an input that must be >= 0 + LessThanZero = 203, + /// Indicates an error when an input that must be > 0 + LessThanOrEqualToZero = 204, + /// Indicates overflow when adding two values + MathOverflow = 205, } // ################## EVENTS ################## diff --git a/contracts/token/fungible/src/storage.rs b/contracts/token/fungible/src/storage.rs index a717486..d7c646b 100644 --- a/contracts/token/fungible/src/storage.rs +++ b/contracts/token/fungible/src/storage.rs @@ -161,6 +161,7 @@ pub fn approve(e: &Env, owner: &Address, spender: &Address, amount: i128, live_u /// * [`FungibleTokenError::InvalidLiveUntilLedger`] - Occurs when attempting to /// set `live_until_ledger` that is less than the current ledger number and /// greater than `0`. +/// * [`FungibleTokenError::LessThanZero`] - Occurs when `amount < 0`. /// /// # Events /// @@ -180,7 +181,7 @@ pub fn set_allowance( emit: bool, ) { if amount < 0 { - panic!("amount cannot be negative") + panic_with_error!(e, FungibleTokenError::LessThanZero) } let allowance = AllowanceData { amount, live_until_ledger }; @@ -346,13 +347,15 @@ pub fn do_transfer(e: &Env, from: &Address, to: &Address, amount: i128) { /// /// * [`FungibleTokenError::InsufficientBalance`] - When attempting to transfer /// more tokens than `from` current balance. +/// * [`FungibleTokenError::LessThanOrEqualToZero`] - When `amount <= 0`. +/// * [`FungibleTokenError::MathOverflow`] - When `total_supply` overflows. /// /// # Notes /// /// No authorization is required. pub fn update(e: &Env, from: Option<&Address>, to: Option<&Address>, amount: i128) { if amount <= 0 { - panic!("amount must be > 0") + panic_with_error!(e, FungibleTokenError::LessThanOrEqualToZero) } if let Some(account) = from { @@ -365,17 +368,21 @@ pub fn update(e: &Env, from: Option<&Address>, to: Option<&Address>, amount: i12 e.storage().persistent().set(&StorageKey::Balance(account.clone()), &from_balance); } else { let mut total_supply = total_supply(e); - total_supply = total_supply.checked_add(amount).expect("total_supply overflow"); + total_supply = match total_supply.checked_add(amount) { + Some(num) => num, + _ => panic_with_error!(e, FungibleTokenError::MathOverflow), + }; e.storage().instance().set(&StorageKey::TotalSupply, &total_supply); } if let Some(account) = to { - let mut to_balance = balance(e, account); - to_balance = to_balance.checked_add(amount).expect("to_balance overflow"); + // NOTE: can't overflow because balance + amoount is at most total_supply. + let to_balance = balance(e, account) + amount; e.storage().persistent().set(&StorageKey::Balance(account.clone()), &to_balance); } else { - let mut total_supply = total_supply(e); - total_supply = total_supply.checked_sub(amount).expect("total_supply underflow"); + // NOTE: can't overflow because amount <= total_supply or amount <= from_balance + // <= total_supply. + let total_supply = total_supply(e) - amount; e.storage().instance().set(&StorageKey::TotalSupply, &total_supply); } } diff --git a/contracts/token/fungible/src/test.rs b/contracts/token/fungible/src/test.rs index 24c8701..54a7638 100644 --- a/contracts/token/fungible/src/test.rs +++ b/contracts/token/fungible/src/test.rs @@ -132,7 +132,7 @@ fn spend_allowance_reduces_amount() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #201)")] fn spend_allowance_insufficient_allowance_fails() { let e = Env::default(); e.mock_all_auths(); @@ -147,7 +147,7 @@ fn spend_allowance_insufficient_allowance_fails() { } #[test] -#[should_panic(expected = "Error(Contract, #3)")] +#[should_panic(expected = "Error(Contract, #202)")] fn set_allowance_with_expired_ledger_fails() { let e = Env::default(); let address = e.register(MockContract, ()); @@ -160,6 +160,19 @@ fn set_allowance_with_expired_ledger_fails() { }); } +#[test] +#[should_panic(expected = "Error(Contract, #203)")] +fn set_allowance_with_neg_amount_fails() { + let e = Env::default(); + let address = e.register(MockContract, ()); + let owner = Address::generate(&e); + let spender = Address::generate(&e); + + e.as_contract(&address, || { + set_allowance(&e, &owner, &spender, -1, 5, true); + }); +} + #[test] fn set_allowance_with_zero_amount() { let e = Env::default(); @@ -249,7 +262,7 @@ fn approve_and_transfer_from() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #200)")] fn transfer_insufficient_balance_fails() { let e = Env::default(); e.mock_all_auths(); @@ -264,7 +277,7 @@ fn transfer_insufficient_balance_fails() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #201)")] fn transfer_from_insufficient_allowance_fails() { let e = Env::default(); e.mock_all_auths(); @@ -323,7 +336,7 @@ fn update_burns_tokens() { } #[test] -#[should_panic(expected = "amount must be > 0")] +#[should_panic(expected = "Error(Contract, #204)")] fn update_with_invalid_amount_panics() { let e = Env::default(); let address = e.register(MockContract, ()); @@ -336,7 +349,20 @@ fn update_with_invalid_amount_panics() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #205)")] +fn update_overflow_panics() { + let e = Env::default(); + let address = e.register(MockContract, ()); + let account = Address::generate(&e); + + e.as_contract(&address, || { + mint(&e, &account, i128::MAX); + update(&e, None, Some(&account), 1); + }); +} + +#[test] +#[should_panic(expected = "Error(Contract, #200)")] fn update_with_insufficient_balance_panics() { let e = Env::default(); let address = e.register(MockContract, ()); diff --git a/contracts/token/fungible/test_snapshots/test/set_allowance_with_neg_amount_fails.1.json b/contracts/token/fungible/test_snapshots/test/set_allowance_with_neg_amount_fails.1.json new file mode 100644 index 0000000..129890f --- /dev/null +++ b/contracts/token/fungible/test_snapshots/test/set_allowance_with_neg_amount_fails.1.json @@ -0,0 +1,75 @@ +{ + "generators": { + "address": 3, + "nonce": 0 + }, + "auth": [ + [] + ], + "ledger": { + "protocol_version": 22, + "sequence_number": 0, + "timestamp": 0, + "network_id": "0000000000000000000000000000000000000000000000000000000000000000", + "base_reserve": 0, + "min_persistent_entry_ttl": 4096, + "min_temp_entry_ttl": 16, + "max_entry_ttl": 6312000, + "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": null + } + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], + [ + { + "contract_code": { + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_code": { + "ext": "v0", + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "code": "" + } + }, + "ext": "v0" + }, + 4095 + ] + ] + ] + }, + "events": [] +} \ No newline at end of file diff --git a/contracts/token/fungible/test_snapshots/test/update_overflow_panics.1.json b/contracts/token/fungible/test_snapshots/test/update_overflow_panics.1.json new file mode 100644 index 0000000..b8e02b4 --- /dev/null +++ b/contracts/token/fungible/test_snapshots/test/update_overflow_panics.1.json @@ -0,0 +1,150 @@ +{ + "generators": { + "address": 2, + "nonce": 0 + }, + "auth": [ + [] + ], + "ledger": { + "protocol_version": 22, + "sequence_number": 0, + "timestamp": 0, + "network_id": "0000000000000000000000000000000000000000000000000000000000000000", + "base_reserve": 0, + "min_persistent_entry_ttl": 4096, + "min_temp_entry_ttl": 16, + "max_entry_ttl": 6312000, + "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "Balance" + }, + { + "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4" + } + ] + }, + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "Balance" + }, + { + "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4" + } + ] + }, + "durability": "persistent", + "val": { + "i128": { + "hi": 9223372036854775807, + "lo": 18446744073709551615 + } + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": null + } + } + } + }, + "ext": "v0" + }, + 120960 + ] + ], + [ + { + "contract_code": { + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_code": { + "ext": "v0", + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "code": "" + } + }, + "ext": "v0" + }, + 120960 + ] + ] + ] + }, + "events": [ + { + "event": { + "ext": "v0", + "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", + "type_": "contract", + "body": { + "v0": { + "topics": [ + { + "symbol": "mint" + }, + { + "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4" + } + ], + "data": { + "i128": { + "hi": 9223372036854775807, + "lo": 18446744073709551615 + } + } + } + } + }, + "failed_call": false + } + ] +} \ No newline at end of file diff --git a/contracts/utils/pausable/src/pausable.rs b/contracts/utils/pausable/src/pausable.rs index 88c45b4..c918127 100644 --- a/contracts/utils/pausable/src/pausable.rs +++ b/contracts/utils/pausable/src/pausable.rs @@ -64,9 +64,9 @@ pub trait Pausable { #[repr(u32)] pub enum PausableError { /// The operation failed because the contract is paused. - EnforcedPause = 1, + EnforcedPause = 100, /// The operation failed because the contract is not paused. - ExpectedPause = 2, + ExpectedPause = 101, } // ################## EVENTS ################## diff --git a/contracts/utils/pausable/src/test.rs b/contracts/utils/pausable/src/test.rs index e344a9a..efe044d 100644 --- a/contracts/utils/pausable/src/test.rs +++ b/contracts/utils/pausable/src/test.rs @@ -82,7 +82,7 @@ fn unpause_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn errors_pause_when_paused() { let e = Env::default(); e.mock_all_auths(); @@ -98,7 +98,7 @@ fn errors_pause_when_paused() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #101)")] fn errors_unpause_when_not_paused() { let e = Env::default(); e.mock_all_auths(); diff --git a/examples/fungible-pausable/src/test.rs b/examples/fungible-pausable/src/test.rs index 27b08a8..c2d2ad8 100644 --- a/examples/fungible-pausable/src/test.rs +++ b/examples/fungible-pausable/src/test.rs @@ -39,7 +39,7 @@ fn transfer_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn transfer_fails_when_paused() { let e = Env::default(); let owner = Address::generate(&e); @@ -67,7 +67,7 @@ fn transfer_from_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn transfer_from_fails_when_paused() { let e = Env::default(); let owner = Address::generate(&e); @@ -93,7 +93,7 @@ fn mint_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn mint_fails_when_paused() { let e = Env::default(); let owner = Address::generate(&e); @@ -117,7 +117,7 @@ fn burn_works() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn burn_fails_when_paused() { let e = Env::default(); let owner = Address::generate(&e); diff --git a/examples/pausable/src/contract.rs b/examples/pausable/src/contract.rs index f37a190..2301169 100644 --- a/examples/pausable/src/contract.rs +++ b/examples/pausable/src/contract.rs @@ -21,7 +21,6 @@ pub enum DataKey { #[contracterror] pub enum ExampleContractError { - // ATTENTION !!! - overwrites PausableError Unauthorized = 1, } diff --git a/examples/pausable/src/test.rs b/examples/pausable/src/test.rs index 3795eb5..f2acc56 100644 --- a/examples/pausable/src/test.rs +++ b/examples/pausable/src/test.rs @@ -72,7 +72,7 @@ fn errors_unpause_unauthorized() { } #[test] -#[should_panic(expected = "Error(Contract, #1)")] +#[should_panic(expected = "Error(Contract, #100)")] fn errors_increment_when_paused() { let e = Env::default(); let owner = Address::generate(&e); @@ -84,7 +84,7 @@ fn errors_increment_when_paused() { } #[test] -#[should_panic(expected = "Error(Contract, #2)")] +#[should_panic(expected = "Error(Contract, #101)")] fn errors_emergency_reset_when_not_paused() { let e = Env::default(); let owner = Address::generate(&e);