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

Solidityscan report API endpoint #8908

Merged
merged 7 commits into from Dec 6, 2023
Merged

Solidityscan report API endpoint #8908

merged 7 commits into from Dec 6, 2023

Conversation

vbaranov
Copy link
Member

@vbaranov vbaranov commented Nov 29, 2023

Resolves #8907

Motivation

Adding API V2 endpoint for proxying request of Solidityscan report.

Changelog

New API v2 endpoint added:

api/v2/smart-contracts/#{contract_address_hash}/solidityscan-report

Sample success response:

{
    "scan_report": {
        "contract_address": "0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B",
        "contract_chain": "eth",
        "contract_platform": "blockscout",
        "contract_url": "https://eth.blockscout.com/address/0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B",
        "contractname": "Unitroller",
        "node_reference_id": null,
        "scan_status": "scan_done",
        "scan_summary": {
            "issue_severity_distribution": {
                "critical": 0,
                "gas": 95,
                "high": 1,
                "informational": 18,
                "low": 31,
                "medium": 1
            },
            "lines_analyzed_count": 2449,
            "scan_time_taken": 6,
            "score": "4.63",
            "score_v2": "92.57",
            "threat_score": "88.89"
        },
        "scanner_reference_url": "https://solidityscan.com/quickscan/0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B/blockscout/eth?ref=blockscout"
    },
    "status": "success"
}

Sample failed response:

{
    "message": "...",
    "status": "error"
}

Checklist for your Pull Request (PR)

end
end

defp base_url(address_hash_string) do
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename it to quickscan_url

Copy link
Member Author

@vbaranov vbaranov Dec 4, 2023

Choose a reason for hiding this comment

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

What is the purpose? What if they change the api path? I'd say we shouldn't depend on their naming in our intertnal namings.

Copy link
Member

Choose a reason for hiding this comment

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

Usually base_url in our modules stands for the base path to the api (https://api.solidityscan.com/api/v1). In this case base_url doesn't suit to this paradigm, therefore I suggested to rename the function

apps/explorer/lib/explorer/helper.ex Outdated Show resolved Hide resolved
Comment on lines 209 to 216
{:format, :error} ->
conn
|> put_status(400)
|> json(%{status: "error", message: "Invalid address hash"})

{:restricted_access, true} ->
conn
|> put_status(403)
|> json(%{status: "error", message: "Access restricted"})

{:error, :not_found} ->
conn
|> put_status(404)
|> json(%{status: "error", message: "Address not found"})

{:is_smart_contract, false} ->
conn
|> put_status(404)
|> json(%{status: "error", message: "Smart-contract not found"})

{:is_empty_response, true} ->
conn
|> put_status(500)
|> json(%{status: "error", message: "Empty response"})
end
end

Copy link
Member

Choose a reason for hiding this comment

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

In API v2 we usually state all mismatched with patters in BlockScoutWeb.API.V2.FallbackController. Most likely all of the patterns you stated, already exist in FallbackController

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, extended fallback controller with new responses in 381a6eb

|> render(:message, %{message: @address_not_found})
end

def call(conn, {:is_smart_contract, false}) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that Address.is_smart_contract/1 may return nil as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 48ac396.

|> fetch_compiler_versions(:vyper)
end

defp fetch_compiler_versions(compiler_list, compiler_type) do
if RustVerifierInterface.enabled?() do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we will pass function instead of list to not to execute request if rust verifier is disabled?
Like this

  defp fetch_solc_versions do
    fetch_compiler_versions(&RustVerifierInterface.get_versions_list/0, :solc)
  end

  defp fetch_vyper_versions do
    fetch_compiler_versions(&RustVerifierInterface.vyper_get_versions_list/0, :vyper)
  end

  defp fetch_compiler_versions(compiler_list_fn, compiler_type) do
    if RustVerifierInterface.enabled?() do
      compiler_list_fn.()
    else
      headers = [{"Content-Type", "application/json"}]

      case HTTPoison.get(source_url(compiler_type), headers) do
        {:ok, %{status_code: 200, body: body}} ->
          {:ok, format_data(body, compiler_type)}

        {:ok, %{status_code: _status_code, body: body}} ->
          {:error, Helper.decode_json(body)["error"]}

        {:error, %{reason: reason}} ->
          {:error, reason}
      end
    end
  end

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. Applied in 48ac396

.dialyzer-ignore Outdated
Comment on lines 16 to 17
lib/explorer/exchange_rates/source.ex:134
lib/explorer/exchange_rates/source.ex:137
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about these dialyzer warnings? It seems that HTTPoison.get may return only %Response{} or %Error{} indeed, maybe we can remove this clauses https://github.com/blockscout/blockscout/blob/vb-solidityscan-support/apps/explorer/lib/explorer/exchange_rates/source.ex#L134-L138?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. Implemented in 48ac396

@vbaranov vbaranov requested a review from varasev December 5, 2023 10:59
@vbaranov vbaranov merged commit 355c3c3 into master Dec 6, 2023
41 checks passed
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.

CredShields - frontend proxy request
4 participants