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

[TT-1435] use chatgpt to find new issues in modified Solidity files #14104

Merged
merged 9 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/solidity-foundry-artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ jobs:
- name: Generate basic info and modified contracts list
shell: bash
run: |
echo "Commit SHA used to generate artifacts: ${{ 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: ${{ 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 }}"
Expand Down Expand Up @@ -213,11 +214,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' }}
Expand Down Expand Up @@ -350,6 +358,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: **${{ github.sha }}**" >> $GITHUB_STEP_SUMMARY
echo "[Artifact URL](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts/$ARTIFACT_ID)" >> $GITHUB_STEP_SUMMARY
Expand Down
174 changes: 164 additions & 10 deletions .github/workflows/solidity-foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }}
Expand All @@ -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:
Expand Down Expand Up @@ -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 }}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Tofel marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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
Expand All @@ -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' }}
Expand Down
94 changes: 94 additions & 0 deletions contracts/scripts/ci/find_slither_report_diff.sh
Original file line number Diff line number Diff line change
@@ -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-first-report> <path-to-second-report> <path-to-diff-report-output> <path-to-prompt> [path-to-validation-prompt]"
exit 1
fi

if [[ -z "${OPEN_API_KEY+x}" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

If preferred, you can safely remove this as $openai_result below will fail with ::error:: log telling you that you haven't supplied the key and we won't be charged for the call.

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 prefer to have it, because it's running in strict mode and will error if env var is not set, when read. I prefer to return a more explicit error than what bash would.

>&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"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest that we pin the mode version to the current one that you are testing with (or make it a variable.)
Otherwise you are at risk of behavioral changes as newer versions are deployed to the 4o endpoints, we have seen breaking changes in format / responses before.

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment for version pinning. To check the latest model version number, see: https://platform.openai.com/docs/models

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea!

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
Loading
Loading