QuarsLab Assessment Review

[mnaamani]

Some general points

• MEDIUM_1 - update substrate and old dependencies
  Updating to newer version of substrate is certainly in the roadmap for carthage and mainnet - MEDIUM_1 v1.1: Upgrade the Joystream runtime to the latest Substrate version. #3487
  We could also bump versions of substrate dependencies with cargo-update
  Perhaps we can add cargo audit step to our CI checks - as an informative check
• INFO_1 - #[transactional] I believe this will be available in substrate v3+ so we should certainly make use of it
• 5.1.3 Benchmarking - We certainly have made progress here but still many pallets are incomplete.
  • Currently we have a github action that builds runtime with benchmarks features and generates weights. This happens on every PR. It takes several hours and always times out. We should re-visit this. It doesn't make sense to generate weights on every PR. Instead this should be a manual invocation that generates and checks in the newly generated weights. It should also be performed on the "reference" hardware.
• LOW_8 - content::delete_post - extrinsic does not exist
• INFO_8 - content::create_post - extrinsic does not exit

[Lezek123]

Since I already did some research on #[transactional], I'm just going to add to this:

The #[transactional] macro is already available with our version of frame_support, the way you can use it is:

use frame_support::transactional;

// ...
        #[weight = 10_000_000]
        #[transactional]
        pub fn some_transactional_extrinsic() -> DispatchResult {
            // ...
        }

However, it seems that our version of frame_support pallet doesn't include a fix (paritytech/substrate#7301), which makes the #[transactional] work with try_do_something()?; syntax:

So it's quite clunky to use unless we bump frame_support to a version that includes the fix.

It's worth noting that #[transactional] was recently made a default behavior for all extrinsics in substrate:
paritytech/substrate#11431

[bedeho]

Related: #3827