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

Add min/max duration and quorum #111

Merged
merged 11 commits into from
May 12, 2022
Merged
4 changes: 3 additions & 1 deletion contracts/starknet/lib/proposal.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ from starkware.cairo.common.uint256 import Uint256

struct Proposal:
member execution_hash : Uint256
member quorum : Uint256
member start_timestamp : felt
member end_timestamp : felt
pscott marked this conversation as resolved.
Show resolved Hide resolved
member min_end_timestamp : felt
member max_end_timestamp : felt
member ethereum_block_number : felt
member execution_params_hash : felt
member executor : felt
Expand Down
146 changes: 120 additions & 26 deletions contracts/starknet/space.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
from starkware.starknet.common.syscalls import get_caller_address, get_block_timestamp
from starkware.cairo.common.cairo_builtins import HashBuiltin
from starkware.cairo.common.alloc import alloc
from starkware.cairo.common.uint256 import Uint256, uint256_add, uint256_lt
from starkware.cairo.common.uint256 import Uint256, uint256_add, uint256_lt, uint256_le
from starkware.cairo.common.bool import TRUE, FALSE
from starkware.cairo.common.hash_state import hash_init, hash_update
from starkware.cairo.common.math import (
assert_lt, assert_le, assert_nn, assert_not_zero, assert_lt_felt
Expand Down Expand Up @@ -32,13 +33,21 @@ func voting_delay() -> (delay : felt):
end

@storage_var
func voting_duration() -> (period : felt):
func min_voting_duration() -> (period : felt):
end

@storage_var
func max_voting_duration() -> (period : felt):
end

@storage_var
func proposal_threshold() -> (threshold : Uint256):
end

@storage_var
func quorum() -> (value : Uint256):
end

@storage_var
func authenticators(authenticator_address : felt) -> (is_valid : felt):
end
Expand Down Expand Up @@ -94,9 +103,11 @@ end
@event
func space_created(
pscott marked this conversation as resolved.
Show resolved Hide resolved
_voting_delay : felt,
_voting_duration : felt,
_min_voting_duration : felt,
_max_voting_duration : felt,
_proposal_threshold : Uint256,
_controller : felt,
_quorum : Uint256,
_voting_strategies_len : felt,
_voting_strategies : felt*,
_authenticators_len : felt,
Expand All @@ -110,12 +121,20 @@ end
func controller_updated(previous : felt, new_controller : felt):
end

@event
func quorum_updated(previous : Uint256, new_quorum : Uint256):
end

@event
func voting_delay_updated(previous : felt, new_voting_delay : felt):
end

@event
func voting_duration_updated(previous : felt, new_voting_duration : felt):
func min_voting_duration_updated(previous : felt, new_voting_duration : felt):
end

@event
func max_voting_duration_updated(previous : felt, new_voting_duration : felt):
end

@event
Expand Down Expand Up @@ -153,9 +172,11 @@ end
@constructor
func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
_voting_delay : felt,
_voting_duration : felt,
_min_voting_duration : felt,
_max_voting_duration : felt,
_proposal_threshold : Uint256,
_controller : felt,
_quorum : Uint256,
_voting_strategies_len : felt,
_voting_strategies : felt*,
_authenticators_len : felt,
Expand All @@ -168,7 +189,7 @@ func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_p
# Sanity checks
with_attr error_message("Invalid constructor parameters"):
assert_nn(_voting_delay)
assert_nn(_voting_duration)
assert_le(_min_voting_duration, _max_voting_duration)
assert_not_zero(_controller)
assert_not_zero(_voting_strategies_len)
assert_not_zero(_authenticators_len)
Expand All @@ -178,9 +199,11 @@ func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_p

# Initialize the storage variables
voting_delay.write(_voting_delay)
voting_duration.write(_voting_duration)
min_voting_duration.write(_min_voting_duration)
max_voting_duration.write(_max_voting_duration)
proposal_threshold.write(_proposal_threshold)
Ownable_initializer(_controller)
quorum.write(_quorum)

unchecked_add_voting_strategies(_voting_strategies_len, _voting_strategies)
unchecked_add_authenticators(_authenticators_len, _authenticators)
Expand All @@ -190,9 +213,11 @@ func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_p

space_created.emit(
_voting_delay,
_voting_duration,
_min_voting_duration,
_max_voting_duration,
_proposal_threshold,
_controller,
_quorum,
_voting_strategies_len,
_voting_strategies,
_authenticators_len,
Expand Down Expand Up @@ -380,6 +405,20 @@ func update_controller{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
return ()
end

@external
func update_quorum{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
new_quorum : Uint256
):
Ownable_only_owner()

let (previous_quorum) = quorum.read()

quorum.write(new_quorum)

quorum_updated.emit(previous_quorum, new_quorum)
return ()
end

@external
func update_voting_delay{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
new_delay : felt
Expand All @@ -396,16 +435,39 @@ func update_voting_delay{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range
end

@external
func update_voting_duration{
func update_min_voting_duration{
syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt
}(new_min_duration : felt):
Ownable_only_owner()

let (previous_duration) = min_voting_duration.read()

let (max_duration) = max_voting_duration.read()

assert_le(new_min_duration, max_duration)

min_voting_duration.write(new_min_duration)

min_voting_duration_updated.emit(previous_duration, new_min_duration)

return ()
end

@external
func update_max_voting_duration{
syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt
}(new_duration : felt):
}(new_max_duration : felt):
Ownable_only_owner()

let (previous_duration) = voting_duration.read()
let (previous_duration) = max_voting_duration.read()

let (min_duration) = min_voting_duration.read()

assert_le(min_duration, new_max_duration)

voting_duration.write(new_duration)
max_voting_duration.write(new_max_duration)

voting_duration_updated.emit(previous_duration, new_duration)
max_voting_duration_updated.emit(previous_duration, new_max_duration)

return ()
end
Expand Down Expand Up @@ -523,12 +585,18 @@ func vote{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : fe
# Verify that the caller is the authenticator contract.
assert_valid_authenticator()

# Make sure proposal has not already been executed
with_attr error_message("Proposal already executed"):
let (has_been_executed) = executed_proposals.read(proposal_id)
assert has_been_executed = 0
end

let (proposal) = proposal_registry.read(proposal_id)
let (current_timestamp) = get_block_timestamp()

# Make sure proposal is not closed
# Make sure proposal is still open for voting
with_attr error_message("Voting period has ended"):
assert_lt(current_timestamp, proposal.end_timestamp)
assert_lt(current_timestamp, proposal.max_end_timestamp)
end

# Make sure proposal has started
Expand Down Expand Up @@ -612,11 +680,14 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :

let (current_timestamp) = get_block_timestamp()
let (delay) = voting_delay.read()
let (duration) = voting_duration.read()

# Define start_timestamp and end_timestamp based on current timestamp, delay and duration variables.
let (_min_voting_duration) = min_voting_duration.read()
let (_max_voting_duration) = max_voting_duration.read()

# Define start_timestamp, min_end and max_end
let start_timestamp = current_timestamp + delay
let end_timestamp = start_timestamp + duration
let min_end_timestamp = start_timestamp + _min_voting_duration
let max_end_timestamp = start_timestamp + _max_voting_duration

let (voting_power) = get_cumulative_voting_power(
start_timestamp,
Expand All @@ -640,9 +711,18 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
# Hash the execution params
let (hash) = hash_array(execution_params_len, execution_params)

let (_quorum) = quorum.read()

# Create the proposal and its proposal id
let proposal = Proposal(
execution_hash, start_timestamp, end_timestamp, ethereum_block_number, hash, executor
execution_hash,
_quorum,
start_timestamp,
min_end_timestamp,
max_end_timestamp,
ethereum_block_number,
hash,
executor,
)

let (proposal_id) = next_proposal_nonce.read()
Expand Down Expand Up @@ -689,14 +769,10 @@ func finalize_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
end

# Make sure proposal period has ended
# NOTE: commented out the `with_attr` block because it needs 0.8.1 to work
# with_attr error_message("Min voting period has not elapsed"):
let (current_timestamp) = get_block_timestamp()
# ------------------------------------------------
# IMPORTANT
# ------------------------------------------------
# This has been commented to allow for easier testing.
# Please uncomment before pushing to prod.
# with_attr error_message("Voting period has not ended yet"):
# assert_lt_felt(proposal.end_timestamp, current_timestamp)
assert_le(proposal.min_end_timestamp, current_timestamp)
# end

# Make sure execution params match the stored hash
Expand All @@ -708,9 +784,27 @@ func finalize_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
# Count votes for
let (for) = vote_power.read(proposal_id, Choice.FOR)

# Count votes against
let (abstain) = vote_power.read(proposal_id, Choice.ABSTAIN)

# Count votes against
let (against) = vote_power.read(proposal_id, Choice.AGAINST)

let (partial_power, overflow1) = uint256_add(for, abstain)

let (total_power, overflow2) = uint256_add(partial_power, against)

let _quorum = proposal.quorum
let (is_lower_or_equal) = uint256_le(_quorum, total_power)

# If overflow1 or overflow2 happened, then quorum has necessarily been reached because `quorum` is by definition smaller or equal to Uint256::MAX.
# If `is_lower_or_equal` (meaning `_quorum` is smaller than `total_power`), then quorum has been reached (definition of quorum).
# So if `overflow1 || overflow2 || is_lower_or_equal`, we have reached quorum. If we sum them and find `0`, then they're all equal to 0, which means
# quorum has not been reached.
with_attr error_message("Quorum has not been reached"):
assert_not_zero(overflow1 + overflow2 + is_lower_or_equal)
end

# Set proposal outcome accordingly
let (has_passed) = uint256_lt(against, for)

Expand Down
4 changes: 2 additions & 2 deletions test/starknet/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Controller', () => {
await new_controller.invoke(vanillaSpace, 'update_controller', {
new_controller: account.starknetContract.address,
});
}).timeout(60000);
}).timeout(600000);

it('Should not be able to update controller if not properly called', async () => {
const fake_controller = await starknet.deployAccount('OpenZeppelin');
Expand All @@ -42,5 +42,5 @@ describe('Controller', () => {
} catch (error: any) {
expect(error.message).to.contain('Ownable: caller is not the owner');
}
}).timeout(60000);
}).timeout(600000);
});
13 changes: 10 additions & 3 deletions test/starknet/shared/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export const VOTE_METHOD = 'vote';
export const GET_PROPOSAL_INFO = 'get_proposal_info';
export const GET_VOTE_INFO = 'get_vote_info';
export const VOTING_DELAY = BigInt(0);
export const VOTING_DURATION = BigInt(20);
export const MIN_VOTING_DURATION = BigInt(0);
export const MAX_VOTING_DURATION = BigInt(2000);
export const VITALIK_ADDRESS = BigInt('0xd8da6bf26964af9d7eed9e03e53415d37aa96045');
export const VITALIK_STRING_ADDRESS = VITALIK_ADDRESS.toString(16);

Expand Down Expand Up @@ -44,6 +45,7 @@ export async function vanillaSetup() {
const voting_strategy = BigInt(vanillaVotingStrategy.address);
const authenticator = BigInt(vanillaAuthenticator.address);
const zodiac_relayer = BigInt(zodiacRelayer.address);
const quorum = SplitUint256.fromUint(BigInt(0));

// This should be declared along with the other const but doing so will make the compiler unhappy as `SplitUin256`
// will be undefined for some reason?
Expand All @@ -52,8 +54,10 @@ export async function vanillaSetup() {
console.log('Deploying space contract...');
const vanillaSpace = (await vanillaSpaceFactory.deploy({
_voting_delay: VOTING_DELAY,
_voting_duration: VOTING_DURATION,
_min_voting_duration: MIN_VOTING_DURATION,
_max_voting_duration: MAX_VOTING_DURATION,
_proposal_threshold: PROPOSAL_THRESHOLD,
_quorum: quorum,
_controller: BigInt(account.starknetContract.address),
_voting_strategies: [voting_strategy],
_authenticators: [authenticator],
Expand Down Expand Up @@ -109,11 +113,14 @@ export async function ethTxAuthSetup(signer: SignerWithAddress) {
// This should be declared along with the other const but doing so will make the compiler unhappy as `SplitUin256`
// will be undefined for some reason?
const PROPOSAL_THRESHOLD = SplitUint256.fromUint(BigInt(1));
const quorum = SplitUint256.fromUint(BigInt(0));

const space = (await SpaceFactory.deploy({
_voting_delay: VOTING_DELAY,
_voting_duration: VOTING_DURATION,
_min_voting_duration: MIN_VOTING_DURATION,
_max_voting_duration: MAX_VOTING_DURATION,
_proposal_threshold: PROPOSAL_THRESHOLD,
_quorum: quorum,
_controller: 1,
_voting_strategies: [voting_strategy],
_authenticators: [authenticator],
Expand Down
8 changes: 3 additions & 5 deletions test/starknet/vanilla_space.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
EXECUTE_METHOD,
PROPOSAL_METHOD,
VOTE_METHOD,
VOTING_DURATION,
MIN_VOTING_DURATION,
MAX_VOTING_DURATION,
} from './shared/setup';
import { StarknetContract } from 'hardhat/types';

Expand Down Expand Up @@ -75,12 +76,9 @@ describe('Space testing', () => {
});

// We can't directly compare the `info` object because we don't know for sure the value of `start_block` (and hence `end_block`),
// so we compare it element by element (except start_block and end_block for which we simply compare their difference to `VOTING_PERIOD`).
// so we compare it element by element.
const _executionHash = SplitUint256.fromObj(proposal_info.proposal.execution_hash);
expect(_executionHash).to.deep.equal(executionHash);
expect(
proposal_info.proposal.end_timestamp - proposal_info.proposal.start_timestamp
).to.deep.equal(VOTING_DURATION);

const _for = SplitUint256.fromObj(proposal_info.power_for).toUint();
expect(_for).to.deep.equal(BigInt(0));
Expand Down