Skip to content

Gloas add spans for deposit signature verification#9302

Closed
eserilev wants to merge 1 commit into
sigp:unstablefrom
eserilev:gloas-deposit-verification-span
Closed

Gloas add spans for deposit signature verification#9302
eserilev wants to merge 1 commit into
sigp:unstablefrom
eserilev:gloas-deposit-verification-span

Conversation

@eserilev

Copy link
Copy Markdown
Member

Issue Addressed

We want to be able to measure the time it takes to sig verify deposits. We'll probably have to make optimizations to this code path (maybe a pre-processing cache?) so it'll be nice to have metrics we can look at.

• lh_process_deposit_requests_post_gloas — total time for all deposits in the block
• lh_is_pending_validator — sig verification per validator deposit
• lh_apply_deposit_for_builder — sig verification for builder deposit

@eserilev eserilev added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! gloas tracing labels May 13, 2026
@eserilev eserilev changed the title Gloas add spans for builder depsoit verification Gloas add spans for deposit signature verification May 13, 2026
#[instrument(
name = "lh_process_deposit_requests_post_gloas",
skip_all,
level = "debug"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be worth adding count = deposit_requests.len()?

}

#[instrument(
name = "lh_process_deposit_requests_post_gloas",

@jimmygchen jimmygchen May 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The lh_ prefix are for top level root span only. These three spans should appear in all the block processing code paths without it.

The only uninstrumented code path I've found is when it's called from the state_advance_timer:

.prepare_beacon_proposer(current_slot)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah right, thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A bit of context on the lh_ prefix: the main purpose is to only export spans that belongs to code paths that are instrumented end-to-end, so we don't end up with lots of individual spans appearing on uninstrumented code paths that we're less interested in.

If we prefix all spans, then it defeats the purpose of this filtering, so we should only the prefix on the top level span.

#8647

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 14, 2026
@eserilev eserilev closed this May 14, 2026
@eserilev eserilev reopened this May 14, 2026
@eserilev

Copy link
Copy Markdown
Member Author

pawan is handling this in his deposit Pr

@eserilev eserilev closed this May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gloas low-hanging-fruit Easy to resolve, get it before someone else does! tracing waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants