near-daos / sputnik-dao-contract

Smart contracts for https://app.astrodao.com
https://astrodao.com/
MIT License
107 stars 79 forks source link

Proposal bond amount doesnt reimburse exceeded bond amount #158

Closed TrevorJTClarke closed 2 years ago

TrevorJTClarke commented 2 years ago

Fix: Change

env::attached_deposit() >= policy.proposal_bond.0

to an assert_eq - so the attached deposit is not different than bond.

Description: When a proposer calls the function add_proposal to add a new proposal, he or she may attach more NEAR tokens than the proposal_bond specified in Contract.policy . sputnikdao2/src/proposals.rs #Line 484

/// Add proposal to this DAO.
#[payable]
pub fn add_proposal(&mut self, proposal: ProposalInput) -> u64 {
    // 0. validate bond attached.
    // TODO: consider bond in the token of this DAO.
    let policy = self.policy.get().unwrap().to_policy();
    assert!(
        env::attached_deposit() >= policy.proposal_bond.0,
        "ERR_MIN_BOND"
);
...
    // 3. Actually add proposal to the current list of proposals.
    let id = self.last_proposal_id;
    self.proposals
        .insert(&id, &VersionedProposal::Default(proposal.into()));
    self.last_proposal_id += 1;
    self.locked_amount += env::attached_deposit();
    id
}

However, this contract does not individually record the actual NEAR tokens deposited for each proposer, but simply accumulates these amounts into the balance of Contract.locked_amount . As a result, such proposers can only receive the proposal_bond specified in Contract.policy returned by the function internal_return_bonds when the proposal is executed or rejected. sputnikdao2/src/proposals.rs #Line 288

fn internal_return_bonds(&mut self, policy: &Policy, proposal: &Proposal) ->
Promise {
        match &proposal.kind {
            ProposalKind::BountyDone { .. } => {
                self.locked_amount -= policy.bounty_bond.0;
                Promise::new(proposal.proposer.clone()).transfer(policy.bounty_bond.0);
            }
_ => {} }
        self.locked_amount -= policy.proposal_bond.0;
        Promise::new(proposal.proposer.clone()).transfer(policy.proposal_bond.0)
    }

Impact: NEAR tokens deposited by the proposer may be locked in this contract.

ctindogaru commented 2 years ago

Thank you for creating an issue on this, Trevor!