External evaluation of ScoutCoinFabrik milestone 2

[dastansam]

Research is well-done, new detectors are added. However, there are some issues I encountered, specifically with extension and documentation.

Application PR: w3f/Grants-Program#1666
Milestone delivery PR: #907

[valeriacaracciolo]

Hello everyone, thank you @dastansam for the comments. Below you can find our answers to the items, one by one. Please let us know in case we are missing anything.

rust-analyzer error

This is a limitation of how rust-analyzer works. When opening a new workspace on vscode, rust-analyzer expects a Cargo.toml file on the root folder in order to work. Given this is a multi-folder repository, with different apps and libraries, we don’t have a shared Cargo.toml to put on the root. A quick solution is to only open the folder for the app or library you’d like to check, e.g. code scout/apps/cargo-scout-audit. Another solution is to add in the vscode workspace settings.json, the following configuration:

"rust-analyzer.linkedProjects": [
    "apps/cargo-scout-audit",
    "detectors/delegate-call",
    // ... rest of apps/libraries
],

This would lead to the correct detection by rust-analyzer, but also would mean that when opening the scout repository, all apps, detectors and test-cases would begin to compile, which in practice proved to be too cumbersome for the utility it provides.
Any case, we can apply the fix if you think this is a better option.

Typos in integration tests

It is fixed. Sorry about that.

unused-return-enum

Thanks for noting this. It is fixed now.

Logs

| This is also a bit slow, is there any specific reason for that?
In the context of integration tests for the Scout-audit project, on our first iteration of the tests, both Dylint and Scout-audit had to be compiled in each of the test cases in addition to the mandatory compilation of each smart contract. These repetitive compilations demanded over an hour for the whole ran, and highlighted an area of optimization.
We then adopted an improved strategy that included compiling Scout-audit only once, and then transferring the compiled detectors to each corresponding directory of the test cases. This reduced running time by an order of magnitude. However, due to the intrinsic characteristics of Dylint, its compilation remains necessary in each test case, and since our tool is coupled with all the vulnerable and remediated smart contracts for each vulnerability class, this is a lot. We understand there is room for improvement, and this is something that we want to peruse in upcoming stages of this work.
We could have replaced dylint by clippy, and this would have reduced running time in a small percentage, but with the problem that collaboration is made less easy--and this is something that we do not want.
However, at one point we plan to decouple the vulnerable/remediated smart contracts which we use for benchmarking, from the specific integration tests we need for our code.
Please let us know if there is there are any other questions for this section.

[dastansam]

issues were addressed and it looks good to me @semuelle

[keeganquigley]

Thanks @dastansam this looks really good. Yeah I often encounter this same issue with rust-analyzer so no worries there. Happy to accept and will forward your payment.

[RouvenP]

hi @dastansam we transferred the payment yesterday