Skip to content
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

various Solidity Foundry updates #14866

Merged
merged 5 commits into from
Oct 23, 2024

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Oct 21, 2024

This PR introduces a couple of changes:

  • adds two new jobs that gather result of matrix jobs for tests and formatting, so that we can use them as required checks
  • modifies formatting job condition in such a way that it also runs for test files
  • adds back static analysis with Slither
  • post Solidity Review Ticket comment only if such Jira issue was found/created

Tests:

  • if all test jobs are skipped, check job passes
  • if any of the test jobs fails, check job fails
  • if any of the test jobs is cancelled, check job fails
  • if all format jobs are skipped, check job passes
  • if any of the format jobs fails, check job fails
  • if any of the format jobs is cancelled, check job fails
  • if neither tests nor format jobs run, both checks pass
  • slither with diff runs only if Sol files are modified
  • slither config upload works
  • if slither config upload fails, workflow is failed
  • formatting jobs run also if test files are modified
  • no Solidity Review comment is created, if ticket is not found
  • Solidity Review comment is created, if ticket is found

@Tofel Tofel force-pushed the tt-1728_tt-1784_tt-1785-various-solidity-fixes branch from de6e810 to 500f4ec Compare October 21, 2024 16:00
@Tofel Tofel marked this pull request as ready for review October 22, 2024 07:37
@Tofel Tofel requested review from a team as code owners October 22, 2024 07:37
Copy link
Contributor

github-actions bot commented Oct 22, 2024

AER Report: CI Core ran successfully ✅

aer_workflow , commit

@@ -252,9 +252,286 @@ jobs:
artifact-name: code-coverage-report-${{ matrix.product.name }}
working-directory: ./contracts

# runs only if non-test contracts were modified; scoped only to modified or added contracts
analyze:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added back whole static analysis job

path: contracts/configs
retention-days: 7
if-no-files-found: error
include-hidden-files: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the only real change vs previous version, required as config files start with a .

solidity-forge-fmt:
name: Forge fmt ${{ matrix.product.name }}
if: ${{ needs.changes.outputs.non_src_changes == 'true' || needs.changes.outputs.not_test_sol_modified == 'true' }}
if: ${{ needs.changes.outputs.non_src_changes == 'true' || needs.changes.outputs.sol_modified_added == 'true' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will now run forge fmt when any Solidity files change, not only non-test ones

with:
name: tmp-*

check-tests-results:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

new job that checks for results of the test matrix, we will set it as a required job

@@ -285,3 +562,16 @@ jobs:
working-directory: contracts
env:
FOUNDRY_PROFILE: ${{ matrix.product.name }}

check-fmt-results:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

new job that checks for results of the formatting matrix, we will set it as a required job

Copy link
Contributor

Choose a reason for hiding this comment

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

typo traceability

Copy link
Contributor

Choose a reason for hiding this comment

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

also the name at line 2

@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@Tofel Tofel added this pull request to the merge queue Oct 23, 2024
Merged via the queue into develop with commit 0c58b3c Oct 23, 2024
103 checks passed
@Tofel Tofel deleted the tt-1728_tt-1784_tt-1785-various-solidity-fixes branch October 23, 2024 08:29
momentmaker added a commit that referenced this pull request Oct 23, 2024
* develop:
  rm gethwrappers from codeowners offchain (#14919)
  CCIP-3899 fix sender encoding (#14877)
  ccip: use unknown address type. (#14836)
  [chore] Add changeset entry for RequestRoundTracker fix (#14912)
  various Solidity Foundry updates (#14866)
  CCIP-3710 create new custom calldata L1 (DA) gas oracle (#14710)
  CCIP owns smoke test (#14906)
  core/services/ocr2/plugins/ccip/internal/ccipdata/factory: check error from TypeAndVersion (#14861)
  CCIP-3730 Misc ccip onchain changes (#14845)
  Chain fee integration tests (#14829)
  release/2.17.0 -> develop (#14721)
  Solana: re-enable disabled test with updated version (#14892)
cedric-cordenier pushed a commit that referenced this pull request Oct 24, 2024
* add two new aggregate jobs for tests and formatting, make formatting run also for tests, add back static analysis

* update AER version, post solidity review ticket comment only if such ticket was found

* revert AER version change

* revert changes to llm aer file
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.

4 participants