Skip to content

various Solidity Foundry updates - #14866

Merged
Tofel merged 5 commits into
developfrom
tt-1728_tt-1784_tt-1785-various-solidity-fixes
Oct 23, 2024
Merged

various Solidity Foundry updates#14866
Tofel merged 5 commits into
developfrom
tt-1728_tt-1784_tt-1785-various-solidity-fixes

Conversation

@Tofel

@Tofel Tofel commented Oct 21, 2024

Copy link
Copy Markdown
Contributor

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
@github-actions

github-actions Bot commented Oct 22, 2024

Copy link
Copy Markdown
Contributor

AER Report: CI Core ran successfully ✅

aer_workflow , commit

working-directory: ./contracts

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

env:
FOUNDRY_PROFILE: ${{ matrix.product.name }}

check-fmt-results:

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
@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)
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