fix(zk): properly mark unlinked contract #40
Conversation
refactor(zk): get output from `eravm` object
Karrq
requested review from
nbaztec,
elfedy,
Jrigada,
dutterbutter and
hedgar2017
as code owners
There was a problem hiding this comment.
Unless I am missing something on why it is done this way, I think we should consider separating compiler outputs from the artifacts we use and not join them into one, hiding the conversion inside the deserialization. The abstractions for this are there already:
- We deserialize
Contractthat has aBytecodeand we respect the output fields (no ad hoc deserialization logic). - We have
ZkContractArtifactwithZkArtifactBytecodethat contains themissing_libraries(if we need them there for some reason) and that is constucted on thezksync_contract_to_artifact_function
| } | ||
|
|
||
| // NOTE: distinction between bytecode and deployed bytecode make no sense of zkEvm, but | ||
| // we implement these conversions in order to be able to use the Artifacts trait. | ||
| impl From<Bytecode> for CompactBytecode { | ||
| fn from(bcode: Bytecode) -> Self { | ||
| Self { object: bcode.object, source_map: None, link_references: BTreeMap::default() } | ||
| let link_references = bcode.link_references(); | ||
| Self { object: bcode.object, source_map: None, link_references } |
There was a problem hiding this comment.
We need to be careful now because bytecode and deployed bytecode will NOT be the same thing as bytecode might be an unlinked and link_references has meaningful info . Maybe update the NOTE above.
Also is having this conversion the reason why you need missing_libraries in the Bytecode struct
There was a problem hiding this comment.
in EraVM they still are the same thing, not sure why the unlinked means it's different
There was a problem hiding this comment.
CompactBytecode will have the link references for the bytecode CompactDeployedBytecode will not. Previously we only cared about the bytecode field and could use both interchangebly. It may not be the case now if we used the linked_references somewhere we need CompactBytecode
fix: remove serde logic in favor of keeping artifact and compiler format separate
* feat: add zksolc support * fix: codeowner change (#32,#34) * feat: zksolc v1.5.7 support (#35) * feat: zksolc 1.5.7 support * chore: update version constants * feat: Support error / warning suppression in zksolc (#33) feat: add FromStr to ZkSolcWarnings and ZkSolcErrors (#37) * fix: add back skip filter over avoid-contracts * feat(zk): properly mark unlinked contract (#40,#41,#43,#44) feat(zk): mark bytecode as unlinked refactor(zk): get output from `eravm` object feat: retain pre-1.5.7 zksolc compatibility test(artifact:zk): unprefixed bytecode test * chore: bump version * chore: merge conflicts 11.6->12.3 feat(zk): per-source profile & settings feat(zk): stub our zksolc settings restrictions * fix(ci): update rust toolchain version * fix(ci): proper rust toolchain name * chore: clippy --------- Co-authored-by: Federico Rodríguez <[email protected]> Co-authored-by: Tomi Moreno <[email protected]> Co-authored-by: Alex Ostrovski <[email protected]> Co-authored-by: Dustin Brickwood <[email protected]>
With this PR we can now properly detect when a contract is unlinked, mark it as such and track the missing libraries associated with the unlinked bytecode.
Changes:
evmtoeravmobject in json outputmethodIdentifiersmissingLibrariesand store it next to the unlinked bytecode