From 6d072dd415c5b5ed0a247b9fd53873b6790e5a02 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Tue, 20 Aug 2024 14:10:54 +0200 Subject: [PATCH] [TT-1435] use chatgpt to find new issues in modified Solidity files (#14104) * use chatgpt to find new issues in modified Solidity files * update prompts * CR changes * use exit instead of return * add product name to solidity artifact summary * prune lcov report in artifact pipeline * pin llm versions --- .../workflows/solidity-foundry-artifacts.yml | 13 +- .github/workflows/solidity-foundry.yml | 174 +++++++++++++++++- .../scripts/ci/find_slither_report_diff.sh | 94 ++++++++++ contracts/scripts/ci/prompt-difference.md | 21 +++ contracts/scripts/ci/prompt-validation.md | 33 ++++ 5 files changed, 323 insertions(+), 12 deletions(-) create mode 100755 contracts/scripts/ci/find_slither_report_diff.sh create mode 100644 contracts/scripts/ci/prompt-difference.md create mode 100644 contracts/scripts/ci/prompt-validation.md diff --git a/.github/workflows/solidity-foundry-artifacts.yml b/.github/workflows/solidity-foundry-artifacts.yml index 7a96c812771..9bba72b2e4c 100644 --- a/.github/workflows/solidity-foundry-artifacts.yml +++ b/.github/workflows/solidity-foundry-artifacts.yml @@ -143,7 +143,8 @@ jobs: - name: Generate basic info and modified contracts list shell: bash run: | - echo "Commit SHA used to generate artifacts: ${{ inputs.commit_to_use || github.sha }}" > contracts/commit_sha_base_ref.txt + echo "Product: ${{ inputs.product }}" > contracts/commit_sha_base_ref.txt + echo "Commit SHA used to generate artifacts: ${{ inputs.commit_to_use || github.sha }}" >> contracts/commit_sha_base_ref.txt echo "Base reference SHA used to find modified contracts: ${{ inputs.base_ref }}" >> contracts/commit_sha_base_ref.txt IFS=',' read -r -a modified_files <<< "${{ needs.changes.outputs.product_files }}" @@ -221,11 +222,18 @@ jobs: env: FOUNDRY_PROFILE: ${{ inputs.product }} + - name: Prune lcov report + if: ${{ !contains(fromJson(steps.prepare-exclusion-list.outputs.coverage_exclusions), inputs.product) && needs.changes.outputs.product_changes == 'true' }} + shell: bash + working-directory: contracts + run: | + ./scripts/lcov_prune ${{ inputs.product }} ./code-coverage/lcov.info ./code-coverage/lcov.info.pruned + - name: Generate Code Coverage HTML report for product contracts if: ${{ !contains(fromJson(steps.prepare-exclusion-list.outputs.coverage_exclusions), inputs.product) && needs.changes.outputs.product_changes == 'true' }} shell: bash working-directory: contracts - run: genhtml code-coverage/lcov.info --branch-coverage --output-directory code-coverage + run: genhtml code-coverage/lcov.info.pruned --branch-coverage --output-directory code-coverage - name: Run Forge doc for product contracts if: ${{ needs.changes.outputs.product_changes == 'true' }} @@ -359,6 +367,7 @@ jobs: echo "Artifact ID: $ARTIFACT_ID" echo "# Solidity Review Artifact Generated" >> $GITHUB_STEP_SUMMARY + echo "Product: **${{ inputs.product }}**" >> $GITHUB_STEP_SUMMARY echo "Base Ref used: **${{ inputs.base_ref }}**" >> $GITHUB_STEP_SUMMARY echo "Commit SHA used: **${{ inputs.commit_to_use || github.sha }}**" >> $GITHUB_STEP_SUMMARY echo "[Artifact URL](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts/$ARTIFACT_ID)" >> $GITHUB_STEP_SUMMARY diff --git a/.github/workflows/solidity-foundry.yml b/.github/workflows/solidity-foundry.yml index 906cb76ffe5..c1da33dc6f4 100644 --- a/.github/workflows/solidity-foundry.yml +++ b/.github/workflows/solidity-foundry.yml @@ -8,6 +8,11 @@ env: # * use the top-level matrix to decide, which checks should run for each product. # * when enabling code coverage, remember to adjust the minimum code coverage as it's set to 98.5% by default. +# This pipeline will run product tests only if product-specific contracts were modified or if broad-impact changes were made (e.g. changes to this pipeline, Foundry configuration, etc.) +# For modified contracts we use a LLM to extract new issues introduced by the changes. For new contracts full report is delivered. +# Slither has a default configuration, but also supports per-product configuration. If a product-specific configuration is not found, the default one is used. +# Changes to test files do not trigger static analysis or formatting checks. + jobs: define-matrix: name: Define test matrix @@ -53,8 +58,10 @@ jobs: runs-on: ubuntu-latest outputs: non_src_changes: ${{ steps.changes.outputs.non_src }} - sol_modified: ${{ steps.changes.outputs.sol }} - sol_modified_files: ${{ steps.changes.outputs.sol_files }} + sol_modified_added: ${{ steps.changes.outputs.sol }} + sol_modified_added_files: ${{ steps.changes.outputs.sol_files }} + sol_mod_only: ${{ steps.changes.outputs.sol_mod_only }} + sol_mod_only_files: ${{ steps.changes.outputs.sol_mod_only_files }} not_test_sol_modified: ${{ steps.changes.outputs.not_test_sol }} not_test_sol_modified_files: ${{ steps.changes.outputs.not_test_sol_files }} all_changes: ${{ steps.changes.outputs.changes }} @@ -73,6 +80,8 @@ jobs: - 'contracts/package.json' sol: - modified|added: 'contracts/src/v0.8/**/*.sol' + sol_mod_only: + - modified: 'contracts/src/v0.8/**/!(*.t).sol' not_test_sol: - modified|added: 'contracts/src/v0.8/**/!(*.t).sol' automation: @@ -199,7 +208,6 @@ jobs: || needs.changes.outputs.non_src_changes == 'true') && matrix.product.setup.run-coverage }} run: | - sudo apt-get install lcov ./contracts/scripts/lcov_prune ${{ matrix.product.name }} ./contracts/lcov.info ./contracts/lcov.info.pruned - name: Report code coverage for ${{ matrix.product.name }} @@ -229,6 +237,7 @@ jobs: this-job-name: Foundry Tests ${{ matrix.product.name }} continue-on-error: true + # runs only if non-test contracts were modified; scoped only to modified or added contracts analyze: needs: [ changes, define-matrix ] name: Run static analysis @@ -268,25 +277,165 @@ jobs: # modify remappings so that solc can find dependencies ./contracts/scripts/ci/modify_remappings.sh contracts contracts/remappings.txt mv remappings_modified.txt remappings.txt + + # without it Slither sometimes fails to use remappings correctly + cp contracts/foundry.toml foundry.toml + + FILES="${{ needs.changes.outputs.not_test_sol_modified_files }}" + + for FILE in $FILES; do + PRODUCT=$(echo "$FILE" | awk -F'src/[^/]*/' '{print $2}' | cut -d'/' -f1) + echo "::debug::Running Slither for $FILE in $PRODUCT" + SLITHER_CONFIG="contracts/configs/slither/.slither.config-$PRODUCT-pr.json" + if [[ ! -f $SLITHER_CONFIG ]]; then + echo "::debug::No Slither config found for $PRODUCT, using default" + SLITHER_CONFIG="contracts/configs/slither/.slither.config-default-pr.json" + fi + ./contracts/scripts/ci/generate_slither_report.sh "${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/" "$SLITHER_CONFIG" "." "$FILE" "contracts/slither-reports-current" "--solc-remaps @=contracts/node_modules/@" + done + + # all the actions below, up to printing results, run only if any existing contracts were modified + # in that case we extract new issues introduced by the changes by using an LLM model + - name: Upload Slither results for current branch + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4 + timeout-minutes: 2 + continue-on-error: true + with: + name: slither-reports-current-${{ github.sha }} + path: contracts/slither-reports-current + retention-days: 7 + + # we need to upload scripts and configuration in case base_ref doesn't have the scripts, or they are in different version + - name: Upload Slither scripts + if: needs.changes.outputs.sol_mod_only == '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: Upload configs + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4 + timeout-minutes: 2 + continue-on-error: true + with: + name: tmp-configs-${{ github.sha }} + path: contracts/configs + retention-days: 7 + + - name: Checkout the repo + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2 + with: + ref: ${{ github.base_ref }} + + - name: Download Slither scripts + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7 + with: + name: tmp-slither-scripts-${{ github.sha }} + path: contracts/scripts/ci - FILES="${{ needs.changes.outputs.not_test_sol_modified_files }}" + - name: Download configs + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7 + with: + name: tmp-configs-${{ github.sha }} + path: contracts/configs + + # since we have just checked out the repository again, we lose NPM dependencies installs previously, we need to install them again to compile contracts + - name: Setup NodeJS + if: needs.changes.outputs.sol_mod_only == 'true' + uses: ./.github/actions/setup-nodejs + + - name: Run Slither for base reference + if: needs.changes.outputs.sol_mod_only == 'true' + shell: bash + run: | + # we need to set file permission again since they are lost during download + for file in contracts/scripts/ci/*.sh; do + chmod +x "$file" + done + + # modify remappings so that solc can find dependencies + ./contracts/scripts/ci/modify_remappings.sh contracts contracts/remappings.txt + mv remappings_modified.txt remappings.txt + + # without it Slither sometimes fails to use remappings correctly + cp contracts/foundry.toml foundry.toml + + FILES="${{ needs.changes.outputs.sol_mod_only_files }}" for FILE in $FILES; do PRODUCT=$(echo "$FILE" | awk -F'src/[^/]*/' '{print $2}' | cut -d'/' -f1) echo "::debug::Running Slither for $FILE in $PRODUCT" SLITHER_CONFIG="contracts/configs/slither/.slither.config-$PRODUCT-pr.json" - if [ ! -f $SLITHER_CONFIG ]; then + if [[ ! -f $SLITHER_CONFIG ]]; then echo "::debug::No Slither config found for $PRODUCT, using default" SLITHER_CONFIG="contracts/configs/slither/.slither.config-default-pr.json" fi - ./contracts/scripts/ci/generate_slither_report.sh "${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/" "$SLITHER_CONFIG" "." "$FILE" "contracts/slither-reports" "--solc-remaps @=contracts/node_modules/@" + ./contracts/scripts/ci/generate_slither_report.sh "${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/" "$SLITHER_CONFIG" "." "$FILE" "contracts/slither-reports-base-ref" "--solc-remaps @=contracts/node_modules/@" + done + + - name: Upload Slither report + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4 + timeout-minutes: 10 + continue-on-error: true + with: + name: slither-reports-base-${{ github.sha }} + path: | + contracts/slither-reports-base-ref + retention-days: 7 + + - name: Download Slither results for current branch + if: needs.changes.outputs.sol_mod_only == 'true' + uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7 + with: + name: slither-reports-current-${{ github.sha }} + path: contracts/slither-reports-current + + - name: Generate diff of Slither reports for modified files + if: needs.changes.outputs.sol_mod_only == 'true' + env: + OPEN_API_KEY: ${{ secrets.OPEN_AI_SLITHER_API_KEY }} + shell: bash + run: | + set -euo pipefail + for base_report in contracts/slither-reports-base-ref/*.md; do + filename=$(basename "$base_report") + current_report="contracts/slither-reports-current/$filename" + new_issues_report="contracts/slither-reports-current/${filename%.md}_new_issues.md" + if [ -f "$current_report" ]; then + if ./contracts/scripts/ci/find_slither_report_diff.sh "$base_report" "$current_report" "$new_issues_report" "contracts/scripts/ci/prompt-difference.md" "contracts/scripts/ci/prompt-validation.md"; then + if [[ -s $new_issues_report ]]; then + awk 'NR==2{print "*This new issues report has been automatically generated by LLM model using two Slither reports. One based on `${{ github.base_ref}}` and another on `${{ github.sha }}` commits.*"}1' $new_issues_report > tmp.md && mv tmp.md $new_issues_report + echo "Replacing full Slither report with diff for $current_report" + rm $current_report && mv $new_issues_report $current_report + else + echo "No difference detected between $base_report and $current_report reports. Won't include any of them." + rm $current_report + fi + else + echo "::warning::Failed to generate a diff report with new issues for $base_report using an LLM model, will use full report." + fi + + else + echo "::error::Failed to find current commit's equivalent of $base_report (file $current_file doesn't exist, but should have been generated). Please check Slither logs." + exit 1 + fi done + # actions that execute only if any existing contracts were modified end here - name: Print Slither summary shell: bash run: | echo "# Static analysis results " >> $GITHUB_STEP_SUMMARY - for file in "contracts/slither-reports"/*.md; do + for file in "contracts/slither-reports-current"/*.md; do if [ -e "$file" ]; then cat "$file" >> $GITHUB_STEP_SUMMARY fi @@ -296,17 +445,17 @@ jobs: uses: ./.github/actions/validate-solidity-artifacts with: validate_slither_reports: 'true' - slither_reports_path: 'contracts/slither-reports' + slither_reports_path: 'contracts/slither-reports-current' sol_files: ${{ needs.changes.outputs.not_test_sol_modified_files }} - - name: Upload Slither report + - name: Upload Slither reports uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4.3.4 timeout-minutes: 10 continue-on-error: true with: name: slither-reports-${{ github.sha }} path: | - contracts/slither-reports + contracts/slither-reports-current retention-days: 7 - name: Collect Metrics @@ -320,6 +469,11 @@ jobs: this-job-name: Run static analysis continue-on-error: true + - name: Remove temp artifacts + uses: geekyeggo/delete-artifact@24928e75e6e6590170563b8ddae9fac674508aa1 # v5.0 + with: + name: tmp-* + 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' }} diff --git a/contracts/scripts/ci/find_slither_report_diff.sh b/contracts/scripts/ci/find_slither_report_diff.sh new file mode 100755 index 00000000000..d0b5238a1ac --- /dev/null +++ b/contracts/scripts/ci/find_slither_report_diff.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash + +set -euo pipefail + +if [[ "$#" -lt 4 ]]; then + >&2 echo "Generates a markdown file with diff in new issues detected by ChatGPT between two Slither reports." + >&2 echo "Usage: $0 [path-to-validation-prompt]" + exit 1 +fi + +if [[ -z "${OPEN_API_KEY+x}" ]]; then + >&2 echo "OPEN_API_KEY is not set." + exit 1 +fi + +first_report_path=$1 +second_report_path=$2 +new_issues_report_path=$3 +report_prompt_path=$4 +if [[ "$#" -eq 5 ]]; then + validation_prompt_path=$5 +else + validation_prompt_path="" +fi + +first_report_content=$(cat "$first_report_path" | sed 's/"//g' | sed -E 's/\\+$//g' | sed -E 's/\\+ //g') +second_report_content=$(cat "$second_report_path" | sed 's/"//g' | sed -E 's/\\+$//g' | sed -E 's/\\+ //g') +openai_prompt=$(cat "$report_prompt_path" | sed 's/"/\\"/g' | sed -E 's/\\+$//g' | sed -E 's/\\+ //g') +openai_model="gpt-4o-2024-05-13" +openai_result=$(echo '{ + "model": "'$openai_model'", + "temperature": 0.01, + "messages": [ + { + "role": "system", + "content": "'$openai_prompt' \nreport1:\n```'$first_report_content'```\nreport2:\n```'$second_report_content'```" + } + ] +}' | envsubst | curl https://api.openai.com/v1/chat/completions \ + -w "%{http_code}" \ + -o prompt_response.json \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer $OPEN_API_KEY" \ + -d @- +) + +# throw error openai_result when is not 200 +if [ "$openai_result" != '200' ]; then + echo "::error::OpenAI API call failed with status $openai_result: $(cat prompt_response.json)" + exit 1 +fi + +# replace lines starting with ' -' (1space) with ' -' (2spaces) +response_content=$(cat prompt_response.json | jq -r '.choices[0].message.content') +new_issues_report_content=$(echo "$response_content" | sed -e 's/^ -/ -/g') +echo "$new_issues_report_content" > "$new_issues_report_path" + +if [[ -n "$validation_prompt_path" ]]; then + echo "::debug::Validating the diff report using the validation prompt" + openai_model="gpt-4-turbo-2024-04-09" + report_input=$(echo "$new_issues_report_content" | sed 's/"//g' | sed -E 's/\\+$//g' | sed -E 's/\\+ //g') + validation_prompt_content=$(cat "$validation_prompt_path" | sed 's/"/\\"/g' | sed -E 's/\\+$//g' | sed -E 's/\\+ //g') + validation_result=$(echo '{ + "model": "'$openai_model'", + "temperature": 0.01, + "messages": [ + { + "role": "system", + "content": "'$validation_prompt_content' \nreport1:\n```'$first_report_content'```\nreport2:\n```'$second_report_content'```\nnew_issues:\n```'$report_input'```" + } + ] + }' | envsubst | curl https://api.openai.com/v1/chat/completions \ + -w "%{http_code}" \ + -o prompt_validation_response.json \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer $OPEN_API_KEY" \ + -d @- + ) + + # throw error openai_result when is not 200 + if [ "$validation_result" != '200' ]; then + echo "::error::OpenAI API call failed with status $validation_result: $(cat prompt_validation_response.json)" + exit 1 + fi + + # replace lines starting with ' -' (1space) with ' -' (2spaces) + response_content=$(cat prompt_validation_response.json | jq -r '.choices[0].message.content') + + echo "$response_content" | sed -e 's/^ -/ -/g' >> "$new_issues_report_path" + echo "" >> "$new_issues_report_path" + echo "*Confidence rating presented above is an automatic validation (self-check) of the differences between two reports generated by ChatGPT ${openai_model} model. It has a scale of 1 to 5, where 1 means that all new issues are missing and 5 that all new issues are present*." >> "$new_issues_report_path" + echo "" >> "$new_issues_report_path" + echo "*If confidence rating is low it's advised to look for differences manually by downloading Slither reports for base reference and current commit from job's artifacts*." >> "$new_issues_report_path" +fi diff --git a/contracts/scripts/ci/prompt-difference.md b/contracts/scripts/ci/prompt-difference.md new file mode 100644 index 00000000000..b7603c97482 --- /dev/null +++ b/contracts/scripts/ci/prompt-difference.md @@ -0,0 +1,21 @@ +You are a helpful expert data engineer with expertise in Blockchain and Decentralized Oracle Networks. + +Given two reports generated by Slither - a Solidity static analysis tool - provided at the bottom of the reply, your task is to help create a report for your peers with new issues introduced in the second report in order to decrease noise resulting from irrelevant changes to the report, by focusing on a single topic: **New Issues**. + +First report is provided under Heading 2 (##) called `report1` and is surrounded by triple backticks (```) to indicate the beginning and end of the report. +Second report is provided under Heading 2 (##) called `report2` and is surrounded by triple backticks (```) to indicate the beginning and end of the report. + +First report is report generated by Slither using default branch of the code repository. Second report is report generated by Slither using a feature branch of the code repository. You want to help your peers understand the impact of changes they introduced in the pull request on the codebase and whether they introduced any new issues. + +**New Issues** + +Provide a bullet point summary of new issues that were introduced in the second report. If a given issue is not present in first report, but is present in the second one, it is considered a new issue. If the count for given issue type is higher in the second report than in the first one, it is considered a new issue. +For each issue include original description text from the report together with severity level, issue ID, line number and a link to problematic line in the code. +Group the issues by their type, which is defined as Heading 2 (##). + +Output your response starting from**New Issues** in escaped, markdown text that can be sent as http body to API. Do not wrap output in code blocks. +Extract the name of the file from the first line of the report and title the new report with it in a following way: "# Slither's new issues in: " + +Remember that it might be possible that second report does not introduce any new issues. In such case, provide an empty report. + +Format **New Issues** as Heading 2 using double sharp characters (##). Otherwise, do not include any another preamble and postamble to your answer. diff --git a/contracts/scripts/ci/prompt-validation.md b/contracts/scripts/ci/prompt-validation.md new file mode 100644 index 00000000000..5fcf08e1462 --- /dev/null +++ b/contracts/scripts/ci/prompt-validation.md @@ -0,0 +1,33 @@ +You are a helpful expert data engineer with expertise in Blockchain and Decentralized Oracle Networks. + +At the bottom of the reply you will find two reports generated by Slither - a Solidity static analysis tool - and another report that contains new issues found in the second report. +Your task is to evaluate how well that new issues report shows all new issues mentioned in the second Slither report and assert its completeness. +Rate your confidence in the completeness of the new issues report on a scale from 1 to 5, where 1 means it's missing all new issues and 5 means that all new issues are present. + +First report is provided under Heading 2 (##) called `report1` and is surrounded by triple backticks (```) to indicate the beginning and end of the report. +Second report is provided under Heading 2 (##) called `report2` and is surrounded by triple backticks (```) to indicate the beginning and end of the report. +New issues report is provided under Heading 2 (##) called `new_issues` and is surrounded by triple backticks (```) to indicate the beginning and end of the report. + +Use the following steps to evaluate the new issues report: +* each report begins with a summary with types of issues found and number of issues found for each type, called "# Summary for " +* group issues by type and count for each report and calculate the expected difference in number of issues for each type for each report +* exclude all issue types, for which the count for is higher in the first report than in the second one +* for each remaining issue type, compare the number of issues found in the new issues report with the expected difference +* evaluate if the new issues report captures all new issues introduced in the second report + +Do not focus on: +* the quality of the Slither reports themselves, but rather on whether all new issues from the second report are present in the new issues report +* how well the new issues report is structured or written and how well it presents new issues + +It is crucial that you ignore all differences in the reports that are not related to new issues, such as resolved issues or issues, which count has decreased. + +If a given issue is not present in first report, but is present in the second one, it is considered a new issue. Similar behaviour is expected from the new issues report. +If the count for given issue type is higher in the second report than in the first one, it is considered a new issue. + +Your report should include only a single section titled "Confidence level". +Your evaluation of the completeness of the new issues report should be displayed as a Heading 3 using triple sharp characters (###). In a new line a brief explanation of the scale used, with minimum and maximum possible values. + +Remember that it might be possible that second report does not introduce any new issues. In such case, confidence rating should be 5. + +Output your response as escaped, markdown text that can be sent as http body to API. Do not wrap output in code blocks. Do not include any partial results or statistics regarding the number of new and resolved issues in any of the reports. +Format **Confidence level** as Heading 2 using double sharp characters (##). Otherwise, do not include any another preamble and postamble to your answer.