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

Initial commit #2

Merged
merged 26 commits into from
Aug 27, 2024
Merged

Initial commit #2

merged 26 commits into from
Aug 27, 2024

Conversation

cpb8010
Copy link
Contributor

@cpb8010 cpb8010 commented Aug 23, 2024

Import the state of the world from my example application

Also use nx and pnpm to setup monorepo tooling for other PRs

@cpb8010 cpb8010 self-assigned this Aug 23, 2024
Copy link
Member

@JackHamer09 JackHamer09 left a comment

Choose a reason for hiding this comment

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

I think we need to stick to using npm instead of other custom package managers. It might be a bit slower but it's always pain for devex, since everyone is using different package managers but everyone knows npm and have it installed.

Copy link
Member

@JackHamer09 JackHamer09 left a comment

Choose a reason for hiding this comment

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

Also I think we need to follow common monorepo practice of storing each module in the packages folder.

@JackHamer09
Copy link
Member

JackHamer09 commented Aug 26, 2024

I pushed SDK and Gateway but didn't add any CI logic yet, I think we can do it a bit later cause need to figure out how tests should be set up to work together

@JackHamer09 JackHamer09 changed the title Contracts folder Initial commit Aug 26, 2024
Copy link
Contributor

@MexicanAce MexicanAce left a comment

Choose a reason for hiding this comment

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

LGTM

cpb8010 and others added 19 commits August 27, 2024 08:16
Expecting this to be a monorepo, and while there are still missing
pieces for the contracts, most of the components are there.
set up to run CI for a folder
hardhat already has caching, so had to specify some folders for nx so it
knows where to look to avoid duplicating hard-hats de-deduplication
Update in root and in contracts
Also import latest lockfile from main, as local was outdated and
inconsistent (the latest package versions have breaking changes)
Use workspace versions instead, we'll see if this by default installs
packages or if there's another step for that
Based on PR feedback for where other packages are going
The peer dep conflict with openzepplin still exists, but other devs on
the project prefer the default package manager since we're not yet large
enough to need the smaller package footprints of pnpm.

Tests should now run in CI again!
Also pass arguments correctly to nx with npm, as it needs the split
between arguments to npm and arguments to nx
might be giving zksolc corruption issues that don't happen locally
Updating packages as well!
Updating lockfile with latest tested versions (from the other repo)
We've exceeded the free plan and instead of falling back to gh actions
it just fails
Still assuming something with the caching wrapper is breaking it, as the
other repo with the onchain job downloads just fine.
cpb8010 and others added 7 commits August 27, 2024 08:16
Because npm won't auto alias to scripts unlike most other custom package mangers
It's looking like the download isn't designed to be multi-process safe!
Because the test step includes a build, the download was being run
twice.
Fails with gas limit, will debug in tests instead of setup
@MexicanAce MexicanAce merged commit 42fde51 into main Aug 27, 2024
@MexicanAce MexicanAce deleted the contracts-folder branch August 27, 2024 15:42
aon added a commit that referenced this pull request Jan 30, 2025
feat: add function to retrieve guarded accounts
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.

3 participants