Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List of issues related to Polls module logic #589

Open
5 tasks
kmlbgn opened this issue Oct 14, 2023 · 0 comments
Open
5 tasks

List of issues related to Polls module logic #589

kmlbgn opened this issue Oct 14, 2023 · 0 comments
Assignees
Labels
error handling invalid This doesn't seem right Priority: High x/gov Governance module

Comments

@kmlbgn
Copy link
Collaborator

kmlbgn commented Oct 14, 2023

  • (k msgServer) PollVote function is missing error handling logic for properties.MaxProposalPollOptionSize and utf8string.NewString(v).IsASCII() when user provide non-existing custom values.

    if msg.Option == types.PollOptionCustom && poll.Options.Count > uint64(len(poll.Options.Values)) && !inSlice(poll.Options.Values, msg.Value) {
    poll.Options.Values = append(poll.Options.Values, msg.Value)
    k.keeper.SavePoll(ctx, poll)
    }

  • Wrong error type here

    if len(msg.Checksum) > int(properties.MaxProposalChecksumSize) {
    return nil, types.ErrProposalTitleSizeExceeds
    }

  • Duration logic between proposal and poll should be normalized. We prioritise simple logic and let abstraction work to front-end. Hence current Poll duration logic, where user can provide any duration in a form of string, should be switched for precise end-date as unix timestamp like for Proposals.

    duration, err := time.ParseDuration(msg.Duration)
    if err != nil || blockTime.Add(duration).Before(blockTime) {
    return nil, fmt.Errorf("invalid duration: %w", err)
    }

    with a simple check like below :
    if proposal.VotingEndTime.Before(ctx.BlockTime()) {
    return nil, types.ErrVotingTimeEnded
    }

  • There is no min-max bounds on poll duration value, there should be. Polls can use existing minimum_proposal_end_time network property. Additionally a new network property maximum_proposal_end_time should be created and enforced both for polls and proposals because KIRA will potentially be trimming historical state in the future which could break some L2 app logic.

  • 128 as default value for network property on option count limit seem way too high

    MaxProposalPollOptionCount: 128,

@kmlbgn kmlbgn added invalid This doesn't seem right x/gov Governance module error handling Priority: High labels Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling invalid This doesn't seem right Priority: High x/gov Governance module
Projects
None yet
Development

No branches or pull requests

2 participants