diff --git a/backend/consensus/vrf.py b/backend/consensus/vrf.py index 55c953600..c1ebe6061 100644 --- a/backend/consensus/vrf.py +++ b/backend/consensus/vrf.py @@ -1,17 +1,27 @@ -from datetime import datetime import numpy as np +from numpy.random import Generator as RNG from backend.consensus.base import DEFAULT_VALIDATORS_COUNT def get_validators_for_transaction( nodes: list[dict], num_validators: int | None = None, - rng=np.random.default_rng(seed=int(datetime.now().timestamp())), + rng: RNG | None = None, ) -> list[dict]: """ Returns subset of validators for a transaction. The selelction and order is given by a random sampling based on the stake of the validators. + + Args: + nodes: List of validator dicts, each with a "stake" key. + num_validators: How many validators to select. Defaults to DEFAULT_VALIDATORS_COUNT. + rng: Optional numpy Generator instance. A fresh default_rng() is created per call + when not provided — avoids the mutable-default-argument pitfall where a single + shared Generator would make the selection sequence predictable across calls. """ + if rng is None: + rng = np.random.default_rng() + if num_validators is None: num_validators = DEFAULT_VALIDATORS_COUNT diff --git a/tests/unit/test_vrf.py b/tests/unit/test_vrf.py index 95473770e..15f6869ea 100644 --- a/tests/unit/test_vrf.py +++ b/tests/unit/test_vrf.py @@ -66,3 +66,26 @@ def choice_mock(a, p, size, replace): rng.choice.assert_called_once() assert validators == [{"stake": 3}, {"stake": 2}, {"stake": 1}] + + +def test_get_validators_for_transaction_independent_rng_per_call() -> None: + """ + Regression test for #1733: mutable default argument caused a single RNG instance + to be shared across all calls, making the selection sequence predictable. + + Each call without an explicit rng must use an independent generator so that + the selections are not correlated across invocations. + """ + nodes = [{"stake": i + 1} for i in range(10)] + + # Collect first-element selections over many independent calls. + # With a shared RNG the sequence would be perfectly deterministic; with + # independent generators the distribution should spread across all nodes. + first_selections = [get_validators_for_transaction(nodes, 1)[0] for _ in range(200)] + unique_first = {tuple(v.items()) for v in first_selections} + + # All 10 nodes should appear as first choice at least once over 200 trials. + assert len(unique_first) > 1, ( + "Only one unique first-validator returned across 200 calls — " + "the RNG is likely being shared (mutable default argument bug)." + )