Skip to content

Conversation

@RembrandtK
Copy link
Contributor

@RembrandtK RembrandtK commented Oct 15, 2025

Status:

  • Pending audit to confirm no impact on production deployed bytecode or other need for a full audit.
  • Likely to additionally be delayed post-Horizon upgrade completion to avoid intereference.

This PR resolves all Solidity linting issues across the monorepo, enabling clean pnpm lint execution.

Additionally:

  • Duplicate interface contracts have been removed with interfaces package consistently used.
  • Standardised open-ended interface compiler versions to allow wider consumer compatibility with interfaces. Dual-version files use ^0.8.0 since they can't leverage v0.8 only features anyway. V0.8-only files use ^0.8.22 to include custom errors and named mappings.

Summary

  • 260 files changed (+5,087 / -1,569 lines)
  • ~3,000 NatSpec comments added (@title, @author, @notice, @param, @return, @dev)
  • ~220 solhint suppressions with TODO comments for unavoidable violations

Solhint Suppressions Added

Most common suppressed rules (all include TODO comments for future fixes):

  • gas-indexed-events (36) - Event parameters without indexed
  • use-natspec (36) - Legacy code without documentation
  • named-parameters-mapping (25) - Requires Solidity ≥0.8.18
  • immutable-vars-naming (19) - Non-UPPER_CASE immutables
  • gas-strict-inequalities (15) - Using <= vs <
  • gas-custom-errors (7) - Using require (0.7.6 limitation)
  • no-inline-assembly (5) - Critical assembly blocks
  • const-name-snakecase (3) - Non-UPPER_CASE constants
  • Other gas optimizations and formatting rules

Verification

Bytecode comparison (compare script): Only mock/test contracts have functional changes (added indexed to events). All production contracts are functionally identical.

All packages pass pnpm lint with zero errors/warnings

- Add NatSpec documentation (@title, @author, @notice, @param)
- Add solhint-disable directives with TODO comments for future fixes
- Minor code formatting changes to address lint rules

All packages now pass pnpm lint with no reported issues.

Verified with scripts/compare-repo-contract-bytecode-excluding-metadata.mjs:
only mock contracts have functional bytecode changes.
@openzeppelin-code
Copy link

openzeppelin-code bot commented Oct 15, 2025

chore(contracts): fix solidity linting issues across all packages

Generated at commit: 4b63fadb00fade821e7e5eda34009b8121ddb03a

🚨 Report Summary

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

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.06%. Comparing base (cd59452) to head (4b63fad).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1239      +/-   ##
==========================================
+ Coverage   82.84%   84.06%   +1.21%     
==========================================
  Files          47       42       -5     
  Lines        2093     2071      -22     
  Branches      620      615       -5     
==========================================
+ Hits         1734     1741       +7     
+ Misses        359      330      -29     
Flag Coverage Δ
unittests 84.06% <100.00%> (+1.21%) ⬆️

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 self-assigned this Oct 17, 2025
RembrandtK and others added 3 commits October 20, 2025 13:47
- Move 36 duplicate interface files from contracts and token-distribution
  to interfaces package
- Update all import statements across contracts, token-distribution,
  horizon, and subgraph-service to use @graphprotocol/interfaces
- Delete duplicate interface files from source packages
- Fix pragma versions for compatibility across compiler configurations
- Add @graphprotocol/interfaces dependency to token-distribution

All builds and tests passing.
- Dual-version interfaces: ^0.7.x || ^0.8.0 (max compatibility)
- V0.8-only interfaces: ^0.8.22 (modern features support)

Interfaces should allow wider compiler version ranges for external
consumer compatibility, while implementations use exact versions for
predictable compilation. Dual-version files use ^0.8.0 since they
can't leverage v0.8 features anyway. V0.8-only files use ^0.8.22 for
custom errors and named mappings.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add explanation of open-ended pragma ranges for interface compatibility
and document available TypeScript type generation formats.
@RembrandtK
Copy link
Contributor Author

Replaced by: #1242

@RembrandtK RembrandtK closed this Oct 20, 2025
@RembrandtK RembrandtK deleted the solidity-lint-fixes-2 branch October 22, 2025 20:52
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