contracts icon indicating copy to clipboard operation
contracts copied to clipboard

chore: move all interfaces to dedicated package

Open tmigone opened this issue 5 months ago • 4 comments

List of changes:

  • Fixed some issues due to previous sync merge with main. These only surfaced with integration tests.
  • Rename common to interfaces. I feel this represents more accurately what this package should be, but I'm happy to further discuss the name and/or use of it.
  • Updated horizon, subgraph-service, toolshed and hardhat-graph-protocol to use interfaces. Most notably this eliminates the circular dependency allowing for arbitrary building of the packages and static import of hardhat-graph-protocol.
  • Fix linting issues in contracts package.
  • Move sdk package to it's own repository at https://github.com/graphprotocol/sdk. contracts package (actually child packages) remains the only place where it's used, it's now pinned to latest published version.
  • hardhat-graph-protocol now accepts a flag to specify if address book should be created or not if it doesn't exist.
  • Moved test fixtures to toolshed
  • Refactored how toolshed manages types and interface imports, its now much simpler.

Pending:

  • While bytecode shouldn't have changed for horizon and subgraph-service contracts it's worth checking with audit team wether or not they want to take a look at the changes.
  • contracts package still uses it's own interfaces (though they are duplicated in interfaces). This seemed like a big lift atm.
  • IGraphToken interface is widely used and for the most part being imported from contracts and not interfaces. This is due to the usage of TokenUtils.sol which also imports the file. Importing it from two sources creates problems. Perhaps this warrants a common shared solidity package, not interfaces but implementations/libs?

tmigone avatar Jun 11 '25 19:06 tmigone

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedjson5@​2.2.310010010079100
Addedlodash@​4.17.211001008680100
Updateddebug@​4.4.0 ⏵ 4.4.1100 +110010082100
Addedconsola@​2.15.39910010083100
Updatedhardhat@​2.23.0 ⏵ 2.25.094 +110090 +199 -180

View full report

socket-security[bot] avatar Jun 11 '25 19:06 socket-security[bot]

chore: move all interfaces to dedicated package

Generated at commit: 523d3b1e656b14f328ccf4383aa3ace4ddded3df

🚨 Report Summary

Severity Level Results
Contracts Critical High Medium Low Note Total 2 4 0 15 39 60
Dependencies Critical High Medium Low Note Total 0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

openzeppelin-code[bot] avatar Jun 11 '25 19:06 openzeppelin-code[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.84%. Comparing base (77007c0) to head (523d3b1). Report is 39 commits behind head on horizon.

Additional details and impacted files
@@           Coverage Diff            @@
##           horizon    #1187   +/-   ##
========================================
  Coverage    82.84%   82.84%           
========================================
  Files           47       47           
  Lines         2093     2093           
  Branches       620      620           
========================================
  Hits          1734     1734           
  Misses         359      359           
Flag Coverage Δ
unittests 82.84% <ø> (ø)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Jun 11 '25 20:06 codecov[bot]

[!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 High
[email protected] is Protestware or potentially unwanted behavior.

Note: This package prints a protestware console message on install regarding Ukraine for users with Russian language locale

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

ℹ Read more on: This package | This alert | What is protestware?

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: Consider that consuming this package may come along with functionality unrelated to its primary purpose.

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.

Warn High
[email protected] is Protestware or potentially unwanted behavior.

Note: The script attempts to run a local post-install script, which could potentially contain malicious code. The error handling suggests that it is designed to fail silently, which is a common tactic in malicious scripts.

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

ℹ Read more on: This package | This alert | What is protestware?

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: Consider that consuming this package may come along with functionality unrelated to its primary purpose.

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

socket-security[bot] avatar Jun 12 '25 14:06 socket-security[bot]