fix(lucidly): distinct keys for same-chain parks at the same clock value#121
Open
kite-builds wants to merge 1 commit into
Open
fix(lucidly): distinct keys for same-chain parks at the same clock value#121kite-builds wants to merge 1 commit into
kite-builds wants to merge 1 commit into
Conversation
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
LucidlyAutoPark.rebalance()keys eachParkedPositionbyf"{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 secondParkedPositionsilently overwrites the first._total_parked(andvault.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.yield_accrued_usdfor the overwritten park is lost (eachParkedPositiontracks 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 readk.split(":")[0]andk.startswith(f"{chain}:"), so the extra:<seq>suffix is backward-compatible.Test
Added
test_two_parks_same_chain_same_clock_are_distinct_positionsusing the injectable clock:report["positions"] == 1(collision) → fails.report["positions"] == 2and matchesr1+r2parked amounts.Full suite: 223 passed, 62 skipped.
ruff checkclean on changed files.