feat(pallet-smart-contract): allow collective approval to cancel contract

[renauter]

Description

This PR add cancel_contract_collective() extrinsic, allowing a collective proposal (council or farmers) to cancel a contract.

Related Issues:

• Fixes Allow collective approval to cancel contracts #884

Checklist:

Please delete options that are not relevant.

• My change requires a change to the documentation and I have updated it accordingly.
• I have added tests to cover my changes.
• I followed the Release document.
• My commits follow this conventional commits guide.

[LeeSmet]

I have a couple of issues with this PR:

First, it seems that a single council member can, by itself, decide to cancel contracts (https://github.com/threefoldtech/tfchain/pull/886/files#diff-d61b60964693bfe471211e6b0f22c898f88febe40aa00b4c551c93168228858dR291-R294). While council members are indeed privileged in a way, the design of the council is such that no one member individually has these privileges, and they are only granted if sufficient council members agree. As such, I believe this check should be changed to check if the extrinsic was called through a council motion.

Secondly, the current implementation overrides the cancel contract call and allows multiple origins, using different behavior (setting a different cancel cause) depending on the origin. This means that figuring out why a contract was cancelled is only possible by checking the cause, which is a subfield, of the generated event. I think that it would be better to have 2 extrinsics, both the original one for users, and a new one for the DAO/council. This way, while the generated event is still the same, the extrinsic call is different and can be used to immediately figure out if a contract is cancelled by the user or not. Additionally, both extrinsics just need to check their own condition regarding the caller, and set the proper cause. This removes some branching code, meaning the extrinsics themselves are simpler (and use somewhat less weight).

[renauter]

@LeeSmet
I see
I guess the following implementation better fits what you mean:

development...development_rework_cancel_contracts

Correct ?

[LeeSmet]

Yes, something like that is indeed what I mean

[renauter]

Yes, something like that is indeed what I mean

Ok, I ll rework it

[LeeSmet]

Looks good as far as I can see, though I'd rename the functions/cause from xxx_collective to xxx_council, since it is specifically the council and not any collective which can do this.

[renauter]

I'd rename the functions/cause from xxx_collective to xxx_council, since it is specifically the council and not any collective which can do this.

Since we have for smart-contract pallet RestrictedOrigin => EnsureRootOrCouncilApproval with

type EnsureRootOrCouncilApproval = EitherOfDiverse<
    EnsureRoot<AccountId>,
    pallet_collective::EnsureProportionAtLeast<AccountId, CouncilCollective, 3, 5>,
>;

a dao approval - which can be considered a farmer collective decision and uses Root origin - can also cancel contracts.

That s why I chose a more generic name, but feel free to suggest another one

[LeeSmet]

I suppose it's fine, can't immediately think of something better. It explains the intention anyway so should be good.