Skip to content

Conversation

RembrandtK
Copy link
Contributor

@RembrandtK RembrandtK commented Oct 6, 2025

This PR has two main commits that can be split into separate PRs if required. (A a few trailing ones to address issues discovered.) The second is quite large and involves changing natspec for many Solidity files, while the first is smaller concentrating on build and lint processing.

feat: upgrade build processing and linting
• Standardize linting configs across all packages (.solhint.json, .markdownlint.json)
• Remove deprecated natspec-smells and storage verification scripts
• Add new utility scripts (check-todos.sh, lint-staged-run.sh, verify-solhint-disables.js)
• Update CI workflows and pnpm workspace configuration

fix(lint): resolving or suppressing all Solhint lint issues
• Add comprehensive NatSpec documentation to all contract interfaces and functions
• Suppress unavoidable lint warnings with inline comments where appropriate
• Fix visibility and naming convention issues across all contract files
• Add missing SPDX license identifiers and pragma statements
• Resolve import ordering and unused variable warnings
• Update function parameter and return value documentation
• Fix modifier and event documentation formatting

• Standardize linting configs across all packages (.solhint.json, .markdownlint.json)
• Remove deprecated natspec-smells and storage verification scripts
• Add new utility scripts (check-todos.sh, lint-staged-run.sh, verify-solhint-disables.js)
• Update CI workflows and pnpm workspace configuration
@RembrandtK RembrandtK requested a review from Copilot October 6, 2025 19:07
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 upgrades build and lint processing across all packages, standardizing linting configurations and clearing lint issues. The changes include standardizing linting configs, removing deprecated tools, adding new utility scripts, updating CI workflows, and fixing comprehensive lint issues across Solidity files with NatSpec documentation improvements and lint suppressions.

Reviewed Changes

Copilot reviewed 298 out of 324 changed files in this pull request and generated 2 comments.

File Description
Multiple packages Standardized .solhint.json configurations and updated package.json scripts
Multiple Solidity files Added comprehensive NatSpec documentation and lint suppressions
Build scripts Updated build commands to use dependency-aware builds
Contract interfaces Enhanced documentation with @author and improved @notice tags
Comments suppressed due to low confidence (1)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

- Replace deprecated GlobSync class with globSync function
- Update import to use named import from glob package
- Fixes TypeScript compilation errors in CI build
Copy link

openzeppelin-code bot commented Oct 6, 2025

Upgrade build and lint processing, and clear all lint issues

Generated at commit: 27cade3493ca389601a460b8d3e0a5a7a48e1235

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
4
0
15
38
59
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 47.36842% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.15%. Comparing base (cd59452) to head (27cade3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...acts/tests/L1GraphTokenLockTransferToolBadMock.sol 0.00% 4 Missing ⚠️
...ntracts/tests/L1GraphTokenLockTransferToolMock.sol 0.00% 4 Missing ⚠️
...ntracts/contracts/tests/MockERC165OnlyContract.sol 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1233      +/-   ##
==========================================
+ Coverage   82.84%   83.15%   +0.31%     
==========================================
  Files          47       48       +1     
  Lines        2093     2096       +3     
  Branches      620      620              
==========================================
+ Hits         1734     1743       +9     
+ Misses        359      353       -6     
Flag Coverage Δ
unittests 83.15% <47.36%> (+0.31%) ⬆️

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
Copy link
Contributor Author

The second commit message got overriden somehow to describe a minor subsequent tweak rather than the main point, the lint fixes.

@RembrandtK RembrandtK marked this pull request as ready for review October 6, 2025 22:26
@RembrandtK RembrandtK self-assigned this Oct 6, 2025
@RembrandtK RembrandtK requested a review from tmigone October 6, 2025 22:27
…mplementation

- Replace missing @graphql-mesh/utils dependency with simple local gql function
- Update extraction script to generate self-contained artifacts
- Fixes Hardhat initialization error: 'Cannot find module @graphql-mesh/utils'
- Maintains functionality while avoiding unnecessary dependency
- Update ethereumjs-util from 7.1.3 to 7.1.5
- Ensures consistent dependency versions across environments
- Update @openzeppelin/contracts from 3.4.1 to 3.4.2 in all packages
- Verified 98 contracts across 4 packages show no functional differences
- Only metadata hashes changed as expected

Package updates:
- packages/contracts/package.json
- packages/interfaces/package.json
- packages/token-distribution/package.json
- packages/contracts/task/package.json
- packages/contracts/test/package.json

Script improvements:
- Add compare-repo-contract-bytecode-excluding-metadata.py for reliable repo comparisons
- Remove obsolete bytecode-diff-no-metadata.sh script

Verification completed:
- contracts: 55 contracts ✅ functionally identical
- token-distribution: 20 contracts ✅ functionally identical
- horizon: 33 contracts ✅ functionally identical
- subgraph-service: 12 contracts ✅ functionally identical
…uilding

- Add timestamp-based checks for WAGMI, ethers-v5, and TypeScript compilation
- Skip type generation when output files are newer than source files
- Improve file-by-file comparison for dist directory organization
- Reduce build time from 5+ seconds to ~600ms when no changes needed
- Maintain build correctness while improving developer experience
The testSlash_RoundDown_TokensThawing_Delegation test was failing because
it could generate scenarios where undelegating would leave less than the
minimum delegation amount (1 ether) in the pool, which violates the
protocol's minimum delegation constraint.

Added assumption to ensure undelegation either removes all tokens or
leaves at least MIN_DELEGATION in the pool, making the test compliant
with the protocol's validation rules introduced in commit 91cda56.
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.

1 participant