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

Astra: add option to act_proposal to not execute the proposal #5

Merged
merged 16 commits into from
Sep 21, 2023

Conversation

amityadav0
Copy link

  • add no_execute: Option
  • add ExecuteProposal Action
  • add tests
  • documentation

astra/src/lib.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
Copy link

@sczembor sczembor 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 we should add another status Executed since now we have a possibility to have an approved proposal but not executed

@amityadav0
Copy link
Author

I don't think Executed is needed. This will create ambiguity around Approved, Rejected or Expired.
We could add a new boolean but I think current implementation makes more sense. we keep the proposal in InProgress state unless it is executed with any result

@amityadav0
Copy link
Author

amityadav0 commented Sep 20, 2023

Also we don't have Approved with not executed possibility, If due to some reason proposal is not executed, the state will change to Failed which can be finalised using Finalize action

@sczembor
Copy link

Also we don't have Approved with not executed possibility, If due to some reason proposal is not executed, the state will change to Failed which can be finalised using Finalize action

but we have an approved proposal (in terms of votes) but not executed. Since we can decide to execute the proposal once the last vote is submitted or not. That's why I think it would make more sense to create a new status. How a user will now if the proposal has reached the approved status if we change the state only after its executed?

@amityadav0
Copy link
Author

amityadav0 commented Sep 20, 2023

I still think this will be Ambiguous to know status of proposal if we put Executed as status.

  • About last question, the current logic is like that, if proposal status is changed that means it is executed. so I kept it same and added an extra Execute action.
  • IF we still do this, I think having an bool executed is better. true for executed false for not executed
    @robert-zaremba What do you think ?

astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
Copy link

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

few small comments

README.md Outdated Show resolved Hide resolved
astra/src/lib.rs Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
astra/src/proposals.rs Show resolved Hide resolved
astra/src/proposals.rs Outdated Show resolved Hide resolved
@amityadav0 amityadav0 merged commit 1a4a44b into master Sep 21, 2023
1 check passed
@amityadav0 amityadav0 deleted the execute branch September 21, 2023 14:53
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