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

avoid deployment of library contract #19

Closed
wants to merge 2 commits into from
Closed

Conversation

jjoshm
Copy link

@jjoshm jjoshm commented Mar 17, 2024

currently every script that is using the library is deploying an additional "DevOpsTools" contract because of
foundry-rs/foundry#3295

that foundry bug makes the lib completely useless for mainnet.

I changed the lib function to be internal.
(includes also a small fix for the interactwithstuff script using a wrong contract name.)

closes #18

@PatrickAlphaC
Copy link
Member

I see and like what you're going for here, but this PR breaks all the tests

@jjoshm
Copy link
Author

jjoshm commented Mar 21, 2024

ok so after some debugging I found out that the bug only happens if you provide 3 arguments to the library function on master branch and always happens on the 0.1.0 tag.


on the master branch:

you can see the difference between 2 and 3 arguments is that the one taking 2 is internal and the one taking 3 is public

function get_most_recent_deployment(string memory contractName, uint256 chainId) internal view returns (address) {
return get_most_recent_deployment(contractName, chainId, RELATIVE_BROADCAST_PATH);
}
function get_most_recent_deployment(
string memory contractName,
uint256 chainId,
string memory relativeBroadcastPath
) public view returns (address) {


on the 0.1.0 tag:

both function are public. thats why the bug is happening all the time

function get_most_recent_deployment(string memory contractName, uint256 chainId) public view returns (address) {
return get_most_recent_deployment(contractName, chainId, RELATIVE_BROADCAST_PATH);
}
function get_most_recent_deployment(
string memory contractName,
uint256 chainId,
string memory relativeBroadcastPath
) public view returns (address) {


so my conclusion is that, in fact, the only problem was that the function was public and not internal.

Still I wanted to validate that the internal library is not bundled with the deployed contract. I checked that by looking at the gasUsed in a transaction with using the library and without. And it looks like both deployments are using the same amount of gas which means the lib is not bundled with the contract, correct?

@PatrickAlphaC
Copy link
Member

Updated in the most recent commit. Thanks for making this issue. I'm going to do a bit of a refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

avoid deployment of library contract
2 participants