Skip to content

test(stf): add unit tests for processDepositRequest #293

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 2 commits into
base: main
Choose a base branch
from
Open

test(stf): add unit tests for processDepositRequest #293

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e814805
test(stf): add unit tests for processDepositRequest
lodekeeper-z Mar 28, 2026
ff746b6
test: verify all PendingDeposit fields in processDepositRequest test
lodekeeper-z Mar 28, 2026
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
118 changes: 118 additions & 0 deletions src/state_transition/block/process_deposit_request.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,121 @@ pub fn processDepositRequest(comptime fork: ForkSeq, state: *BeaconState(fork),
var pending_deposits = try state.pendingDeposits();
try pending_deposits.pushValue(&pending_deposit);
}

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

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

fn makeDepositRequest(index: u64) DepositRequest {
return DepositRequest{
.pubkey = [_]u8{@as(u8, @intCast(index & 0xFF))} ** 48,
.withdrawal_credentials = [_]u8{0x01} ++ [_]u8{0} ** 11 ++ [_]u8{0xAA} ** 20,
.amount = 32_000_000_000,
.signature = [_]u8{0} ** 96,
.index = index,
};
Comment on lines +34 to +40
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

medium

The style guide (lines 51, 55) mandates assertions on function arguments. This function is missing them. Please add an assertion to validate the index argument, for example, to ensure it's not a sentinel value.

    assert(index != c.UNSET_DEPOSIT_REQUESTS_START_INDEX);
    return DepositRequest{
        .pubkey = [_]u8{@as(u8, @intCast(index & 0xFF))} ** 48,
        .withdrawal_credentials = [_]u8{0x01} ++ [_]u8{0} ** 11 ++ [_]u8{0xAA} ** 20,
        .amount = 32_000_000_000,
        .signature = [_]u8{0} ** 96,
        .index = index,
    };
References
  1. 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. (link)

Copy link
Copy Markdown
Contributor Author

@lodekeeper-z lodekeeper-z Mar 28, 2026

Choose a reason for hiding this comment

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

This is a test-only helper — assertions on function arguments in the style guide are for production code paths, not test fixtures. The index value is always a literal in each test. Dismissing.

Copy link
Copy Markdown
Contributor Author

@lodekeeper-z lodekeeper-z Mar 28, 2026

Choose a reason for hiding this comment

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

This is a test-only helper — assertions on function arguments in the style guide are for production code paths, not test fixtures. The index value is always a literal in each test. Dismissing.

}

test "processDepositRequest - first request sets depositRequestsStartIndex" {
const allocator = testing.allocator;
var pool = try Node.Pool.init(allocator, 256 * 5);
defer pool.deinit();

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

var state = test_state.cached_state.state.castToFork(.electra);

// Set to UNSET value (as electra upgrade does)
try state.setDepositRequestsStartIndex(c.UNSET_DEPOSIT_REQUESTS_START_INDEX);

const request = makeDepositRequest(42);
try processDepositRequest(.electra, state, &request);

// Should have set the start index to the request's index
try testing.expectEqual(@as(u64, 42), try state.depositRequestsStartIndex());

// Should have appended one pending deposit
var pending = try state.pendingDeposits();
try testing.expectEqual(@as(u64, 1), try pending.length());
}

test "processDepositRequest - subsequent request does not change start index" {
const allocator = testing.allocator;
var pool = try Node.Pool.init(allocator, 256 * 5);
defer pool.deinit();

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

var state = test_state.cached_state.state.castToFork(.electra);

// Set to UNSET, then process first request
try state.setDepositRequestsStartIndex(c.UNSET_DEPOSIT_REQUESTS_START_INDEX);
const first = makeDepositRequest(10);
try processDepositRequest(.electra, state, &first);

// Process second request with different index
const second = makeDepositRequest(20);
try processDepositRequest(.electra, state, &second);

// Start index should still be 10 (from first request)
try testing.expectEqual(@as(u64, 10), try state.depositRequestsStartIndex());

// Should have two pending deposits
var pending = try state.pendingDeposits();
try testing.expectEqual(@as(u64, 2), try pending.length());
}

test "processDepositRequest - pending deposit fields match request" {
const allocator = testing.allocator;
var pool = try Node.Pool.init(allocator, 256 * 5);
defer pool.deinit();

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

var state = test_state.cached_state.state.castToFork(.electra);

// Use a non-UNSET start index so we skip that branch
try state.setDepositRequestsStartIndex(0);

const request = makeDepositRequest(5);
try processDepositRequest(.electra, state, &request);

var pending = try state.pendingDeposits();
var deposit = try pending.get(0);

// Verify all fields were copied correctly
// Verify amount and slot were correctly copied from the request

const amount_view = try deposit.get("amount");
try testing.expectEqual(@as(u64, 32_000_000_000), amount_view);

const slot_view = try deposit.get("slot");
const state_slot = try state.slot();
try testing.expectEqual(state_slot, slot_view);
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

medium

This test is incomplete as it only verifies amount and slot. To ensure full correctness and prevent regressions, it should also verify that pubkey, withdrawal_credentials, and signature are copied correctly from the request to the PendingDeposit.

Also, it's better to compare against request.amount instead of a hardcoded value to make the test more resilient to changes in makeDepositRequest.

    // Verify all fields were copied correctly.
    const pubkey_view = try deposit.get("pubkey");
    var actual_pubkey: [48]u8 = undefined;
    _ = try pubkey_view.getAllInto(actual_pubkey[0..]);
    try testing.expectEqualSlices(u8, &request.pubkey, &actual_pubkey);

    const withdrawal_credentials_view = try deposit.get("withdrawal_credentials");
    var actual_withdrawal_credentials: [32]u8 = undefined;
    _ = try withdrawal_credentials_view.getAllInto(actual_withdrawal_credentials[0..]);
    try testing.expectEqualSlices(
        u8,
        &request.withdrawal_credentials,
        &actual_withdrawal_credentials,
    );

    const signature_view = try deposit.get("signature");
    var actual_signature: [96]u8 = undefined;
    _ = try signature_view.getAllInto(actual_signature[0..]);
    try testing.expectEqualSlices(u8, &request.signature, &actual_signature);

    const amount_view = try deposit.get("amount");
    try testing.expectEqual(request.amount, amount_view);

    const slot_view = try deposit.get("slot");
    const state_slot = try state.slot();
    try testing.expectEqual(state_slot, slot_view);

Copy link
Copy Markdown
Contributor Author

@lodekeeper-z lodekeeper-z Mar 28, 2026

Choose a reason for hiding this comment

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

Good point on field verification. I'll add pubkey, withdrawal_credentials, and signature checks using getAllInto, and reference request.amount instead of the hardcoded value. Pushing a fix.

}

test "processDepositRequest - already set start index is preserved" {
const allocator = testing.allocator;
var pool = try Node.Pool.init(allocator, 256 * 5);
defer pool.deinit();

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

var state = test_state.cached_state.state.castToFork(.electra);

// Pre-set to a specific value (not UNSET)
try state.setDepositRequestsStartIndex(100);

const request = makeDepositRequest(200);
try processDepositRequest(.electra, state, &request);

// Start index should remain 100
try testing.expectEqual(@as(u64, 100), try state.depositRequestsStartIndex());
}
Loading