feat: drain credit lots soonest-expiring first#1123
Conversation
Switch the eager-write drain policy from emission-order to expiration-order: both _consume_address_credits and the repair replay in _rebuild_credit_lots_for_address now spend the soonest-expiring lot first, with emission order as the tiebreaker for replay stability. Non-expiring lots are drained last (NULLS LAST). This matches the user-facing "use it or lose it" model: a spend should consume credits that are about to lapse before consuming longer-lived ones. Reads are unchanged: get_credit_balance(_with_details) already sum over still-valid lots regardless of order. Existing tests that encoded emission-first expectations are updated. The two FIFO scenario tests are renamed to describe the new policy directly, and the two details tests now assert the expiration-first breakdown of the remaining lots.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Correctly implements expiration-first credit lot draining in both the eager-writer SQL path and the repair replay. The drain order (expiration_date ASC NULLS LAST with emission-order tiebreakers) is consistent between both code paths. Test expectations have been accurately recalculated for the new policy. The consensus risk is acknowledged in the PR description and is a deployment strategy decision, not a code defect.
src/aleph/db/accessors/balances.py (line 314): The SQL ORDER BY correctly implements expiration-first with NULLS LAST. The emission-order tiebreakers ensure deterministic replay-stable ordering for lots sharing the same expiration.
src/aleph/repair.py (line 85): The _drain_order_key correctly mirrors the SQL ordering. Python's False < True ensures non-NULL expiration dates sort before NULL, matching NULLS LAST.
Summary
Follow-up to #1122 (the eager-write lot cache). Switches the drain policy from emission-order to expiration-order:
_consume_address_creditsnow drains lotsORDER BY expiration_date ASC NULLS LAST, message_timestamp ASC, credit_ref ASC, credit_index ASC. Emission order remains the tiebreaker for replay stability._rebuild_credit_lots_for_addressmirrors the same drain policy when replaying history.This is the end state #1121 was aiming for, but on top of the per-lot keying we already shipped instead of the bucket-per-expiration shape. Reads (
get_credit_balance,get_credit_balance_with_details, paginated listings) are unchanged: they already sum over still-valid lots regardless of order.Behavioural change
User-facing: a spend now consumes the credit lot that is closest to expiring first, before touching longer-lived (or non-expiring) credits. This matches the "use it or lose it" model end-users expect.
The switch is unconditional — there is no time gate. After this PR ships:
_rebuild_credit_lots_for_address) replays the entirecredit_historyunder the new policy, so per-address lot states derived from history may differ from what was actually used to validate past transfers (which were processed under emission-first).In practice every node converges on the same balances after restart because they all replay the same
credit_historywith the same code. But any consumer that observed pre-deploy balances and acted on them under the old policy is no longer reproducible from the post-deploy node.If we want a hard cutover that preserves historical state byte-for-byte, the alternative is to gate on
message_timestampwith a newCREDIT_EXPIRATION_FIRST_CUTOFF_TIMESTAMP(mirroringCREDIT_PRECISION_CUTOFF_TIMESTAMP): emission-first for< cutoff, expiration-first for>= cutoff. Happy to add that gate before merge — say the word.Test plan