Skip to content
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.

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

vm: Change execution module to use execution::Config #3430

Merged
merged 7 commits into from
Jan 28, 2025

Conversation

herr-seppia
Copy link
Member

@herr-seppia herr-seppia commented Jan 27, 2025

This PR refactors the execution module to utilize the vm::execution::Config structure, enhancing configuration management within the VM.

Why?

  1. Improved Code Modularity:
    This change eliminates the need to pass individual variables across multiple functions, reducing clutter and ensuring that related parameters are logically grouped together.

  2. Ease of Adding New Parameters:
    By centralizing configuration into a single structure, the codebase is better prepared for future expansions, such as incorporating new ABIs or additional VM parameters. (See also rusk: add Moonlight TX sender metadata as host function #3341)

Additionally, this refactor introduces a new RuskVMConfig struct, which consolidates additional VM-related configurations not required by the execution module itself. Examples include generation_timeout and block_gas_limit, which are now properly scoped and managed within this structure.

moCello
moCello previously approved these changes Jan 28, 2025
Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

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

Nice change. Just two small questions, nothing blocking

vm/CHANGELOG.md Outdated Show resolved Hide resolved
rusk/src/lib/node/vm/config.rs Outdated Show resolved Hide resolved
@herr-seppia herr-seppia merged commit 0082152 into master Jan 28, 2025
16 checks passed
@herr-seppia herr-seppia deleted the execution_config branch January 28, 2025 15:25
@moCello moCello linked an issue Jan 29, 2025 that may be closed by this pull request
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.

rusk: group execution params together
2 participants