fix: exclude empty artifacts from ContractsByArtifact
#7713
Conversation
klkvr
changed the title
ContractsByArtifact
crates/script/src/verify.rs
Outdated
| @@ -106,6 +106,10 @@ impl VerifyBundle { | |||
| libraries: &[String], | |||
| ) -> Option<VerifyArgs> { | |||
| for (artifact, contract) in self.known_contracts.iter() { | |||
| // Avoid comparing to empty bytecode. | |||
| if contract.bytecode.is_empty() { | |||
There was a problem hiding this comment.
we don't need this check if we add the check in ContractsByArtifact i think?
There was a problem hiding this comment.
yeah, but I've seen this bug before so would like to keep this as impact of this bug is pretty high and it's pretty easy to miss while working with artifacts
There was a problem hiding this comment.
also I think this would've been caught by verification tests which are disabled rn
cc @mattsse
crates/common/src/contracts.rs
Outdated
| @@ -45,6 +45,13 @@ impl ContractsByArtifact { | |||
| let bytecode = artifact.bytecode.and_then(|b| b.into_bytes())?; | |||
| let deployed_bytecode = | |||
| artifact.deployed_bytecode.and_then(|b| b.into_bytes())?; | |||
|
|
|||
| if bytecode.is_empty() { | |||
There was a problem hiding this comment.
nit: add small comment on what empty bytecode means, just like the pr description
|
ah it actually seems that we depend on |
|
instead of excluding them we now keep both bytecodes as |
* fix: exclude empty artifacts from ContractsByArtifact * additional check * fmt * doc * use Option for bytecodes
Motivation
Closes #7702
Solution
Exclude artifacts with present but empty bytecode from
ContractsByArtifact. Such contracts are usually abstract contracts or interfaces. Also adds additional safety check toVerifyArgs