-
Notifications
You must be signed in to change notification settings - Fork 0
added factory dependencies for Resolc Compiler #39
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
Conversation
@@ -83,6 +83,7 @@ impl From<ResolcContract> for foundry_compilers_artifacts_solc::Contract { | |||
ewasm: None, | |||
ir_optimized: contract.ir_optimized, | |||
ir_optimized_ast: None, | |||
factory_dependencies: contract.factory_dependencies, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe extensions
?
namely:
enum Extensions {
None,
Resolc(ResolcExtras)
}
and corresponding methods like
contract.resolc_extras : () -> Option<ResolcExtras>
simillar to what is already was implemented for Bytecode
in this repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why extensions went to resolcContract
when the solc
contract is the one to be extended?
It doesnt make any sense semantically, we need to extend the solc
contract, resolc
contract is already a superset of solc
contract (for a subset of values, e.g. factory_dependencies
and others).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to solc
@@ -59,6 +59,9 @@ pub struct ConfigurableContractArtifact { | |||
/// The identifier of the source file | |||
#[serde(default, skip_serializing_if = "Option::is_none")] | |||
pub id: Option<u32>, | |||
/// Factory dependencies for the contract. | |||
#[serde(default, skip_serializing_if = "Option::is_none")] | |||
pub factory_dependencies: Option<BTreeMap<String, String>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why Option
is needed?
BtreeMap::empty
should pretty much be equivalent to Option::None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
a37ad61
to
d00695b
Compare
Added an optional factory_dependencies field (a BTreeMap<String, String>) to Contract and ConfigurableContractArtifact in the resolc, solc, and vyper crates, with serde defaults to avoid breaking existing artifacts.
Propagated this new field through all From<…> conversions so parsed contracts now carry factory‐dependency metadata.
Updated the ArtifactOutput logic in the compilers crate to include factory_dependencies in emitted artifacts.