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

Implement precompile mock contracts and foundry mock util test contracts #462

Merged
merged 9 commits into from
Oct 17, 2023

Conversation

mshakeg
Copy link
Contributor

@mshakeg mshakeg commented Oct 14, 2023

Description

Replaces #236 for a cleaner review.

  • Add foundry test support files
  • Add husky commit-msg to help devs not forget signing off
  • Add foundry testing readme file
  • Add npm package script for easy runs of foundry, hardhat and localnode
  • Add HTS mock and test contracts and use for reference foundry tests
  • Add github CI action to run foundry tests

@mshakeg
Copy link
Contributor Author

mshakeg commented Oct 14, 2023

Hey @Nana-EC I opened this PR as the git rebase to sign-off all commits wasn't clean and ended up showing a bunch of other changes.

There are still quite a lot of TODOs documented in the code, when I free up a bit I'll create issues for those and address them.

@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 16, 2023

Hey @Nana-EC I opened this PR as the git rebase to sign-off all commits wasn't clean and ended up showing a bunch of other changes.

There are still quite a lot of TODOs documented in the code, when I free up a bit I'll create issues for those and address them.

Np.
Once you break out the issues we can see how to get this in so maybe you don't have to do it all.
Thanks

@mshakeg
Copy link
Contributor Author

mshakeg commented Oct 16, 2023

@Nana-EC do you want to merge this PR as is or should I first remove the TODOs scattered throughout the code?

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Some minor docs, naming and workflow changes to make it ready to get in

README.md Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test-workflow.yml Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 17, 2023

@Nana-EC do you want to merge this PR as is or should I first remove the TODOs scattered throughout the code?

@mshakeg gonna work with you to get this in hopefuly this week.
I've reviewed and I think getting DCO (remember to sign-off) to pass and addressing these minor comments and we can get this in

@Nana-EC Nana-EC added enhancement New feature or request P2 Tooling tooling labels Oct 17, 2023
@Nana-EC Nana-EC added this to the 0.6.0 milestone Oct 17, 2023
Signed-off-by: Mo Shaikjee <[email protected]>
@mshakeg
Copy link
Contributor Author

mshakeg commented Oct 17, 2023

@Nana-EC thanks for the review, I added a husky hook to prevent committing without signing off as I don't always remember to do so.

@mshakeg mshakeg requested a review from Nana-EC October 17, 2023 11:31
@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 17, 2023

@Nana-EC thanks for the review, I added a husky hook to prevent committing without signing off as I don't always remember to do so.

@mshakeg Looks good to go. Just waiting on CI to pass and I'll merge this in.
Appreciate the contributions here and as always if you see any gaps feel free to open tickets to get on our radar or awesome PRs like this to accelerate the process.
Thanks

@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 17, 2023

@Nana-EC thanks for the review, I added a husky hook to prevent committing without signing off as I don't always remember to do so.

@mshakeg Looks good to go. Just waiting on CI to pass and I'll merge this in. Appreciate the contributions here and as always if you see any gaps feel free to open tickets to get on our radar or awesome PRs like this to accelerate the process. Thanks

And just after I commented of course all the CI tests fail - https://github.com/hashgraph/hedera-smart-contracts/actions/runs/6546507450/job/17787629512?pr=462

@mshakeg
Copy link
Contributor Author

mshakeg commented Oct 17, 2023

@Nana-EC thanks, will do. Once this is in, I'll continue with the foundry-example project in the json rpc relay

Copy link
Contributor

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

Incredible work, I will have to come back later to complete review, but overall it looks really good.
Only 2 nits and 1 question.

.gitignore Outdated Show resolved Hide resolved
foundry.toml Outdated Show resolved Hide resolved
Signed-off-by: Mo Shaikjee <[email protected]>
@mshakeg mshakeg requested a review from AlfredoG87 October 17, 2023 18:26
Signed-off-by: Mo Shaikjee <[email protected]>
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG
Nice work

@Nana-EC Nana-EC merged commit b7f9d1e into hashgraph:main Oct 17, 2023
20 checks passed
@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 17, 2023

@Nana-EC thanks, will do. Once this is in, I'll continue with the foundry-example project in the json rpc relay

@mshakeg you're all set, only took a little bit 🤣

@mshakeg mshakeg deleted the hedera-main branch October 18, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Tooling tooling
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants