Baseline upgrades in preparation for service quality contracts PRs #1213

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

Jump to bottom

Draft

RembrandtK wants to merge 4 commits into main

from baseline-upgrade

Draft

Baseline upgrades in preparation for service quality contracts PRs #1213

RembrandtK wants to merge 4 commits into main from baseline-upgrade

Conversation

RembrandtK

Copy link

Contributor

@RembrandtK RembrandtK commented Aug 14, 2025

Build improvements and global fixing of linting issues.

pnpm updated to latest version
dependency improvements (mainly for building dependencies before running tests)
reducing unnecessary token-distribution builds
linting configuration upgrades
some patches to get clean test runs
global fix of reported lint issues to get clean lint run (with local issue suppression where necessary)
minor dev container updates for working dir and Claude

Many files changes. I should provide a better list of changes and split into separate commits (and perhaps even PRs). Currently short on time, but can improve later if required. At this stage mostly all merged together with history lost.

I will be using this as the baseline for contract PR(s), to use this foundation of update builds and clean lint run.

RembrandtK added 3 commits

August 13, 2025 12:02

@RembrandtK


 fix(pnpm): ignore .pnpm-store/

0420c16

@RembrandtK


 feat(container): ugrade dev container to use Claude, /work dir, and J...

1eec283

...SON5

@RembrandtK


 feat: upgrade of pnpm, linting, package dependencies, and linting fixes

85bc48f

@openzeppelin-code OpenZeppelin Code

Copy link

openzeppelin-code bot commented Aug 14, 2025 •

edited

Loading

Baseline upgrades in preparation for service quality contracts PRs

Generated at commit: 383951cf95df74e5f6f699720e210f8ad459f43c

🚨 Report Summary

Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 5 0 15 33 55
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


 fix(contracts/task): fix TypeScript rootDir configuration

383951c

- Remove restrictive rootDir setting that prevented imports from parent directories
- Set rootDir to '..' to allow access to parent contracts types
- Fixes CI build failure in packages/contracts/task TypeScript compilation
Resolves TypeScript error TS6059 where files outside rootDir were being imported

@codecov Codecov

Copy link

codecov bot commented Aug 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.51%. Comparing base (cbda0fc) to head (383951c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@
## main #1213 +/- ##
==========================================
- Coverage 86.06% 83.51% -2.56% 
==========================================
 Files 47 47 
 Lines 2074 2074 
 Branches 613 613 
==========================================
- Hits 1785 1732 -53 
- Misses 289 342 +53

Flag	Coverage Δ
unittests	`83.51% <100.00%> (-2.56%)`	⬇️

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 requested a review from Copilot

August 14, 2025 14:55

Copilot

Copilot AI reviewed

Aug 14, 2025

View reviewed changes

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 implements baseline upgrades in preparation for service quality contracts, focusing on build improvements and global fixing of linting issues. The changes update build tooling, improve dependency management, and establish a clean linting baseline across the codebase.

Key changes include:

Updated pnpm workspace configuration with centralized dependency catalog
Build system improvements for token-distribution package with offline GraphClient support
Global linting configuration upgrades and systematic fixing of Solidity and TypeScript lint issues

Reviewed Changes

Copilot reviewed 175 out of 179 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pnpm-workspace.yaml	Updated workspace configuration with catalog for centralized dependency management
scripts/*.js	Added verification scripts for solhint-disables and lint-staged runner
packages/*/package.json	Converted to use catalog dependencies and improved build scripts
packages/token-distribution/scripts/*	Added GraphClient extraction for offline builds and improved build process
packages/contracts/contracts/*/.sol	Applied systematic linting fixes with TODO sections for disabled rules
packages//test//.ts	Updated test configurations and fixed import paths

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a 100ドル gift card. Take the survey.}

scripts/verify-solhint-disables.js

fs.writeFileSync(tempFile, cleanedContent)

try {

const result = execSync(`npx solhint ${tempFile} -f json`, {

Copy link

Preview

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The execSync call uses unsanitized file path input which could lead to command injection. Consider using proper path validation or a safer alternative.

Suggested change

const result = execSync(`npxsolhint${tempFile} -f json`, {

const result = execFileSync('npx',['solhint',tempFile,'-f','json'], {

Copilot uses AI. Check for mistakes.

packages/token-distribution/scripts/extract-graphclient.js

Object.defineProperty(exports, "__esModule", { value: true });

// Minimal GraphClient for offline builds - contains only what ops/info.ts uses

const { gql } = require('@graphql-mesh/utils');

Copy link

Preview

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Hard-coded dependency on '@graphql-mesh/utils' in generated code could break if the dependency is not available. Consider making this configurable or handling the missing dependency gracefully.

Suggested change

const{gql }=require('@graphql-mesh/utils');

// Local no-op gql tag to avoid dependency on @graphql-mesh/utils

function gql(strings, ...values) {

let str = '';

for (let i = 0; i < strings.length; i++) {

str += strings[i];

if (i < values.length) str += values[i];

}

return str;

}

Copilot uses AI. Check for mistakes.

packages/contracts/test/tests/unit/staking/configuration.test.ts

const impl = await proxyAdmin.getProxyImplementation(staking.address)

const factory = await ethers.getContractFactory('StakingExtension')

const factory = await ethers.getContractFactory('contracts/staking/StakingExtension.sol:StakingExtension')

Copy link

Preview

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Hard-coded contract path makes the test brittle to file reorganization. Consider using a constant or helper function to manage contract paths.

Suggested change

const factory = await ethers.getContractFactory('contracts/staking/StakingExtension.sol:StakingExtension')

const factory = await ethers.getContractFactory(STAKING_EXTENSION_CONTRACT_PATH)

Copilot uses AI. Check for mistakes.

packages/token-distribution/scripts/build.js

} else {

// No API key and no cached artifacts - cannot proceed

// To fix: Set STUDIO_API_KEY or GRAPH_API_KEY environment variable

console.error('❌ No API key or cached GraphClient artifacts available')

Copy link

Preview

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

Error message could be more helpful by explaining exactly which environment variables need to be set (STUDIO_API_KEY or GRAPH_API_KEY).

Suggested change

console.error('❌ No API key or cached GraphClient artifacts available')

console.error('❌ No API key or cached GraphClient artifacts available. Please set the STUDIO_API_KEY or GRAPH_API_KEY environment variable and try again.')

Copilot uses AI. Check for mistakes.

@RembrandtK RembrandtK requested a review from tmigone

August 15, 2025 15:00

@RembrandtK RembrandtK self-assigned this

Aug 15, 2025

tmigone

tmigone reviewed

Sep 1, 2025

View reviewed changes

package.json

@@ -48,7 +45,7 @@

"prettier": "^3.5.3",

"prettier-plugin-solidity": "^1.0.0",

"pretty-quick": "^4.1.1",

"solhint": "^5.1.0",

"solhint": "catalog:",

Copy link

Member

@tmigone tmigone Sep 1, 2025

Choose a reason for hiding this comment

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

oh wow, TIL about this feature. Would have saved much pain aligning ethers versions in the past.

Labels

None yet

2 participants

@RembrandtK @tmigone

Baseline upgrades in preparation for service quality contracts PRs #1213

Are you sure you want to change the base?

Baseline upgrades in preparation for service quality contracts PRs #1213

Uh oh!

Conversation

@RembrandtK RembrandtK commented Aug 14, 2025

Uh oh!

openzeppelin-code bot commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Baseline upgrades in preparation for service quality contracts PRs

🚨 Report Summary

Uh oh!

codecov bot commented Aug 14, 2025

Codecov Report

Uh oh!

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

@tmigone tmigone Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openzeppelin-code bot commented Aug 14, 2025 •

edited

Loading