Skip to content

Conversation

@fusmanii
Copy link
Contributor

@fusmanii fusmanii commented Aug 14, 2025

Splitting this #1044 into multiple PRs

Part 1: Extract addresses script that generates DeployedAddresses.sol that can be used in foundry deploy scripts

After every deploy, just need to run yarn extract-addresses and deployed addresses contract will get updated with the latest addresses from broadcast folder and deployments.json file

More info in the attached EXTRACT_ADDRESSES.md

Diff is large because of the broadcast/deployed-addresses.json and broadcast/deployed-addresses.md files

@socket-security
Copy link

socket-security bot commented Aug 14, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​openzeppelin/​foundry-upgrades@​0.4.0571009185100

View full report

Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
@fusmanii fusmanii force-pushed the faisal/foundry-migration-addresses branch from ed6035f to 6a2adf7 Compare August 14, 2025 16:30
Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
@fusmanii fusmanii changed the title feat(Foundry): Added address extraction script feat(Foundry): Part 1 - Added address extraction script Aug 17, 2025
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two q's

Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
bmzig
bmzig previously approved these changes Aug 19, 2025
Copy link
Contributor

@bmzig bmzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@pxrl
Copy link
Contributor

pxrl commented Aug 19, 2025

@fusmanii Did we look at forge-deploy for this purpose? It's made by the same developer as hardhat-deploy, so I wonder if it might be more of a drop-in replacement for use with foundry.

https://github.com/wighawag/forge-deploy

Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
@fusmanii fusmanii force-pushed the faisal/foundry-migration-addresses branch from 37c4b53 to 1db9230 Compare August 20, 2025 16:04
@fusmanii
Copy link
Contributor Author

@pxrl first time hearing about https://github.com/wighawag/forge-deploy, taking a look

@grasphoper
Copy link
Contributor

@pxrl @fusmanii For forge-deploy, I see that the last commit to it was 9 months ago. Do you think it's worth adding a dependency that's not actively maintained? IMO we should avoid using deps like these if feasible. Thoughts?

@fusmanii
Copy link
Contributor Author

@grasphoper yeah I agree, I just tried forge-deploy, the implementation is not complete and it crashes on deployments file generation

@pxrl
Copy link
Contributor

pxrl commented Aug 22, 2025

@pxrl @fusmanii For forge-deploy, I see that the last commit to it was 9 months ago. Do you think it's worth adding a dependency that's not actively maintained? IMO we should avoid using deps like these if feasible. Thoughts?

Ah, I hadn't spotted that. 9 months with no activity is a bit of a red flag, though the author does otherwise have a really solid track record. If it worked and solved our use case then I think it'd at least be worth evaluating further, but if it doesn't work that's a showstopper.

In general I just want to be sure that we've researched and evaluated any off-the-shelf tooling for this - we're not the only project with this use-case so it seems really odd that it's so under-served.

Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
@fusmanii fusmanii force-pushed the faisal/foundry-migration-addresses branch from 912607e to ebc36ce Compare August 22, 2025 17:43
@fusmanii fusmanii merged commit 64e113c into master Aug 25, 2025
10 checks passed
@fusmanii fusmanii deleted the faisal/foundry-migration-addresses branch August 25, 2025 21:36
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.

6 participants