Skip to content

cas-107 / 156 Increase time before proposals can start#105

Merged
dbslone merged 5 commits intomainfrom
rich/cas-156
Jul 12, 2022
Merged

cas-107 / 156 Increase time before proposals can start#105
dbslone merged 5 commits intomainfrom
rich/cas-156

Conversation

@0xmovses
Copy link
Copy Markdown
Contributor

@0xmovses 0xmovses commented Jul 6, 2022

In Production environment only we need to increase the time a proposal can go live to 1 hour after the current time for proposals.

This should be reflected in the frontend UI and also validated on the backend as well. The reason for this change is because the snapshotter tool will need time to run and capture the chain so we know which accounts had the correct balance at the time the proposal was created:

• Adds SetStatus method on createProposal and getProposal controllers
• Checks if grace period has passed, only if strategy has a contract and environment is prod. Grace period is one hour

@0xmovses 0xmovses requested review from dbslone and jacksonConrad July 6, 2022 14:20
Copy link
Copy Markdown
Contributor

@jacksonConrad jacksonConrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would take a different approach here. I think its cleaner to ditch SetStatus completely.

We have computedStatus that is being set in the getProposal functions -- this feels like it belongs there, since it is basically part of that same set of conditional logic. You could basically just do:

if p.computedStatus == `active` && p.contractName != nil && !time.Now().After(p.createdAt.Add(time.Hour)) {
    p.computedStatus = 'pending'
}

Although.... honestly the best way to do this might just to enforce it in the createProposal controller:

// assert that
p.Start_time >= time.Now().Add(time.Minute * 60)

That runs into the issue where if a user selects an hour in the future, time will go by before they click "submit" and this condition will fail.

Sooo maybe the best option is actually to add this logic in createProposal:

if p.contractName != nil && p.Start_time.Before(time.Now.Add(time.Hour)) {
  p.startTime = time.Now.Add(time.Hour)
}

@0xmovses
Copy link
Copy Markdown
Contributor Author

0xmovses commented Jul 8, 2022

ok, updated.

Comment thread backend/main/server/app.go Outdated
Comment on lines +830 to +832
if strategy.Contract.Name != nil && p.Start_time.Before(time.Now().Add(time.Hour)) {
p.Start_time = time.Now().Add(time.Hour)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rvmelkonian Is this taking into account that it should only be applied in the production environment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just added an environment variable check before this

Comment thread backend/main/server/app.go Outdated
return
}

if os.Getenv("APP_ENV") != "PRODUCTION" {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be if os.Getenv("APP_ENV") == "PRODUCTION" { we only want to add 1 hour in production

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad! that's fixed

Copy link
Copy Markdown
Contributor

@dbslone dbslone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just need @jacksonConrad to review

@dbslone dbslone merged commit 5aa8731 into main Jul 12, 2022
@dbslone dbslone deleted the rich/cas-156 branch July 12, 2022 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants