diff --git a/app/bounty_api.py b/app/bounty_api.py index 7595a81..68ffc92 100644 --- a/app/bounty_api.py +++ b/app/bounty_api.py @@ -244,7 +244,7 @@ async def api_pay_bounty( if CONTROL_CHAR_RE.search(note): raise HTTPException( status_code=400, - detail="verifier_result.note must not contain control characters", + detail="note must not contain control characters", ) verifier_result["note"] = note[:240] with session_scope(db_url) as session: diff --git a/app/treasury.py b/app/treasury.py index b7d042d..5318ba3 100644 --- a/app/treasury.py +++ b/app/treasury.py @@ -265,13 +265,19 @@ def _validate_create_bounty_proposal(session: Session, payload: dict[str, Any]) issue_url = str(payload["issue_url"]) if not _github_issue_url_matches(repo, issue_number, issue_url): raise LedgerError("issue_url must match repo and issue_number") - if _has_pending_create_bounty_proposal(session, repo=repo, issue_number=issue_number): + exclude_id = int(payload["_proposal_id"]) if "_proposal_id" in payload else None + if _has_pending_create_bounty_proposal( + session, repo=repo, issue_number=issue_number, exclude_proposal_id=exclude_id + ): raise LedgerError("create_bounty proposal already pending") reserved = parse_mrwk_amount(str(payload["reward_mrwk"])) * int(payload["max_awards"]) + pending_reserved = _pending_create_bounty_reserved_microunits( + session, through_proposal_id=exclude_id + ) + if exclude_id is not None: + pending_reserved -= reserved if ( - _epoch_reserved_microunits(session, _db_now()) - + _pending_create_bounty_reserved_microunits(session) - + reserved + _epoch_reserved_microunits(session, _db_now()) + pending_reserved + reserved > TREASURY_EPOCH_RESERVE_CAP_MICRO ): raise LedgerError("treasury epoch reserve cap exceeded") @@ -294,7 +300,10 @@ def _validate_pay_bounty_proposal(session: Session, payload: dict[str, Any]) -> raise LedgerError("bounty not found") if bounty.status != "open": raise LedgerError("bounty is not open") - if _has_pending_close_bounty_proposal(session, bounty_id=bounty_id): + exclude_id = int(payload["_proposal_id"]) if "_proposal_id" in payload else None + if _has_pending_close_bounty_proposal( + session, bounty_id=bounty_id, exclude_proposal_id=exclude_id + ): raise LedgerError("bounty has pending close proposal") submission_url = str(payload["submission_url"]) if ( @@ -304,7 +313,11 @@ def _validate_pay_bounty_proposal(session: Session, payload: dict[str, Any]) -> is not None ): raise LedgerError("submission already paid") - pending_payouts = _pending_pay_bounty_payloads(session, bounty_id) + pending_payouts = [ + (proposal, pending_payload) + for proposal, pending_payload in _pending_pay_bounty_payloads(session, bounty_id) + if exclude_id is None or proposal.id != exclude_id + ] if any( str(pending_payload["submission_url"]) == submission_url for _, pending_payload in pending_payouts @@ -326,9 +339,14 @@ def _validate_close_bounty_proposal(session: Session, payload: dict[str, Any]) - raise LedgerError("bounty not found") if bounty.status != "open": raise LedgerError("bounty is not open") - if _has_pending_close_bounty_proposal(session, bounty_id=bounty_id): + exclude_id = int(payload["_proposal_id"]) if "_proposal_id" in payload else None + if _has_pending_close_bounty_proposal( + session, bounty_id=bounty_id, exclude_proposal_id=exclude_id + ): raise LedgerError("close_bounty proposal already pending") - if _has_pending_pay_bounty_proposal(session, bounty_id=bounty_id): + if _has_pending_pay_bounty_proposal( + session, bounty_id=bounty_id, exclude_proposal_id=exclude_id + ): raise LedgerError("bounty has pending payout proposals") @@ -528,11 +546,15 @@ def execute_treasury_proposal( payload = _canonical_payload(proposal.action, proposal_payload(proposal)) if _proposal_hash(proposal.action, payload) != proposal.payload_hash: raise LedgerError("proposal payload hash mismatch") + validation_payload = {**payload, "_proposal_id": proposal.id} if proposal.action == "create_bounty": + _validate_create_bounty_proposal(session, validation_payload) result, ledger_sequence = _execute_create_bounty(session, payload, now) elif proposal.action == "pay_bounty": + _validate_pay_bounty_proposal(session, validation_payload) result, ledger_sequence = _execute_pay_bounty(session, payload) elif proposal.action == "close_bounty": + _validate_close_bounty_proposal(session, validation_payload) result, ledger_sequence = _execute_close_bounty(session, payload) else: raise LedgerError("unsupported treasury action") diff --git a/app/treasury_routes.py b/app/treasury_routes.py index e24aee8..a02b468 100644 --- a/app/treasury_routes.py +++ b/app/treasury_routes.py @@ -30,7 +30,22 @@ def _proposal_error(exc: LedgerError) -> HTTPException: detail = str(exc) if detail in {"proposal not found", "bounty not found"}: return HTTPException(status_code=404, detail=detail) - if detail in {"proposal already executed", "submission already paid"}: + if detail in { + "bounty has pending close proposal", + "bounty has pending payout proposals", + "bounty is not open", + "close_bounty proposal already pending", + "create_bounty proposal already pending", + "pay_bounty proposal already pending for submission", + "pending payout proposals exceed bounty remaining awards", + "pending payout proposals exceed bounty reserve", + "proposal already executed", + "proposal delay has not elapsed", + "proposal has blocking challenge", + "proposal is not pending", + "submission already paid", + "treasury epoch reserve cap exceeded", + }: return HTTPException(status_code=409, detail=detail) return HTTPException(status_code=400, detail=detail) diff --git a/tests/test_security.py b/tests/test_security.py index e6b3a7b..1dba649 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -975,9 +975,7 @@ def test_bounty_payment_proof_rejects_control_character_metadata(sqlite_url: str accepted_by="maintainer\nops", verifier_result={"label": "mrwk:accepted"}, ) - with pytest.raises( - LedgerError, match="verifier_result.note must not contain control characters" - ): + with pytest.raises(LedgerError, match="note must not contain control characters"): pay_bounty( session, bounty_id=second_bounty.id, @@ -1027,7 +1025,7 @@ def test_admin_payout_api_rejects_control_character_note( ) assert response.status_code == 400 - assert response.json()["detail"] == ("verifier_result.note must not contain control characters") + assert response.json()["detail"] == ("note must not contain control characters") with session_scope(sqlite_url) as session: assert get_balance(session, "github:alice") == 0 diff --git a/tests/test_treasury_proposals.py b/tests/test_treasury_proposals.py index 3b93e2b..81702a9 100644 --- a/tests/test_treasury_proposals.py +++ b/tests/test_treasury_proposals.py @@ -154,7 +154,7 @@ def test_proposal_execution_requires_admin_delay_and_is_idempotent( ) assert unauthenticated.status_code == 401 - assert too_early.status_code == 400 + assert too_early.status_code == 409 assert too_early.json()["detail"] == "proposal delay has not elapsed" assert executed.status_code == 200 assert executed.json()["status"] == "executed" @@ -543,6 +543,52 @@ def test_manual_payout_freezes_github_destination_at_proposal_creation( assert get_balance(session, wallet_address) == 0 +def test_execution_revalidates_stale_pay_proposal_after_close( + sqlite_url: str, monkeypatch: pytest.MonkeyPatch +) -> None: + client = _client(sqlite_url, monkeypatch) + with session_scope(sqlite_url) as session: + ensure_genesis(session) + bounty = create_bounty( + session, + repo="ramimbo/mergework", + issue_number=89, + issue_url="https://github.com/ramimbo/mergework/issues/89", + title="Stale payout proposal", + reward_mrwk="5", + acceptance="Contributor comments with proof.", + ) + bounty_id = bounty.id + proposal = client.post( + f"/api/v1/bounties/{bounty_id}/pay", + headers=ADMIN_HEADERS, + json={ + "to_account": "github:bob", + "submission_url": "https://github.com/ramimbo/mergework/pull/89", + "accepted_by": "maintainer", + }, + ).json() + with session_scope(sqlite_url) as session: + from app.ledger.service import close_bounty + + close_bounty( + session, + bounty_id=bounty_id, + closed_by="maintainer", + reference="https://github.com/ramimbo/mergework/issues/89", + ) + _make_executable(sqlite_url, proposal["id"]) + + executed = client.post( + f"/api/v1/treasury/proposals/{proposal['id']}/execute", headers=ADMIN_HEADERS + ) + + assert executed.status_code == 409 + assert executed.json()["detail"] == "bounty is not open" + with session_scope(sqlite_url) as session: + assert get_balance(session, "github:bob") == 0 + + def test_challenges_require_accepted_work_and_can_block_invalid_proposals( sqlite_url: str, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -595,7 +641,7 @@ def test_challenges_require_accepted_work_and_can_block_invalid_proposals( assert subjective.json()["status"] == "noted" assert blocking.status_code == 200 assert blocking.json()["status"] == "accepted_blocking" - assert execute.status_code == 400 + assert execute.status_code == 409 assert execute.json()["detail"] == "proposal has blocking challenge" with session_scope(sqlite_url) as session: assert session.scalar(select(func.count(TreasuryChallenge.id))) == 2