# This is a combination of 14 commits.

# This is the 1st commit message: update contract # This is the commit message #2: make scripts executable # This is the commit message #3: upload base too # This is the commit message #4: try again # This is the commit message #5: gather changesets, print products out of scope # This is the commit message #6: fix product finding # This is the commit message #7: try again # This is the commit message #8: debug # This is the commit message #9: debug 2 # This is the commit message #10: grab only modified or added changesets # This is the commit message #11: try again 1 # This is the commit message #12: remove early exit # This is the commit message #13: place changesets in a subfolder in final artifact # This is the commit message #14: validate whether everything was generated
smartcontractkit · Aug 2, 2024 · d81404f · d81404f
1 parent 0eec5b6
commit d81404f
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 98 deletions.
diff --git a/.github/workflows/solidity-foundry.yml b/.github/workflows/solidity-foundry.yml
@@ -200,87 +200,14 @@ jobs:
           python -m pip install --upgrade pip
           pip install slither-analyzer
 
-      - name: Run Slither for current changes
+      - name: Run Slither
         shell: bash
         run: |
           CHANGED="${{ needs.changes.outputs.sol_files }}"
           CHANGED_ARRAY=$(echo "$CHANGED" | tr ' ' ',')
           
           ./contracts/scripts/ci/generate_slither_report.sh "${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/" contracts/.slither.config-artifacts.json "." "$CHANGED_ARRAY" "contracts/slither-reports" "--solc-remaps @=contracts/node_modules/@"
 
-        # Run this step only if some existing files were modified
-      - name: Upload Slither results for current commit
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
-        timeout-minutes: 2
-        continue-on-error: true
-        with:
-          name: tmp-slither-reports-${{ github.sha }}
-          path: contracts/slither-reports
-          retention-days: 7
-
-        # Run this step only if some existing files were modified
-      - name: Upload Slither scripts
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
-        timeout-minutes: 2
-        continue-on-error: true
-        with:
-          name: tmp-slither-scripts-${{ github.sha }}
-          path: contracts/scripts/ci
-          retention-days: 7
-
-      - name: Checkout the repo
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
-        with:
-          submodules: recursive
-          ref: ${{ github.base_ref }}
-
-      - name: Download Slither scripts
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
-        with:
-          name: tmp-slither-scripts-${{ github.sha }}
-          path: contracts/scripts/ci
-
-      # Run this step only if some existing files were modified
-      - name: Run Slither for base ref
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        shell: bash
-        run: |
-          MODIFIED="${{ needs.changes.outputs.mod_sol_files }}"
-          MODIFIED_ARRAY=$(echo "$MODIFIED" | tr ' ' ',')
-          
-          ./contracts/scripts/ci/generate_slither_report.sh "${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/" contracts/.slither.config-pr.json "." "$MODIFIED_ARRAY" "contracts/slither-reports-original" "--solc-remaps @=contracts/node_modules/@"
-
-      - name: Download Slither artifact
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
-        with:
-          name: tmp-slither-reports-${{ github.sha }}
-          path: contracts/slither-reports
-
-      - name: Merge Slither reports
-        if: needs.changes.outputs.mod_sol_changes == 'true'
-        run: |
-          for original_file in contracts/slither-reports-original/*.md; do
-            base_file=$(basename "$original_file")
-            new_file="contracts/slither-reports/$base_file"
-            diff_file="contracts/slither-reports/${base_file%.md}_diff.md"
-            if [ -f "$new_file" ]; then
-              # we need to use awk to skip the first line of the diff output, which contains info about line number and type of change that do not interest us
-              diff "$original_file" "$new_file" | awk 'NR>1' > "$diff_file" 
-            else
-              echo "File $new_file does not exist, skipping diff."
-            fi
-          done
-          
-          for original_file in contracts/slither-reports/*_diff.md; do
-            echo "$original_file:"
-            cat "$original_file"
-          done
-
       - name: Print Slither summary
         shell: bash
         run: |
@@ -314,10 +241,8 @@ jobs:
 
   fmt:
     needs: [ changes ]
-    name: Check formatting
-    # TODO remove me
-    if: needs.changes.outputs.sol_changes == 'mietek'
-    # if: needs.changes.outputs.sol_changes == 'true'
+    name: Check formatting for modified files
+    if: needs.changes.outputs.sol_changes == 'true'
     runs-on: ubuntu-22.04
     steps:
       - name: Checkout the repo
@@ -333,17 +258,16 @@ jobs:
         with:
           version: ${{ env.FOUNDRY_VERSION }}
 
-      # TODO think about running this with correct FOUNDRY PROFILE
-
       - name: Run Forge fmt
         run: |
           SOL_FILES="${{ needs.changes.outputs.sol_files }}"
           FILE_LIST=$(echo "$SOL_FILES" | tr ' ' '\n')
           format_errors=0
           for file in $FILE_LIST; do
             sanitized_file="${file/contracts\//}"
-            echo "Running forge fmt --check on $sanitized_file"
-            forge fmt --check "$sanitized_file" || format_errors=$((format_errors+1))
+            PROFILE=$(echo "$sanitized_file" | awk -F'src/[^/]*/' '{print $2}' | cut -d'/' -f1)            
+            echo "::debug::Running forge fmt $sanitized_file with FOUNDRY_PROFILE=$PROFILE"
+            FOUNDRY_PROFILE=$PROFILE forge fmt --check "$sanitized_file" || format_errors=$((format_errors+1))
           done
           if [ $format_errors -ne 0 ]; then
             echo "There were $format_errors formatting errors."

diff --git a/.github/workflows/solidity-hardhat.yml b/.github/workflows/solidity-hardhat.yml
@@ -5,14 +5,15 @@ on:
     inputs:
       product:
         type: choice
-        description: 'product for which to generate artifacts; should be the same as foundry profile'
+        description: 'product for which to generate artifacts; should be the same as Foundry profile'
         required: true
         options:
           - "automation"
           - "ccip"
           - "functions"
           - "keystone"
           - "l2ep"
+          - "liquiditymanager"
           - "llo-feeds"
           - "operatorforwarder"
           - "transmission"
@@ -37,27 +38,51 @@ jobs:
       product_changes: ${{ steps.changes.outputs.product }}
       shared_changes: ${{ steps.changes.outputs.shared }}
       files: ${{ steps.changes.outputs.included_files }}
+      changeset_changes: ${{ steps.changes-dorny.outputs.changeset }}
+      changeset_files: ${{ steps.changes-dorny.outputs.changeset_files }}
     steps:
       - name: Checkout the repo
         uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
       - name: Find modified contracts
-        uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+        uses: AurorNZ/paths-filter@3b1f3abc3371cca888d8eb03dfa70bc8a9867629 # v4.0.0
         id: changes
         with:
           list-files: 'csv'
           base: ${{ env.BASE_REF }} #TODO change to inputs.base_ref after testing
           filters: |
+            ignored: &ignored
+              - '!contracts/src/v0.8/**/test/**'
+              - '!contracts/src/v0.8/**/tests/**'
+              - '!contracts/src/v0.8/**/*.t.sol'
+              - '!contracts/src/v0.8/*.t.sol'
+              - '!contracts/src/v0.8/**/testhelpers/**'
+              - '!contracts/src/v0.8/testhelpers/**'
+              - '!contracts/src/v0.8/vendor/**'
             shared: &shared
-              - modified|added: 'contracts/src/!(v0.8/**/test/**|v0.8/**/tests/**|v0.8/**/*.t.sol|v0.8/*.t.sol|v0.8/**/testhelpers/**|v0.8/testhelpers/**|v0.8/vendor/**)/(shared/**/*.sol|interfaces/**/*.sol|*.sol)'
-            all_sol:
-              - modified|added: 'contracts/src/v0.8/**/*.sol'
+              - 'contracts/src/v0.8/shared/**/*.sol'
+              - 'contracts/src/v0.8/interfaces/**/*.sol'
+              - 'contracts/src/v0.8/*.sol'
+              - *ignored     
             sol:
-              - modified|added: 'contracts/src/v0.8/!(test|tests|testhelpers|vendor)/**/!(test|tests|testhelpers|vendor)/!(test|tests|testhelpers|vendor|*.t.sol|vendor)/**/*.sol'
+              - 'contracts/src/v0.8/**/*.sol'
+              - *ignored   
             product: &product
-              - modified|added: 'contracts/src/!(v0.8/**/test/**|v0.8/**/tests/**|v0.8/**/*.t.sol|v0.8/*.t.sol|v0.8/**/testhelpers/**|v0.8/testhelpers/**|v0.8/vendor/**)/${{ env.PRODUCT }}/**/*.sol'
+              - 'contracts/src/v0.8/${{ env.PRODUCT }}/**/*.sol'         
+              - *ignored            
             included:
               - *product
               - *shared
+              - *ignored
+
+      - name: Find modified changesets
+        uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+        id: changes-dorny
+        with:
+          list-files: 'shell'
+          base: ${{ env.BASE_REF }} #TODO change to inputs.base_ref after testing
+          filters: |
+            changeset:
+              - modified|added: 'contracts/.changeset/!(README)*.md'
 
       - name: Check for changes outside of artifact scope
         if: ${{ steps.changes.outputs.sol == 'true' }}
@@ -70,22 +95,39 @@ jobs:
           excluded_paths_pattern="!/^contracts\/src\/v0\.8\/shared/ && !/^contracts\/src\/v0\.8\/${{ env.PRODUCT}}/ && !/^contracts\/src\/v0\.8\/[^\/]+\.sol$/"
           echo "::debug::Excluded paths: $excluded_paths_pattern"
           unexpected_files=$(echo ${{ steps.changes.outputs.sol_files }} | tr ',' '\n' | awk "$excluded_paths_pattern")
+          set -e 
+          set -o pipefail
           if [[ -n "$unexpected_files" ]]; then
+            products=()
+            productsStr=""
+            IFS=$'\n' read -r -d '' -a files <<< "$unexpected_files" || true
+            echo "Files: ${files[@]}"
+          
+            for file in "${files[@]}"; do
+              product=$(echo "$file" | awk -F'src/[^/]*/' '{print $2}' | cut -d'/' -f1)
+              if [[ ! " ${products[@]} " =~ " ${product} " ]]; then
+                products+=("$product")
+                productsStr+="$product, "
+              fi
+            done
+            productsStr=${productsStr%, }
+          
+            set +e 
+            set +o pipefail
+          
             echo "Error: Found modified contracts outside of the expected scope: ${{ env.PRODUCT }}, shared"
             echo "Files:"
             echo "$unexpected_files"
-            echo "Please run this workflow separately for other products, if you want to generate the artifacts"
+            echo "Action required: If you want to generate artifacts for other products ($productsStr) run this workflow again with updated configuration"
           
             echo "# Warning!" >> $GITHUB_STEP_SUMMARY
             echo "## Reason: Found modified contracts outside of the expected scope: ${{ env.PRODUCT }}, shared" >> $GITHUB_STEP_SUMMARY
             echo "### Files:" >> $GITHUB_STEP_SUMMARY
             echo "$unexpected_files" >> $GITHUB_STEP_SUMMARY
-            echo "## Action required: Please run this workflow separately for other products, if you want to generate the artifacts" >> $GITHUB_STEP_SUMMARY            
+            echo "## Action required: If you want to generate artifacts for other products ($productsStr) run this workflow again with updated configuration" >> $GITHUB_STEP_SUMMARY            
           else
             echo "No unexpected files found."
           fi
-          
-          exit 1
 
       - name: Check for changes only in shared
         if: ${{ steps.changes.outputs.product == 'false' && steps.changes.outputs.shared == 'true' }}
@@ -98,6 +140,34 @@ jobs:
           #TODO change to inputs.base_ref after testing
           echo "## Base Ref: ${{ env.BASE_REF }}" >> $GITHUB_STEP_SUMMARY
 
+  gather-changesets:
+    if: ${{ needs.changes.outputs.changeset_changes == 'true' }}
+    name: Gather modified changesets
+    runs-on: ubuntu-22.04
+    needs: [ changes ]
+    steps:
+      - name: Checkout the repo
+        uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
+        with:
+          submodules: recursive
+
+      - name: Copy modified changesets
+        run: |
+          mkdir -p contracts/changesets/changesets
+          for changeset in "${{ needs.changes.outputs.changeset_files }}"; do
+            echo "::debug:: Copying $changeset"
+            cp $changeset contracts/changesets/changesets
+          done
+
+      - name: Upload changesets
+        uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
+        timeout-minutes: 2
+        continue-on-error: true
+        with:
+          name: tmp-changesets
+          path: contracts/changesets
+          retention-days: 7
+
   # some of the artifacts can only be generated on product level, and we cannot scope them to single contracts
   # some product-level modifications might also require shared contracts changes, so if these happened we need to generate artifacts for shared contracts as well
   coverage-and-book:
@@ -309,23 +379,58 @@ jobs:
             contracts/commit_sha_base_ref.txt
           retention-days: 7
 
+      - name: Validate UMLs and Slither reports
+        if: ${{ needs.changes.outputs.product_changes == 'true' || needs.changes.outputs.shared_changes == 'true' }}
+        run: |
+          echo "Validating UMLs and Slither reports"
+          IFS=',' read -r -a modified_files <<< "${{ needs.changes.outputs.files }}"
+          missing_svgs=()
+          missing_reports=()
+          for file in "${modified_files[@]}"; do
+            svg_file="$(basename "${file%.sol}").svg"
+            if [ ! -f "uml-diagrams/$svg_file" ]; then
+              echo "Error: UML diagram for $file not found"
+              missing_svgs+=("$file")
+            fi
+            
+            report_file="$(basename "${file%.sol}")-slither-report.md"            
+            if [ ! -f "slither-reports/$report_file" ]; then
+              echo "Error: Slither report for $file not found"
+              missing_reports+=("$file")
+            fi
+          done
+          
+          if [ ${#missing_svgs[@]} -gt 0 ]; then
+              echo "Error: Missing UML diagrams for files: ${missing_svgs[@]}"
+              echo "# Warning!" >> $GITHUB_STEP_SUMMARY
+              echo "## Reason: Missing UML diagrams for files: ${missing_svgs[@]}" >> $GITHUB_STEP_SUMMARY
+              echo "## Action required: Please try generate artifacts for them locally or using a different tool" >> $GITHUB_STEP_SUMMARY
+          fi
+          
+          if [ ${#missing_reports[@]} -gt 0 ]; then
+              echo "Error: Missing Slither reports for files: ${missing_reports[@]}"
+              echo "# Warning!" >> $GITHUB_STEP_SUMMARY
+              echo "## Reason: Missing Slither reports for files: ${missing_reports[@]}" >> $GITHUB_STEP_SUMMARY
+              echo "## Action required: Please try generate artifacts for them locally" >> $GITHUB_STEP_SUMMARY
+          fi
+
   gather-all-artifacts:
     name: Gather all artifacts
     if: ${{ needs.changes.outputs.product_changes == 'true' || needs.changes.outputs.shared_changes == 'true' }}
     runs-on: ubuntu-latest
-    needs: [coverage-and-book, uml-static-analysis, changes]
+    needs: [coverage-and-book, uml-static-analysis, gather-changesets, changes]
     steps:
       - name: Download all artifacts
         uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
         with:
-          path: review_artifacts-${{ env.PRODUCT }}-${{ github.sha }}
+          path: review_artifacts
           merge-multiple: true
 
       - name: Upload all artifacts as single package
         uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4
         with:
           name: review-artifacts-${{ env.PRODUCT }}-${{ github.sha }}
-          path: review_artifacts-${{ env.PRODUCT }}-${{ github.sha }}
+          path: review_artifacts
 
       - name: Remove temporary artifacts
         uses: geekyeggo/delete-artifact@24928e75e6e6590170563b8ddae9fac674508aa1 # v5.0
@@ -358,7 +463,7 @@ jobs:
           #TODO change to inputs.base_ref after testing
           echo "Base Ref used: **${{ env.BASE_REF }}**" >> $GITHUB_STEP_SUMMARY
           echo "Commit SHA used: **${{ github.sha }}**" >> $GITHUB_STEP_SUMMARY
-          echo "## Reason: No modified Solidity files found." >> $GITHUB_STEP_SUMMARY
+          echo "## Reason: No modified Solidity files found for ${{ env.PRODUCT }}" >> $GITHUB_STEP_SUMMARY
           #TODO change to inputs.base_ref after testing
           echo "No modified Solidity files found between ${{ env.BASE_REF }} and ${{ github.sha }} commits or they are located outside of ./contracts/src/v0.8 folder" >> $GITHUB_STEP_SUMMARY
           exit 1

diff --git a/contracts/src/v0.8/automation/v2_3/AutomationRegistrar2_3.sol b/contracts/src/v0.8/automation/v2_3/AutomationRegistrar2_3.sol
@@ -258,7 +258,7 @@ contract AutomationRegistrar2_3 is TypeAndVersionInterface, ConfirmedOwner, IERC
     emit ConfigChanged();
   }
 
-  uint256 upkeepId2;
+  uint256 upkeepId4;
   uint256 upkeepId3;
 
   /**