Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/auth-audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
47 changes: 44 additions & 3 deletions contracts/deposit_handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -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)),
}
}

Expand Down Expand Up @@ -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
)))
);
}
}
85 changes: 84 additions & 1 deletion contracts/market_token/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, ());
Expand Down Expand Up @@ -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
)))
);
}
}
138 changes: 138 additions & 0 deletions scripts/test_gen_auth_audit.py
Original file line number Diff line number Diff line change
@@ -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<Address> {
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()
Loading