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

chore: add set_compiled_artifacts to ProjectCompileOutput impl #33

Merged
merged 2 commits into from
Dec 12, 2023
Merged

chore: add set_compiled_artifacts to ProjectCompileOutput impl #33

merged 2 commits into from
Dec 12, 2023

Conversation

dutterbutter
Copy link
Contributor

Motivation

This pull request proposes a change to the ProjectCompileOutput impl by creating a method set_compiled_artifacts. The modification is required for the usage of extending foundry to work with non-solc compilers like zksolc for zkSync.

Context:

Currently, the compiled_artifacts field is marked as pub(crate), making it accessible only within the same crate. This encapsulation poses a limitation when the struct is used as part of a larger build process where access to compiled_artifacts is needed to be modified. Providing a set method to update compiled_artifacts it the proposed solution.

Rationale:

In effort to extend foundry support to zkSync ecosystem, the ability to set the modified Artifact is needed. You can see where we make use of set_compiled_artifacts here.

  • Enhanced Flexibility: Providing set_compiled_artifacts allows for more flexible post-compilation processes, including custom artifact handling, storage, and modifications tailored to specific use cases.

Functionality Impact:

The proposed change will have no impact on existing functionality. It merely extends the accessibility of the compiled_artifacts field, enabling external crates to utilize the compiled bytecode. This change is backward-compatible and does not alter the behavior of any existing code that depends on the ProjectCompileOutput struct.

Solution

Solution:

pub fn set_compiled_artifacts(&mut self, new_compiled_artifacts: Artifacts<T::Artifact>) {
        self.compiled_artifacts = new_compiled_artifacts;
}

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

this is only additive, so good w/ me.

@DaniPopes DaniPopes merged commit b1561d8 into foundry-rs:main Dec 12, 2023
20 checks passed
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.

3 participants