search

tezos-commons / baseDAO

BaseDAO - a generic smart contract framework for DAOs on Tezos
57 stars 15 forks source link

Verify the limit on `max_votes` and what to suggest for its default value #271

Closed pasqu4le closed 2 years ago

pasqu4le commented 3 years ago

Clarification and motivation

Sibling issue to #68 (or rather its solution, but for max_votes.

We currently suggest a number of max_votes of 1000, but in most cases that might be too small.

However, a limit should be put, mainly because the number of votes affects the size of the voters field in a proposal, which has to be de-serialized for flush, which means it can get to be too large to fit in a single operation's gas cost and hit a constant failure.

While this problem is not entirely catastrophic (such a proposal could be dropped) we should still verify that 1000 is a sensible value to suggest and if possible perhaps suggest an even higher number (and limit it in storage creation).

Acceptance criteria

  1. Some evidence is given that either:
    • no limit is actually needed on max_votes
    • there is a number x that is a hard limit for max_votes
  2. In case there is an x:
    • add a check in defaults.mligo to prohibit more than x (or even less, for safety) for max_votes
    • suggest a number of max_votes that's closer to x/2
sras commented 3 years ago

Since we have changed the proposal to track voters instead of votes, this issue should instead track the number of voters, instead of votes.

It has been confirmed that a proposal that contain 1000 voters can continue to operate with out issues. After sending 1000 votes to the contract, each from a different voter address, It has been tested that the contract can successfully unpack the proposal in context (one with 1000 voters) and other operations like freeze.

To do this test, the contract was slightly modified temporarly to extend the potentioal voting period indefinitely, instead of restricting it to a very specific period, and the test was done on this modified contract (or else the period would pass when sending the 1000 separate votes).

The code for the test is preserved in branch sras/#271-stress-test-voters-do-not-merge

pasqu4le commented 3 years ago

@sras to avoid changing the contract I think that here too we could instead use something more like this, which is to say to originate the contract with a large number of voters in its initial storage instead.

It should be just as effective and much less painful, don't you think?

pasqu4le commented 2 years ago

I'm closing this because #311 removed the limit in question, making this issue obsolete.