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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ near view $ASTRA_ID get_policy
- `VoteRemove` - _Votes to remove given proposal or bounty (this may be because the proposal is spam or otherwise invalid)._
- `Finalize` - _Finalizes proposal which is cancelled when proposal has expired (this action also returns funds)._
- `MoveToHub` - _Moves a proposal to the hub (this is used to move a proposal into another DAO)._

- `ExecuteProposal` - Execute a proposal if it was not executed in final voting.
---

## Proposals
Expand Down
6 changes: 3 additions & 3 deletions astra/src/bounties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ mod tests {
},
});
assert_eq!(contract.get_last_bounty_id(), id);
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
id
}

Expand Down Expand Up @@ -267,7 +267,7 @@ mod tests {
"bounty_done"
);

contract.act_proposal(1, Action::VoteApprove, None);
contract.act_proposal(1, Action::VoteApprove, None, None);
testing_env!(
context.build(),
near_sdk::VMConfig::test(),
Expand All @@ -282,7 +282,7 @@ mod tests {

contract.bounty_claim(0, U64::from(500));
contract.bounty_done(0, None, "Bounty is done 2".to_string());
contract.act_proposal(2, Action::VoteApprove, None);
contract.act_proposal(2, Action::VoteApprove, None, None);
testing_env!(
context.build(),
near_sdk::VMConfig::test(),
Expand Down
62 changes: 54 additions & 8 deletions astra/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ mod tests {
assert_eq!(contract.get_proposals(0, 10).len(), 1);

let id = create_proposal(&mut context, &mut contract);
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
assert_eq!(
contract.get_proposal(id).proposal.status,
ProposalStatus::Approved
Expand All @@ -218,7 +218,7 @@ mod tests {
testing_env!(context
.block_timestamp(1_000_000_000 * 24 * 60 * 60 * 8)
.build());
contract.act_proposal(id, Action::Finalize, None);
contract.act_proposal(id, Action::Finalize, None, None);
assert_eq!(
contract.get_proposal(id).proposal.status,
ProposalStatus::Expired
Expand Down Expand Up @@ -249,7 +249,7 @@ mod tests {
);
let id = create_proposal(&mut context, &mut contract);
assert_eq!(contract.get_proposal(id).proposal.description, "test");
contract.act_proposal(id, Action::RemoveProposal, None);
contract.act_proposal(id, Action::RemoveProposal, None, None);
}

#[test]
Expand All @@ -263,7 +263,7 @@ mod tests {
let mut contract = Contract::new(Config::test_config(), policy);
let id = create_proposal(&mut context, &mut contract);
assert_eq!(contract.get_proposal(id).proposal.description, "test");
contract.act_proposal(id, Action::RemoveProposal, None);
contract.act_proposal(id, Action::RemoveProposal, None, None);
assert_eq!(contract.get_proposals(0, 10).len(), 0);
}

Expand All @@ -279,7 +279,7 @@ mod tests {
testing_env!(context
.block_timestamp(1_000_000_000 * 24 * 60 * 60 * 8)
.build());
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
}

#[test]
Expand All @@ -292,8 +292,8 @@ mod tests {
VersionedPolicy::Default(vec![accounts(1), accounts(2)]),
);
let id = create_proposal(&mut context, &mut contract);
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
}

#[test]
Expand All @@ -312,12 +312,58 @@ mod tests {
role: "missing".to_string(),
},
});
contract.act_proposal(id, Action::VoteApprove, None);
contract.act_proposal(id, Action::VoteApprove, None, None);
let x = contract.get_policy();
// still 2 roles: all and council.
assert_eq!(x.roles.len(), 2);
}

#[test]
fn test_proposal_execution() {
let mut context = VMContextBuilder::new();
testing_env!(context.predecessor_account_id(accounts(1)).build());
let mut contract = Contract::new(
Config::test_config(),
VersionedPolicy::Default(vec![accounts(1)]),
);

let id = create_proposal(&mut context, &mut contract);
contract.act_proposal(id, Action::VoteApprove, None, Some(false));
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
// verify proposal wasn't approved during final vote
assert_eq!(
contract.get_proposal(id).proposal.status,
ProposalStatus::InProgress
);

contract.act_proposal(id, Action::ExecuteProposal, None, None);
assert_eq!(
contract.get_proposal(id).proposal.status,
ProposalStatus::Approved
);
}

#[test]
#[should_panic(expected = "ERR_PROPOSAL_NOT_IN_PROGRESS")]
fn test_proposal_double_execution() {
let mut context = VMContextBuilder::new();
testing_env!(context.predecessor_account_id(accounts(1)).build());
let mut contract = Contract::new(
Config::test_config(),
VersionedPolicy::Default(vec![accounts(1)]),
);

let id = create_proposal(&mut context, &mut contract);
contract.act_proposal(id, Action::VoteApprove, None, None);
// verify proposal was approved and executed
assert_eq!(
contract.get_proposal(id).proposal.status,
ProposalStatus::Approved
);
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved

// panics if we try to execute again
contract.act_proposal(id, Action::ExecuteProposal, None, None);
}

#[test]
#[should_panic(expected = "ERR_INVALID_POLICY")]
fn test_fails_adding_invalid_policy() {
Expand Down
2 changes: 2 additions & 0 deletions astra/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ pub fn default_policy(council: Vec<AccountId>) -> Policy {
"*:VoteReject".to_string(),
"*:VoteRemove".to_string(),
"*:Finalize".to_string(),
"*:ExecuteProposal".to_string(),
]
.into_iter()
.collect(),
Expand Down Expand Up @@ -494,6 +495,7 @@ mod tests {
"*:VoteReject".to_string(),
"*:VoteRemove".to_string(),
"*:Finalize".to_string(),
"*:ExecuteProposal".to_string(),
]
.into_iter()
.collect();
Expand Down
56 changes: 42 additions & 14 deletions astra/src/proposals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ impl Contract {

/// Act on given proposal by id, if permissions allow.
/// Memo is logged but not stored in the state. Can be used to leave notes or explain the action.
pub fn act_proposal(&mut self, id: u64, action: Action, memo: Option<String>) {
pub fn act_proposal(&mut self, id: u64, action: Action, memo: Option<String>, no_execute: Option<bool>) {
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
let execute = no_execute.unwrap_or(true);
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
let mut proposal: Proposal = self.proposals.get(&id).expect("ERR_NO_PROPOSAL").into();
let policy = self.policy.get().unwrap().to_policy();
// Check permissions for the given action.
Expand All @@ -564,21 +565,26 @@ impl Contract {
&policy,
self.get_user_weight(&sender_id),
);
// Updates proposal status with new votes using the policy.
proposal.status =

if execute {
// Updates proposal status with new votes using the policy.
proposal.status =
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
policy.proposal_status(&proposal, roles, self.total_delegation_amount);
if proposal.status == ProposalStatus::Approved {
self.internal_execute_proposal(&policy, &proposal, id);
true
} else if proposal.status == ProposalStatus::Removed {
self.internal_reject_proposal(&policy, &proposal, false);
self.proposals.remove(&id);
false
} else if proposal.status == ProposalStatus::Rejected {
self.internal_reject_proposal(&policy, &proposal, true);
true
if proposal.status == ProposalStatus::Approved && execute {
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
self.internal_execute_proposal(&policy, &proposal, id);
true
} else if proposal.status == ProposalStatus::Removed && execute {
self.internal_reject_proposal(&policy, &proposal, false);
self.proposals.remove(&id);
false
} else if proposal.status == ProposalStatus::Rejected && execute {
self.internal_reject_proposal(&policy, &proposal, true);
true
} else {
// Still in progress or expired.
true
}
} else {
// Still in progress or expired.
true
}
}
Expand Down Expand Up @@ -609,6 +615,28 @@ impl Contract {
true
}
Action::MoveToHub => false,
Action::ExecuteProposal => {
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
// Note: If proposal was executed already `proposal_status` will throw error
proposal.status = policy.proposal_status(&proposal, roles, self.total_delegation_amount);
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
let mut update = true;
match proposal.status {
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
ProposalStatus::Approved => {
self.internal_execute_proposal(&policy, &proposal, id);
}
ProposalStatus::Removed => {
self.internal_reject_proposal(&policy, &proposal, false);
self.proposals.remove(&id);
update = false;
}
ProposalStatus::Rejected => {
self.internal_reject_proposal(&policy, &proposal, true);
}
_ => {
// do nothing
}
}
update
}
amityadav0 marked this conversation as resolved.
Show resolved Hide resolved
};
if update {
self.proposals
Expand Down
2 changes: 2 additions & 0 deletions astra/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ pub enum Action {
Finalize,
/// Move a proposal to the hub to shift into another DAO.
MoveToHub,
/// Execute proposal and update proposal status
ExecuteProposal
}

impl Action {
Expand Down
Loading