crytic / crytic-compile

Abstraction layer for smart contract build systems
GNU Affero General Public License v3.0
157 stars 84 forks source link

crytic_compile.platform.exceptions.InvalidCompilation: Unknown file #572

Open Uhan19 opened 2 months ago

Uhan19 commented 2 months ago

Hi team,

running into some issues when I am reusing a slither workflow from one of my solidity repos (core-v1) in another repo (peripheral-v1). Core-v1 repo is submodule in the Peripheral-v1, lib/core-v1. The slither analysis passes in CI on the core-v1 repo, however it fails when the workflow is being called by the peripheral CI workflow. Here is the error:

crytic_compile.platform.exceptions.InvalidCompilation: Unknown file: contracts/interfaces/callbacks/ITransferValidator.sol

ITransferValidator.sol is a file in the core-v1 repo, and it is not used or imported in the peripheral-v1 repo, so I am not sure why slither is trying to compile this file.

here is my slither.yml in core-v1:

name: Slither Analysis

on:
  workflow_call:
    secrets:
      PAT_TOKEN:
        required: false
    inputs:
      cache-path:
        default: |
          cache
          out
        required: false
        type: 'string'

      restore-cache:
        default: true
        required: false
        type: 'boolean'

      target:
        default: '.'
        required: false
        type: 'string'

jobs:
  analyze:
    runs-on: ubuntu-latest
    steps:
      - name: 'Check out the repo'
        uses: 'actions/checkout@v4'
        with:
          fetch-depth: 0
          submodules: recursive
          token: ${{ secrets.PAT_TOKEN || github.token }}

      # - name: 'Restore the cached build'
      #   if: ${{ inputs.restore-cache }}
      #   uses: 'actions/cache/restore@v4'
      #   with:
      #     fail-on-cache-miss: true
      #     key: 'build-${{ github.sha }}'
      #     path: ${{ inputs.cache-path }}

      - name: 'Install Foundry'
        uses: 'foundry-rs/foundry-toolchain@v1'

      - name: Initialize Forge
        run: |
          forge --version
          forge config
          forge install --quiet ## suppress git logs

      - name: 'Compile contracts'
        run: |
          forge clean
          forge config --json
          forge build --build-info --skip */test/** */script/** --force

      - name: 'List directories'
        run: ls -la contracts/

      - name: Run Slither Static Analysis
        env:
          TERM: xterm-color
        uses: crytic/slither-action@v0.4.0
        id: slither
        with:
          slither-version: 'dev'
          fail-on: 'low'
          target: ${{ inputs.target }}
          slither-config: slither.config.json
          ignore-compile: true

      - name: 'Add summary'
        run: |
          echo "### Slither Analysis result" >> $GITHUB_STEP_SUMMARY
          echo "Passed" >> $GITHUB_STEP_SUMMARY

      - name: 'Add summary'
        run: |
          echo "### Slither Analysis" >> $GITHUB_STEP_SUMMARY
          echo "Passed" >> $GITHUB_STEP_SUMMARY

and this is the calling workflow CI.yml in the Peripheral-v1 repo:

name: CI # Think about changing this name or remove it.

concurrency:
  cancel-in-progress: true
  group: ${{ github.workflow }}-${{ github.ref }}

on:
  push:
    branches:
      - master
  pull_request:
  release:
    types: [published]
  workflow_dispatch:

env:
  FOUNDRY_PROFILE: ci
  FACTORY_ADDRESS: '0xC427715e2428A5a99fDC0159A61b9F6ea875Eb39'

jobs:
  format:
    name: 'Run format'
    runs-on: ubuntu-latest
    steps:
      - name: check out repository
        uses: actions/checkout@v4
      - name: Format
        uses: Ammalgam-Protocol/core-v1/.github/actions/format@feature/ci-pat-token-permissions

  build:
    name: 'Run build'
    secrets: inherit
    uses: Ammalgam-Protocol/core-v1/.github/workflows/build.yml@feature/ci-pat-token-permissions

  test:
    name: 'Run tests'
    needs: ['format', 'build']
    secrets: inherit
    uses: Ammalgam-Protocol/core-v1/.github/workflows/test.yml@feature/ci-pat-token-permissions

  coverage:
    name: 'Run coverage'
    needs: ['format', 'build']
    secrets:
      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
      PAT_TOKEN: ${{ secrets.PAT_TOKEN || github.token }}
    uses: Ammalgam-Protocol/core-v1/.github/workflows/coverage.yml@feature/ci-pat-token-permissions

  slither:
    name: 'Run slither'
    needs: ['format', 'build']
    secrets: inherit
    uses: Ammalgam-Protocol/core-v1/.github/workflows/slither.yml@feature/ci-pat-token-permissions
    with:
      target: ./

  old:
    needs: ['build', 'test']
    strategy:
      fail-fast: true

    name: Ammalgam peripheral-v1
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0
          submodules: recursive
          token: ${{ secrets.PAT_TOKEN }}

      - name: Install Foundry
        uses: foundry-rs/foundry-toolchain@v1
        with:
          version: nightly

      - name: Test Fork Deploy
        env:
          LOCAL: 0
          MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }}
          MNEMONIC: ${{ secrets.MNEMONIC }}

        run: |
          forge script script/LocalForkDeploy.s.sol -vvvv --skip-simulation --fork-url $MAINNET_RPC_URL
        id: deploy

      - name: Test SEPOLIA Deploy
        env:
          SEPOLIA_RPC_URL: ${{ secrets.SEPOLIA_RPC_URL }}
          MNEMONIC: ${{ secrets.MNEMONIC }}
        run: forge script script/TestnetDeploy.s.sol --rpc-url $SEPOLIA_RPC_URL -vvvv

I also have added slither.config.json in both repos:

{
  "filter_paths": "lib|test|script"
}

I had thought that perhaps I needed to define the targets for slither to run and perhaps it was running in the run context or root level, but after adding a step to list the directories, I could see that the file structure is as I expected. I also commented out the cache restore and build the contracts as part of the slither workflow, however this did not resolve the issue either. Would love some input in terms of why the compiler is reading a file in the submodule when it should've been filtered out. Please let me know if I can provide any additional information. Thanks.

elopez commented 2 months ago

Hi! Thanks for the report; can you confirm if it works locally or if you see the same error there? e.g.

git clone --recursive https://github.com/Ammalgam-Protocol/peripheral-v1
cd peripheral-v1
forge install --quiet
forge build --build-info --skip */test/** */script/** --force
slither . --ignore-compile --config-file=slither.config.json --fail-low
Uhan19 commented 2 months ago

Hi @elopez - I had to adjust to use this build command forge build --build-info --skip test/** /script/** --force

but running slither . --ignore-compile --config-file=slither.config.json --fail-low generated the same error on local.

crytic_compile.platform.exceptions.InvalidCompilation: Unknown file: contracts/interfaces/callbacks/ITransferValidator.sol

elopez commented 2 months ago

Alright, so this is not an issue on the action then, but on either crytic-compile or Foundry's artifact generation. Out of curiosity, can you try again without the skip flags and report back? i.e. do a plain forge build --force

Uhan19 commented 2 months ago

@elopez - that makes sense.

so running: forge build --force and then slither . --ignore-compile --config-file=slither.config.json --fail-low generates this error:

Error in .
Traceback (most recent call last):
  File "/Library/Python/3.9/site-packages/slither_analyzer-0.9.2-py3.9.egg/slither/__main__.py", line 834, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/Library/Python/3.9/site-packages/slither_analyzer-0.9.2-py3.9.egg/slither/__main__.py", line 87, in process_all
    compilations = compile_all(target, **vars(args))
  File "/Library/Python/3.9/site-packages/crytic_compile-0.3.0-py3.9.egg/crytic_compile/crytic_compile.py", line 620, in compile_all
    compilations.append(CryticCompile(target, **kwargs))
  File "/Library/Python/3.9/site-packages/crytic_compile-0.3.0-py3.9.egg/crytic_compile/crytic_compile.py", line 110, in __init__
    self._compile(**kwargs)
  File "/Library/Python/3.9/site-packages/crytic_compile-0.3.0-py3.9.egg/crytic_compile/crytic_compile.py", line 530, in _compile
    self._platform.compile(self, **kwargs)
  File "/Library/Python/3.9/site-packages/crytic_compile-0.3.0-py3.9.egg/crytic_compile/platform/foundry.py", line 90, in compile
    hardhat_like_parsing(crytic_compile, self._target, build_directory, self._target)
  File "/Library/Python/3.9/site-packages/crytic_compile-0.3.0-py3.9.egg/crytic_compile/platform/hardhat.py", line 66, in hardhat_like_parsing
    targets_json = loaded_json["output"]
KeyError: 'output'
elopez commented 2 months ago

@Uhan19 can you upgrade your slither and crytic-compile and see if that still happens? I see you're using quite old versions (0.9.2, 0.3.0 respectively)

Uhan19 commented 2 months ago

@elopez - I have just upgraded to 0.10.4 and 0.3.7, but unfortunately the error is still occuring. Trying to look into the build artifact and see if I can figure out where the issue is. Althought I do see the ITransferValidator.sol in the out/ directory.

Also running a simple forge build still generates the KeyError: output error when running: slither . --ignore-compile --config-file=slither.config.json --fail-low

elopez commented 2 months ago

It's possible the artifacts don't reflect the correct path of the file, does contracts/interfaces/callbacks/ITransferValidator.sol exist in that path? or is it in some other location? What do the artifacts mentioning contracts/interfaces/callbacks/ITransferValidator.sol say?

In the meantime I'll move this issue to the crytic-compile repo to keep things organized.

Uhan19 commented 2 months ago

It's possible the artifacts don't reflect the correct path of the file, does contracts/interfaces/callbacks/ITransferValidator.sol exist in that path? or is it in some other location? What do the artifacts mentioning contracts/interfaces/callbacks/ITransferValidator.sol say?

so contracts/interfaces/callbacks/ITransferValidator.sol is the path for the file in the core-v1 repo, which is a submodule of peripheral-v1. In the peripheral-v1 the path should be lib/core-v1/contracts/interfaces/callbacks/ITransferValidator.sol

I just tooked at the the build-info json that was generated and I see that under sources:

{ "sources": { ..., "contracts/interfaces/callbacks/ITransferValidator.sol": { "id": 2, "ast": {} }, ..., "lib/core-v1/contracts/interfaces/callbacks/ITransferValidator.sol": { "id": 9, "ast": {...} }, ... } }

The second source with the correct path, has the ast fully populated with data. So I think this could potentially be the issue, since that first path is incorrect.

elopez commented 2 months ago

I wonder if this could be related to https://github.com/foundry-rs/foundry/issues/7591

@0xalpharush have you seen this issue with the paths in the Foundry artifacts before?

Uhan19 commented 2 months ago

@elopez - yea that looks very familiar to my issue:

what's interesting is that some files are duped, but not all of them. For example:

cat $(find out/build-info/*.json) | jq '.output.sources | keys' | grep 'ITransferValidator.sol' "contracts/interfaces/callbacks/ITransferValidator.sol", "lib/core-v1/contracts/interfaces/callbacks/ITransferValidator.sol",

cat $(find out/build-info/*.json) | jq '.output.sources | keys' | grep 'IAmmalgamCallee.sol'
"lib/core-v1/contracts/interfaces/callbacks/IAmmalgamCallee.sol",

elopez commented 2 months ago

It could also be an instance of https://github.com/gakonst/ethers-rs/issues/2609 as it seems like the remappings may not be preserving the leading path prefix.

Would you mind reporting this on the foundry repo so it gets looked at by them?

Uhan19 commented 2 months ago

sure will do,

I just checked in the core-v1 repo build-info and it looks like there's no dupe there, so I am not running into the same issue:

❯ cat $(find out/build-info/*.json) | jq '.output.sources | keys' | grep 'ITransferValidator.sol' "contracts/interfaces/callbacks/ITransferValidator.sol",