Skip to content

chore: add set_compiled_artifacts to ProjectCompileOutput impl#33

Merged
DaniPopes merged 2 commits intofoundry-rs:mainfrom
dutterbutter:db/add_set_get_for_compiler_artifact_object
Dec 12, 2023
Merged

chore: add set_compiled_artifacts to ProjectCompileOutput impl#33
DaniPopes merged 2 commits intofoundry-rs:mainfrom
dutterbutter:db/add_set_get_for_compiler_artifact_object

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
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