Skip to content

Prevent cached settings footgun in rust bindings#585

Merged
jtraglia merged 7 commits intoethereum:mainfrom
jtraglia:better-precompute-caching
Aug 5, 2025
Merged

Prevent cached settings footgun in rust bindings#585
jtraglia merged 7 commits intoethereum:mainfrom
jtraglia:better-precompute-caching

Conversation

@jtraglia
Copy link
Member

This creates caches for each possible precompute value.

Fixes #584.

@ethereum-code-reviewer

This comment was marked as resolved.

@ethereum-code-reviewer

This comment was marked as duplicate.

@ethereum-code-reviewer

This comment was marked as duplicate.

@ethereum-code-reviewer

This comment was marked as duplicate.

Comment on lines 42 to 47
pub fn ethereum_kzg_settings(precompute: u64) -> &'static KzgSettings {
ethereum_kzg_settings_inner(precompute).as_ref()
let arc = ethereum_kzg_settings_arc(precompute);
// Intentionally leak the Arc to create a static reference.
// This is safe because the settings are cached and meant to live for the program's lifetime.
// The memory will be cleaned up when the program terminates.
&*Box::leak(Box::new(arc))
Copy link
Member Author

Choose a reason for hiding this comment

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

@rakita how do you feel about this function? We don't really like it. Is there a better way to do this? Maybe we could delete this function and only provide the arc'd version (ethereum_kzg_settings_arc); would this work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Previously we had:
fn ethereum_kzg_settings_inner(precompute: u64) -> &'static Arc<KzgSettings> {

that would allow returning 'static reference. I would try to bring back this fn

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah. I didn't realize I changed that function's return type. Good call, thanks.

Copy link
Contributor

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm

@jtraglia jtraglia merged commit b6231a9 into ethereum:main Aug 5, 2025
43 checks passed
@jtraglia jtraglia deleted the better-precompute-caching branch August 5, 2025 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

footgun on ethereum_kzg_settings

2 participants