diff --git a/src/pycyphal2/__init__.py b/src/pycyphal2/__init__.py index 3881714b..30a93f5e 100644 --- a/src/pycyphal2/__init__.py +++ b/src/pycyphal2/__init__.py @@ -155,7 +155,7 @@ async def main(): from ._transport import Transport as Transport from ._transport import TransportArrival as TransportArrival -__version__ = "2.0.0.dev4" +__version__ = "2.0.0.dev5" # pdoc needs __all__ to display re-exported members. __all__ = [ diff --git a/src/pycyphal2/_node.py b/src/pycyphal2/_node.py index f3045223..9cb510ec 100644 --- a/src/pycyphal2/_node.py +++ b/src/pycyphal2/_node.py @@ -246,7 +246,9 @@ def compute_subject_id(topic_hash: int, evictions: int, modulus: int) -> int: """Compute the subject-ID for a topic given its hash, evictions, and subject-ID modulus.""" if evictions >= EVICTIONS_PINNED_MIN: return 0xFFFFFFFF - evictions - return SUBJECT_ID_PINNED_MAX + 1 + ((topic_hash + (evictions * evictions)) % modulus) + h = topic_hash % modulus + e = evictions % modulus + return SUBJECT_ID_PINNED_MAX + 1 + ((h + ((e * e) % modulus)) % modulus) # ===================================================================================================================== diff --git a/tests/test_integration.py b/tests/test_integration.py index adf91a83..407114cb 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -270,12 +270,12 @@ async def test_subject_id_computation(): """Verify subject-ID computation matches the reference formula.""" modulus = 8378431 # 23bit - # Non-pinned: 0x2000 + ((hash + evictions^2) % modulus) + # Non-pinned: 0x2000 + (((hash % modulus) + ((evictions % modulus)^2 % modulus)) % modulus) sid = compute_subject_id(0xDEADBEEF, 0, modulus) assert sid == 0x2000 + (0xDEADBEEF % modulus) sid = compute_subject_id(0xDEADBEEF, 3, modulus) - assert sid == 0x2000 + ((0xDEADBEEF + 9) % modulus) + assert sid == 0x2000 + (((0xDEADBEEF % modulus) + ((3 % modulus) ** 2 % modulus)) % modulus) # Pinned: UINT32_MAX - evictions sid = compute_subject_id(0xDEADBEEF, EVICTIONS_PINNED_MIN, modulus) diff --git a/tests/test_topic.py b/tests/test_topic.py index 96f040f2..8e7dcb98 100644 --- a/tests/test_topic.py +++ b/tests/test_topic.py @@ -40,21 +40,40 @@ def test_compute_subject_id_pinned_boundary(): def test_compute_subject_id_non_pinned_zero_evictions(): """Non-pinned with zero evictions: offset + hash % modulus.""" - topic_hash = rapidhash("my/topic") - sid = compute_subject_id(topic_hash, 0, DEFAULT_MODULUS) - expected = SUBJECT_ID_PINNED_MAX + 1 + (topic_hash % DEFAULT_MODULUS) - assert sid == expected + for topic_hash in (0, 1, rapidhash("my/topic"), (1 << 64) - 1): + sid = compute_subject_id(topic_hash, 0, DEFAULT_MODULUS) + expected = SUBJECT_ID_PINNED_MAX + 1 + (topic_hash % DEFAULT_MODULUS) + assert sid == expected def test_compute_subject_id_non_pinned_with_evictions(): - """Non-pinned formula: offset + (hash + evictions^2) % modulus.""" + """Non-pinned formula: offset + ((hash % modulus) + ((evictions % modulus)^2 % modulus)) % modulus.""" topic_hash = rapidhash("some/topic") for ev in (1, 2, 5, 100): sid = compute_subject_id(topic_hash, ev, DEFAULT_MODULUS) - expected = SUBJECT_ID_PINNED_MAX + 1 + ((topic_hash + ev * ev) % DEFAULT_MODULUS) + expected = ( + SUBJECT_ID_PINNED_MAX + + 1 + + ( + ((topic_hash % DEFAULT_MODULUS) + (((ev % DEFAULT_MODULUS) * (ev % DEFAULT_MODULUS)) % DEFAULT_MODULUS)) + % DEFAULT_MODULUS + ) + ) assert sid == expected +def test_compute_subject_id_non_pinned_does_not_wrap_uint64_sum(): + topic_hash = (1 << 64) - 1 + evictions = EVICTIONS_PINNED_MIN - 1 + sid = compute_subject_id(topic_hash, evictions, DEFAULT_MODULUS) + uint64_wrapping = ( + SUBJECT_ID_PINNED_MAX + 1 + (((topic_hash + (evictions * evictions)) & ((1 << 64) - 1)) % DEFAULT_MODULUS) + ) + assert sid == 49564 + assert uint64_wrapping == 74897 + assert sid != uint64_wrapping + + def test_compute_subject_id_evictions_changes_sid(): """Different eviction counts should generally produce different subject-IDs.""" topic_hash = rapidhash("test/evictions") @@ -70,7 +89,14 @@ def test_compute_subject_id_just_below_pinned(): ev = EVICTIONS_PINNED_MIN - 1 topic_hash = 12345 sid = compute_subject_id(topic_hash, ev, DEFAULT_MODULUS) - expected = SUBJECT_ID_PINNED_MAX + 1 + ((topic_hash + ev * ev) % DEFAULT_MODULUS) + expected = ( + SUBJECT_ID_PINNED_MAX + + 1 + + ( + ((topic_hash % DEFAULT_MODULUS) + (((ev % DEFAULT_MODULUS) * (ev % DEFAULT_MODULUS)) % DEFAULT_MODULUS)) + % DEFAULT_MODULUS + ) + ) assert sid == expected @@ -361,9 +387,10 @@ async def test_gossip_unknown_collision_we_lose(): # produces the same subject-ID and a very high lage. remote_lage = 50 # Very old. # Build a fake hash that produces the same SID as our topic. - # Since sid = PINNED_MAX + 1 + (hash + ev^2) % modulus, we need: - # (remote_hash + 0) % modulus == (topic.hash + old_evictions^2) % modulus - target_remainder = (topic.hash + old_evictions * old_evictions) % DEFAULT_MODULUS + target_remainder = ( + (topic.hash % DEFAULT_MODULUS) + + (((old_evictions % DEFAULT_MODULUS) * (old_evictions % DEFAULT_MODULUS)) % DEFAULT_MODULUS) + ) % DEFAULT_MODULUS # Pick remote_hash such that remote_hash % modulus == target_remainder AND remote_hash != topic.hash. remote_hash = target_remainder + DEFAULT_MODULUS # Different from topic.hash but same modular result.