Add current_keys()/current_mac_addresses() to RollingKeyPairSource

[ubrt]

Same change as the upstream PR at malmeloo#262, for parawanderer/OpenTagViewer#17.

On the target branch, since it isn't main: OpenTagViewer pins parawanderer/FindMy.py@337381d, which is the tip of this branch rather than of main. The app's app/src/main/python/main.py imports MobileMeDelegateError, which exists only here — not in this fork's main, and not in malmeloo/main. So a merge into main alone would produce a commit the app cannot move its pin to; it needs one carrying both this change and the iCloud keychain work.

Happy to retarget if you'd rather take it on main and merge it across yourself — I just couldn't do the pin bump from there.

This should also become unnecessary in time: once the upstream PR lands and you rebase this branch onto a release containing it, the method arrives on its own.

[parawanderer]

Thanks for this — and for working out the base-branch question before opening it; your reasoning there is right, MobileMeDelegateError only exists on this branch so a main merge would give the app nothing to pin to.

I applied it on top of the branch tip (now 4f94015, so it'll want a rebase — no conflicts, it touches files nothing else has). 816 tests pass, ruff and basedpyright clean. The Python 3.10 to_bytes fix is correct and belongs in this PR rather than separately: mac_address is what the new method returns, so without it the feature is simply broken on the oldest supported Python.

Three things, in order of how much they'd change the code.

1. test_a_margin_widens_the_candidate_set_both_ways only tests one way

Every FindMyAccessory test here uses paired_at=now with no alignment update, so alignment_index == 0. Then get_min_index(now - 12h) returns -48, and keys_at returns set() for every negative index — so the widening those tests observe comes entirely from the forward side.

I checked by deleting the backwards half (keys_between(now, now + margin)): all six tests still pass.

That matters because the backwards half is the part your docstring says is the whole point — the accessory advertising a metre from the scanner and absent from its own candidate set. It can only happen when alignment is above zero, which is also the only place it can be tested:

accessory.update_alignment(now, 2880)   # a month old, as a real one would be

behind = {k.adv_key_bytes for k in accessory.keys_at(2879)}
assert behind <= {k.adv_key_bytes for k in accessory.current_keys(now, timedelta(hours=12))}

Passes as written; fails with the backwards half removed.

2. The return type makes the cheap path unreachable

Measured on a 30-day-old accessory aligned at index 2880:

call	cost	keys
current_keys(now, margin=12h), repeated	42 ms every call	100
margin=1h	4 ms	12
no margin, alignment fresh	0.4 ms	3

The SK ratchet is already cached, which is why a cold call is only ~20% slower than a warm one. What is not cached is derive_ps_key plus KeyPair construction — an EC operation per key, ~0.42 ms, on every call.

update_alignment(now, index) is what collapses that range, and it's already in the API. But current_keys returns set[KeyPair] with no index attached, so a caller that matches an advertisement cannot tell which index matched and cannot feed it back. keys_between yields (ind, key) pairs and the index is dropped one line later.

Since the docstring's use case is a scanning loop, that's the loop's cheap path being unreachable through this method — a caller who wants it has to bypass current_keys and call keys_between directly. An index-preserving variant, or returning the pairs, would fix it.

3. margin defaults to the failure the docstring describes

The docstring argues the margin is what makes an accessory findable, cites is_from's 12-hour window as precedent, and then defaults to timedelta(0). Someone who reads the signature and not the prose gets exactly the bug you're describing.

Worth either defaulting to the 12 hours you cite, or saying plainly in the docstring that the bare call is the unsafe one. Related: the margin covers drift since the last alignment, so it arguably wants to be a function of how stale that alignment is rather than a constant — near zero right after a hit, wider after a day.

---

Separately, and not for this PR: a memo on _AccessoryKeyGenerator._get_keypair takes the repeated 12-hour call from 42 ms to 6.7 ms, and halves the first one (both generators reach the same indices). That's a memory-for-time trade for @parawanderer to weigh, not something to fold in here.

[parawanderer]

Checked all three, and the rebase. 817 pass (809 on the branch plus your 8), ruff and basedpyright clean over the library.

The margin test now catches what it is for. Deleting the backwards half fails two tests where it previously failed none. The negative assertion — assert not (behind <= forward_only) — is the part I would not have thought to ask for: it stops the test quietly ceasing to test anything if the fixture's alignment ever drifts back to zero. That is the failure mode the original had, guarded against directly.

The index is right, not just present. I checked the lower-bound claim rather than taking it: of 100 candidates from a 12-hour margin, every reported index genuinely holds the key it is reported for. And update_alignment does ignore an index below the one it holds — fed 2800 against a held 2880, it stayed at 2880 — so the lower bound is safe in the direction it can be wrong.

Re-measured your numbers: 46 ms for the 12-hour margin, 0.5 ms after feeding the index back. The set-like reads survive the dict: in, iteration and len all behave as before on both methods.

On leaving the default at no margin — agreed, and your reasoning is better than my suggestion was. I offered "default to 12 hours, or say plainly the bare call is unsafe" as alternatives. Returning the index changes which of those is right: keeping alignment fresh is now something a caller can actually do, so the wide range is the exception rather than the thing every call should pay for. The docstring saying so, with the two working together, is the correct resolution.

Nothing further from me. (Edited: I first wrote that this was ahead of malmeloo#262 — it is not. You had already pushed the same commit there. The two now differ only in line wrapping applied by pre-commit.ci, so the API shape is the same in both places.)