From 7f89da6131aa4311a3adfdfd57538953710fcf04 Mon Sep 17 00:00:00 2001
From: Emmy123222 <emmanuelogheneovo17@gmail.com>
Date: Sat, 28 Mar 2026 10:19:02 +0000
Subject: [PATCH] Fix invoice-token approve to reject negative allowances

- Add validation in approve function to reject amount < 0
- Reuse existing Error::InvalidAmount variant
- Add comprehensive unit tests for negative amount rejection
- Tests cover: negative amount rejection, zero amount acceptance, invalid expiration rejection

Closes #28
---
 contracts/invoice-token/src/lib.rs  |  3 ++
 contracts/invoice-token/src/test.rs | 57 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/contracts/invoice-token/src/lib.rs b/contracts/invoice-token/src/lib.rs
index 2f136bf..ab77e99 100644
--- a/contracts/invoice-token/src/lib.rs
+++ b/contracts/invoice-token/src/lib.rs
@@ -114,6 +114,9 @@ impl InvoiceToken {
         expiration_ledger: u32,
     ) -> Result<(), Error> {
         from.require_auth();
+        if amount < 0 {
+            return Err(Error::InvalidAmount);
+        }
         let ledger = env.ledger().sequence();
         if amount != 0 && expiration_ledger < ledger {
             return Err(Error::InvalidExpiration);
diff --git a/contracts/invoice-token/src/test.rs b/contracts/invoice-token/src/test.rs
index 2782aec..ca441bc 100644
--- a/contracts/invoice-token/src/test.rs
+++ b/contracts/invoice-token/src/test.rs
@@ -1037,3 +1037,60 @@ fn test_approve_update_expiration_shortens() {
         .with_mut(|li| li.sequence_number = new_expiration + 1);
     assert_eq!(client.allowance(&admin, &spender), 0);
 }
+
+// ========== Negative Amount Tests ==========
+
+#[test]
+fn test_approve_negative_amount_rejected() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let (client, admin, _minter) = setup_token(&env);
+
+    let spender = Address::generate(&env);
+    let expiration = env.ledger().sequence() + 100;
+
+    // Attempt to approve with negative amount should fail
+    let result = client.try_approve(&admin, &spender, &(-100i128), &expiration);
+    assert_eq!(result, Err(Ok(crate::errors::Error::InvalidAmount)));
+
+    // Verify no allowance was set
+    assert_eq!(client.allowance(&admin, &spender), 0);
+}
+
+#[test]
+fn test_approve_zero_amount_accepted() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let (client, admin, _minter) = setup_token(&env);
+
+    let spender = Address::generate(&env);
+    let expiration = env.ledger().sequence() + 100;
+
+    // First set a non-zero allowance
+    client.approve(&admin, &spender, &500, &expiration);
+    assert_eq!(client.allowance(&admin, &spender), 500);
+
+    // Approve with zero amount should remove allowance (current behavior)
+    client.approve(&admin, &spender, &0, &expiration);
+    assert_eq!(client.allowance(&admin, &spender), 0);
+}
+
+#[test]
+fn test_approve_positive_amount_invalid_expiration_rejected() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let (client, admin, _minter) = setup_token(&env);
+
+    let spender = Address::generate(&env);
+
+    // Advance ledger to ensure we can test past expiration
+    env.ledger().with_mut(|li| li.sequence_number = 10);
+    let current_ledger = env.ledger().sequence();
+
+    // Attempt to approve with positive amount and invalid expiration should fail
+    let result = client.try_approve(&admin, &spender, &500, &(current_ledger - 1));
+    assert_eq!(result, Err(Ok(crate::errors::Error::InvalidExpiration)));
+
+    // Verify no allowance was set
+    assert_eq!(client.allowance(&admin, &spender), 0);
+}