tfchain icon indicating copy to clipboard operation
tfchain copied to clipboard

Any council member can take action without the support of other members/farmers

Open sameh-farouk opened this issue 2 years ago • 3 comments

Describe the bug

I noticed by reviewing the council pallet code base that a council member can propose a DAO proposal with a threshold of 1, vote for it himself (given that he is a farmer as well), and close it without requiring other farmers votes. I tested this locally and found that it works. the proposal was executed.

This seems like a flaw to me, as it allows any council member to take action without the support of other farmers.

It is not advisable to allow the proposal creator to set the approval threshold, as this could lead to a major risk associated with a poor threshold. A low threshold with a short voting time could result in a proposal passing without farmers even knowing that the proposal took place. Conversely, a high threshold that cannot be achieved with a long voting time could cause the proposal to get stuck.

sameh-farouk avatar Nov 08 '23 12:11 sameh-farouk

Here is a sum of what needs to be done:

  • Set a Minimum and a Maximum Duration : Ensure the duration is neither too short nor too long.
  • Establish a Threshold: Specify a minimum weight required. could be calculated as a percentage of all weight available on the network?
  • Disallow Self-Voting: If a DAO member is a Farmer as well, He should not vote for their own proposals.

sameh-farouk avatar Mar 21 '24 18:03 sameh-farouk

  • Set a Minimum and a Maximum Duration : Ensure the duration is neither too short nor too long.

Actually DAO proposal duration is fixed to 30 days (default) which can be seen as a maximum duration. But there is no minimum duration control indeed. See here: https://github.com/threefoldtech/tfchain/blob/eb36aa90df2d60cb1a534997903821fc68a096f1/substrate-node/pallets/pallet-dao/src/dao.rs#L45-L51 What would be good is to add an extra minimum duration.

  • Establish a Threshold: Specify a minimum weight required. could be calculated as a percentage of all weight available on the network?

The threshold correspond to the minimal number of farmer votes required to be able to close proposal before its end. See here: https://github.com/threefoldtech/tfchain/blob/eb36aa90df2d60cb1a534997903821fc68a096f1/substrate-node/pallets/pallet-dao/src/dao.rs#L212-L216 So yes, it could be good to harden the closing for a single council member to make a proposal, vote for it and close it to get a favorable result. Dealing with farm weight would imply to track grid total weight or compute it each time close() extrinsic is called. Maybe having a minimum threshold > 1 would be enough.

  • Disallow Self-Voting: If a DAO member is a Farmer as well, He should not vote for their own proposals.

The idea is good but it implies for vote() extrinsic to check each time if origin is not DAO member (actually not stored) which is not so relevant if we put in the balance of integrating this exception. I don't see any problem to vote for the proposal I submitted since I am not the only one being able to vote. Previous restrictions (threshold + duration) should be enough.

Repo documentation to probably update:

  • https://github.com/threefoldtech/tfchain/blob/development/docs/misc/minimal_DAO.md
  • https://github.com/threefoldtech/tfchain/blob/development/substrate-node/pallets/pallet-dao/creating_proposal_farmers.md Maybe open issue to TF site documentation (but not sure there is something related ?)

renauter avatar May 02 '24 17:05 renauter

So 2 things:

  1. In propose() I would "hard code" min threshold and if threshold lower then return something like Error::<T>::TresholdToLow

  2. In propose() I would "hard code" min motion duration and return Error::<T>::InvalidProposalDuration if min duration < motion_duration < 30 days not met

To keep it as simple as possible

renauter avatar May 02 '24 17:05 renauter