Skip to content

Conversation

RembrandtK
Copy link
Contributor

@RembrandtK RembrandtK commented Jun 9, 2025

Large number of changes consolidating tooling, configuration, package structure, and linting. Attempts to consolidate and merge several aspects from Horizon and other WIP to both create a cleaner baseline for contract code PRs and converge upgrades across branches. A PR will also be created to merge this upgrade into horizon, creating a cleaner more consistent baseline.

Key features:

  • Upgrade to pnpm, improving package management and converging with Horizon.
  • Restructure packages to remove dependency cycles. Instead of the sdk depending on contracts and contracts on sdk, contracts has child packages contracts-test and contracts-task that depend on sdk but that sdk does not depend on. These child packages depend on sdk, while contracts does not. A common package has also been created for shared dependencies of contract packages. There are now no dependency cycles, no SKIP_LOAD compiles, and dynamic loading has been converted to static imports.
  • Numerous lint issues have been fixed. ESLint upgraded to v9 and natspec-smells linting added.
  • GH workflows have been simplified. One for linting, one for build, test, and coverage test. Workflows use pnpm to find build, test, and test:coverage targets rather than needing separate configuration or per package workflows.

Copy link

socket-security bot commented Jun 9, 2025

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert (click for details)
Warn Critical
[email protected] has a Critical CVE.

CVE: GHSA-67hx-6x53-jw92 Babel vulnerable to arbitrary code execution when compiling specifically crafted malicious code (CRITICAL)

Affected versions: >= 0

Patched version: No patched versions

From: pnpm-lock.yamlnpm/[email protected]

ℹ Read more on: This package | This alert | What is a critical CVE?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@RembrandtK RembrandtK requested a review from Copilot June 9, 2025 09:33
Copilot

This comment was marked as outdated.

Copy link

openzeppelin-code bot commented Jun 9, 2025

Main upgrades in preparation for contract PRs

Generated at commit: 4013b60eab576ab72ed3ab960fb375c78ffc2693

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
4
0
15
36
57
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@RembrandtK RembrandtK requested a review from Copilot June 9, 2025 10:04
@RembrandtK RembrandtK self-assigned this Jun 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates tooling and configuration upgrades to prepare for upcoming contract PRs by transitioning from Yarn to pnpm, restructuring packages to eliminate dependency cycles, and streamlining workflows and lint configurations.

  • Upgrade from Yarn to pnpm across build, publish, and dependency installation tooling
  • Removal of now-obsolete workflows and adjustments to configuration files for improved consistency
  • Restructuring package relationships to remove dependency cycles

Reviewed Changes

Copilot reviewed 385 out of 385 changed files in this pull request and generated no comments.

Show a summary per file
File Description
natspec-smells.config.js Added configuration for natspec documentation linting
eslint.config.mjs Updated ESLint configuration to switch from Yarn/TS plugin to pnpm-based rules
count-patterns.txt Added pattern file for counting Solidity contracts documentation changes
README.md Updated instructions and package table for pnpm and package restructuring
.solhint.json Removed Prettier plugin from Solhint configuration
.prettierignore Updated ignore file with pnpm and additional cache/build directories
.github/workflows/* Updated various workflows including build, publish, lint, and verify steps to use pnpm and new paths
.github/actions/setup/action.yml Revised setup script to use pnpm along with Foundry setup and Node.js configuration
.devcontainer/* Updated devcontainer scripts and configurations to support pnpm
.changeset/smooth-balloons-stare.md Removed a changeset file, consolidating the changeset configuration
Comments suppressed due to low confidence (2)

.github/workflows/publish.yml:36

  • Please verify that the '--no-git-checks' flag is supported by the current version of pnpm and that skipping git checks will not bypass any important validations during the publish process.
pnpm publish --tag ${{ inputs.tag }} --access public --no-git-checks

.changeset/smooth-balloons-stare.md:1

  • The removal of this changeset file may affect the release process; please document the rationale for its removal to ensure clarity on how version bumps and changelog management will be handled going forward.
----

Copy link

codecov bot commented Jun 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.12%. Comparing base (cbda0fc) to head (4013b60).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1184      +/-   ##
==========================================
- Coverage   86.06%   83.12%   -2.95%     
==========================================
  Files          47       47              
  Lines        2074     2074              
  Branches      613      613              
==========================================
- Hits         1785     1724      -61     
- Misses        289      350      +61     
Flag Coverage Δ
unittests 83.12% <ø> (-2.95%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RembrandtK RembrandtK marked this pull request as ready for review June 9, 2025 10:19
@RembrandtK RembrandtK requested review from pcarranzav and tmigone June 9, 2025 10:20
@RembrandtK RembrandtK merged commit bdc6613 into main Jun 9, 2025
5 of 6 checks passed
@RembrandtK RembrandtK deleted the main.upgrade branch June 9, 2025 12:42
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.

2 participants