Proposals: Feedback

[mnaamani]

I think the implementation is just what we need for sparta and test coverage looks good.

There are only two important issues to address immediately.

• In create_proposal(Origin, stake, name, description, wasm_code) We should have an upper bound check on the length of the name, description and wasm_code arrays

• tally() is called on every block - because quorum might be reached before proposal expires.
  Maybe its a bit excessive to do a full tally on all pending proposals. Check every N blocks instead?

The remaining points are suggested renaming/refactoring, that can be done later.

ProposalStatus enum can probably be split into two enums, ProposalStatus, and ProposalOutcome

ProposalStatus {
 Pending, // Maybe rename to Active
 Cancelled,
 Finalized
}

ProposalOutcome {
   Approved,
   Rejected,
   Slashed,
   Expired,
}

In struct TallyResult rename a field status to outcome of type ProposalOutcome.

In struct Proposal field name title would be better than name

ProposalCount get(proposal_count): ProposalId.
ProposalId is of type u32 so first impression was that it should really be of type usize
But the way it is used, it would be suitable to rename ProposalCount to NextProposalId

Prefixing DEFAULT_ to the const values used as defaults for the storage values.

There is a case when majority of councilors vote to reject a proposal but it isn't finalized until the proposal expires.

[siman]

• Check name, description and wasm_code for max length
• Use DEFAULT_ prefix for default constant values
• Rename in ProposalStatus: Pending -> Active and check all related names that use pending
• Split into two enums: ProposalStatus and ProposalOutcome
• In struct TallyResult rename a field status to outcome of type ProposalOutcome
• tally() is called on every block - because quorum might be reached before proposal expires.
  Maybe its a bit excessive to do a full tally on all pending proposals. Check every N blocks instead?