Skip to content

fix: prevent backdated expense from draining future-dated grant#1124

Open
odesenfans wants to merge 2 commits into
mainfrom
od/credit-eager-timestamp-floor
Open

fix: prevent backdated expense from draining future-dated grant#1124
odesenfans wants to merge 2 commits into
mainfrom
od/credit-eager-timestamp-floor

Conversation

@odesenfans
Copy link
Copy Markdown
Collaborator

Summary

The eager-write drain in `_consume_address_credits` was filtering lots by the expense's expiration bound only, not by the lower bound on the lot's own `message_timestamp`. Messages can arrive out of order in the P2P pipeline, so a future-dated grant may already exist as a lot in the cache when an older expense lands. Without the lower bound the eager writer happily deducts from a lot that didn't yet exist at the expense's instant.

This is a gap that was introduced in #1122 (the eager-write lot cache PR) and hadn't been spotted before. `_rebuild_credit_lots_for_address` already enforces both bounds because it walks `credit_history` chronologically, so the next restart silently heals any divergence the eager writer produces. The fix brings the eager-write filter to parity with what the replay derives.

Change

One-line filter addition in `_consume_address_credits`:

```python
AlephCreditBalanceDb.message_timestamp <= message_timestamp,
```

Docstring updated to spell out both bounds.

Test plan

  • New regression test: backdated expense at T=2024-05 against a grant whose `message_timestamp` is T=2024-06 leaves the grant fully intact (`tests/db/test_credit_balances.py::test_backdated_expense_cannot_drain_future_grant`)
  • `hatch run linting:all` passes (ruff, black, isort, mypy, yamlfix, pyproject-fmt, check-sdist)
  • CI green on the full credit suite

Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ autodev could not generate a review for this PR (empty output from the model).

Click Re-request review to retry.

@odesenfans odesenfans requested a review from foxpatch-aleph May 12, 2026 23:03
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ autodev could not generate a review for this PR (empty output from the model).

Click Re-request review to retry.

The eager-write drain in _consume_address_credits filtered lots by the
expense's expiration bound but not by its lower-bound message_timestamp.
Messages can arrive out of order in the P2P pipeline, so an older expense
could land after a future-dated grant has already been cached as a lot, at
which point the eager writer would happily deduct from that lot. The
repair replay enforces the lower bound by walking credit_history
chronologically, so the next restart would silently correct the eager
state.

Add the missing ``AlephCreditBalanceDb.message_timestamp <= message_timestamp``
predicate so the eager writer matches what the replay derives, and cover
it with a regression test that processes a backdated expense against a
future-dated grant.
test_credit_transfer_non_whitelisted_sender seeded the sender with a
distribution dated 2023-01-01 but processed a transfer whose embedded
``time`` field is 1651050219.0 (2022-04-27). With the new timestamp
lower-bound filter on the eager-write drain, a transfer dated 2022 cannot
deduct from a lot whose message_timestamp is 2023 — that is the bug the
filter is supposed to catch. Move the seed back to 2022-01-01 so the lot
genuinely precedes the transfer in time.
@odesenfans odesenfans force-pushed the od/credit-eager-timestamp-floor branch from 2e097c6 to 9899c70 Compare May 13, 2026 21:54
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR correctly addresses a gap where backdated expenses could drain future-dated grants in the eager-write credit lot cache. The one-line filter addition (AlephCreditBalanceDb.message_timestamp <= message_timestamp) in _consume_address_credits brings the eager writer's eligibility logic to parity with the chronologically-walked repair replay. The regression test directly validates the fix, and the existing transfer test was adjusted to maintain correctness under the new constraint. The code is well-documented and clean.

src/aleph/db/accessors/balances.py (line 308): Correctly adds the lower bound on message_timestamp. The <= is the correct operator since grants at exactly the same timestamp as the expense should be drainable (the repair replay, which processes entries chronologically, would see the grant before the expense at the same timestamp).

src/aleph/db/accessors/balances.py (line 283): The updated docstring is thorough and clearly explains the rationale for both bounds. Good documentation.

tests/message_processing/test_process_posts.py (line 224): The adjusted timestamp (2022-01-01) correctly seeds the grant before the transfer's time field (2022-04-27), matching the new invariant. The explanatory comment is helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants