Skip to content

test(stf): add unit tests for initiateValidatorExit #281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lodekeeper-z wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test(stf): add unit tests for initiateValidatorExit #281

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions src/state_transition/block/initiate_validator_exit.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,131 @@ pub fn initiateValidatorExit(
try std.math.add(u64, try validator.get("exit_epoch"), config.chain.MIN_VALIDATOR_WITHDRAWABILITY_DELAY),
);
}

// ─── Tests ───────────────────────────────────────────────────────────

const std_testing = std.testing;
const TestCachedBeaconState = @import("../test_utils/root.zig").TestCachedBeaconState;
const Node = @import("persistent_merkle_tree").Node;
const preset = @import("preset").preset;

test "initiateValidatorExit - no-op if already exited" {
const allocator = std_testing.allocator;
const num_validators = 256;
const pool_size = num_validators * 5;
var pool = try Node.Pool.init(allocator, pool_size);
defer pool.deinit();

var test_state = try TestCachedBeaconState.init(allocator, &pool, num_validators);
defer test_state.deinit();

const state = test_state.cached_state.state.castToFork(.electra);
Comment on lines +80 to +89
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is significant code duplication in the setup phase of all four new tests. Consider extracting this common setup logic into a helper function to improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle. A helper function could initialize TestCachedBeaconState and return the necessary context (test_state, state, epoch_cache, etc.), reducing boilerplate in each test.

References
  1. The style guide emphasizes simplicity and avoiding duplication (line 311: "Don't duplicate variables or take aliases to them"). While this rule is about runtime variables, the principle of avoiding duplication to improve maintainability is a core concept. (link)

const epoch_cache = test_state.cached_state.epoch_cache;

// Set validator 0's exit_epoch to something other than FAR_FUTURE_EPOCH
var validators = try state.validators();
var validator = try validators.get(0);
try validator.set("exit_epoch", 10);
try validator.set("withdrawable_epoch", 10 + test_state.config.chain.MIN_VALIDATOR_WITHDRAWABILITY_DELAY);

// Call initiateValidatorExit — should be a no-op
try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);

// exit_epoch should remain unchanged
try std_testing.expectEqual(@as(u64, 10), try validator.get("exit_epoch"));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test correctly checks that exit_epoch is unchanged for an already-exited validator. However, the setup on line 96 also sets withdrawable_epoch. To make the test more robust and ensure there are no unintended side effects, please also assert that withdrawable_epoch remains unchanged. This also improves assertion density, as recommended by the style guide.

    try std_testing.expectEqual(@as(u64, 10), try validator.get("exit_epoch"));
    try std_testing.expectEqual(10 + test_state.config.chain.MIN_VALIDATOR_WITHDRAWABILITY_DELAY, try validator.get("withdrawable_epoch"));
References
  1. The style guide recommends a high density of assertions to ensure correctness (lines 51-55: "Assert all function arguments and return values, pre/postconditions and invariants... The assertion density of the code must average a minimum of two assertions per function."). Adding this check improves test robustness and assertion density. (link)

}

test "initiateValidatorExit - sets exit and withdrawable epochs" {
const allocator = std_testing.allocator;
const num_validators = 256;
const pool_size = num_validators * 5;
var pool = try Node.Pool.init(allocator, pool_size);
defer pool.deinit();

var test_state = try TestCachedBeaconState.init(allocator, &pool, num_validators);
defer test_state.deinit();

const state = test_state.cached_state.state.castToFork(.electra);
const epoch_cache = test_state.cached_state.epoch_cache;

var validators = try state.validators();
var validator = try validators.get(0);

// Validator should start with FAR_FUTURE_EPOCH
try std_testing.expectEqual(FAR_FUTURE_EPOCH, try validator.get("exit_epoch"));

try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);

const exit_epoch = try validator.get("exit_epoch");
try std_testing.expect(exit_epoch != FAR_FUTURE_EPOCH);
try std_testing.expectEqual(
exit_epoch + test_state.config.chain.MIN_VALIDATOR_WITHDRAWABILITY_DELAY,
try validator.get("withdrawable_epoch"),
);
}

test "initiateValidatorExit - multiple exits" {
const allocator = std_testing.allocator;
const num_validators = 256;
const pool_size = num_validators * 5;
var pool = try Node.Pool.init(allocator, pool_size);
defer pool.deinit();

var test_state = try TestCachedBeaconState.init(allocator, &pool, num_validators);
defer test_state.deinit();

const state = test_state.cached_state.state.castToFork(.electra);
const epoch_cache = test_state.cached_state.epoch_cache;

var validators = try state.validators();

// Exit multiple validators and verify each gets a valid exit epoch
var v0 = try validators.get(0);
var v1 = try validators.get(1);
var v2 = try validators.get(2);
Comment on lines +150 to +152
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable names v0, v1, and v2 are abbreviations. The repository style guide (line 221) advises: "Do not abbreviate variable names". Using more descriptive names like validator_0, validator_1, and validator_2 would improve readability and align with the style guide. Please update their usages throughout the test as well.

    var validator_0 = try validators.get(0);
    var validator_1 = try validators.get(1);
    var validator_2 = try validators.get(2);
References
  1. The style guide (line 221) states: "Do not abbreviate variable names". This improves code clarity and maintainability. (link)


try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, v0);
try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, v1);
try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, v2);

const exit0 = try v0.get("exit_epoch");
const exit1 = try v1.get("exit_epoch");
const exit2 = try v2.get("exit_epoch");

// All should have valid exit epochs
try std_testing.expect(exit0 != FAR_FUTURE_EPOCH);
try std_testing.expect(exit1 != FAR_FUTURE_EPOCH);
try std_testing.expect(exit2 != FAR_FUTURE_EPOCH);
Comment on lines +163 to +165
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test verifies that the validators have been assigned a valid exit epoch. To make the test more robust, it should also assert the relationship between the exit epochs. Since the validators are exited sequentially, their exit epochs should be non-decreasing (i.e., exit0 <= exit1 <= exit2).

    try std_testing.expect(exit0 != FAR_FUTURE_EPOCH);
    try std_testing.expect(exit1 != FAR_FUTURE_EPOCH);
    try std_testing.expect(exit2 != FAR_FUTURE_EPOCH);
    try std_testing.expect(exit0 <= exit1);
    try std_testing.expect(exit1 <= exit2);


// All should have valid withdrawable epochs
const delay = test_state.config.chain.MIN_VALIDATOR_WITHDRAWABILITY_DELAY;
try std_testing.expectEqual(exit0 + delay, try v0.get("withdrawable_epoch"));
try std_testing.expectEqual(exit1 + delay, try v1.get("withdrawable_epoch"));
try std_testing.expectEqual(exit2 + delay, try v2.get("withdrawable_epoch"));
}

test "initiateValidatorExit - second call is no-op" {
const allocator = std_testing.allocator;
const num_validators = 256;
const pool_size = num_validators * 5;
var pool = try Node.Pool.init(allocator, pool_size);
defer pool.deinit();

var test_state = try TestCachedBeaconState.init(allocator, &pool, num_validators);
defer test_state.deinit();

const state = test_state.cached_state.state.castToFork(.electra);
const epoch_cache = test_state.cached_state.epoch_cache;

var validators = try state.validators();
var validator = try validators.get(0);

// First call — should set exit epoch
try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);
const exit_epoch = try validator.get("exit_epoch");
try std_testing.expect(exit_epoch != FAR_FUTURE_EPOCH);

// Second call — should be a no-op
try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);
try std_testing.expectEqual(exit_epoch, try validator.get("exit_epoch"));
Comment on lines +191 to +197
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test correctly asserts that exit_epoch is not changed on the second call, confirming idempotency. For completeness, it should also assert that withdrawable_epoch is not changed, as this is also set by initiateValidatorExit. This ensures the no-op behavior applies to all fields modified by the function.

    try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);
    const exit_epoch = try validator.get("exit_epoch");
    const withdrawable_epoch = try validator.get("withdrawable_epoch");
    try std_testing.expect(exit_epoch != FAR_FUTURE_EPOCH);

    // Second call — should be a no-op
    try initiateValidatorExit(.electra, test_state.config, epoch_cache, state, validator);
    try std_testing.expectEqual(exit_epoch, try validator.get("exit_epoch"));
    try std_testing.expectEqual(withdrawable_epoch, try validator.get("withdrawable_epoch"));
References
  1. The style guide recommends a high density of assertions to ensure correctness (lines 51-55). Adding this check for withdrawable_epoch ensures all aspects of the function's idempotency are tested. (link)

}
Loading