Merge pull request #1237 from richardiyamura/fix/subscription-895-896-897-898

fix(subscription): validate error codes, optimize gas, integration tests, wasm CI verify

Author: aji70

.github/workflows/contract-ci.yml

@@ -39,3 +39,29 @@ jobs:
       - name: Build wasm target
         run: cargo build --release --target wasm32-unknown-unknown --manifest-path Cargo.toml
         working-directory: contract
+
+      - name: Verify wasm artifacts
+        run: |
+          WASM_DIR="target/wasm32-unknown-unknown/release"
+          EXPECTED=(
+            subscription
+            myfans_token
+            content_access
+            creator_registry
+            earnings
+          )
+          MISSING=0
+          for pkg in "${EXPECTED[@]}"; do
+            WASM_FILE="${WASM_DIR}/${pkg}.wasm"
+            if [[ -f "$WASM_FILE" && -s "$WASM_FILE" ]]; then
+              echo "✅ ${pkg}.wasm ($(du -sh "$WASM_FILE" | cut -f1))"
+            else
+              echo "❌ ${pkg}.wasm: missing or empty"
+              MISSING=1
+            fi
+          done
+          if [[ "$MISSING" -eq 1 ]]; then
+            echo "::error::One or more expected wasm artifacts are missing or empty after build."
+            exit 1
+          fi
+        working-directory: contract

contract/contracts/myfans-lib/src/error_codes.rs

@@ -27,6 +27,8 @@ pub mod subscription {
     pub const INVALID_FEE_BPS: u32 = 7;
     pub const INVALID_TOKEN_ADDRESS: u32 = 8;
     pub const INVALID_PRICE: u32 = 9;
+    /// Plan ID does not exist; never created or out of range.
+    pub const PLAN_NOT_FOUND: u32 = 10;
 }
 
 /// Error codes for the **content-access** contract.

contract/contracts/subscription/src/lib.rs

@@ -68,6 +68,7 @@ impl DataKey {
 /// | 7 | `InvalidFeeBps` |
 /// | 8 | `InvalidTokenAddress` |
 /// | 9 | `InvalidPrice` |
+/// | 10 | `PlanNotFound` |
 #[contracterror]
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum Error {
@@ -89,6 +90,8 @@ pub enum Error {
     InvalidTokenAddress = 8,
     /// Code 9 – subscription price must be strictly positive.
     InvalidPrice = 9,
+    /// Code 10 – plan ID does not exist; never created or out of range.
+    PlanNotFound = 10,
 }
 
 /// Stellar "null" account (GAAA...WHF) — not a valid fee recipient.
@@ -214,20 +217,20 @@ impl MyfansContract {
             .storage()
             .instance()
             .get(&DataKey::Plan(plan_id))
-            .unwrap();
+            .unwrap_or_else(|| panic_with_error!(&env, Error::PlanNotFound));
         let fee_bps: u32 = env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0);
-        let fee_recipient: Address = env
-            .storage()
-            .instance()
-            .get(&DataKey::FeeRecipient)
-            .unwrap();
-
         let fee = (plan.amount * fee_bps as i128) / 10000;
         let creator_amount = plan.amount - fee;
 
         let token_client = token::Client::new(&env, &plan.asset);
         token_client.transfer(&fan, &plan.creator, &creator_amount);
         if fee > 0 {
+            // Deferred read: only fetch fee_recipient when a fee is actually owed.
+            let fee_recipient: Address = env
+                .storage()
+                .instance()
+                .get(&DataKey::FeeRecipient)
+                .unwrap();
             token_client.transfer(&fan, &fee_recipient, &fee);
         }
 
@@ -284,7 +287,9 @@ impl MyfansContract {
             .instance()
             .get(&DataKey::Paused)
             .unwrap_or(false);
-        assert!(!paused, "contract is paused");
+        if paused {
+            panic_with_error!(&env, Error::Paused);
+        }
 
         let sub: Subscription = env
             .storage()
@@ -300,21 +305,21 @@ impl MyfansContract {
             .storage()
             .instance()
             .get(&DataKey::Plan(sub.plan_id))
-            .unwrap();
+            .unwrap_or_else(|| panic_with_error!(&env, Error::PlanNotFound));
 
         let fee_bps: u32 = env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0);
-        let fee_recipient: Address = env
-            .storage()
-            .instance()
-            .get(&DataKey::FeeRecipient)
-            .unwrap();
-
         let fee = (plan.amount * fee_bps as i128) / 10000;
         let creator_amount = plan.amount - fee;
 
         let token_client = token::Client::new(&env, &token);
         token_client.transfer(&fan, &creator, &creator_amount);
         if fee > 0 {
+            // Deferred read: only fetch fee_recipient when a fee is actually owed.
+            let fee_recipient: Address = env
+                .storage()
+                .instance()
+                .get(&DataKey::FeeRecipient)
+                .unwrap();
             token_client.transfer(&fan, &fee_recipient, &fee);
         }
 
@@ -375,7 +380,9 @@ impl MyfansContract {
             .instance()
             .get(&DataKey::Paused)
             .unwrap_or(false);
-        assert!(!paused, "contract is paused");
+        if paused {
+            panic_with_error!(&env, Error::Paused);
+        }
 
         let token: Address = env
             .storage()
@@ -384,18 +391,18 @@ impl MyfansContract {
             .unwrap();
         let price: i128 = env.storage().instance().get(&DataKey::Price).unwrap();
         let fee_bps: u32 = env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0);
-        let fee_recipient: Address = env
-            .storage()
-            .instance()
-            .get(&DataKey::FeeRecipient)
-            .unwrap();
-
         let fee = (price * fee_bps as i128) / 10000;
         let creator_amount = price - fee;
 
         let token_client = token::Client::new(&env, &token);
         token_client.transfer(&fan, &creator, &creator_amount);
         if fee > 0 {
+            // Deferred read: only fetch fee_recipient when a fee is actually owed.
+            let fee_recipient: Address = env
+                .storage()
+                .instance()
+                .get(&DataKey::FeeRecipient)
+                .unwrap();
             token_client.transfer(&fan, &fee_recipient, &fee);
         }
 

contract/contracts/subscription/src/test.rs

@@ -1046,7 +1046,6 @@ fn test_subscribe_fails_when_paused() {
 }
 
 #[test]
-#[should_panic(expected = "contract is paused")]
 fn test_extend_subscription_fails_when_paused() {
     let (env, client, admin, token, token_admin) = setup_test();
     let fee_recipient = Address::generate(&env);
@@ -1057,7 +1056,11 @@ fn test_extend_subscription_fails_when_paused() {
     let plan_id = client.create_plan(&creator, &token.address, &1000, &30);
     client.subscribe(&fan, &plan_id, &token.address);
     client.pause();
-    client.extend_subscription(&fan, &creator, &17280, &token.address);
+    let result = client.try_extend_subscription(&fan, &creator, &17280, &token.address);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(Error::Paused as u32)))
+    );
 }
 
 #[test]
@@ -1076,10 +1079,13 @@ fn test_cancel_fails_when_paused() {
 }
 
 #[test]
-#[should_panic]
 fn test_create_subscription_fails_when_paused() {
     let (_env, client, _admin, creator, fan, _token, _token_admin) = setup_paused();
-    client.create_subscription(&fan, &creator, &518400);
+    let result = client.try_create_subscription(&fan, &creator, &518400);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(Error::Paused as u32)))
+    );
 }
 
 /// Views must remain available while paused.
@@ -1479,3 +1485,70 @@ fn test_ping_works_on_uninitialized_contract() {
     let seq = client.ping();
     assert_eq!(seq, env.ledger().sequence());
 }
+
+// ── #895 – error code validation ─────────────────────────────────────────────
+
+/// subscribe with a plan_id that was never created returns Error::PlanNotFound (code 10).
+#[test]
+fn test_subscribe_nonexistent_plan_returns_plan_not_found() {
+    let (env, client, admin, token, _token_admin) = setup_test();
+    let fee_recipient = Address::generate(&env);
+    client.init(&admin, &500, &fee_recipient, &token.address, &1000);
+    let fan = Address::generate(&env);
+
+    let result = client.try_subscribe(&fan, &9999u32, &token.address);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(
+            Error::PlanNotFound as u32
+        )))
+    );
+}
+
+/// create_plan when paused returns Error::Paused (code 2).
+#[test]
+fn test_create_plan_paused_returns_typed_error() {
+    let (_env, client, _admin, creator, _fan, token, _token_admin) = setup_paused();
+    let result = client.try_create_plan(&creator, &token.address, &1000, &30);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(Error::Paused as u32)))
+    );
+}
+
+/// subscribe when paused returns Error::Paused (code 2).
+#[test]
+fn test_subscribe_paused_returns_typed_error() {
+    let (env, client, admin, token, token_admin) = setup_test();
+    let fee_recipient = Address::generate(&env);
+    let creator = Address::generate(&env);
+    let fan = Address::generate(&env);
+    client.init(&admin, &500, &fee_recipient, &token.address, &1000);
+    token_admin.mint(&fan, &50000);
+    let plan_id = client.create_plan(&creator, &token.address, &1000, &30);
+    client.pause();
+    let result = client.try_subscribe(&fan, &plan_id, &token.address);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(Error::Paused as u32)))
+    );
+}
+
+/// cancel when paused returns Error::Paused (code 2).
+#[test]
+fn test_cancel_paused_returns_typed_error() {
+    let (env, client, admin, token, token_admin) = setup_test();
+    let fee_recipient = Address::generate(&env);
+    let creator = Address::generate(&env);
+    let fan = Address::generate(&env);
+    client.init(&admin, &0, &fee_recipient, &token.address, &1000);
+    token_admin.mint(&fan, &50000);
+    let plan_id = client.create_plan(&creator, &token.address, &1000, &30);
+    client.subscribe(&fan, &plan_id, &token.address);
+    client.pause();
+    let result = client.try_cancel(&fan, &creator, &0);
+    assert_eq!(
+        result,
+        Err(Ok(SorobanError::from_contract_error(Error::Paused as u32)))
+    );
+}