Omniscia Beanstalk Audit

GovernanceFacet Code Style Findings

GovernanceFacet Code Style Findings

GFT-01C: Code Duplication

Description:

The linked code commits a particular BIP using the same statements and a single literal value difference.

Example:

protocol/contracts/farm/facets/GovernanceFacet/GovernanceFacet.sol
97function commit(uint32 bip) external {
98 require(isNominated(bip), "Governance: Not nominated.");
99 require(!isActive(bip), "Governance: Not ended.");
100 require(!isExpired(bip), "Governance: Expired.");
101 require(
102 endedBipVotePercent(bip).greaterThanOrEqualTo(C.getGovernancePassThreshold()),
103 "Governance: Must have majority."
104 );
105
106 s.g.bips[bip].executed = true;
107
108 cutBip(bip);
109 pauseOrUnpauseBip(bip);
110
111 incentivize(msg.sender, true, bip, C.getCommitIncentive());
112 emit Commit(msg.sender, bip);
113}
114
115function emergencyCommit(uint32 bip) external {
116 require(isNominated(bip), "Governance: Not nominated.");
117 require(
118 block.timestamp >= timestamp(bip).add(C.getGovernanceEmergencyPeriod()),
119 "Governance: Too early.");
120 require(isActive(bip), "Governance: Ended.");
121 require(
122 bipVotePercent(bip).greaterThanOrEqualTo(C.getGovernanceEmergencyThreshold()),
123 "Governance: Must have super majority."
124 );
125
126 endBip(bip);
127 s.g.bips[bip].executed = true;
128
129 cutBip(bip);
130 pauseOrUnpauseBip(bip);
131
132 incentivize(msg.sender, false, bip, C.getCommitIncentive());
133 emit Commit(msg.sender, bip);
134}
135
136function pauseOrUnpause(uint32 bip) external {
137 require(isNominated(bip), "Governance: Not nominated.");
138 require(diamondCutIsEmpty(bip),"Governance: Has diamond cut.");
139 require(isActive(bip), "Governance: Ended.");
140 require(
141 bipVotePercent(bip).greaterThanOrEqualTo(C.getGovernanceEmergencyThreshold()),
142 "Governance: Must have super majority."
143 );
144
145 endBip(bip);
146 s.g.bips[bip].executed = true;
147
148 pauseOrUnpauseBip(bip);
149
150 incentivize(msg.sender, false, bip, C.getCommitIncentive());
151 emit Commit(msg.sender, bip);
152}

Recommendation:

We advise the code to be refactored to utilise a single internal or private function that accepts the literal changed as an argument instead, greatly optimising the bytecode size of the contract.

Alleviation:

The linked duplicate statements were refactored to a single _execute private function thereby alleviating this exhibit.

GFT-02C: Inefficient First Vote

Description:

The first vote cast to a BIP by the proposer themselves is inefficiently done so as it invokes the vote function performing 4 redundant require checks.

Example:

protocol/contracts/farm/facets/GovernanceFacet/GovernanceFacet.sol
31/**
32 * Proposition
33**/
34
35function propose(
36 IDiamondCut.FacetCut[] calldata _diamondCut,
37 address _init,
38 bytes calldata _calldata,
39 uint8 _pauseOrUnpause
40)
41 external
42{
43 require(canPropose(msg.sender), "Governance: Not enough Stalk.");
44 require(notTooProposed(msg.sender), "Governance: Too many active BIPs.");
45 require(
46 _init != address(0) || _diamondCut.length > 0 || _pauseOrUnpause > 0,
47 "Governance: Proposition is empty."
48 );
49
50 uint32 bipId = createBip(
51 _diamondCut,
52 _init,
53 _calldata,
54 _pauseOrUnpause,
55 C.getGovernancePeriod(),
56 msg.sender
57 );
58
59 emit Proposal(msg.sender, bipId, season(), C.getGovernancePeriod());
60
61 vote(bipId);
62}
63
64/**
65 * Voting
66**/
67
68function vote(uint32 bip) public {
69 require(isNominated(bip), "Governance: Not nominated.");
70 require(balanceOfRoots(msg.sender) > 0, "Governance: Must have Stalk.");
71 require(isActive(bip), "Governance: Ended.");
72 require(!voted(msg.sender, bip), "Governance: Already voted.");
73
74 recordVote(msg.sender, bip);
75 placeLock(msg.sender, bip);
76
77 emit Vote(msg.sender, bip, balanceOfRoots(msg.sender));
78}

Recommendation:

We advise the propose function to instead cast the vote directly by only using the recordVote and placeLock functions. If bytecode efficiency is desired, both functions can utilise a shared _vote function that is internally / privately available.

Alleviation:

The code was optimized as advised with a shared internal _vote function in the VotingBooth contract implementation.