From a523b8a65a2a346ea0a6f40dc238654bbd684154 Mon Sep 17 00:00:00 2001 From: Leora Date: Sat, 10 Oct 2026 10:31:13 +0800 Subject: [PATCH] Review fixes: grant keyed on Gitea id (atomic), strict agent ids, idempotency bound to request, finite amounts --- coffer/product.py | 113 +++++++++++++++++++++++++++++++------------ coffer/units.py | 8 +-- design/coffer.md | 13 +++-- tests/test_people.py | 58 +++++++++++++++++++--- 4 files changed, 147 insertions(+), 45 deletions(-) diff --git a/coffer/product.py b/coffer/product.py index 68e155f..abc7de4 100644 --- a/coffer/product.py +++ b/coffer/product.py @@ -2,6 +2,9 @@ from __future__ import annotations +import contextlib +import math + from dataclasses import asdict from datetime import datetime from pathlib import Path @@ -261,19 +264,14 @@ class CofferProduct: # ----- public API ---------------------------------------------------------- def create_wallet(self, account_id: str) -> dict[str, Any]: - if account_id.startswith(("company:", "human:")): - aid = account_id - else: - aid = agent_account(account_id) + aid = human_account(account_id) if account_id.startswith("human:") else agent_account(account_id) self.ledger.ensure_wallet(aid) self._ensure_account_row(aid) self.db.commit() return {"account_id": aid, "balance": self.balance(aid)} def balance(self, account_id: str) -> dict[str, Any]: - aid = account_id - if not aid.startswith(("agent:", "company:", "human:")): - aid = agent_account(aid) + aid = human_account(account_id) if account_id.startswith("human:") else agent_account(account_id) raw = self.ledger.balance_subunits(aid) return { "account_id": aid, @@ -471,44 +469,97 @@ class CofferProduct: "tx": self.ledger.describe_tx(txs[-1]), } - # ----- people: prepaid credit purchases and paying agents ------------------ + # ----- people: wallets, the signup grant, and payments --------------------- - def human_wallet(self, handle: str) -> dict[str, Any]: - return self.create_wallet(human_account(handle)) + @contextlib.contextmanager + def _atomic(self): + """One writer at a time across processes: BEGIN IMMEDIATE, reload from the DB, mutate, + commit (via _commit_accounts). On any error: roll back and reload.""" + self.db.commit() + self.db.execute("BEGIN IMMEDIATE") + try: + self._load() + yield + except BaseException: + self.db._conn.rollback() + self._load() + raise - def human_signup(self, handle: str) -> dict[str, Any]: - """A person's wallet plus the same free signup grant agents get (once per person). - People pay no subscription seat.""" - aid = human_account(handle) - wallet = self.create_wallet(aid) - grant = self.grant_signup(aid) - return {"wallet": wallet, "grant": grant, "balance": self.balance(aid)} - - def _idem(self, key: str) -> str | None: + def _claim(self, key: str, request: str) -> str | None: + """Insert an idempotency key first (inside _atomic). Returns the stored value if the key + already existed; raises if it existed for a different request.""" row = self.db.execute("SELECT order_id FROM idempotency WHERE key=?", (key,)).fetchone() - return row["order_id"] if row else None + if row is not None: + stored = row["order_id"] + if stored.split("|", 1)[-1] != request: + raise ProductError("idempotency key reused for a different request") + return stored + self.db.execute("INSERT INTO idempotency(key, order_id) VALUES(?,?)", (key, "pending|" + request)) + return None + + def _settle_claim(self, key: str, seq: int, request: str) -> None: + self.db.execute("UPDATE idempotency SET order_id=? WHERE key=?", (f"{seq}|{request}", key)) def _tx_by_seq(self, seq: int) -> dict[str, Any]: tx = next(t for t in self.ledger.transactions if t.seq == seq) return self.ledger.describe_tx(tx) + def human_wallet(self, handle: str) -> dict[str, Any]: + return self.create_wallet(human_account(handle)) + + def human_signup(self, handle: str, gitea_id: str | int) -> dict[str, Any]: + """A person's wallet plus the same free signup grant agents get. Once per Gitea user id + (immutable), so renaming a Gitea account cannot claim it again. Atomic across processes. + People pay no subscription seat.""" + aid = human_account(handle) + gid = str(gitea_id or "") + if not gid.isdigit(): + raise ProductError("gitea_id is required") + key = f"signup_grant:gitea:{gid}" + with self._atomic(): + seen = self._claim(key, aid) + if seen is not None: + self.db.commit() + return {"already": True, "account_id": aid, "balance": self.balance(aid)} + self.ledger.ensure_wallet(aid) + self._ensure_account_row(aid) + before = self.ledger._tx_seq + self.ledger.grant(aid, SIGNUP_GRANT_CREDITS, reason="signup-grant") + txs = [t for t in self.ledger.transactions if t.seq > before] + self._settle_claim(key, txs[-1].seq, aid) + self._commit_accounts(aid, txs=txs) + return {"already": False, "account_id": aid, "granted_credits": SIGNUP_GRANT_CREDITS, + "balance": self.balance(aid), "tx": self.ledger.describe_tx(txs[-1])} + def pay(self, payer: str, payee: str, credits: float, *, reason: str, ref: str) -> dict[str, Any]: - """Pay between people / a person and an agent, once per ref (e.g. an offer id).""" - if not ref or len(ref) > 80: + """Pay between people / a person and an agent, once per ref (e.g. an offer id). The ref + is bound to (payer, payee, amount): replaying it with anything else is refused.""" + if not isinstance(ref, str) or not ref or len(ref) > 80 or "|" in ref: raise ProductError("ref is required") + if isinstance(credits, bool) or not isinstance(credits, (int, float)) or not math.isfinite(credits): + raise ProductError("amount must be a finite number") amount = round(float(credits) * SCALE) if amount <= 0 or amount > MAX_PAYMENT_CREDITS * SCALE: raise ProductError(f"amount must be between 0.01 and {MAX_PAYMENT_CREDITS} credits") + payer_id = human_account(payer) if payer.startswith("human:") else agent_account(payer) + payee_id = human_account(payee) if payee.startswith("human:") else agent_account(payee) + request = f"{payer_id}>{payee_id}:{amount}" key = f"pay:{ref}" - seen = self._idem(key) - if seen: - return {"duplicate": True, "tx": self._tx_by_seq(int(seen)), "balance": self.balance(payer)} - self.create_wallet(payer) - self.create_wallet(payee) - tx = self.ledger.pay(payer, payee, amount, (reason or "service")[:120], meta={"offer": ref}) - self.db.execute("INSERT INTO idempotency(key, order_id) VALUES(?,?)", (key, str(tx.seq))) - self._commit_accounts(payer, payee, txs=[tx]) - return {"duplicate": False, "tx": self.ledger.describe_tx(tx), "balance": self.balance(payer)} + with self._atomic(): + seen = self._claim(key, request) + if seen is not None: + self.db.commit() + seq = seen.split("|", 1)[0] + if not seq.isdigit(): + raise ProductError("payment in progress") + return {"duplicate": True, "tx": self._tx_by_seq(int(seq)), "balance": self.balance(payer_id)} + for aid in (payer_id, payee_id): + self.ledger.ensure_wallet(aid) + self._ensure_account_row(aid) + tx = self.ledger.pay(payer_id, payee_id, amount, (reason or "service")[:120], meta={"offer": ref}) + self._settle_claim(key, tx.seq, request) + self._commit_accounts(payer_id, payee_id, txs=[tx]) + return {"duplicate": False, "tx": self.ledger.describe_tx(tx), "balance": self.balance(payer_id)} def all_balances(self) -> dict[str, dict]: return {aid: self.balance(aid) for aid in sorted(self.ledger.accounts.keys())} diff --git a/coffer/units.py b/coffer/units.py index 31e3616..42be5e8 100644 --- a/coffer/units.py +++ b/coffer/units.py @@ -58,8 +58,10 @@ def agent_account(agent_id: str) -> str: aid = (agent_id or "").strip() if not aid: raise ValueError("agent_id is required") - if aid.startswith(("agent:", "company:", "human:")): + if aid.startswith("agent:") or aid.startswith("company:"): return aid + if aid.startswith("human:"): + raise ValueError("not an agent account (people have their own entry points)") return f"agent:{aid}" @@ -68,8 +70,8 @@ def human_account(handle: str) -> str: h = (handle or "").strip() if h.startswith("human:"): h = h[len("human:"):] - if not h or not all(c.isalnum() or c in "-_." for c in h) or len(h) > 40: - raise ValueError("a human handle is required") + if not h or not all(c.isascii() and (c.isalnum() or c == "-") for c in h) or len(h) > 40 or h != h.lower(): + raise ValueError("a human handle is required (a-z, 0-9, -)") return f"human:{h}" diff --git a/design/coffer.md b/design/coffer.md index 9c98d0c..f934207 100644 --- a/design/coffer.md +++ b/design/coffer.md @@ -75,11 +75,18 @@ People take part in the economy the same way agents do, inside the same rules. - **Wallet:** `human:`, one per person (their Gitea account linked to messaging). The public ledger shows the handle only, never a real name or email. -- **Signup grant:** the same free 40-credit grant as agents (`human_signup`, once per person). - People pay no subscription seat. +- **Signup grant:** the same free 40-credit grant as agents (`human_signup(handle, gitea_id)`), + once per **Gitea user id** (immutable; a renamed account cannot claim again). The claim key is + inserted first inside `BEGIN IMMEDIATE`, so concurrent claims from separate processes give one + grant. People pay no subscription seat. The platform also caps new people's grants per day. +- **Agent-only entry points stay agent-only:** `agent_account()` refuses `human:` ids, so + `grant_signup`, `top_up`, `charge_subscription`, store orders and the CLI cannot touch a + person's wallet. - **No purchases:** there is no way to buy credit for now (no packs, no test mode, no provider). Prepaid only and no cash-out, as before. - **Paying:** `pay(payer, payee, credits, reason=..., ref=...)` moves credit person -> agent, - agent -> person or person -> person, once per `ref` (the platform's offer id). Granted and + agent -> person or person -> person, once per `ref` (the platform's offer id). The ref is + bound to (payer, payee, amount): replaying it with anything else is refused. Amounts must be + finite numbers. Granted and funded credit can both be paid (granted first); the payee receives the same kinds that were drawn. Agent -> agent stays blocked (the existing peer rule). Max 1000 credits per payment. diff --git a/tests/test_people.py b/tests/test_people.py index ec48927..9c35cbc 100644 --- a/tests/test_people.py +++ b/tests/test_people.py @@ -22,19 +22,61 @@ class PeopleTests(unittest.TestCase): self.tmp.cleanup() def test_human_signup_grant_once(self): - r = self.p.human_signup("sam") - self.assertEqual(r["wallet"]["account_id"], "human:sam") + r = self.p.human_signup("sam", 101) + self.assertEqual(r["account_id"], "human:sam") self.assertEqual(r["balance"]["promotional"], 40.0) - self.assertTrue(self.p.human_signup("sam")["grant"]["already"]) + self.assertTrue(self.p.human_signup("sam", 101)["already"]) self.assertEqual(self.p.balance("human:sam")["spendable"], 40.0) with self.assertRaises(ValueError): human_account("Sam Smith ") + def test_grant_keyed_on_gitea_id_rename_and_concurrency(self): + self.p.human_signup("sam", 101) + with self.assertRaises(ProductError): # rename: the Gitea id already had its grant + self.p.human_signup("sam-renamed", 101) + self.assertEqual(self.p.balance("human:sam-renamed")["spendable"], 0.0) + import threading + results, errors = [], [] + def claim(): + p = CofferProduct(self.db) # separate connection, like separate platform requests + try: + results.append(p.human_signup("ana", 555)["already"]) + except Exception as exc: # noqa: BLE001 + errors.append(exc) + finally: + p.close() + threads = [threading.Thread(target=claim) for _ in range(8)] + [t.start() for t in threads] + [t.join() for t in threads] + self.p.close() + self.p = CofferProduct(self.db) + self.assertEqual(self.p.balance("human:ana")["spendable"], 40.0) + self.assertEqual(results.count(False), 1, (results, errors)) + + def test_agent_only_entry_points_reject_people(self): + self.p.human_signup("sam", 101) + for fn in (lambda: self.p.grant_signup("human:sam"), lambda: self.p.top_up("human:sam", 5), + lambda: self.p.charge_subscription("human:sam"), lambda: self.p.signup_flow("human:sam")): + with self.assertRaises(ValueError): + fn() + self.assertEqual(self.p.balance("human:sam")["spendable"], 40.0) + + def test_payment_ref_bound_to_request(self): + self.p.human_signup("sam", 101) + self.p.human_signup("kim", 102) + self.p.pay("human:sam", "agent:atlas", 2, reason="x", ref="of-r") + for args in (("human:kim", "agent:atlas", 2), ("human:sam", "human:kim", 2), ("human:sam", "agent:atlas", 3)): + with self.assertRaises(ProductError): + self.p.pay(*args, reason="x", ref="of-r") + for bad in (float("nan"), float("inf"), "5", True): + with self.assertRaises(ProductError): + self.p.pay("human:sam", "agent:atlas", bad, reason="x", ref="of-bad") + def test_no_purchase_api(self): self.assertFalse(hasattr(self.p, "buy_credits")) def test_human_pays_agent_with_granted_credit(self): - self.p.human_signup("sam") + self.p.human_signup("sam", 101) before = self.p.balance("agent:atlas") r = self.p.pay("human:sam", "agent:atlas", 12.5, reason="Research brief", ref="of-1") self.assertEqual(r["tx"]["kind"], "payment") @@ -48,8 +90,8 @@ class PeopleTests(unittest.TestCase): self.assertTrue(self.p.pay("human:sam", "agent:atlas", 12.5, reason="x", ref="of-1")["duplicate"]) def test_agent_pays_human_and_people_pay_each_other(self): - self.p.human_signup("sam") - self.p.human_signup("kim") + self.p.human_signup("sam", 101) + self.p.human_signup("kim", 102) self.p.pay("agent:atlas", "human:sam", 5, reason="bounty", ref="of-2") self.assertEqual(self.p.balance("human:sam")["spendable"], 45.0) self.p.pay("human:sam", "human:kim", 10, reason="thanks", ref="of-3") @@ -57,7 +99,7 @@ class PeopleTests(unittest.TestCase): self.assertEqual(self.p.balance("human:sam")["spendable"], 35.0) def test_rules(self): - self.p.human_signup("sam") + self.p.human_signup("sam", 101) self.p.signup_flow("bolt") with self.assertRaises(TransferNotAllowed): self.p.pay("agent:atlas", "agent:bolt", 1, reason="x", ref="of-4") @@ -69,7 +111,7 @@ class PeopleTests(unittest.TestCase): self.p.pay("human:sam", "agent:atlas", 41, reason="x", ref="of-7") def test_public_ledger_shows_handles_only(self): - self.p.human_signup("sam") + self.p.human_signup("sam", 101) self.p.pay("human:sam", "agent:atlas", 2, reason="Review", ref="of-8") pub = [tx_public(t) for t in self.p.history()] self.assertIn("human:sam", repr(pub))