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

Migrate from Foundry Foreign Interface to File System #10

Merged
merged 5 commits into from
Nov 20, 2023

Conversation

rocknet
Copy link
Contributor

@rocknet rocknet commented Nov 19, 2023

  • Updated forge-std
  • Added additional tests
  • Added new contract deployment (in the new run-latest.json) to exercise what seems to be a newer broadcast json output that doesn't have the rpc property. If this is no longer required, could perhaps remove the LegacyBroadcastTransaction type and logic around it.
  • Explored areas like readTx1559s and Tx1559, but those also seem different from what I was seeing in the run-latest.json files. Might be good to use this, but it seemed inapplicable.
  • Considered and rejected string parsing the json.
  • Don't like the code duplication in processRun, but I don't know of a way to pass in a type to something like a generic function, don't think that's a thing in Solidity.
  • Sorry for the magic number I just found. vm.readDir(relativeBroadcastPath, 3) goes three level down from ./broadcast.

@PatrickAlphaC
Copy link
Member

Love this!!!

So it's not building for me, forge build is failing.

Error: 
Compiler run failed:
Error (7920): Identifier not found or not unique.
  --> src/DevOpsTools.sol:88:9:
   |
88 |         Vm.DirEntry[] memory entries = vm.readDir(relativeBroadcastPath, 3);
   |         ^^^^^^^^^^^

Am I doing something wrong?

@rocknet
Copy link
Contributor Author

rocknet commented Nov 19, 2023

So it's not building for me, forge build is failing.

Looks like Vm.DirEntry, along with the other newer FS cheatcodes, where added in 1.5.5. Added the makefile, but a quick forge update will do it also. https://github.com/foundry-rs/forge-std/releases/tag/v1.5.5

Let me know if what you were thinking for the makefile would be to actually do a forge rm forge-std -f and forge install foundry-rd/forge-std@v1.7.0 for example. Not sure that's necessary, but can do if requested. Technically it seems the minimum version required is likely v1.6.1, because I'm using vm.keyExists.

@PatrickAlphaC
Copy link
Member

LETS GOOOO!!!!!!!!!

THANK YOU SO MUCH THIS IS GREAT!!!

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.

None yet

2 participants