fix refund bugs

[k-nkym]

概要

melon-db の返金処理 restore()(refund/void 共通実装)に、独立した金銭正当性バグを 2 件見つけたので修正しました。restore() の隣接コードを触るため 1 本の PR にまとめています。

バグ ① 並行返金による過剰返金

決済行をロックせず「返金済み額」を集計していたため、同一決済への並行返金が上限チェックの競合状態を通過し、過剰返金が成立していました。バケット共有時(例: ¥1000 バケットから ¥500 決済 2 件)に発生し、バケット側の CHECK (remaining_amount <= original_amount)(migrations/0001_init.sql:59)は合計がバケット原資を超えない限り発火しないため捕まりません。

pay() が候補バケットに既に行っている防御(ops.rs:1258 の FOR UPDATE)を、restore() の決済行にも適用しました。

再現テスト concurrent_refunds_never_over_refund は修正前 10/10 回失敗(典型例: 5 件中 4 件が成功してしまう)。

バグ ② 失効済みバケットへの返金

失効・スイープ済みバケットへ返金すると status が 'active' に戻る一方 expires_at は過去のまま残ります。balance() は expires_at > now でフィルタする(ops.rs:343)ため、利用者からは残高が消えたまま見えず、次回スイープで同じ金額が再度失効益に計上されます(二重計上)。

返金計画の段階で対象バケットの expires_at を確認し、過去なら新エラー DbError::RefundIntoExpiredBucket(422 REFUND_INTO_EXPIRED_BUCKET)で返金全体を拒否するようにしました。部分成立は許しません。

再現テスト refund_does_not_resurrect_an_expired_bucket で、修正前は返金が成功し直後の再スイープで同額(¥400)が再計上されることを確認しています。

変更

• fix refund race condition — バグ①
• reorder refund restoration steps — 挙動不変のリファクタ(バグ②の下ごしらえ、返金計画を書き込み前に確定)
• reject refund into an expired bucket — バグ②、および melon-server/docs/api.md/端末 UI への反映

各コミットは単体で clippy・全テストが通ります(git rebase --exec で確認)。

検証

• cargo fmt --all --check / cargo clippy --workspace --all-targets --locked -- -D warnings / cargo test --workspace --locked 全通過
• 新規テスト 2 件は、対応する修正を一時的に外して失敗(過剰返金・二重失効益)を再現した上でコミット

既知の制約

フォークからの PR には FELICA_RS_TOKEN リポジトリシークレットが渡らないため、この PR の CI (.github/workflows/ci.yml) は失敗します(ワークフロー冒頭のコメントに明記されている既知の制約)。ローカルでは上記の通り全チェック通過済みです。

[soltia48]

レビューしました。結論として race condition の修正そのものは妥当 です。

SELECT … FROM transactions WHERE id = $1 FOR UPDATE によって、同一 payment に対する並行 restore() が READ COMMITTED 下でも正しく直列化されます（ロック取得後の exists / already の読み取りは新しいスナップショットを見るため）。テストが狙っている二重返金は実際に塞がっています。復元プランを INSERT … ON CONFLICT DO NOTHING より前に移す変更も、このロックがある前提では挙動中立です。デッドロック環も増えていません（pay() は merchants→buckets、expire_due は SKIP LOCKED、payment の transaction 行より先に bucket をロックする経路はありません）。

その上で 3 点コメントします。1 と 2 は実装の指摘、3 はドキュメントです。

---

1. 有効期限ガードが bucket をロックしていない（crates/melon-db/src/ops.rs）

復元プランを組む際の SELECT は tb.expires_at をロックなしで読んでいます。

SELECT le.bucket_id, le.amount, tb.expires_at FROM ledger_entries le
  JOIN topup_buckets tb ON tb.id = le.bucket_id
 WHERE le.transaction_id = $1 AND le.kind = 'payment' ORDER BY le.seq

このチェックと、後段の

UPDATE topup_buckets SET remaining_amount = remaining_amount + $1, status = 'active' WHERE id = $2

の間に並行する expire_due() のスイープが入り込むと、失効直後の bucket を status = 'active' で復活させてしまいます。結果として breakage の二重計上が起こり得ます。本 PR が閉じようとしているのと同じ TOCTOU の形です。

SELECT に FOR UPDATE OF tb を付けて bucket 行ごとロックしてください。pay() が有効期限の早い順に消費する以上、le.seq 順のロック順序は payment 間で一貫するので、追加のデッドロック要因にはならないはずです。

なお下の 2 を採用する場合、「有効か失効か」で処理を分岐させることになるため、判定根拠である expires_at をロックして読むことは必須になります。1 と 2 は独立ではなく、セットで直す形になります。

2. void() を失効済みバケットでも成功させたい

void() も同じ restore() を通るため、寄与バケットが 1 つでも失効した時点で、誤請求に対する技術的リバーサルが恒久的に不可能になります。返金の拒否は妥当ですが、void は技術的訂正なので常に成功できる必要があると考えます。

そこで refund は現状の 422 拒否のまま、void のみ「復元して即失効」させる 形を提案します。restore() に kind 由来のポリシーを渡し、reversal かつバケットが失効済みの場合だけ次のように書きます。

• status = 'active' への書き戻しはしない（バケットは失効のまま）
• reversal ポスティング +restore_amt は通常どおり記帳する（取引の打ち消しが台帳上で完結する）
• 同時に expiry ポスティング -restore_amt を記帳し、remaining_amount を 0 に戻す

会計的にはこれが実態と一致します。その支払いが最初から無かったとしたら、その価値はバケットに残ったまま期限到来で失効益（breakage）になっていたはずなので、void 後に breakage へ計上するのが正しい姿です。二重計上にもなりません — expire_due() は status = 'active' の行しか拾わないので、二度目のスイープは来ません。利用者から見た復活残高はゼロで、これも正しい挙動です。

スキーマ変更は不要です。0001_init.sql の CHECK は expiry の transaction_id NULL を許容し、amount < 0 も満たします。balance() は expires_at > now で除外するので影響ありません。

インターフェース面では、RefundIntoExpiredBucket は refund 専用エラーとして残し、void 側は成功レスポンスに「失効により復元されなかった額」を含める形が素直だと思います。運用上の注意として、expired_amount / 未使用残高レポートが void のたびに動く点は共有が必要です。

3. ドキュメントの更新

docs/domain.md の返金/取消の節に「元バケットが既に失効済みなら復元は即時利用不可(失効価値は復活しない)」という記述が残っており、本 PR のハード拒否と食い違っています。2 を入れると refund と void で挙動が分かれるので、両方を明記する形に書き換えてください。

docs/api.md も、新しい 422 が refund セクションにしか記載されておらず、void セクションは「レスポンスは常に refund 形式」のままです。2 を入れるなら void のレスポンスに失効分のフィールドが増えるため、あわせて更新をお願いします。