-
Notifications
You must be signed in to change notification settings - Fork 329
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
Erc721 votes and general Votes component #1114
Draft
ggonzalez94
wants to merge
16
commits into
OpenZeppelin:main
Choose a base branch
from
ggonzalez94:erc721-votes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+754
−651
Draft
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
4324006
votes first iteration
ggonzalez94 dd0eb38
consolidate votes and delegation to governace module
ggonzalez94 5141fb5
fmt
ggonzalez94 2a76341
add ERC20Votes impl, code docs and remove interface for internal iml
ggonzalez94 6c02234
add basic tests and mocks
ggonzalez94 64461b9
refactor, remove unused usings
ggonzalez94 6f8c2d0
cleanup mocks, remove public impl
ggonzalez94 0836234
merge main and fix compile issues
ggonzalez94 05b69d1
merge main
ggonzalez94 de076af
refactor
ggonzalez94 6f5a8a6
export VotesComponent for easier use
ggonzalez94 fefcfeb
remove erc20votes from token module
ggonzalez94 0116f03
fmt
ggonzalez94 175af4e
add more tests
ggonzalez94 ce72985
fmt
ggonzalez94 4ca0ae4
add changelog entry
ggonzalez94 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
mod tests; | ||
|
||
pub mod timelock; | ||
pub mod utils; | ||
pub mod votes; |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
pub mod interface; | ||
pub mod utils; | ||
pub mod votes; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// | ||
// Offchain message hash generation helpers. | ||
// | ||
|
||
// sn_keccak("\"Delegation\"(\"delegatee\":\"ContractAddress\",\"nonce\":\"felt\",\"expiry\":\"u128\")") | ||
// | ||
// Since there's no u64 type in SNIP-12, we use u128 for `expiry` in the type hash generation. | ||
|
||
use core::hash::{HashStateTrait, HashStateExTrait}; | ||
use core::poseidon::PoseidonTrait; | ||
use openzeppelin_utils::cryptography::snip12::{StructHash}; | ||
use starknet::ContractAddress; | ||
|
||
pub const DELEGATION_TYPE_HASH: felt252 = | ||
0x241244ac7acec849adc6df9848262c651eb035a3add56e7f6c7bcda6649e837; | ||
|
||
#[derive(Copy, Drop, Hash)] | ||
pub struct Delegation { | ||
pub delegatee: ContractAddress, | ||
pub nonce: felt252, | ||
pub expiry: u64 | ||
} | ||
|
||
impl StructHashImpl of StructHash<Delegation> { | ||
fn hash_struct(self: @Delegation) -> felt252 { | ||
let hash_state = PoseidonTrait::new(); | ||
hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,312 @@ | ||
// SPDX-License-Identifier: MIT | ||
// OpenZeppelin Contracts for Cairo v0.15.1 (governance/votes/votes.cairo) | ||
|
||
use core::hash::{HashStateTrait, HashStateExTrait}; | ||
use core::poseidon::PoseidonTrait; | ||
use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, StructHash, SNIP12Metadata}; | ||
use starknet::ContractAddress; | ||
|
||
/// # Votes Component | ||
/// | ||
/// The Votes component provides a flexible system for tracking voting power and delegation | ||
/// that is currently implemented for ERC20 and ERC721 tokens. It allows accounts to delegate | ||
/// their voting power to a representative, who can then use the pooled voting power in | ||
/// governance decisions. Voting power must be delegated to be counted, and an account can | ||
/// delegate to itself if it wishes to vote directly. | ||
/// | ||
/// This component offers a unified interface for voting mechanisms across ERC20 and ERC721 | ||
/// token standards, with the potential to be extended to other token standards in the future. | ||
/// It's important to note that only one token implementation (either ERC20 or ERC721) should | ||
/// be used at a time to ensure consistent voting power calculations. | ||
#[starknet::component] | ||
pub mod VotesComponent { | ||
// We should not use Checkpoints or StorageArray as they are for ERC721Vote | ||
// Instead we can rely on Vec | ||
use core::num::traits::Zero; | ||
use openzeppelin_account::dual_account::{DualCaseAccount, DualCaseAccountTrait}; | ||
use openzeppelin_governance::votes::interface::{IVotes, TokenVotesTrait}; | ||
use openzeppelin_governance::votes::utils::{Delegation}; | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
use openzeppelin_introspection::src5::SRC5Component; | ||
use openzeppelin_token::erc20::ERC20Component; | ||
use openzeppelin_token::erc20::interface::IERC20; | ||
use openzeppelin_token::erc721::ERC721Component; | ||
use openzeppelin_token::erc721::interface::IERC721; | ||
use openzeppelin_utils::nonces::NoncesComponent::InternalTrait as NoncesInternalTrait; | ||
use openzeppelin_utils::nonces::NoncesComponent; | ||
use openzeppelin_utils::structs::checkpoint::{Checkpoint, Trace, TraceTrait}; | ||
use starknet::ContractAddress; | ||
use starknet::storage::Map; | ||
use super::{OffchainMessageHash, SNIP12Metadata}; | ||
|
||
#[storage] | ||
struct Storage { | ||
Votes_delegatee: Map::<ContractAddress, ContractAddress>, | ||
Votes_delegate_checkpoints: Map::<ContractAddress, Trace>, | ||
Votes_total_checkpoints: Trace, | ||
} | ||
|
||
#[event] | ||
#[derive(Drop, PartialEq, starknet::Event)] | ||
pub enum Event { | ||
DelegateChanged: DelegateChanged, | ||
DelegateVotesChanged: DelegateVotesChanged, | ||
} | ||
|
||
#[derive(Drop, PartialEq, starknet::Event)] | ||
pub struct DelegateChanged { | ||
#[key] | ||
pub delegator: ContractAddress, | ||
#[key] | ||
pub from_delegate: ContractAddress, | ||
#[key] | ||
pub to_delegate: ContractAddress | ||
} | ||
|
||
#[derive(Drop, PartialEq, starknet::Event)] | ||
pub struct DelegateVotesChanged { | ||
#[key] | ||
pub delegate: ContractAddress, | ||
pub previous_votes: u256, | ||
pub new_votes: u256 | ||
} | ||
|
||
pub mod Errors { | ||
pub const FUTURE_LOOKUP: felt252 = 'Votes: future Lookup'; | ||
pub const EXPIRED_SIGNATURE: felt252 = 'Votes: expired signature'; | ||
pub const INVALID_SIGNATURE: felt252 = 'Votes: invalid signature'; | ||
} | ||
|
||
#[embeddable_as(VotesImpl)] | ||
impl Votes< | ||
TContractState, | ||
+HasComponent<TContractState>, | ||
impl Nonces: NoncesComponent::HasComponent<TContractState>, | ||
impl TokenTrait: TokenVotesTrait<ComponentState<TContractState>>, | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
+SNIP12Metadata, | ||
+Drop<TContractState> | ||
> of IVotes<ComponentState<TContractState>> { | ||
/// Returns the current amount of votes that `account` has. | ||
fn get_votes(self: @ComponentState<TContractState>, account: ContractAddress) -> u256 { | ||
self.Votes_delegate_checkpoints.read(account).latest() | ||
} | ||
|
||
/// Returns the amount of votes that `account` had at a specific moment in the past. | ||
/// | ||
/// Requirements: | ||
/// | ||
/// - `timepoint` must be in the past. | ||
fn get_past_votes( | ||
self: @ComponentState<TContractState>, account: ContractAddress, timepoint: u64 | ||
) -> u256 { | ||
let current_timepoint = starknet::get_block_timestamp(); | ||
assert(timepoint < current_timepoint, Errors::FUTURE_LOOKUP); | ||
self.Votes_delegate_checkpoints.read(account).upper_lookup_recent(timepoint) | ||
} | ||
|
||
/// Returns the total supply of votes available at a specific moment in the past. | ||
/// | ||
/// Requirements: | ||
/// | ||
/// - `timepoint` must be in the past. | ||
fn get_past_total_supply(self: @ComponentState<TContractState>, timepoint: u64) -> u256 { | ||
let current_timepoint = starknet::get_block_timestamp(); | ||
assert(timepoint < current_timepoint, Errors::FUTURE_LOOKUP); | ||
self.Votes_total_checkpoints.read().upper_lookup_recent(timepoint) | ||
} | ||
|
||
/// Returns the delegate that `account` has chosen. | ||
fn delegates( | ||
self: @ComponentState<TContractState>, account: ContractAddress | ||
) -> ContractAddress { | ||
self.Votes_delegatee.read(account) | ||
} | ||
|
||
/// Delegates votes from the sender to `delegatee`. | ||
/// | ||
/// Emits a `DelegateChanged` event. | ||
/// May emit one or two `DelegateVotesChanged` events. | ||
fn delegate(ref self: ComponentState<TContractState>, delegatee: ContractAddress) { | ||
let sender = starknet::get_caller_address(); | ||
self._delegate(sender, delegatee); | ||
} | ||
|
||
/// Delegates votes from the sender to `delegatee` through a SNIP12 message signature | ||
/// validation. | ||
/// | ||
/// Requirements: | ||
/// | ||
/// - `expiry` must not be in the past. | ||
/// - `nonce` must match the account's current nonce. | ||
/// - `delegator` must implement `SRC6::is_valid_signature`. | ||
/// - `signature` should be valid for the message hash. | ||
/// | ||
/// Emits a `DelegateChanged` event. | ||
/// May emit one or two `DelegateVotesChanged` events. | ||
fn delegate_by_sig( | ||
ref self: ComponentState<TContractState>, | ||
delegator: ContractAddress, | ||
delegatee: ContractAddress, | ||
nonce: felt252, | ||
expiry: u64, | ||
signature: Array<felt252> | ||
) { | ||
assert(starknet::get_block_timestamp() <= expiry, Errors::EXPIRED_SIGNATURE); | ||
|
||
// Check and increase nonce. | ||
let mut nonces_component = get_dep_component_mut!(ref self, Nonces); | ||
nonces_component.use_checked_nonce(delegator, nonce); | ||
|
||
// Build hash for calling `is_valid_signature`. | ||
let delegation = Delegation { delegatee, nonce, expiry }; | ||
let hash = delegation.get_message_hash(delegator); | ||
|
||
let is_valid_signature_felt = DualCaseAccount { contract_address: delegator } | ||
.is_valid_signature(hash, signature); | ||
|
||
// Check either 'VALID' or True for backwards compatibility. | ||
let is_valid_signature = is_valid_signature_felt == starknet::VALIDATED | ||
|| is_valid_signature_felt == 1; | ||
|
||
assert(is_valid_signature, Errors::INVALID_SIGNATURE); | ||
|
||
// Delegate votes. | ||
self._delegate(delegator, delegatee); | ||
} | ||
} | ||
|
||
// | ||
// Internal for ERC721Votes | ||
// | ||
|
||
pub impl ERC721VotesImpl< | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
TContractState, | ||
+HasComponent<TContractState>, | ||
+SRC5Component::HasComponent<TContractState>, | ||
impl ERC721: ERC721Component::HasComponent<TContractState>, | ||
+ERC721Component::ERC721HooksTrait<TContractState>, | ||
+Drop<TContractState> | ||
> of TokenVotesTrait<ComponentState<TContractState>> { | ||
/// Returns the number of voting units for a given account. | ||
/// | ||
/// This implementation is specific to ERC721 tokens, where each token | ||
/// represents one voting unit. The function returns the balance of | ||
/// ERC721 tokens for the specified account. | ||
fn get_voting_units( | ||
self: @ComponentState<TContractState>, account: ContractAddress | ||
) -> u256 { | ||
let mut erc721_component = get_dep_component!(self, ERC721); | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
erc721_component.balance_of(account).into() | ||
} | ||
} | ||
|
||
// | ||
// Internal for ERC20Votes | ||
// | ||
|
||
pub impl ERC20VotesImpl< | ||
TContractState, | ||
+HasComponent<TContractState>, | ||
+SRC5Component::HasComponent<TContractState>, | ||
impl ERC20: ERC20Component::HasComponent<TContractState>, | ||
+ERC20Component::ERC20HooksTrait<TContractState>, | ||
+Drop<TContractState> | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> of TokenVotesTrait<ComponentState<TContractState>> { | ||
/// Returns the number of voting units for a given account. | ||
/// | ||
/// This implementation is specific to ERC20 tokens, where the balance | ||
/// of tokens directly represents the number of voting units. | ||
fn get_voting_units( | ||
self: @ComponentState<TContractState>, account: ContractAddress | ||
) -> u256 { | ||
let mut erc20_component = get_dep_component!(self, ERC20); | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
erc20_component.balance_of(account) | ||
} | ||
} | ||
|
||
// | ||
// Common internal for Votes | ||
// | ||
|
||
#[generate_trait] | ||
pub impl InternalImpl< | ||
TContractState, | ||
+HasComponent<TContractState>, | ||
impl TokenTrait: TokenVotesTrait<ComponentState<TContractState>>, | ||
+NoncesComponent::HasComponent<TContractState>, | ||
+SNIP12Metadata, | ||
+Drop<TContractState> | ||
> of InternalTrait<TContractState> { | ||
/// Delegates all of `account`'s voting units to `delegatee`. | ||
/// | ||
/// Emits a `DelegateChanged` event. | ||
/// May emit one or two `DelegateVotesChanged` events. | ||
fn _delegate( | ||
ref self: ComponentState<TContractState>, | ||
account: ContractAddress, | ||
delegatee: ContractAddress | ||
) { | ||
let from_delegate = self.delegates(account); | ||
self.Votes_delegatee.write(account, delegatee); | ||
self | ||
.emit( | ||
DelegateChanged { delegator: account, from_delegate, to_delegate: delegatee } | ||
); | ||
self | ||
.move_delegate_votes( | ||
from_delegate, delegatee, TokenTrait::get_voting_units(@self, account) | ||
); | ||
} | ||
|
||
/// Moves delegated votes from one delegate to another. | ||
/// | ||
/// May emit one or two `DelegateVotesChanged` events. | ||
fn move_delegate_votes( | ||
ref self: ComponentState<TContractState>, | ||
from: ContractAddress, | ||
to: ContractAddress, | ||
amount: u256 | ||
) { | ||
let zero_address = Zero::zero(); | ||
let block_timestamp = starknet::get_block_timestamp(); | ||
if (from != to && amount > 0) { | ||
if (from != zero_address) { | ||
let mut trace = self.Votes_delegate_checkpoints.read(from); | ||
let (previous_votes, new_votes) = trace | ||
.push(block_timestamp, trace.latest() - amount); | ||
self.emit(DelegateVotesChanged { delegate: from, previous_votes, new_votes }); | ||
} | ||
if (to != zero_address) { | ||
let mut trace = self.Votes_delegate_checkpoints.read(to); | ||
let (previous_votes, new_votes) = trace | ||
.push(block_timestamp, trace.latest() + amount); | ||
self.emit(DelegateVotesChanged { delegate: to, previous_votes, new_votes }); | ||
} | ||
} | ||
ggonzalez94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// Transfers, mints, or burns voting units. | ||
/// | ||
/// To register a mint, `from` should be zero. To register a burn, `to` | ||
/// should be zero. Total supply of voting units will be adjusted with mints and burns. | ||
/// | ||
/// May emit one or two `DelegateVotesChanged` events. | ||
fn transfer_voting_units( | ||
ref self: ComponentState<TContractState>, | ||
from: ContractAddress, | ||
to: ContractAddress, | ||
amount: u256 | ||
) { | ||
let zero_address = Zero::zero(); | ||
let block_timestamp = starknet::get_block_timestamp(); | ||
if (from == zero_address) { | ||
let mut trace = self.Votes_total_checkpoints.read(); | ||
trace.push(block_timestamp, trace.latest() + amount); | ||
} | ||
if (to == zero_address) { | ||
let mut trace = self.Votes_total_checkpoints.read(); | ||
trace.push(block_timestamp, trace.latest() - amount); | ||
} | ||
self.move_delegate_votes(self.delegates(from), self.delegates(to), amount); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update accordingly, or remove the comment since the user shouldn't implement this trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean removing the trait or removing the comment or making it just a code comment and not
///
? I think having the trait is valuable since it forces contracts that use theVotesComponent
to have an implementation of this trait and it allows someone that wants to use their own token voting mechanism to implement itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd adjust the comment because it's not an interface. This is an important distinction because trait and interface definitions look (unfortunately) similar. I might even consider moving this away from
interface.cairo
and into the Votes file or mod to make it clear this isn't meant to be exposed