Skip to content

fix(lucidly): distinct keys for same-chain parks at the same clock value#121

Merged
abhicris merged 1 commit into
kcolbchain:mainfrom
kite-builds:fix/lucidly-position-key-collision
Jun 10, 2026
Merged

fix(lucidly): distinct keys for same-chain parks at the same clock value#121
abhicris merged 1 commit into
kcolbchain:mainfrom
kite-builds:fix/lucidly-position-key-collision

Conversation

@kite-builds

Copy link
Copy Markdown
Contributor

Bug

LucidlyAutoPark.rebalance() keys each ParkedPosition by f"{chain}:{self.clock()}". The adapter accepts an injectable clock (the docstring states "Pluggable clock for deterministic tests"), so two parks on the same chain at the same clock value produce the same dict key — the second ParkedPosition silently overwrites the first.

_total_parked (and vault.balance) still account for both deposits, so the in-memory positions map diverges from the parked total. Concretely:

  • yield_report()["positions"] under-counts active positions.
  • Per-position yield_accrued_usd for the overwritten park is lost (each ParkedPosition tracks its own yield).

With a fixed/deterministic clock this is guaranteed on the 2nd same-chain park; with real time.time() it can still collide on rapid successive parks.

Fix

Append a monotonic per-instance sequence to the position key so distinct park events never collide. yield_report() / status() only read k.split(":")[0] and k.startswith(f"{chain}:"), so the extra :<seq> suffix is backward-compatible.

Test

Added test_two_parks_same_chain_same_clock_are_distinct_positions using the injectable clock:

  • Before: report["positions"] == 1 (collision) → fails.
  • After: report["positions"] == 2 and matches r1+r2 parked amounts.

Full suite: 223 passed, 62 skipped. ruff check clean on changed files.

ParkedPositions were keyed `chain:clock()`. Since LucidlyAutoPark takes
an injectable deterministic clock (documented "Pluggable clock for
deterministic tests"), two parks on the same chain at the same clock
value mapped to the same dict key, so the second ParkedPosition silently
overwrote the first. `_total_parked` still summed both, so the positions
dict (and yield_report()["positions"] / per-position yield_accrued_usd)
diverged from the parked total. Real time.time() can also collide on
rapid successive parks.

Fix: append a monotonic per-instance sequence to the key so distinct
park events never collide. yield_report()/status() only parse
`k.split(":")[0]` and `k.startswith(f"{chain}:")`, so the extra suffix
is safe. +1 regression test (red before: positions==1, green after: 2).
@abhicris

Copy link
Copy Markdown
Contributor

🤖 Audit verdict: safe

Legitimate bug fix adding a monotonic counter to prevent position key collisions when multiple parks occur at identical clock values; no malicious payloads, credential leaks, supply-chain risks, or remaining bugs detected.

Audited by the kcolbchain PR pipeline. See pipeline docs.

@abhicris abhicris merged commit 69c5004 into kcolbchain:main Jun 10, 2026
2 checks passed
@abhicris

Copy link
Copy Markdown
Contributor

Merged — thanks, @kite-builds. That's 8 merged PRs to kcolbchain now.

You're a Fellow — when paid research, partner-org work, or grant milestones come up, you're in the first invite pool.

Next-up issues across the org: https://kcolbchain.com/invitations/

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants