Skip to content
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

Improve library support #139

Open
5 tasks
anishnaik opened this issue May 1, 2023 · 4 comments · May be fixed by #183
Open
5 tasks

Improve library support #139

anishnaik opened this issue May 1, 2023 · 4 comments · May be fixed by #183
Labels
help wanted Extra attention is needed medium-priority on hold This issue was marked as not desirable to complete for now

Comments

@anishnaik
Copy link
Collaborator

anishnaik commented May 1, 2023

There are two features that medusa needs to better support libraries:

  1. medusa does not support external libraries. External libraries are unique since you must first deploy the library, resolve their addresses, update the contract bytecode with those addresses, and then deploy the contracts.
  2. medusa is not great at decoding events that are emitted in internal libraries. See Bug: medusa fails to decode events emitted in internal library functions #131. The current fix is a brute-force approach to a better solution that would only parse the ABIs of libraries that the contract depends on.

Here are the technical requirements for the first feature:

  • Figure out a way to identify whether a contract is a library or not based on the runtime bytecode.
  • Identify which external libraries that a contract depends on based on the provided placeholders. This is an annoying and weirdly difficult problem. crytic-compile's solc and standard export has the necessary information to resolve this. However, it might be better to come up with an in-house solution for it so that we can re-use it for other compilation platforms (e.g., first-party support for foundry / hardhat).
  • Update the test chain deployment process to first deploy the external libraries, update the contract bytecodes with resolved deployed library addresses, and then finally deploy the contracts.

For whoever picks this issue up, please reach out to me since I have done a lot of the legwork in the dev/library-support branch (requirements 1 and 3 are basically sort of done but not in the most elegant way). The hardest part is the second technical requirement which has an effective yet not-so-elegant solution that we can discuss.

Here are the technical requirements for the second feature:

  • Identify which libraries a contract depends on. Note that this is different from a similar requirement in the first feature since internal libraries will not have placeholders but are rather inlined. I am not sure the best way to do this but ideally finding library dependencies is a single solution that can handle it for both feature 1 and 2.
  • Update the execution tracer to navigate the library dependencies if an event does not match the ABI for the calling contract.
@aviggiano
Copy link

Hi

I'd like to add a request for Medusa to also track the line coverage for libraries.

I'm working on a project with architecture similar to the "Diamond Pattern" (many libraries) and I'm seeing 0% for all files except the target contract.

@anishnaik
Copy link
Collaborator Author

Are the libraries external or internal?

@aviggiano
Copy link

Are the libraries external or internal?

External: https://github.com/aviggiano/medusa-libraries
Curiously, on my real project at least I can deploy the contract, but when I tried to create this MWE it instantly reverts. Maybe I'm missing a config?

@aviggiano
Copy link

Workaround https://gist.github.com/aviggiano/d0c31c26c18535b25a0e75d185436e8b

Just sed your lib's external and public to internal, and calldata to memory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed medium-priority on hold This issue was marked as not desirable to complete for now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants