sphinx-labs / sphinx

DevOps platform for smart contract deployments
https://sphinx.dev/
MIT License
236 stars 14 forks source link

fix(pg): Fix error when script artifact not found due to absolute paths in build info file #1702

Closed RPate97 closed 3 months ago

RPate97 commented 3 months ago

Purpose

This fixes a bug where the assertNoLinkedLibraries function fails to find the artifact of the target script, causing an error to be thrown. Calnix reported a similar issue on Windows a while ago, but I was not able to replicate it until now, and it's occurring on all platforms for me. I don't think the issue I am resolving here is directly related to that problem, but it may be similar.

Bug

This bug occurs because Foundry sometimes outputs a set of contracts with keys that are absolute paths instead of relative paths. As a result, we fail to locate the correct script artifact because our logic relies on the path in the build-info file being a relative path.

I am not totally sure what causes this to happen. I only observed this issue occurring in the mono repo. I was not able to replicate it in an external repository. I would speculate that this is happening because of our mono repo structure, where we have multiple packages that use foundry. So, I would imagine it is possible for a user to encounter this issue if they have a similar setup, but I haven't confirmed this.

Solution

This PR resolves the issue by implementing logic to search for fully qualified names using absolute and relative paths. If we find matches for both relative and absolute paths, then an error an invariant error is thrown. During my testing, I observed that Foundry either uses relative or absolute paths and not both, so I expect this to not occur.

I tested out our production packages, and this issue does not appear to affect them, but as I said above, I think it may be possible for this issue to affect users that have complex setups like ours.

Todo

mergify[bot] commented 3 months ago

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

RPate97 commented 3 months ago

This was fixed in Foundry: https://github.com/foundry-rs/foundry/issues/7878