diff --git a/.github/workflows/auth-audit.yml b/.github/workflows/auth-audit.yml index f527a9a..7a383eb 100644 --- a/.github/workflows/auth-audit.yml +++ b/.github/workflows/auth-audit.yml @@ -7,6 +7,7 @@ on: - "contracts/**/src/lib.rs" - "docs/auth-audit.md" - "scripts/gen_auth_audit.py" + - "scripts/test_gen_auth_audit.py" permissions: contents: read @@ -20,5 +21,8 @@ jobs: - name: Checkout uses: actions/checkout@v4 + - name: Run gen_auth_audit.py unit tests + run: python3 scripts/test_gen_auth_audit.py -v + - name: Verify auth-audit.md matches source run: python3 scripts/gen_auth_audit.py --check diff --git a/contracts/deposit_handler/src/lib.rs b/contracts/deposit_handler/src/lib.rs index 9397ff3..1c154cb 100644 --- a/contracts/deposit_handler/src/lib.rs +++ b/contracts/deposit_handler/src/lib.rs @@ -46,6 +46,10 @@ pub enum Error { BelowMinimumDeposit = 8, /// Issue #370: execution_fee is below the configured global minimum. InsufficientExecutionFee = 9, + /// Issue #371: market_token has no registered index/long/short tokens in + /// data_store. Distinct from DepositNotFound, which means "no such + /// deposit id" — this means "no such market". + InvalidMarket = 10, } // ─── Storage ────────────────────────────────────────────────────────────────── @@ -591,13 +595,13 @@ fn load_market_props(env: &Env, data_store: &Address, market_token: &Address) -> market_token: market_token.clone(), index_token: ds .get_address(&market_index_token_key(env, market_token)) - .unwrap_or_else(|| panic_with_error!(env, Error::DepositNotFound)), + .unwrap_or_else(|| panic_with_error!(env, Error::InvalidMarket)), long_token: ds .get_address(&market_long_token_key(env, market_token)) - .unwrap_or_else(|| panic_with_error!(env, Error::DepositNotFound)), + .unwrap_or_else(|| panic_with_error!(env, Error::InvalidMarket)), short_token: ds .get_address(&market_short_token_key(env, market_token)) - .unwrap_or_else(|| panic_with_error!(env, Error::DepositNotFound)), + .unwrap_or_else(|| panic_with_error!(env, Error::InvalidMarket)), } } @@ -2178,4 +2182,41 @@ mod tests { let lp = MtClient::new(env, &w.market_tk).balance(&user); assert!(lp > 0, "normal deposit must still mint LP tokens after vault check added"); } + + // ── Issue #371: unregistered market must raise InvalidMarket ────────────── + + /// create_deposit against a market_token with no registered index/long/short + /// tokens in data_store must revert with InvalidMarket, not DepositNotFound. + /// DepositNotFound is reserved for lookups of an existing deposit id. + #[test] + fn create_deposit_unregistered_market_rejected_with_invalid_market() { + let w = setup(); + let env = &w.env; + let user = Address::generate(env); + let unregistered_market = Address::generate(env); + + StellarAssetClient::new(env, &w.long_tk).mint(&user, &1_000_0000i128); + + let hc = DepositHandlerClient::new(env, &w.handler); + let result = hc.try_create_deposit( + &user, + &CreateDepositParams { + receiver: user.clone(), + market: unregistered_market, + initial_long_token: w.long_tk.clone(), + initial_short_token: w.short_tk.clone(), + long_token_amount: 1_000_0000i128, + short_token_amount: 0, + min_market_tokens: 0, + execution_fee: 0, + }, + ); + + assert_eq!( + result, + Err(Ok(soroban_sdk::Error::from_contract_error( + Error::InvalidMarket as u32 + ))) + ); + } } diff --git a/contracts/market_token/src/lib.rs b/contracts/market_token/src/lib.rs index 5e8c19e..9eaba9e 100644 --- a/contracts/market_token/src/lib.rs +++ b/contracts/market_token/src/lib.rs @@ -365,7 +365,10 @@ fn spend_allowance(env: &Env, from: &Address, spender: &Address, amount: i128) { mod tests { use super::*; use role_store::{RoleStore, RoleStoreClient as RsClient}; - use soroban_sdk::{testutils::Address as _, Env}; + use soroban_sdk::{ + testutils::{Address as _, Ledger as _}, + Env, + }; fn deploy_role_store(env: &Env, admin: &Address) -> Address { let id = env.register(RoleStore, ()); @@ -601,4 +604,84 @@ mod tests { rs.revoke_role(&admin, &handler, &roles::controller(&env)); client.mint(&handler, &user, &100_0000i128); } + + // ── Issue #362: allowance-expiration coverage ───────────────────────────── + + /// Once the ledger sequence passes an approval's expiration_ledger, + /// allowance() must report 0 even though the underlying temporary entry + /// (if not yet TTL-evicted) still holds the original amount. + #[test] + fn allowance_reads_zero_after_expiration_ledger_passes() { + let (env, admin, _, mt_id) = setup(); + let client = MarketTokenClient::new(&env, &mt_id); + let alice = Address::generate(&env); + let spender = Address::generate(&env); + + client.mint(&admin, &alice, &1000_0000i128); + + let expiration = env.ledger().sequence() + 100; + client.approve(&alice, &spender, &500_0000i128, &expiration); + assert_eq!(client.allowance(&alice, &spender), 500_0000); + + // Advance past the approval's expiration_ledger. + env.ledger().set_sequence_number(expiration + 1); + + assert_eq!( + client.allowance(&alice, &spender), + 0, + "allowance() must return 0 once expiration_ledger has passed" + ); + } + + /// transfer_from on an expired allowance must revert with AllowanceExpired, + /// not InsufficientAllowance, even though the stored amount would otherwise + /// be enough to cover the transfer. + #[test] + fn transfer_from_after_expiration_reverts_with_allowance_expired() { + let (env, admin, _, mt_id) = setup(); + let client = MarketTokenClient::new(&env, &mt_id); + let alice = Address::generate(&env); + let bob = Address::generate(&env); + let spender = Address::generate(&env); + + client.mint(&admin, &alice, &1000_0000i128); + + let expiration = env.ledger().sequence() + 100; + client.approve(&alice, &spender, &500_0000i128, &expiration); + + env.ledger().set_sequence_number(expiration + 1); + + let result = client.try_transfer_from(&spender, &alice, &bob, &1_0000i128); + assert_eq!( + result, + Err(Ok(soroban_sdk::Error::from_contract_error( + Error::AllowanceExpired as u32 + ))) + ); + } + + /// burn_from on an expired allowance must revert with AllowanceExpired, + /// exercising the same expiry branch in spend_allowance as transfer_from. + #[test] + fn burn_from_after_expiration_reverts_with_allowance_expired() { + let (env, admin, _, mt_id) = setup(); + let client = MarketTokenClient::new(&env, &mt_id); + let alice = Address::generate(&env); + let spender = Address::generate(&env); + + client.mint(&admin, &alice, &1000_0000i128); + + let expiration = env.ledger().sequence() + 100; + client.approve(&alice, &spender, &500_0000i128, &expiration); + + env.ledger().set_sequence_number(expiration + 1); + + let result = client.try_burn_from(&spender, &alice, &1_0000i128); + assert_eq!( + result, + Err(Ok(soroban_sdk::Error::from_contract_error( + Error::AllowanceExpired as u32 + ))) + ); + } } diff --git a/scripts/test_gen_auth_audit.py b/scripts/test_gen_auth_audit.py new file mode 100644 index 0000000..143e1ca --- /dev/null +++ b/scripts/test_gen_auth_audit.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +""" +test_gen_auth_audit.py — Regression coverage for gen_auth_audit.py's --check mode. + +Issue #356: docs/auth-audit.md previously certified two functions +(set_account_principal_delta / get_account_principal_delta) that did not exist +anywhere in contracts/data_store/src/lib.rs, and mis-stated get_position_manager +as an auth-checked "✅ PASS" read when it is actually an unrestricted public read. +gen_auth_audit.py --check was added to catch exactly this class of drift, but +had no automated test of its own — this fills that gap by exercising +validate_against_source() against a fabricated source/doc pair with both a +phantom (documented-but-nonexistent) entry and a missing (undocumented-but-real) +entry, then confirms a freshly generated doc round-trips clean. + +Usage: + python3 scripts/test_gen_auth_audit.py +""" + +from __future__ import annotations + +import importlib.util +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +SCRIPT_PATH = Path(__file__).resolve().parent / "gen_auth_audit.py" + + +def _load_gen_auth_audit(): + spec = importlib.util.spec_from_file_location("gen_auth_audit", SCRIPT_PATH) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + # dataclasses' field-type resolution looks the module up via + # sys.modules[cls.__module__], so it must be registered before exec. + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +gen_auth_audit = _load_gen_auth_audit() + +FAKE_SOURCE = """ +#![no_std] + +#[contractimpl] +impl FakeContract { + pub fn get_position_manager(env: Env, owner: Address, market: Address) -> Option
{ + env.storage().persistent().get(&DataKey::Addr(owner)) + } + + pub fn set_u128(env: Env, caller: Address, key: BytesN<32>, value: u128) -> u128 { + caller.require_auth(); + require_controller(&env, &caller); + env.storage().persistent().set(&DataKey::U128(key), &value); + value + } +} +""" + + +class GenAuthAuditTests(unittest.TestCase): + def setUp(self): + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + root = Path(self.tmpdir.name) + + crate_dir = root / "contracts" / "fake_contract" / "src" + crate_dir.mkdir(parents=True) + (crate_dir / "lib.rs").write_text(FAKE_SOURCE) + + self.output_path = root / "docs" / "auth-audit.md" + self.output_path.parent.mkdir(parents=True) + + self.patches = [ + patch.object(gen_auth_audit, "CONTRACTS_DIR", root / "contracts"), + patch.object(gen_auth_audit, "OUTPUT_PATH", self.output_path), + patch.object( + gen_auth_audit, + "CONTRACTS", + [("fake_contract", "fake_contract")], + ), + ] + for p in self.patches: + p.start() + self.addCleanup(p.stop) + + def test_freshly_generated_doc_has_no_discrepancies(self): + """A doc generated straight from source must validate clean (the + property --check relies on to gate CI).""" + self.output_path.write_text(gen_auth_audit.generate_markdown()) + self.assertEqual(gen_auth_audit.validate_against_source(), []) + + def test_phantom_documented_function_is_detected(self): + """A documented function that does not exist in source (#356's exact + failure mode: set_account_principal_delta / get_account_principal_delta) + must be reported as a discrepancy.""" + doc = gen_auth_audit.generate_markdown() + doc = doc.replace( + "| `set_u128` |", + "| `set_account_principal_delta` |\n| `set_u128` |", + ) + self.output_path.write_text(doc) + + discrepancies = gen_auth_audit.validate_against_source() + self.assertTrue( + any("set_account_principal_delta" in d and "does NOT exist" in d for d in discrepancies), + f"expected a phantom-function discrepancy, got: {discrepancies}", + ) + + def test_undocumented_real_function_is_detected(self): + """A real pub fn missing from the doc entirely must be reported — + the other direction of drift --check guards against.""" + doc = gen_auth_audit.generate_markdown() + doc = "\n".join( + line for line in doc.splitlines() if "get_position_manager" not in line + ) + self.output_path.write_text(doc) + + discrepancies = gen_auth_audit.validate_against_source() + self.assertTrue( + any("get_position_manager" in d and "NOT documented" in d for d in discrepancies), + f"expected a missing-function discrepancy, got: {discrepancies}", + ) + + def test_get_position_manager_classified_as_read_only_not_pass(self): + """get_position_manager takes no auth-check action on its own body, so + it must classify as read-only (N/A), never a caller-checked PASS — + the specific mis-statement #356 flagged.""" + fns = gen_auth_audit.extract_pub_fns(FAKE_SOURCE) + by_name = {fn.name: fn for fn in fns} + status, _expected = gen_auth_audit.classify_fn(by_name["get_position_manager"]) + self.assertEqual(status, "➖ N/A") + + +if __name__ == "__main__": + unittest.main()