From 4324006a50a00f3c6a560cdd18e174953c7b49a8 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Mon, 19 Aug 2024 17:30:07 -0300 Subject: [PATCH 01/15] votes first iteration --- packages/governance/src/lib.cairo | 1 + packages/governance/src/votes.cairo | 1 + packages/governance/src/votes/votes.cairo | 243 ++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 packages/governance/src/votes.cairo create mode 100644 packages/governance/src/votes/votes.cairo diff --git a/packages/governance/src/lib.cairo b/packages/governance/src/lib.cairo index 852e24521..05ce4b158 100644 --- a/packages/governance/src/lib.cairo +++ b/packages/governance/src/lib.cairo @@ -2,3 +2,4 @@ mod tests; pub mod timelock; pub mod utils; +pub mod votes; diff --git a/packages/governance/src/votes.cairo b/packages/governance/src/votes.cairo new file mode 100644 index 000000000..d86a74c7d --- /dev/null +++ b/packages/governance/src/votes.cairo @@ -0,0 +1 @@ +pub mod votes; diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo new file mode 100644 index 000000000..bff69d058 --- /dev/null +++ b/packages/governance/src/votes/votes.cairo @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: MIT + +use core::hash::{HashStateTrait, HashStateExTrait}; +use core::poseidon::PoseidonTrait; +use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, StructHash, SNIP12Metadata}; +use starknet::ContractAddress; + +#[starknet::interface] +pub trait IVotesInternal { + fn get_voting_units(self: @TState, account: ContractAddress) -> u256; +} + +#[starknet::component] +pub mod VotesComponent { + use core::num::traits::Zero; + use openzeppelin_account::dual_account::{DualCaseAccount, DualCaseAccountTrait}; + use openzeppelin_introspection::src5::SRC5Component; + use openzeppelin_governance::utils::interfaces::IVotes; + 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::{Delegation, OffchainMessageHash, SNIP12Metadata, IVotesInternal}; + + #[storage] + struct Storage { + Votes_delegatee: Map::, + Votes_delegate_checkpoints: Map::, + 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, + impl Nonces: NoncesComponent::HasComponent, + impl TokenTrait: IVotesInternal>, + +SNIP12Metadata, + +Drop + > of IVotes> { + // Common implementation for both ERC20 and ERC721 + fn get_votes(self: @ComponentState, account: ContractAddress) -> u256 { + self.Votes_delegate_checkpoints.read(account).latest() + } + + fn get_past_votes( + self: @ComponentState, 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) + } + + fn get_past_total_supply(self: @ComponentState, 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) + } + + fn delegates( + self: @ComponentState, account: ContractAddress + ) -> ContractAddress { + self.Votes_delegatee.read(account) + } + + fn delegate(ref self: ComponentState, delegatee: ContractAddress) { + let sender = starknet::get_caller_address(); + self._delegate(sender, delegatee); + } + + fn delegate_by_sig( + ref self: ComponentState, + delegator: ContractAddress, + delegatee: ContractAddress, + nonce: felt252, + expiry: u64, + signature: Array + ) { + 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); + } + } + + #[embeddable_as(ERC721VotesImpl)] + // Should we also use a trait bound to make sure that the Votes trait is implemented? + impl ERC721Votes< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + impl ERC721: ERC721Component::HasComponent, + +ERC721Component::ERC721HooksTrait, + +Drop + > of IVotesInternal> { + // ERC721-specific implementation + fn get_voting_units( + self: @ComponentState, account: ContractAddress + ) -> u256 { + let mut erc721_component = get_dep_component!(self, ERC721); + erc721_component.balance_of(account).into() + } + } + + #[generate_trait] + pub impl InternalImpl< + TContractState, + +HasComponent, + impl TokenTrait: IVotesInternal>, + +NoncesComponent::HasComponent, + +SNIP12Metadata, + +Drop + > of InternalTrait { + // Common internal functions + fn _delegate( + ref self: ComponentState, + 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) + ); + } + + fn move_delegate_votes( + ref self: ComponentState, + 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 }); + } + } + } + + fn transfer_voting_units( + ref self: ComponentState, + 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); + } + } +} + +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 { + fn hash_struct(self: @Delegation) -> felt252 { + let hash_state = PoseidonTrait::new(); + hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() + } +} From dd0eb3856f77d41f04871d84ace5ad009104eb9e Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Thu, 22 Aug 2024 16:37:02 -0300 Subject: [PATCH 02/15] consolidate votes and delegation to governace module --- Scarb.lock | 2 + packages/governance/Scarb.toml | 2 + packages/governance/src/lib.cairo | 1 - packages/governance/src/utils.cairo | 1 - .../governance/src/utils/interfaces.cairo | 3 -- packages/governance/src/votes.cairo | 4 +- .../votes.cairo => votes/interface.cairo} | 6 +++ packages/governance/src/votes/utils.cairo | 29 ++++++++++++ packages/governance/src/votes/votes.cairo | 45 +++++++------------ .../src/erc20/extensions/erc20_votes.cairo | 29 ++---------- .../src/tests/erc20/test_erc20_votes.cairo | 2 +- 11 files changed, 63 insertions(+), 61 deletions(-) delete mode 100644 packages/governance/src/utils.cairo delete mode 100644 packages/governance/src/utils/interfaces.cairo rename packages/governance/src/{utils/interfaces/votes.cairo => votes/interface.cairo} (86%) create mode 100644 packages/governance/src/votes/utils.cairo diff --git a/Scarb.lock b/Scarb.lock index 0fa419a96..48e179b1e 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -46,8 +46,10 @@ name = "openzeppelin_governance" version = "0.15.1" dependencies = [ "openzeppelin_access", + "openzeppelin_account", "openzeppelin_introspection", "openzeppelin_testing", + "openzeppelin_token", "snforge_std", ] diff --git a/packages/governance/Scarb.toml b/packages/governance/Scarb.toml index 644641f1d..68df693ad 100644 --- a/packages/governance/Scarb.toml +++ b/packages/governance/Scarb.toml @@ -19,6 +19,8 @@ fmt.workspace = true starknet.workspace = true openzeppelin_access = { path = "../access" } openzeppelin_introspection = { path = "../introspection" } +openzeppelin_account = { path = "../account"} +openzeppelin_token = {path= "../token"} [dev-dependencies] snforge_std.workspace = true diff --git a/packages/governance/src/lib.cairo b/packages/governance/src/lib.cairo index 05ce4b158..1240e356f 100644 --- a/packages/governance/src/lib.cairo +++ b/packages/governance/src/lib.cairo @@ -1,5 +1,4 @@ mod tests; pub mod timelock; -pub mod utils; pub mod votes; diff --git a/packages/governance/src/utils.cairo b/packages/governance/src/utils.cairo deleted file mode 100644 index 43b15ec08..000000000 --- a/packages/governance/src/utils.cairo +++ /dev/null @@ -1 +0,0 @@ -pub mod interfaces; diff --git a/packages/governance/src/utils/interfaces.cairo b/packages/governance/src/utils/interfaces.cairo deleted file mode 100644 index 7d7abb05b..000000000 --- a/packages/governance/src/utils/interfaces.cairo +++ /dev/null @@ -1,3 +0,0 @@ -pub mod votes; - -pub use votes::IVotes; diff --git a/packages/governance/src/votes.cairo b/packages/governance/src/votes.cairo index d86a74c7d..ca7cd7390 100644 --- a/packages/governance/src/votes.cairo +++ b/packages/governance/src/votes.cairo @@ -1 +1,3 @@ -pub mod votes; +pub mod interface; +pub mod utils; +pub mod votes; \ No newline at end of file diff --git a/packages/governance/src/utils/interfaces/votes.cairo b/packages/governance/src/votes/interface.cairo similarity index 86% rename from packages/governance/src/utils/interfaces/votes.cairo rename to packages/governance/src/votes/interface.cairo index d55c481c2..401c9f7d3 100644 --- a/packages/governance/src/utils/interfaces/votes.cairo +++ b/packages/governance/src/votes/interface.cairo @@ -33,3 +33,9 @@ pub trait IVotes { signature: Array ); } + +/// Common interface for tokens used for voting(e.g. `ERC721Votes` or `ERC20Votes`) +#[starknet::interface] +pub trait IVotesToken { + fn get_voting_units(self: @TState, account: ContractAddress) -> u256; +} diff --git a/packages/governance/src/votes/utils.cairo b/packages/governance/src/votes/utils.cairo new file mode 100644 index 000000000..9c1002d96 --- /dev/null +++ b/packages/governance/src/votes/utils.cairo @@ -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 { + fn hash_struct(self: @Delegation) -> felt252 { + let hash_state = PoseidonTrait::new(); + hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() + } +} diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index bff69d058..10435bbdd 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -1,21 +1,20 @@ // 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; -#[starknet::interface] -pub trait IVotesInternal { - fn get_voting_units(self: @TState, account: ContractAddress) -> u256; -} #[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_introspection::src5::SRC5Component; - use openzeppelin_governance::utils::interfaces::IVotes; + use openzeppelin_governance::votes::interface::{IVotes, IVotesToken}; + use openzeppelin_governance::votes::utils::{Delegation}; use openzeppelin_token::erc721::ERC721Component; use openzeppelin_token::erc721::interface::IERC721; use openzeppelin_utils::nonces::NoncesComponent::InternalTrait as NoncesInternalTrait; @@ -23,7 +22,7 @@ pub mod VotesComponent { use openzeppelin_utils::structs::checkpoint::{Checkpoint, Trace, TraceTrait}; use starknet::ContractAddress; use starknet::storage::Map; - use super::{Delegation, OffchainMessageHash, SNIP12Metadata, IVotesInternal}; + use super::{OffchainMessageHash, SNIP12Metadata}; #[storage] struct Storage { @@ -68,7 +67,7 @@ pub mod VotesComponent { TContractState, +HasComponent, impl Nonces: NoncesComponent::HasComponent, - impl TokenTrait: IVotesInternal>, + impl TokenTrait: IVotesToken>, +SNIP12Metadata, +Drop > of IVotes> { @@ -134,7 +133,10 @@ pub mod VotesComponent { } } - #[embeddable_as(ERC721VotesImpl)] + // + // Internal for ERC721Votes + // + // Should we also use a trait bound to make sure that the Votes trait is implemented? impl ERC721Votes< TContractState, @@ -143,7 +145,7 @@ pub mod VotesComponent { impl ERC721: ERC721Component::HasComponent, +ERC721Component::ERC721HooksTrait, +Drop - > of IVotesInternal> { + > of IVotesToken> { // ERC721-specific implementation fn get_voting_units( self: @ComponentState, account: ContractAddress @@ -153,11 +155,15 @@ pub mod VotesComponent { } } + // + // Internal + // + #[generate_trait] pub impl InternalImpl< TContractState, +HasComponent, - impl TokenTrait: IVotesInternal>, + impl TokenTrait: IVotesToken>, +NoncesComponent::HasComponent, +SNIP12Metadata, +Drop @@ -224,20 +230,3 @@ pub mod VotesComponent { } } } - -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 { - fn hash_struct(self: @Delegation) -> felt252 { - let hash_state = PoseidonTrait::new(); - hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() - } -} diff --git a/packages/token/src/erc20/extensions/erc20_votes.cairo b/packages/token/src/erc20/extensions/erc20_votes.cairo index b13cd160d..32237f84c 100644 --- a/packages/token/src/erc20/extensions/erc20_votes.cairo +++ b/packages/token/src/erc20/extensions/erc20_votes.cairo @@ -18,7 +18,8 @@ use starknet::ContractAddress; pub mod ERC20VotesComponent { use core::num::traits::Zero; use openzeppelin_account::dual_account::{DualCaseAccount, DualCaseAccountTrait}; - use openzeppelin_governance::utils::interfaces::IVotes; + use openzeppelin_governance::votes::interface::IVotes; + use openzeppelin_governance::votes::utils::{Delegation}; use openzeppelin_token::erc20::ERC20Component; use openzeppelin_token::erc20::interface::IERC20; use openzeppelin_utils::nonces::NoncesComponent::InternalTrait as NoncesInternalTrait; @@ -26,7 +27,7 @@ pub mod ERC20VotesComponent { use openzeppelin_utils::structs::checkpoint::{Checkpoint, Trace, TraceTrait}; use starknet::ContractAddress; use starknet::storage::Map; - use super::{Delegation, OffchainMessageHash, SNIP12Metadata}; + use super::{OffchainMessageHash, SNIP12Metadata}; #[storage] struct Storage { @@ -285,27 +286,3 @@ pub mod ERC20VotesComponent { } } } - -// -// 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. -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 { - fn hash_struct(self: @Delegation) -> felt252 { - let hash_state = PoseidonTrait::new(); - hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() - } -} diff --git a/packages/token/src/tests/erc20/test_erc20_votes.cairo b/packages/token/src/tests/erc20/test_erc20_votes.cairo index 53961b4a7..3390ba9e4 100644 --- a/packages/token/src/tests/erc20/test_erc20_votes.cairo +++ b/packages/token/src/tests/erc20/test_erc20_votes.cairo @@ -9,7 +9,7 @@ use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ }; use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ERC20VotesImpl, InternalImpl}; use openzeppelin_token::erc20::extensions::ERC20VotesComponent; -use openzeppelin_token::erc20::extensions::erc20_votes::Delegation; +use openzeppelin_governance::votes::utils::{Delegation}; use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock::SNIP12MetadataImpl; use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; From 5141fb52c90e149ad6e0c71ae991131b0c100b78 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Thu, 22 Aug 2024 17:27:06 -0300 Subject: [PATCH 03/15] fmt --- packages/governance/src/votes.cairo | 2 +- packages/governance/src/votes/votes.cairo | 2 +- packages/token/src/tests/erc20/test_erc20_votes.cairo | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/governance/src/votes.cairo b/packages/governance/src/votes.cairo index ca7cd7390..69e5a8028 100644 --- a/packages/governance/src/votes.cairo +++ b/packages/governance/src/votes.cairo @@ -1,3 +1,3 @@ pub mod interface; pub mod utils; -pub mod votes; \ No newline at end of file +pub mod votes; diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 10435bbdd..43c6f2218 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -12,9 +12,9 @@ pub mod VotesComponent { // Instead we can rely on Vec use core::num::traits::Zero; use openzeppelin_account::dual_account::{DualCaseAccount, DualCaseAccountTrait}; - use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_governance::votes::interface::{IVotes, IVotesToken}; use openzeppelin_governance::votes::utils::{Delegation}; + use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_token::erc721::ERC721Component; use openzeppelin_token::erc721::interface::IERC721; use openzeppelin_utils::nonces::NoncesComponent::InternalTrait as NoncesInternalTrait; diff --git a/packages/token/src/tests/erc20/test_erc20_votes.cairo b/packages/token/src/tests/erc20/test_erc20_votes.cairo index 3390ba9e4..c11e9bf07 100644 --- a/packages/token/src/tests/erc20/test_erc20_votes.cairo +++ b/packages/token/src/tests/erc20/test_erc20_votes.cairo @@ -1,5 +1,6 @@ use core::num::traits::Bounded; use core::num::traits::Zero; +use openzeppelin_governance::votes::utils::{Delegation}; use openzeppelin_testing as utils; use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; use openzeppelin_testing::events::EventSpyExt; @@ -9,7 +10,6 @@ use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ }; use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ERC20VotesImpl, InternalImpl}; use openzeppelin_token::erc20::extensions::ERC20VotesComponent; -use openzeppelin_governance::votes::utils::{Delegation}; use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock::SNIP12MetadataImpl; use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; From 2a763413347fbf30e122e4505ea652cd5f515579 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Tue, 27 Aug 2024 17:41:02 -0400 Subject: [PATCH 04/15] add ERC20Votes impl, code docs and remove interface for internal iml --- packages/governance/src/votes/interface.cairo | 3 +- packages/governance/src/votes/votes.cairo | 102 ++++++++++++++++-- 2 files changed, 92 insertions(+), 13 deletions(-) diff --git a/packages/governance/src/votes/interface.cairo b/packages/governance/src/votes/interface.cairo index 401c9f7d3..707002b3d 100644 --- a/packages/governance/src/votes/interface.cairo +++ b/packages/governance/src/votes/interface.cairo @@ -35,7 +35,6 @@ pub trait IVotes { } /// Common interface for tokens used for voting(e.g. `ERC721Votes` or `ERC20Votes`) -#[starknet::interface] -pub trait IVotesToken { +pub trait TokenVotesTrait { fn get_voting_units(self: @TState, account: ContractAddress) -> u256; } diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 43c6f2218..47d27a927 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -1,20 +1,34 @@ // 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, IVotesToken}; + use openzeppelin_governance::votes::interface::{IVotes, TokenVotesTrait}; use openzeppelin_governance::votes::utils::{Delegation}; 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; @@ -67,15 +81,20 @@ pub mod VotesComponent { TContractState, +HasComponent, impl Nonces: NoncesComponent::HasComponent, - impl TokenTrait: IVotesToken>, + impl TokenTrait: TokenVotesTrait>, +SNIP12Metadata, +Drop > of IVotes> { - // Common implementation for both ERC20 and ERC721 + /// Returns the current amount of votes that `account` has. fn get_votes(self: @ComponentState, 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, account: ContractAddress, timepoint: u64 ) -> u256 { @@ -84,23 +103,45 @@ pub mod VotesComponent { 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, 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, 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, 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, delegator: ContractAddress, @@ -137,16 +178,19 @@ pub mod VotesComponent { // Internal for ERC721Votes // - // Should we also use a trait bound to make sure that the Votes trait is implemented? - impl ERC721Votes< + pub impl ERC721VotesImpl< TContractState, +HasComponent, +SRC5Component::HasComponent, impl ERC721: ERC721Component::HasComponent, +ERC721Component::ERC721HooksTrait, +Drop - > of IVotesToken> { - // ERC721-specific implementation + > of TokenVotesTrait> { + /// 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, account: ContractAddress ) -> u256 { @@ -156,19 +200,46 @@ pub mod VotesComponent { } // - // Internal + // Internal for ERC20Votes + // + + pub impl ERC20VotesImpl< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + impl ERC20: ERC20Component::HasComponent, + +ERC20Component::ERC20HooksTrait, + +Drop + > of TokenVotesTrait> { + /// 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, account: ContractAddress + ) -> u256 { + let mut erc20_component = get_dep_component!(self, ERC20); + erc20_component.balance_of(account) + } + } + + // + // Common internal for Votes // #[generate_trait] pub impl InternalImpl< TContractState, +HasComponent, - impl TokenTrait: IVotesToken>, + impl TokenTrait: TokenVotesTrait>, +NoncesComponent::HasComponent, +SNIP12Metadata, +Drop > of InternalTrait { - // Common internal functions + /// 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, account: ContractAddress, @@ -186,6 +257,9 @@ pub mod VotesComponent { ); } + /// Moves delegated votes from one delegate to another. + /// + /// May emit one or two `DelegateVotesChanged` events. fn move_delegate_votes( ref self: ComponentState, from: ContractAddress, @@ -210,6 +284,12 @@ pub mod VotesComponent { } } + /// 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, from: ContractAddress, From 6c02234078fbcf1de3cb948d43804880f92b4617 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Wed, 4 Sep 2024 17:16:56 -0400 Subject: [PATCH 05/15] add basic tests and mocks --- packages/governance/src/tests.cairo | 2 + packages/governance/src/tests/mocks.cairo | 1 + .../src/tests/mocks/votes_mocks.cairo | 189 +++++++++++ .../governance/src/tests/test_votes.cairo | 309 ++++++++++++++++++ 4 files changed, 501 insertions(+) create mode 100644 packages/governance/src/tests/mocks/votes_mocks.cairo create mode 100644 packages/governance/src/tests/test_votes.cairo diff --git a/packages/governance/src/tests.cairo b/packages/governance/src/tests.cairo index 221670eec..9da570e01 100644 --- a/packages/governance/src/tests.cairo +++ b/packages/governance/src/tests.cairo @@ -4,3 +4,5 @@ pub(crate) mod mocks; mod test_timelock; #[cfg(test)] mod test_utils; +#[cfg(test)] +mod test_votes; diff --git a/packages/governance/src/tests/mocks.cairo b/packages/governance/src/tests/mocks.cairo index 6d38a6715..72e9e6b05 100644 --- a/packages/governance/src/tests/mocks.cairo +++ b/packages/governance/src/tests/mocks.cairo @@ -1,2 +1,3 @@ pub(crate) mod non_implementing_mock; pub(crate) mod timelock_mocks; +pub(crate) mod votes_mocks; diff --git a/packages/governance/src/tests/mocks/votes_mocks.cairo b/packages/governance/src/tests/mocks/votes_mocks.cairo new file mode 100644 index 000000000..1d01470ef --- /dev/null +++ b/packages/governance/src/tests/mocks/votes_mocks.cairo @@ -0,0 +1,189 @@ +#[starknet::contract] +pub(crate) mod ERC721VotesMock { + use openzeppelin_governance::votes::votes::VotesComponent; + use openzeppelin_introspection::src5::SRC5Component; + use openzeppelin_token::erc721::ERC721Component; + // This is temporary - we should actually implement the hooks manually + // and transfer the voting units in the hooks. + use openzeppelin_token::erc721::ERC721HooksEmptyImpl; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + + component!(path: VotesComponent, storage: erc721_votes, event: ERC721VotesEvent); + component!(path: ERC721Component, storage: erc721, event: ERC721Event); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); + + //Votes and ERC721Votes + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl InternalImpl = VotesComponent::InternalImpl; + impl ERC721VotesInternalImpl = VotesComponent::ERC721VotesImpl; + + // ERC721 + #[abi(embed_v0)] + impl ERC721MixinImpl = ERC721Component::ERC721MixinImpl; + impl ERC721InternalImpl = ERC721Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + struct Storage { + #[substorage(v0)] + erc721_votes: VotesComponent::Storage, + #[substorage(v0)] + erc721: ERC721Component::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage, + #[substorage(v0)] + nonces: NoncesComponent::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + ERC721VotesEvent: VotesComponent::Event, + #[flat] + ERC721Event: ERC721Component::Event, + #[flat] + SRC5Event: SRC5Component::Event, + #[flat] + NoncesEvent: NoncesComponent::Event + } + + /// Required for hash computation. + pub(crate) impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + // + // Hooks + // + + // impl ERC721VotesHooksImpl< + // TContractState, + // impl Votes: VotesComponent::HasComponent, + // impl HasComponent: VotesComponent::HasComponent, + // +NoncesComponent::HasComponent, + // +Drop + // > of ERC721Component::ERC721HooksTrait { + // fn after_update( + // ref self: ERC721Component::ComponentState, + // to: ContractAddress, + // token_id: u256, + // auth: ContractAddress + // ) { + // let mut erc721_votes_component = get_dep_component_mut!(ref self, Votes); + // erc721_votes_component.transfer_voting_units(auth, to, 1); + // } + // } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc721.initializer("MyToken", "MTK", ""); + } +} + +#[starknet::contract] +pub(crate) mod ERC20VotesMock { + use openzeppelin_governance::votes::votes::VotesComponent; + use openzeppelin_introspection::src5::SRC5Component; + use openzeppelin_token::erc20::ERC20Component; + // This is temporary - we should actually implement the hooks manually + // and transfer the voting units in the hooks. + use openzeppelin_token::erc20::ERC20HooksEmptyImpl; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + + component!(path: VotesComponent, storage: erc20_votes, event: ERC20VotesEvent); + component!(path: ERC20Component, storage: erc20, event: ERC20Event); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); + + // Votes and ERC20Votes + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl InternalImpl = VotesComponent::InternalImpl; + impl ERC20VotesInternalImpl = VotesComponent::ERC20VotesImpl; + + // ERC20 + #[abi(embed_v0)] + impl ERC20MixinImpl = ERC20Component::ERC20MixinImpl; + impl ERC20InternalImpl = ERC20Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + struct Storage { + #[substorage(v0)] + erc20_votes: VotesComponent::Storage, + #[substorage(v0)] + erc20: ERC20Component::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage, + #[substorage(v0)] + nonces: NoncesComponent::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + ERC20VotesEvent: VotesComponent::Event, + #[flat] + ERC20Event: ERC20Component::Event, + #[flat] + SRC5Event: SRC5Component::Event, + #[flat] + NoncesEvent: NoncesComponent::Event + } + + /// Required for hash computation. + pub(crate) impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + // + // Hooks + // + + // Uncomment and modify this section if you need ERC20 hooks + // impl ERC20VotesHooksImpl< + // TContractState, + // impl Votes: VotesComponent::HasComponent, + // impl HasComponent: VotesComponent::HasComponent, + // +NoncesComponent::HasComponent, + // +Drop + // > of ERC20Component::ERC20HooksTrait { + // fn after_transfer( + // ref self: ERC20Component::ComponentState, + // from: ContractAddress, + // to: ContractAddress, + // amount: u256 + // ) { + // let mut erc20_votes_component = get_dep_component_mut!(ref self, Votes); + // erc20_votes_component.transfer_voting_units(from, to, amount); + // } + // } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc20.initializer("MyToken", "MTK"); + } +} + diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo new file mode 100644 index 000000000..b72477946 --- /dev/null +++ b/packages/governance/src/tests/test_votes.cairo @@ -0,0 +1,309 @@ +use openzeppelin_governance::tests::mocks::votes_mocks::ERC721VotesMock::SNIP12MetadataImpl; +use openzeppelin_governance::tests::mocks::votes_mocks::{ERC721VotesMock, ERC20VotesMock}; +use openzeppelin_governance::votes::interface::TokenVotesTrait; +use openzeppelin_governance::votes::votes::VotesComponent::{ + DelegateChanged, DelegateVotesChanged, VotesImpl, InternalImpl, +}; +use openzeppelin_governance::votes::votes::VotesComponent; +use openzeppelin_testing as utils; +use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; +use openzeppelin_testing::events::EventSpyExt; +use openzeppelin_token::erc20::ERC20Component::InternalTrait; +use openzeppelin_token::erc721::ERC721Component::{ + ERC721MetadataImpl, InternalImpl as ERC721InternalImpl, +}; +use openzeppelin_token::erc721::ERC721Component::{ERC721Impl, ERC721CamelOnlyImpl}; +use openzeppelin_utils::structs::checkpoint::TraceTrait; +use snforge_std::signature::stark_curve::{StarkCurveKeyPairImpl, StarkCurveSignerImpl}; +use snforge_std::{ + cheat_block_timestamp_global, start_cheat_caller_address, spy_events, test_address +}; +use snforge_std::{EventSpy}; +use starknet::ContractAddress; + + +// +// Setup +// + +type ComponentState = VotesComponent::ComponentState; +type ERC20ComponentState = VotesComponent::ComponentState; + +fn COMPONENT_STATE() -> ComponentState { + VotesComponent::component_state_for_testing() +} + +fn ERC20_COMPONENT_STATE() -> ERC20ComponentState { + VotesComponent::component_state_for_testing() +} + +fn ERC721VOTES_CONTRACT_STATE() -> ERC721VotesMock::ContractState { + ERC721VotesMock::contract_state_for_testing() +} + +fn ERC20VOTES_CONTRACT_STATE() -> ERC20VotesMock::ContractState { + ERC20VotesMock::contract_state_for_testing() +} + +fn setup_erc721_votes() -> ComponentState { + let mut state = COMPONENT_STATE(); + let mut mock_state = ERC721VOTES_CONTRACT_STATE(); + // Mint 10 NFTs to OWNER + let mut i: u256 = 0; + loop { + if i >= 10 { + break; + } + mock_state.erc721.mint(OWNER(), i); + // We manually transfer voting units here, since this is usually implemented in the hook + state.transfer_voting_units(ZERO(), OWNER(), 1); + i += 1; + }; + state +} + +fn setup_erc20_votes() -> ERC20ComponentState { + let mut state = ERC20_COMPONENT_STATE(); + let mut mock_state = ERC20VOTES_CONTRACT_STATE(); + + // Mint SUPPLY tokens to owner + mock_state.erc20.mint(OWNER(), SUPPLY); + // We manually transfer voting units here, since this is usually implemented in the hook + state.transfer_voting_units(ZERO(), OWNER(), 1); + + state +} + +fn setup_account(public_key: felt252) -> ContractAddress { + let mut calldata = array![public_key]; + utils::declare_and_deploy("DualCaseAccountMock", calldata) +} + +// +// Common tests for Votes +// + +#[test] +fn test_get_votes() { + let mut state = setup_erc721_votes(); + start_cheat_caller_address(test_address(), OWNER()); + // Before delegating, the owner has 0 votes + assert_eq!(state.get_votes(OWNER()), 0); + state.delegate(OWNER()); + + assert_eq!(state.get_votes(OWNER()), 10); +} + +// This test can be improved by using the api of the component +// to add checkpoints and thus verify the internal state of the component +// instead of using the trace directly. +#[test] +fn test_get_past_votes() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_delegate_checkpoints.read(OWNER()); + + cheat_block_timestamp_global('ts10'); + + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + assert_eq!(state.get_past_votes(OWNER(), 'ts2'), 5); + assert_eq!(state.get_past_votes(OWNER(), 'ts5'), 7); +} + +#[test] +#[should_panic(expected: ('Votes: future Lookup',))] +fn test_get_past_votes_future_lookup() { + let state = setup_erc721_votes(); + cheat_block_timestamp_global('ts1'); + state.get_past_votes(OWNER(), 'ts2'); +} + +#[test] +fn test_get_past_total_supply() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_total_checkpoints.read(); + + cheat_block_timestamp_global('ts10'); + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + assert_eq!(state.get_past_total_supply('ts2'), 5); + assert_eq!(state.get_past_total_supply('ts5'), 7); +} + +#[test] +#[should_panic(expected: ('Votes: future Lookup',))] +fn test_get_past_total_supply_future_lookup() { + let state = setup_erc721_votes(); + cheat_block_timestamp_global('ts1'); + state.get_past_total_supply('ts2'); +} + +#[test] +fn test_self_delegate() { + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, OWNER()); + + state.delegate(OWNER()); + spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), OWNER()); + spy.assert_only_event_delegate_votes_changed(contract_address, OWNER(), 0, 10); + assert_eq!(state.get_votes(OWNER()), 10); +} + +#[test] +fn test_delegate_to_recipient_updates_votes() { + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, OWNER()); + + state.delegate(RECIPIENT()); + spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), RECIPIENT()); + spy.assert_only_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, 10); + assert_eq!(state.get_votes(RECIPIENT()), 10); + assert_eq!(state.get_votes(OWNER()), 0); +} + +#[test] +fn test_delegate_to_recipient_updates_delegates() { + let mut state = setup_erc721_votes(); + start_cheat_caller_address(test_address(), OWNER()); + state.delegate(OWNER()); + assert_eq!(state.delegates(OWNER()), OWNER()); + state.delegate(RECIPIENT()); + assert_eq!(state.delegates(OWNER()), RECIPIENT()); +} + +// #[test] +// fn test_delegate_by_sig() { +// cheat_chain_id_global('SN_TEST'); +// cheat_block_timestamp_global('ts1'); +// let mut state = setup_erc721_votes(); +// let contract_address = test_address(); +// let key_pair = KeyPairTrait::generate(); +// let account = setup_account(key_pair.public_key); +// let nonce = 0; +// let expiry = 'ts2'; +// let delegator = account; +// let delegatee = RECIPIENT(); +// let delegation = Delegation { delegatee, nonce, expiry }; +// let msg_hash = delegation.get_message_hash(delegator); +// let (r, s) = key_pair.sign(msg_hash).unwrap(); +// let mut spy = spy_events(); +// state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s]); +// spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); +// assert_eq!(state.delegates(account), delegatee); +// } + +#[test] +#[should_panic(expected: ('Votes: expired signature',))] +fn test_delegate_by_sig_past_expiry() { + cheat_block_timestamp_global('ts5'); + + let mut state = setup_erc721_votes(); + let expiry = 'ts4'; + let signature = array![0, 0]; + + state.delegate_by_sig(OWNER(), RECIPIENT(), 0, expiry, signature); +} + +#[test] +#[should_panic(expected: ('Nonces: invalid nonce',))] +fn test_delegate_by_sig_invalid_nonce() { + let mut state = setup_erc721_votes(); + let signature = array![0, 0]; + + state.delegate_by_sig(OWNER(), RECIPIENT(), 1, 0, signature); +} + +// #[test] +// #[should_panic(expected: ('Votes: invalid signature',))] +// fn test_delegate_by_sig_invalid_signature() { +// let mut state = setup_erc721_votes(); +// // For some reason this is panicking before we get to delegate_by_sig +// let account = setup_account(0x123); +// let signature = array![0, 0]; + +// state.delegate_by_sig(account, RECIPIENT(), 0, 0, signature); +// } + +// +// Tests specific to ERC721Votes and +// + +#[test] +fn test_erc721_get_voting_units() { + let state = setup_erc721_votes(); + + assert_eq!(state.get_voting_units(OWNER()), 10); + assert_eq!(state.get_voting_units(RECIPIENT()), 0); +} + +#[test] +fn test_erc20_get_voting_units() { + let mut state = setup_erc20_votes(); + + assert_eq!(state.get_voting_units(OWNER()), SUPPLY); + assert_eq!(state.get_voting_units(RECIPIENT()), 0); +} + +// +// Helpers +// + +#[generate_trait] +impl VotesSpyHelpersImpl of VotesSpyHelpers { + fn assert_event_delegate_changed( + ref self: EventSpy, + contract: ContractAddress, + delegator: ContractAddress, + from_delegate: ContractAddress, + to_delegate: ContractAddress + ) { + let expected = VotesComponent::Event::DelegateChanged( + DelegateChanged { delegator, from_delegate, to_delegate } + ); + self.assert_emitted_single(contract, expected); + } + + fn assert_event_delegate_votes_changed( + ref self: EventSpy, + contract: ContractAddress, + delegate: ContractAddress, + previous_votes: u256, + new_votes: u256 + ) { + let expected = VotesComponent::Event::DelegateVotesChanged( + DelegateVotesChanged { delegate, previous_votes, new_votes } + ); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_delegate_changed( + ref self: EventSpy, + contract: ContractAddress, + delegator: ContractAddress, + from_delegate: ContractAddress, + to_delegate: ContractAddress + ) { + self.assert_event_delegate_changed(contract, delegator, from_delegate, to_delegate); + self.assert_no_events_left_from(contract); + } + + fn assert_only_event_delegate_votes_changed( + ref self: EventSpy, + contract: ContractAddress, + delegate: ContractAddress, + previous_votes: u256, + new_votes: u256 + ) { + self.assert_event_delegate_votes_changed(contract, delegate, previous_votes, new_votes); + self.assert_no_events_left_from(contract); + } +} + From 64461b90f7ddbbbcb300776f5400dd46d2301596 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Wed, 4 Sep 2024 17:37:04 -0400 Subject: [PATCH 06/15] refactor, remove unused usings --- packages/governance/src/votes/votes.cairo | 21 +++++-------------- .../src/erc20/extensions/erc20_votes.cairo | 5 +---- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 47d27a927..3b912fe96 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -1,10 +1,7 @@ // 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; +use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, SNIP12Metadata}; /// # Votes Component /// @@ -25,7 +22,7 @@ pub mod VotesComponent { 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}; + use openzeppelin_governance::votes::utils::Delegation; use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_token::erc20::ERC20Component; use openzeppelin_token::erc20::interface::IERC20; @@ -33,7 +30,7 @@ pub mod VotesComponent { 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 openzeppelin_utils::structs::checkpoint::{Trace, TraceTrait}; use starknet::ContractAddress; use starknet::storage::Map; use super::{OffchainMessageHash, SNIP12Metadata}; @@ -81,7 +78,7 @@ pub mod VotesComponent { TContractState, +HasComponent, impl Nonces: NoncesComponent::HasComponent, - impl TokenTrait: TokenVotesTrait>, + +TokenVotesTrait>, +SNIP12Metadata, +Drop > of IVotes> { @@ -175,7 +172,7 @@ pub mod VotesComponent { } // - // Internal for ERC721Votes + // Internal // pub impl ERC721VotesImpl< @@ -199,10 +196,6 @@ pub mod VotesComponent { } } - // - // Internal for ERC20Votes - // - pub impl ERC20VotesImpl< TContractState, +HasComponent, @@ -223,10 +216,6 @@ pub mod VotesComponent { } } - // - // Common internal for Votes - // - #[generate_trait] pub impl InternalImpl< TContractState, diff --git a/packages/token/src/erc20/extensions/erc20_votes.cairo b/packages/token/src/erc20/extensions/erc20_votes.cairo index 32237f84c..b1fdb633e 100644 --- a/packages/token/src/erc20/extensions/erc20_votes.cairo +++ b/packages/token/src/erc20/extensions/erc20_votes.cairo @@ -1,10 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo v0.15.1 (token/erc20/extensions/erc20_votes.cairo) -use core::hash::{HashStateTrait, HashStateExTrait}; -use core::poseidon::PoseidonTrait; -use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, StructHash, SNIP12Metadata}; -use starknet::ContractAddress; +use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, SNIP12Metadata}; /// # ERC20Votes Component /// From 6f8c2d0174da15fc979070ea19fbc2433b15c30e Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Wed, 4 Sep 2024 22:14:55 -0400 Subject: [PATCH 07/15] cleanup mocks, remove public impl --- .../src/tests/mocks/votes_mocks.cairo | 53 ------------------- packages/governance/src/votes/votes.cairo | 4 +- .../src/tests/erc20/test_erc20_votes.cairo | 2 +- 3 files changed, 3 insertions(+), 56 deletions(-) diff --git a/packages/governance/src/tests/mocks/votes_mocks.cairo b/packages/governance/src/tests/mocks/votes_mocks.cairo index 1d01470ef..7c35bbce4 100644 --- a/packages/governance/src/tests/mocks/votes_mocks.cairo +++ b/packages/governance/src/tests/mocks/votes_mocks.cairo @@ -3,8 +3,6 @@ pub(crate) mod ERC721VotesMock { use openzeppelin_governance::votes::votes::VotesComponent; use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_token::erc721::ERC721Component; - // This is temporary - we should actually implement the hooks manually - // and transfer the voting units in the hooks. use openzeppelin_token::erc721::ERC721HooksEmptyImpl; use openzeppelin_utils::cryptography::nonces::NoncesComponent; use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; @@ -17,8 +15,6 @@ pub(crate) mod ERC721VotesMock { //Votes and ERC721Votes #[abi(embed_v0)] impl VotesImpl = VotesComponent::VotesImpl; - impl InternalImpl = VotesComponent::InternalImpl; - impl ERC721VotesInternalImpl = VotesComponent::ERC721VotesImpl; // ERC721 #[abi(embed_v0)] @@ -64,28 +60,6 @@ pub(crate) mod ERC721VotesMock { } } - // - // Hooks - // - - // impl ERC721VotesHooksImpl< - // TContractState, - // impl Votes: VotesComponent::HasComponent, - // impl HasComponent: VotesComponent::HasComponent, - // +NoncesComponent::HasComponent, - // +Drop - // > of ERC721Component::ERC721HooksTrait { - // fn after_update( - // ref self: ERC721Component::ComponentState, - // to: ContractAddress, - // token_id: u256, - // auth: ContractAddress - // ) { - // let mut erc721_votes_component = get_dep_component_mut!(ref self, Votes); - // erc721_votes_component.transfer_voting_units(auth, to, 1); - // } - // } - #[constructor] fn constructor(ref self: ContractState) { self.erc721.initializer("MyToken", "MTK", ""); @@ -97,8 +71,6 @@ pub(crate) mod ERC20VotesMock { use openzeppelin_governance::votes::votes::VotesComponent; use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_token::erc20::ERC20Component; - // This is temporary - we should actually implement the hooks manually - // and transfer the voting units in the hooks. use openzeppelin_token::erc20::ERC20HooksEmptyImpl; use openzeppelin_utils::cryptography::nonces::NoncesComponent; use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; @@ -111,8 +83,6 @@ pub(crate) mod ERC20VotesMock { // Votes and ERC20Votes #[abi(embed_v0)] impl VotesImpl = VotesComponent::VotesImpl; - impl InternalImpl = VotesComponent::InternalImpl; - impl ERC20VotesInternalImpl = VotesComponent::ERC20VotesImpl; // ERC20 #[abi(embed_v0)] @@ -158,29 +128,6 @@ pub(crate) mod ERC20VotesMock { } } - // - // Hooks - // - - // Uncomment and modify this section if you need ERC20 hooks - // impl ERC20VotesHooksImpl< - // TContractState, - // impl Votes: VotesComponent::HasComponent, - // impl HasComponent: VotesComponent::HasComponent, - // +NoncesComponent::HasComponent, - // +Drop - // > of ERC20Component::ERC20HooksTrait { - // fn after_transfer( - // ref self: ERC20Component::ComponentState, - // from: ContractAddress, - // to: ContractAddress, - // amount: u256 - // ) { - // let mut erc20_votes_component = get_dep_component_mut!(ref self, Votes); - // erc20_votes_component.transfer_voting_units(from, to, amount); - // } - // } - #[constructor] fn constructor(ref self: ContractState) { self.erc20.initializer("MyToken", "MTK"); diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 3b912fe96..79574cab5 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -175,7 +175,7 @@ pub mod VotesComponent { // Internal // - pub impl ERC721VotesImpl< + impl ERC721VotesImpl< TContractState, +HasComponent, +SRC5Component::HasComponent, @@ -196,7 +196,7 @@ pub mod VotesComponent { } } - pub impl ERC20VotesImpl< + impl ERC20VotesImpl< TContractState, +HasComponent, +SRC5Component::HasComponent, diff --git a/packages/token/src/tests/erc20/test_erc20_votes.cairo b/packages/token/src/tests/erc20/test_erc20_votes.cairo index c11e9bf07..16bc80827 100644 --- a/packages/token/src/tests/erc20/test_erc20_votes.cairo +++ b/packages/token/src/tests/erc20/test_erc20_votes.cairo @@ -1,6 +1,6 @@ use core::num::traits::Bounded; use core::num::traits::Zero; -use openzeppelin_governance::votes::utils::{Delegation}; +use openzeppelin_governance::votes::utils::Delegation; use openzeppelin_testing as utils; use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; use openzeppelin_testing::events::EventSpyExt; From de076af7da0a803a3059536f3c0b46fa380e8e02 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Fri, 4 Oct 2024 16:41:05 -0400 Subject: [PATCH 08/15] refactor --- Scarb.lock | 2 +- .../src/tests/mocks/votes_mocks.cairo | 6 ---- .../governance/src/tests/test_votes.cairo | 15 ++++------ packages/governance/src/votes/interface.cairo | 4 --- packages/governance/src/votes/votes.cairo | 29 ++++++++++--------- 5 files changed, 23 insertions(+), 33 deletions(-) diff --git a/Scarb.lock b/Scarb.lock index ab2f2fb59..cca249e31 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -62,8 +62,8 @@ dependencies = [ "openzeppelin_account", "openzeppelin_introspection", "openzeppelin_testing", - "openzeppelin_utils", "openzeppelin_token", + "openzeppelin_utils", "snforge_std", ] diff --git a/packages/governance/src/tests/mocks/votes_mocks.cairo b/packages/governance/src/tests/mocks/votes_mocks.cairo index 608280841..89e2d7324 100644 --- a/packages/governance/src/tests/mocks/votes_mocks.cairo +++ b/packages/governance/src/tests/mocks/votes_mocks.cairo @@ -69,7 +69,6 @@ pub(crate) mod ERC721VotesMock { #[starknet::contract] pub(crate) mod ERC20VotesMock { use openzeppelin_governance::votes::votes::VotesComponent; - use openzeppelin_introspection::src5::SRC5Component; use openzeppelin_token::erc20::ERC20Component; use openzeppelin_token::erc20::ERC20HooksEmptyImpl; use openzeppelin_utils::cryptography::nonces::NoncesComponent; @@ -77,7 +76,6 @@ pub(crate) mod ERC20VotesMock { component!(path: VotesComponent, storage: erc20_votes, event: ERC20VotesEvent); component!(path: ERC20Component, storage: erc20, event: ERC20Event); - component!(path: SRC5Component, storage: src5, event: SRC5Event); component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); // Votes and ERC20Votes @@ -100,8 +98,6 @@ pub(crate) mod ERC20VotesMock { #[substorage(v0)] pub erc20: ERC20Component::Storage, #[substorage(v0)] - pub src5: SRC5Component::Storage, - #[substorage(v0)] pub nonces: NoncesComponent::Storage } @@ -113,8 +109,6 @@ pub(crate) mod ERC20VotesMock { #[flat] ERC20Event: ERC20Component::Event, #[flat] - SRC5Event: SRC5Component::Event, - #[flat] NoncesEvent: NoncesComponent::Event } diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo index ed123778c..b49eee489 100644 --- a/packages/governance/src/tests/test_votes.cairo +++ b/packages/governance/src/tests/test_votes.cairo @@ -1,12 +1,12 @@ use openzeppelin_governance::tests::mocks::votes_mocks::ERC721VotesMock::SNIP12MetadataImpl; use openzeppelin_governance::tests::mocks::votes_mocks::{ERC721VotesMock, ERC20VotesMock}; -use openzeppelin_governance::votes::interface::TokenVotesTrait; +use openzeppelin_governance::votes::votes::TokenVotesTrait; use openzeppelin_governance::votes::votes::VotesComponent::{ DelegateChanged, DelegateVotesChanged, VotesImpl, InternalImpl, }; use openzeppelin_governance::votes::votes::VotesComponent; use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; +use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT, OTHER}; use openzeppelin_testing::events::EventSpyExt; use openzeppelin_token::erc20::ERC20Component::InternalTrait; use openzeppelin_token::erc721::ERC721Component::{ @@ -51,10 +51,7 @@ fn setup_erc721_votes() -> ComponentState { let mut mock_state = ERC721VOTES_CONTRACT_STATE(); // Mint 10 NFTs to OWNER let mut i: u256 = 0; - loop { - if i >= 10 { - break; - } + while i < 10 { mock_state.erc721.mint(OWNER(), i); // We manually transfer voting units here, since this is usually implemented in the hook state.transfer_voting_units(ZERO(), OWNER(), 1); @@ -70,7 +67,7 @@ fn setup_erc20_votes() -> ERC20ComponentState { // Mint SUPPLY tokens to owner mock_state.erc20.mint(OWNER(), SUPPLY); // We manually transfer voting units here, since this is usually implemented in the hook - state.transfer_voting_units(ZERO(), OWNER(), 1); + state.transfer_voting_units(ZERO(), OWNER(), SUPPLY); state } @@ -242,7 +239,7 @@ fn test_erc721_get_voting_units() { let state = setup_erc721_votes(); assert_eq!(state.get_voting_units(OWNER()), 10); - assert_eq!(state.get_voting_units(RECIPIENT()), 0); + assert_eq!(state.get_voting_units(OTHER()), 0); } #[test] @@ -250,7 +247,7 @@ fn test_erc20_get_voting_units() { let mut state = setup_erc20_votes(); assert_eq!(state.get_voting_units(OWNER()), SUPPLY); - assert_eq!(state.get_voting_units(RECIPIENT()), 0); + assert_eq!(state.get_voting_units(OTHER()), 0); } // diff --git a/packages/governance/src/votes/interface.cairo b/packages/governance/src/votes/interface.cairo index 707002b3d..597045720 100644 --- a/packages/governance/src/votes/interface.cairo +++ b/packages/governance/src/votes/interface.cairo @@ -34,7 +34,3 @@ pub trait IVotes { ); } -/// Common interface for tokens used for voting(e.g. `ERC721Votes` or `ERC20Votes`) -pub trait TokenVotesTrait { - fn get_voting_units(self: @TState, account: ContractAddress) -> u256; -} diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 5d6ddba60..0f98eda41 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo v0.15.1 (governance/votes/votes.cairo) -use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, SNIP12Metadata}; +use starknet::ContractAddress; + /// # Votes Component /// @@ -21,21 +22,21 @@ pub mod VotesComponent { // 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::interface::IVotes; use openzeppelin_governance::votes::utils::Delegation; 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::cryptography::snip12::{OffchainMessageHash, SNIP12Metadata}; use openzeppelin_utils::nonces::NoncesComponent::InternalTrait as NoncesInternalTrait; use openzeppelin_utils::nonces::NoncesComponent; use openzeppelin_utils::structs::checkpoint::{Trace, TraceTrait}; - use starknet::ContractAddress; use starknet::storage::{ Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess }; - use super::{OffchainMessageHash, SNIP12Metadata}; + use super::{TokenVotesTrait, ContractAddress}; #[storage] pub struct Storage { @@ -193,7 +194,7 @@ pub mod VotesComponent { fn get_voting_units( self: @ComponentState, account: ContractAddress ) -> u256 { - let mut erc721_component = get_dep_component!(self, ERC721); + let erc721_component = get_dep_component!(self, ERC721); erc721_component.balance_of(account).into() } } @@ -201,10 +202,8 @@ pub mod VotesComponent { impl ERC20VotesImpl< TContractState, +HasComponent, - +SRC5Component::HasComponent, impl ERC20: ERC20Component::HasComponent, - +ERC20Component::ERC20HooksTrait, - +Drop + +ERC20Component::ERC20HooksTrait > of TokenVotesTrait> { /// Returns the number of voting units for a given account. /// @@ -213,7 +212,7 @@ pub mod VotesComponent { fn get_voting_units( self: @ComponentState, account: ContractAddress ) -> u256 { - let mut erc20_component = get_dep_component!(self, ERC20); + let erc20_component = get_dep_component!(self, ERC20); erc20_component.balance_of(account) } } @@ -257,16 +256,15 @@ pub mod VotesComponent { 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) { + if from != to && amount > 0 { + if from.is_non_zero() { 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) { + if to.is_non_zero() { let mut trace = self.Votes_delegate_checkpoints.read(to); let (previous_votes, new_votes) = trace .push(block_timestamp, trace.latest() + amount); @@ -301,3 +299,8 @@ pub mod VotesComponent { } } } + +/// Common trait for tokens used for voting(e.g. `ERC721Votes` or `ERC20Votes`) +pub trait TokenVotesTrait { + fn get_voting_units(self: @TState, account: ContractAddress) -> u256; +} From 6f5a8a677d5c9d0645bd60902c6a8e7cd62b298f Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Fri, 4 Oct 2024 16:49:21 -0400 Subject: [PATCH 09/15] export VotesComponent for easier use --- packages/governance/src/votes.cairo | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/governance/src/votes.cairo b/packages/governance/src/votes.cairo index 69e5a8028..b66668c46 100644 --- a/packages/governance/src/votes.cairo +++ b/packages/governance/src/votes.cairo @@ -1,3 +1,5 @@ pub mod interface; pub mod utils; pub mod votes; + +pub use votes::VotesComponent; \ No newline at end of file From fefcfeb8b630a1816998d5037fa365a532ee1fab Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Fri, 4 Oct 2024 17:19:54 -0400 Subject: [PATCH 10/15] remove erc20votes from token module --- Scarb.lock | 1 - packages/token/README.md | 1 - packages/token/Scarb.toml | 1 - packages/token/src/erc20.cairo | 1 - packages/token/src/erc20/extensions.cairo | 3 - .../src/erc20/extensions/erc20_votes.cairo | 287 ------------- packages/token/src/erc20/interface.cairo | 40 -- packages/token/src/tests/erc20.cairo | 1 - .../src/tests/erc20/test_erc20_votes.cairo | 406 ------------------ packages/token/src/tests/mocks.cairo | 1 - .../src/tests/mocks/erc20_votes_mocks.cairo | 94 ---- 11 files changed, 836 deletions(-) delete mode 100644 packages/token/src/erc20/extensions.cairo delete mode 100644 packages/token/src/erc20/extensions/erc20_votes.cairo delete mode 100644 packages/token/src/tests/erc20/test_erc20_votes.cairo delete mode 100644 packages/token/src/tests/mocks/erc20_votes_mocks.cairo diff --git a/Scarb.lock b/Scarb.lock index cca249e31..6cf85b600 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -131,7 +131,6 @@ name = "openzeppelin_token" version = "0.17.0" dependencies = [ "openzeppelin_account", - "openzeppelin_governance", "openzeppelin_introspection", "openzeppelin_test_common", "openzeppelin_testing", diff --git a/packages/token/README.md b/packages/token/README.md index 33982f342..920092019 100644 --- a/packages/token/README.md +++ b/packages/token/README.md @@ -15,7 +15,6 @@ standards. #### Components - [`ERC20Component`](https://docs.openzeppelin.com/contracts-cairo/0.17.0/api/erc20#ERC20Component) -- [`ERC20VotesComponent`](https://docs.openzeppelin.com/contracts-cairo/0.17.0/api/erc20#ERC20VotesComponent) ### ERC721 diff --git a/packages/token/Scarb.toml b/packages/token/Scarb.toml index 158519166..e83f10a7f 100644 --- a/packages/token/Scarb.toml +++ b/packages/token/Scarb.toml @@ -26,7 +26,6 @@ fmt.workspace = true starknet.workspace = true openzeppelin_account = { path = "../account" } openzeppelin_introspection = { path = "../introspection" } -openzeppelin_governance = { path = "../governance" } openzeppelin_utils = { path = "../utils" } [dev-dependencies] diff --git a/packages/token/src/erc20.cairo b/packages/token/src/erc20.cairo index e40fbb2bb..3f63977d5 100644 --- a/packages/token/src/erc20.cairo +++ b/packages/token/src/erc20.cairo @@ -1,6 +1,5 @@ pub mod dual20; pub mod erc20; -pub mod extensions; pub mod interface; pub use erc20::{ERC20Component, ERC20HooksEmptyImpl}; diff --git a/packages/token/src/erc20/extensions.cairo b/packages/token/src/erc20/extensions.cairo deleted file mode 100644 index e45cdcbf3..000000000 --- a/packages/token/src/erc20/extensions.cairo +++ /dev/null @@ -1,3 +0,0 @@ -pub mod erc20_votes; - -pub use erc20_votes::ERC20VotesComponent; diff --git a/packages/token/src/erc20/extensions/erc20_votes.cairo b/packages/token/src/erc20/extensions/erc20_votes.cairo deleted file mode 100644 index de52e9814..000000000 --- a/packages/token/src/erc20/extensions/erc20_votes.cairo +++ /dev/null @@ -1,287 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.17.0 (token/erc20/extensions/erc20_votes.cairo) - -use openzeppelin_utils::cryptography::snip12::{OffchainMessageHash, SNIP12Metadata}; - -/// # ERC20Votes Component -/// -/// The ERC20Votes component tracks voting units from ERC20 balances, which are a measure of voting -/// power that can be transferred, and provides a system of vote delegation, where an account can -/// delegate its voting units to a sort of "representative" that will pool delegated voting units -/// from different accounts and can then use it to vote in decisions. In fact, voting units MUST be -/// delegated in order to count as actual votes, and an account has to delegate those votes to -/// itself if it wishes to participate in decisions and does not have a trusted representative. -#[starknet::component] -pub mod ERC20VotesComponent { - use core::num::traits::Zero; - use crate::erc20::ERC20Component; - use crate::erc20::interface::IERC20; - use openzeppelin_account::dual_account::{DualCaseAccount, DualCaseAccountTrait}; - use openzeppelin_governance::votes::interface::IVotes; - use openzeppelin_governance::votes::utils::{Delegation}; - 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, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess - }; - use super::{OffchainMessageHash, SNIP12Metadata}; - - #[storage] - pub struct Storage { - pub ERC20Votes_delegatee: Map, - pub ERC20Votes_delegate_checkpoints: Map, - pub ERC20Votes_total_checkpoints: Trace - } - - #[event] - #[derive(Drop, PartialEq, starknet::Event)] - pub enum Event { - DelegateChanged: DelegateChanged, - DelegateVotesChanged: DelegateVotesChanged, - } - - /// Emitted when `delegator` delegates their votes from `from_delegate` to `to_delegate`. - #[derive(Drop, PartialEq, starknet::Event)] - pub struct DelegateChanged { - #[key] - pub delegator: ContractAddress, - #[key] - pub from_delegate: ContractAddress, - #[key] - pub to_delegate: ContractAddress - } - - /// Emitted when `delegate` votes are updated from `previous_votes` to `new_votes`. - #[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(ERC20VotesImpl)] - impl ERC20Votes< - TContractState, - +HasComponent, - +ERC20Component::HasComponent, - +ERC20Component::ERC20HooksTrait, - impl Nonces: NoncesComponent::HasComponent, - +SNIP12Metadata, - +Drop - > of IVotes> { - /// Returns the current amount of votes that `account` has. - fn get_votes(self: @ComponentState, account: ContractAddress) -> u256 { - self.ERC20Votes_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, account: ContractAddress, timepoint: u64 - ) -> u256 { - let current_timepoint = starknet::get_block_timestamp(); - assert(timepoint < current_timepoint, Errors::FUTURE_LOOKUP); - - self.ERC20Votes_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. - /// - /// NOTE: This value is the sum of all available votes, which is not necessarily the sum of - /// all delegated votes. - /// Votes that have not been delegated are still part of total supply, even though they - /// would not participate in a vote. - fn get_past_total_supply(self: @ComponentState, timepoint: u64) -> u256 { - let current_timepoint = starknet::get_block_timestamp(); - assert(timepoint < current_timepoint, Errors::FUTURE_LOOKUP); - - self.ERC20Votes_total_checkpoints.read().upper_lookup_recent(timepoint) - } - - /// Returns the delegate that `account` has chosen. - fn delegates( - self: @ComponentState, account: ContractAddress - ) -> ContractAddress { - self.ERC20Votes_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, 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, - delegator: ContractAddress, - delegatee: ContractAddress, - nonce: felt252, - expiry: u64, - signature: Array - ) { - 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 - // - - #[generate_trait] - pub impl InternalImpl< - TContractState, - +HasComponent, - impl ERC20: ERC20Component::HasComponent, - +ERC20Component::ERC20HooksTrait, - +NoncesComponent::HasComponent, - +SNIP12Metadata, - +Drop - > of InternalTrait { - /// Returns the current total supply of votes. - fn get_total_supply(self: @ComponentState) -> u256 { - self.ERC20Votes_total_checkpoints.read().latest() - } - - /// 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, - account: ContractAddress, - delegatee: ContractAddress - ) { - let from_delegate = self.delegates(account); - self.ERC20Votes_delegatee.write(account, delegatee); - - self - .emit( - DelegateChanged { delegator: account, from_delegate, to_delegate: delegatee } - ); - self.move_delegate_votes(from_delegate, delegatee, self.get_voting_units(account)); - } - - /// Moves delegated votes from one delegate to another. - /// - /// May emit one or two `DelegateVotesChanged` events. - fn move_delegate_votes( - ref self: ComponentState, - 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.ERC20Votes_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.ERC20Votes_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 }); - } - } - } - - /// 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, - 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.ERC20Votes_total_checkpoints.read(); - trace.push(block_timestamp, trace.latest() + amount); - } - if (to == zero_address) { - let mut trace = self.ERC20Votes_total_checkpoints.read(); - trace.push(block_timestamp, trace.latest() - amount); - } - self.move_delegate_votes(self.delegates(from), self.delegates(to), amount); - } - - /// Returns the number of checkpoints for `account`. - fn num_checkpoints(self: @ComponentState, account: ContractAddress) -> u32 { - self.ERC20Votes_delegate_checkpoints.read(account).length() - } - - /// Returns the `pos`-th checkpoint for `account`. - fn checkpoints( - self: @ComponentState, account: ContractAddress, pos: u32 - ) -> Checkpoint { - self.ERC20Votes_delegate_checkpoints.read(account).at(pos) - } - - /// Returns the voting units of an `account`. - fn get_voting_units( - self: @ComponentState, account: ContractAddress - ) -> u256 { - let mut erc20_component = get_dep_component!(self, ERC20); - erc20_component.balance_of(account) - } - } -} diff --git a/packages/token/src/erc20/interface.cairo b/packages/token/src/erc20/interface.cairo index 811eb62fc..6ad506ad6 100644 --- a/packages/token/src/erc20/interface.cairo +++ b/packages/token/src/erc20/interface.cairo @@ -67,43 +67,3 @@ pub trait ERC20ABI { ref self: TState, sender: ContractAddress, recipient: ContractAddress, amount: u256 ) -> bool; } - -#[starknet::interface] -pub trait ERC20VotesABI { - // IERC20 - fn total_supply(self: @TState) -> u256; - fn balance_of(self: @TState, account: ContractAddress) -> u256; - fn allowance(self: @TState, owner: ContractAddress, spender: ContractAddress) -> u256; - fn transfer(ref self: TState, recipient: ContractAddress, amount: u256) -> bool; - fn transfer_from( - ref self: TState, sender: ContractAddress, recipient: ContractAddress, amount: u256 - ) -> bool; - fn approve(ref self: TState, spender: ContractAddress, amount: u256) -> bool; - - // IERC20Metadata - fn name(self: @TState) -> ByteArray; - fn symbol(self: @TState) -> ByteArray; - fn decimals(self: @TState) -> u8; - - // IVotes - fn get_votes(self: @TState, account: ContractAddress) -> u256; - fn get_past_votes(self: @TState, account: ContractAddress, timepoint: u64) -> u256; - fn get_past_total_supply(self: @TState, timepoint: u64) -> u256; - fn delegates(self: @TState, account: ContractAddress) -> ContractAddress; - fn delegate(ref self: TState, delegatee: ContractAddress); - fn delegate_by_sig( - ref self: TState, - delegator: ContractAddress, - delegatee: ContractAddress, - nonce: felt252, - expiry: u64, - signature: Array - ); - - // IERC20CamelOnly - fn totalSupply(self: @TState) -> u256; - fn balanceOf(self: @TState, account: ContractAddress) -> u256; - fn transferFrom( - ref self: TState, sender: ContractAddress, recipient: ContractAddress, amount: u256 - ) -> bool; -} diff --git a/packages/token/src/tests/erc20.cairo b/packages/token/src/tests/erc20.cairo index 213861a92..203942da6 100644 --- a/packages/token/src/tests/erc20.cairo +++ b/packages/token/src/tests/erc20.cairo @@ -1,3 +1,2 @@ mod test_dual20; mod test_erc20; -mod test_erc20_votes; diff --git a/packages/token/src/tests/erc20/test_erc20_votes.cairo b/packages/token/src/tests/erc20/test_erc20_votes.cairo deleted file mode 100644 index 775ccc10c..000000000 --- a/packages/token/src/tests/erc20/test_erc20_votes.cairo +++ /dev/null @@ -1,406 +0,0 @@ -use core::num::traits::Bounded; -use core::num::traits::Zero; -use openzeppelin_governance::votes::utils::Delegation; -use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; -use openzeppelin_testing::events::EventSpyExt; -use openzeppelin_token::erc20::ERC20Component::InternalImpl as ERC20Impl; -use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ - DelegateChanged, DelegateVotesChanged -}; -use openzeppelin_token::erc20::extensions::ERC20VotesComponent::{ERC20VotesImpl, InternalImpl}; -use openzeppelin_token::erc20::extensions::ERC20VotesComponent; -use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock::SNIP12MetadataImpl; -use openzeppelin_token::tests::mocks::erc20_votes_mocks::DualCaseERC20VotesMock; -use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; -use openzeppelin_utils::structs::checkpoint::{Checkpoint, TraceTrait}; -use snforge_std::EventSpy; -use snforge_std::signature::KeyPairTrait; -use snforge_std::signature::stark_curve::{StarkCurveKeyPairImpl, StarkCurveSignerImpl}; -use snforge_std::{ - start_cheat_block_timestamp_global, start_cheat_caller_address, spy_events, - start_cheat_chain_id_global, test_address -}; -use starknet::storage::{StorageMapReadAccess, StoragePointerReadAccess}; -use starknet::{ContractAddress, contract_address_const}; - -// -// Setup -// - -type ComponentState = ERC20VotesComponent::ComponentState; - -fn CONTRACT_STATE() -> DualCaseERC20VotesMock::ContractState { - DualCaseERC20VotesMock::contract_state_for_testing() -} -fn COMPONENT_STATE() -> ComponentState { - ERC20VotesComponent::component_state_for_testing() -} - -fn setup() -> ComponentState { - let mut state = COMPONENT_STATE(); - let mut mock_state = CONTRACT_STATE(); - - mock_state.erc20.mint(OWNER(), SUPPLY); - state.transfer_voting_units(ZERO(), OWNER(), SUPPLY); - state -} - -fn setup_account(public_key: felt252) -> ContractAddress { - let mut calldata = array![public_key]; - utils::declare_and_deploy("DualCaseAccountMock", calldata) -} - -// Checkpoints unordered insertion - -#[test] -#[should_panic(expected: ('Unordered insertion',))] -fn test__delegate_checkpoints_unordered_insertion() { - let mut state = setup(); - let mut trace = state.ERC20Votes_delegate_checkpoints.read(OWNER()); - - start_cheat_block_timestamp_global('ts10'); - trace.push('ts2', 0x222); - trace.push('ts1', 0x111); -} - -#[test] -#[should_panic(expected: ('Unordered insertion',))] -fn test__total_checkpoints_unordered_insertion() { - let mut state = setup(); - let mut trace = state.ERC20Votes_total_checkpoints.read(); - - start_cheat_block_timestamp_global('ts10'); - trace.push('ts2', 0x222); - trace.push('ts1', 0x111); -} - -// -// get_votes && get_past_votes -// - -#[test] -fn test_get_votes() { - let mut state = setup(); - start_cheat_caller_address(test_address(), OWNER()); - state.delegate(OWNER()); - - assert_eq!(state.get_votes(OWNER()), SUPPLY); -} - -#[test] -fn test_get_past_votes() { - let mut state = setup(); - let mut trace = state.ERC20Votes_delegate_checkpoints.read(OWNER()); - - // Future timestamp. - start_cheat_block_timestamp_global('ts10'); - trace.push('ts1', 0x111); - trace.push('ts2', 0x222); - trace.push('ts3', 0x333); - - // Big numbers (high different from 0x0) - let big1: u256 = Bounded::::MAX.into() + 0x444; - let big2: u256 = Bounded::::MAX.into() + 0x666; - let big3: u256 = Bounded::::MAX.into() + 0x888; - trace.push('ts4', big1); - trace.push('ts6', big2); - trace.push('ts8', big3); - - assert_eq!(state.get_past_votes(OWNER(), 'ts2'), 0x222, "Should eq ts2"); - assert_eq!(state.get_past_votes(OWNER(), 'ts5'), big1, "Should eq ts4"); - assert_eq!(state.get_past_votes(OWNER(), 'ts8'), big3, "Should eq ts8"); -} - -#[test] -#[should_panic(expected: ('Votes: future Lookup',))] -fn test_get_past_votes_future_lookup() { - let state = setup(); - - // Past timestamp. - start_cheat_block_timestamp_global('ts1'); - state.get_past_votes(OWNER(), 'ts2'); -} - -#[test] -fn test_get_past_total_supply() { - let mut state = setup(); - let mut trace = state.ERC20Votes_total_checkpoints.read(); - - // Future timestamp. - start_cheat_block_timestamp_global('ts10'); - trace.push('ts1', 0x111); - trace.push('ts2', 0x222); - trace.push('ts3', 0x333); - - // Big numbers (high different from 0x0) - let big1: u256 = Bounded::::MAX.into() + 0x444; - let big2: u256 = Bounded::::MAX.into() + 0x666; - let big3: u256 = Bounded::::MAX.into() + 0x888; - trace.push('ts4', big1); - trace.push('ts6', big2); - trace.push('ts8', big3); - - assert_eq!(state.get_past_total_supply('ts2'), 0x222, "Should eq ts2"); - assert_eq!(state.get_past_total_supply('ts5'), big1, "Should eq ts4"); - assert_eq!(state.get_past_total_supply('ts8'), big3, "Should eq ts8"); -} - -#[test] -#[should_panic(expected: ('Votes: future Lookup',))] -fn test_get_past_total_supply_future_lookup() { - let state = setup(); - - // Past timestamp. - start_cheat_block_timestamp_global('ts1'); - state.get_past_total_supply('ts2'); -} - -// -// delegate & delegates -// - -#[test] -fn test_delegate() { - let mut state = setup(); - let contract_address = test_address(); - let mut spy = spy_events(); - - start_cheat_caller_address(contract_address, OWNER()); - - // Delegate from zero - state.delegate(OWNER()); - - spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), OWNER()); - spy.assert_only_event_delegate_votes_changed(contract_address, OWNER(), 0, SUPPLY); - assert_eq!(state.get_votes(OWNER()), SUPPLY); - - // Delegate from non-zero to non-zero - state.delegate(RECIPIENT()); - - spy.assert_event_delegate_changed(contract_address, OWNER(), OWNER(), RECIPIENT()); - spy.assert_event_delegate_votes_changed(contract_address, OWNER(), SUPPLY, 0); - spy.assert_only_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, SUPPLY); - assert!(state.get_votes(OWNER()).is_zero()); - assert_eq!(state.get_votes(RECIPIENT()), SUPPLY); - - // Delegate to zero - state.delegate(ZERO()); - - spy.assert_event_delegate_changed(contract_address, OWNER(), RECIPIENT(), ZERO()); - spy.assert_event_delegate_votes_changed(contract_address, RECIPIENT(), SUPPLY, 0); - assert!(state.get_votes(RECIPIENT()).is_zero()); - - // Delegate from zero to zero - state.delegate(ZERO()); - - spy.assert_only_event_delegate_changed(contract_address, OWNER(), ZERO(), ZERO()); -} - -#[test] -fn test_delegates() { - let mut state = setup(); - start_cheat_caller_address(test_address(), OWNER()); - - state.delegate(OWNER()); - assert_eq!(state.delegates(OWNER()), OWNER()); - - state.delegate(RECIPIENT()); - assert_eq!(state.delegates(OWNER()), RECIPIENT()); -} - -// delegate_by_sig - -#[test] -fn test_delegate_by_sig_hash_generation() { - start_cheat_chain_id_global('SN_TEST'); - - let nonce = 0; - let expiry = 'ts2'; - let delegator = contract_address_const::< - 0x70b0526a4bfbc9ca717c96aeb5a8afac85181f4585662273668928585a0d628 - >(); - let delegatee = RECIPIENT(); - let delegation = Delegation { delegatee, nonce, expiry }; - - let hash = delegation.get_message_hash(delegator); - - // This hash was computed using starknet js sdk from the following values: - // - name: 'DAPP_NAME' - // - version: 'DAPP_VERSION' - // - chainId: 'SN_TEST' - // - account: 0x70b0526a4bfbc9ca717c96aeb5a8afac85181f4585662273668928585a0d628 - // - delegatee: 'RECIPIENT' - // - nonce: 0 - // - expiry: 'ts2' - // - revision: '1' - let expected_hash = 0x314bd38b22b62d576691d8dafd9f8ea0601329ebe686bc64ca28e4d8821d5a0; - assert_eq!(hash, expected_hash); -} - -#[test] -fn test_delegate_by_sig() { - start_cheat_chain_id_global('SN_TEST'); - start_cheat_block_timestamp_global('ts1'); - - let mut state = setup(); - let contract_address = test_address(); - let key_pair = KeyPairTrait::::generate(); - let account = setup_account(key_pair.public_key); - - let nonce = 0; - let expiry = 'ts2'; - let delegator = account; - let delegatee = RECIPIENT(); - - let delegation = Delegation { delegatee, nonce, expiry }; - let msg_hash = delegation.get_message_hash(delegator); - let (r, s) = key_pair.sign(msg_hash).unwrap(); - - let mut spy = spy_events(); - - state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s]); - - spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); - assert_eq!(state.delegates(account), delegatee); -} - -#[test] -#[should_panic(expected: ('Votes: expired signature',))] -fn test_delegate_by_sig_past_expiry() { - start_cheat_block_timestamp_global('ts5'); - - let mut state = setup(); - let expiry = 'ts4'; - let signature = array![0, 0]; - - state.delegate_by_sig(OWNER(), RECIPIENT(), 0, expiry, signature); -} - -#[test] -#[should_panic(expected: ('Nonces: invalid nonce',))] -fn test_delegate_by_sig_invalid_nonce() { - let mut state = setup(); - let signature = array![0, 0]; - - state.delegate_by_sig(OWNER(), RECIPIENT(), 1, 0, signature); -} - -#[test] -#[should_panic(expected: ('Votes: invalid signature',))] -fn test_delegate_by_sig_invalid_signature() { - let mut state = setup(); - let account = setup_account(0x123); - let signature = array![0, 0]; - - state.delegate_by_sig(account, RECIPIENT(), 0, 0, signature); -} - -// -// num_checkpoints & checkpoints -// - -#[test] -fn test_num_checkpoints() { - let state = @setup(); - let mut trace = state.ERC20Votes_delegate_checkpoints.read(OWNER()); - - trace.push('ts1', 0x111); - trace.push('ts2', 0x222); - trace.push('ts3', 0x333); - trace.push('ts4', 0x444); - assert_eq!(state.num_checkpoints(OWNER()), 4); - - trace.push('ts5', 0x555); - trace.push('ts6', 0x666); - assert_eq!(state.num_checkpoints(OWNER()), 6); - - assert!(state.num_checkpoints(RECIPIENT()).is_zero()); -} - -#[test] -fn test_checkpoints() { - let state = @setup(); - let mut trace = state.ERC20Votes_delegate_checkpoints.read(OWNER()); - - trace.push('ts1', 0x111); - trace.push('ts2', 0x222); - trace.push('ts3', 0x333); - trace.push('ts4', 0x444); - - let checkpoint: Checkpoint = state.checkpoints(OWNER(), 2); - assert_eq!(checkpoint.key, 'ts3'); - assert_eq!(checkpoint.value, 0x333); -} - -#[test] -#[should_panic(expected: ('Array overflow',))] -fn test__checkpoints_array_overflow() { - let state = setup(); - state.checkpoints(OWNER(), 1); -} - -// -// get_voting_units -// - -#[test] -fn test_get_voting_units() { - let state = setup(); - assert_eq!(state.get_voting_units(OWNER()), SUPPLY); -} - -// -// Helpers -// - -#[generate_trait] -impl VotesSpyHelpersImpl of VotesSpyHelpers { - fn assert_event_delegate_changed( - ref self: EventSpy, - contract: ContractAddress, - delegator: ContractAddress, - from_delegate: ContractAddress, - to_delegate: ContractAddress - ) { - let expected = ERC20VotesComponent::Event::DelegateChanged( - DelegateChanged { delegator, from_delegate, to_delegate } - ); - self.assert_emitted_single(contract, expected); - } - - fn assert_only_event_delegate_changed( - ref self: EventSpy, - contract: ContractAddress, - delegator: ContractAddress, - from_delegate: ContractAddress, - to_delegate: ContractAddress - ) { - self.assert_event_delegate_changed(contract, delegator, from_delegate, to_delegate); - self.assert_no_events_left_from(contract); - } - - fn assert_event_delegate_votes_changed( - ref self: EventSpy, - contract: ContractAddress, - delegate: ContractAddress, - previous_votes: u256, - new_votes: u256 - ) { - let expected = ERC20VotesComponent::Event::DelegateVotesChanged( - DelegateVotesChanged { delegate, previous_votes, new_votes } - ); - self.assert_emitted_single(contract, expected); - } - - fn assert_only_event_delegate_votes_changed( - ref self: EventSpy, - contract: ContractAddress, - delegate: ContractAddress, - previous_votes: u256, - new_votes: u256 - ) { - self.assert_event_delegate_votes_changed(contract, delegate, previous_votes, new_votes); - self.assert_no_events_left_from(contract); - } -} diff --git a/packages/token/src/tests/mocks.cairo b/packages/token/src/tests/mocks.cairo index 9574a4b7a..849962bbd 100644 --- a/packages/token/src/tests/mocks.cairo +++ b/packages/token/src/tests/mocks.cairo @@ -2,7 +2,6 @@ pub(crate) mod account_mocks; pub(crate) mod erc1155_mocks; pub(crate) mod erc1155_receiver_mocks; pub(crate) mod erc20_mocks; -pub(crate) mod erc20_votes_mocks; pub(crate) mod erc2981_mocks; pub(crate) mod erc721_enumerable_mocks; pub(crate) mod erc721_mocks; diff --git a/packages/token/src/tests/mocks/erc20_votes_mocks.cairo b/packages/token/src/tests/mocks/erc20_votes_mocks.cairo deleted file mode 100644 index c777ec62f..000000000 --- a/packages/token/src/tests/mocks/erc20_votes_mocks.cairo +++ /dev/null @@ -1,94 +0,0 @@ -#[starknet::contract] -pub(crate) mod DualCaseERC20VotesMock { - use crate::erc20::ERC20Component; - use crate::erc20::extensions::ERC20VotesComponent::InternalTrait as ERC20VotesInternalTrait; - use crate::erc20::extensions::ERC20VotesComponent; - use openzeppelin_utils::cryptography::nonces::NoncesComponent; - use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; - use starknet::ContractAddress; - - component!(path: ERC20VotesComponent, storage: erc20_votes, event: ERC20VotesEvent); - component!(path: ERC20Component, storage: erc20, event: ERC20Event); - component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); - - // ERC20Votes - #[abi(embed_v0)] - impl ERC20VotesComponentImpl = - ERC20VotesComponent::ERC20VotesImpl; - - // ERC20Mixin - #[abi(embed_v0)] - impl ERC20MixinImpl = ERC20Component::ERC20MixinImpl; - impl InternalImpl = ERC20Component::InternalImpl; - - // Nonces - #[abi(embed_v0)] - impl NoncesImpl = NoncesComponent::NoncesImpl; - - #[storage] - pub struct Storage { - #[substorage(v0)] - pub erc20_votes: ERC20VotesComponent::Storage, - #[substorage(v0)] - pub erc20: ERC20Component::Storage, - #[substorage(v0)] - pub nonces: NoncesComponent::Storage - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - #[flat] - ERC20VotesEvent: ERC20VotesComponent::Event, - #[flat] - ERC20Event: ERC20Component::Event, - #[flat] - NoncesEvent: NoncesComponent::Event - } - - /// Required for hash computation. - pub(crate) impl SNIP12MetadataImpl of SNIP12Metadata { - fn name() -> felt252 { - 'DAPP_NAME' - } - fn version() -> felt252 { - 'DAPP_VERSION' - } - } - - // - // Hooks - // - - impl ERC20VotesHooksImpl< - TContractState, - impl ERC20Votes: ERC20VotesComponent::HasComponent, - impl HasComponent: ERC20Component::HasComponent, - +NoncesComponent::HasComponent, - +Drop - > of ERC20Component::ERC20HooksTrait { - fn after_update( - ref self: ERC20Component::ComponentState, - from: ContractAddress, - recipient: ContractAddress, - amount: u256 - ) { - let mut erc20_votes_component = get_dep_component_mut!(ref self, ERC20Votes); - erc20_votes_component.transfer_voting_units(from, recipient, amount); - } - } - - /// Sets the token `name` and `symbol`. - /// Mints `fixed_supply` tokens to `recipient`. - #[constructor] - fn constructor( - ref self: ContractState, - name: ByteArray, - symbol: ByteArray, - fixed_supply: u256, - recipient: ContractAddress - ) { - self.erc20.initializer(name, symbol); - self.erc20.mint(recipient, fixed_supply); - } -} From 0116f0385405340a5be7a653d80fc38dce29cc5b Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Fri, 4 Oct 2024 19:44:09 -0400 Subject: [PATCH 11/15] fmt --- packages/governance/src/votes.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/governance/src/votes.cairo b/packages/governance/src/votes.cairo index b66668c46..93d7040ab 100644 --- a/packages/governance/src/votes.cairo +++ b/packages/governance/src/votes.cairo @@ -2,4 +2,4 @@ pub mod interface; pub mod utils; pub mod votes; -pub use votes::VotesComponent; \ No newline at end of file +pub use votes::VotesComponent; From 175af4e4034490ecdc3198895ff839a8335c68f0 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Sat, 5 Oct 2024 19:59:55 -0400 Subject: [PATCH 12/15] add more tests --- packages/governance/src/tests/mocks.cairo | 1 + .../src/tests/mocks/account_mocks.cairo | 145 ++++++++++++++++++ .../governance/src/tests/test_votes.cairo | 104 ++++++++----- 3 files changed, 209 insertions(+), 41 deletions(-) create mode 100644 packages/governance/src/tests/mocks/account_mocks.cairo diff --git a/packages/governance/src/tests/mocks.cairo b/packages/governance/src/tests/mocks.cairo index 72e9e6b05..46c829072 100644 --- a/packages/governance/src/tests/mocks.cairo +++ b/packages/governance/src/tests/mocks.cairo @@ -1,3 +1,4 @@ pub(crate) mod non_implementing_mock; pub(crate) mod timelock_mocks; pub(crate) mod votes_mocks; +pub(crate) mod account_mocks; diff --git a/packages/governance/src/tests/mocks/account_mocks.cairo b/packages/governance/src/tests/mocks/account_mocks.cairo new file mode 100644 index 000000000..f8f222dad --- /dev/null +++ b/packages/governance/src/tests/mocks/account_mocks.cairo @@ -0,0 +1,145 @@ +#[starknet::contract(account)] +pub(crate) mod DualCaseAccountMock { + use openzeppelin_account::AccountComponent; + use openzeppelin_introspection::src5::SRC5Component; + + component!(path: AccountComponent, storage: account, event: AccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6Impl = AccountComponent::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = AccountComponent::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl DeclarerImpl = AccountComponent::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = AccountComponent::DeployableImpl; + impl AccountInternalImpl = AccountComponent::InternalImpl; + + // SCR5 + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub account: AccountComponent::Storage, + #[substorage(v0)] + pub src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + AccountEvent: AccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} + +#[starknet::contract(account)] +pub(crate) mod SnakeAccountMock { + use openzeppelin_account::AccountComponent; + use openzeppelin_introspection::src5::SRC5Component; + + component!(path: AccountComponent, storage: account, event: AccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6Impl = AccountComponent::SRC6Impl; + #[abi(embed_v0)] + impl PublicKeyImpl = AccountComponent::PublicKeyImpl; + impl AccountInternalImpl = AccountComponent::InternalImpl; + + // SCR5 + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub account: AccountComponent::Storage, + #[substorage(v0)] + pub src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + AccountEvent: AccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} + +#[starknet::contract(account)] +pub(crate) mod CamelAccountMock { + use openzeppelin_account::AccountComponent; + use openzeppelin_introspection::src5::SRC5Component; + use starknet::account::Call; + + component!(path: AccountComponent, storage: account, event: AccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = AccountComponent::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = AccountComponent::PublicKeyCamelImpl; + impl SRC6Impl = AccountComponent::SRC6Impl; + impl AccountInternalImpl = AccountComponent::InternalImpl; + + // SCR5 + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub account: AccountComponent::Storage, + #[substorage(v0)] + pub src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + AccountEvent: AccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, publicKey: felt252) { + self.account.initializer(publicKey); + } + + #[abi(per_item)] + #[generate_trait] + impl ExternalImpl of ExternalTrait { + #[external(v0)] + fn __execute__(self: @ContractState, mut calls: Array) -> Array> { + self.account.__execute__(calls) + } + + #[external(v0)] + fn __validate__(self: @ContractState, mut calls: Array) -> felt252 { + self.account.__validate__(calls) + } + } +} diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo index b49eee489..5a9fc8c8a 100644 --- a/packages/governance/src/tests/test_votes.cairo +++ b/packages/governance/src/tests/test_votes.cairo @@ -5,6 +5,7 @@ use openzeppelin_governance::votes::votes::VotesComponent::{ DelegateChanged, DelegateVotesChanged, VotesImpl, InternalImpl, }; use openzeppelin_governance::votes::votes::VotesComponent; +use openzeppelin_governance::votes::utils::Delegation; use openzeppelin_testing as utils; use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT, OTHER}; use openzeppelin_testing::events::EventSpyExt; @@ -14,6 +15,7 @@ use openzeppelin_token::erc721::ERC721Component::{ }; use openzeppelin_token::erc721::ERC721Component::{ERC721Impl, ERC721CamelOnlyImpl}; use openzeppelin_utils::structs::checkpoint::TraceTrait; +use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; use snforge_std::signature::stark_curve::{StarkCurveKeyPairImpl, StarkCurveSignerImpl}; use snforge_std::{ start_cheat_block_timestamp_global, start_cheat_caller_address, spy_events, test_address @@ -22,6 +24,7 @@ use snforge_std::{EventSpy}; use starknet::ContractAddress; use starknet::storage::{StoragePointerReadAccess, StorageMapReadAccess}; +const ERC_721_INITIAL_MINT: u256 = 10; // // Setup @@ -49,9 +52,9 @@ fn ERC20VOTES_CONTRACT_STATE() -> ERC20VotesMock::ContractState { fn setup_erc721_votes() -> ComponentState { let mut state = COMPONENT_STATE(); let mut mock_state = ERC721VOTES_CONTRACT_STATE(); - // Mint 10 NFTs to OWNER + // Mint ERC_721_INITIAL_MINT NFTs to OWNER let mut i: u256 = 0; - while i < 10 { + while i < ERC_721_INITIAL_MINT { mock_state.erc721.mint(OWNER(), i); // We manually transfer voting units here, since this is usually implemented in the hook state.transfer_voting_units(ZERO(), OWNER(), 1); @@ -74,7 +77,7 @@ fn setup_erc20_votes() -> ERC20ComponentState { fn setup_account(public_key: felt252) -> ContractAddress { let mut calldata = array![public_key]; - utils::declare_and_deploy("DualCaseAccountMock", calldata) + utils::declare_and_deploy("SnakeAccountMock", calldata) } // @@ -89,7 +92,7 @@ fn test_get_votes() { assert_eq!(state.get_votes(OWNER()), 0); state.delegate(OWNER()); - assert_eq!(state.get_votes(OWNER()), 10); + assert_eq!(state.get_votes(OWNER()), ERC_721_INITIAL_MINT); } // This test can be improved by using the api of the component @@ -114,6 +117,7 @@ fn test_get_past_votes() { #[should_panic(expected: ('Votes: future Lookup',))] fn test_get_past_votes_future_lookup() { let state = setup_erc721_votes(); + start_cheat_block_timestamp_global('ts1'); state.get_past_votes(OWNER(), 'ts2'); } @@ -149,8 +153,8 @@ fn test_self_delegate() { state.delegate(OWNER()); spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), OWNER()); - spy.assert_only_event_delegate_votes_changed(contract_address, OWNER(), 0, 10); - assert_eq!(state.get_votes(OWNER()), 10); + spy.assert_only_event_delegate_votes_changed(contract_address, OWNER(), 0, ERC_721_INITIAL_MINT); + assert_eq!(state.get_votes(OWNER()), ERC_721_INITIAL_MINT); } #[test] @@ -162,8 +166,8 @@ fn test_delegate_to_recipient_updates_votes() { state.delegate(RECIPIENT()); spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), RECIPIENT()); - spy.assert_only_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, 10); - assert_eq!(state.get_votes(RECIPIENT()), 10); + spy.assert_only_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, ERC_721_INITIAL_MINT); + assert_eq!(state.get_votes(RECIPIENT()), ERC_721_INITIAL_MINT); assert_eq!(state.get_votes(OWNER()), 0); } @@ -177,26 +181,36 @@ fn test_delegate_to_recipient_updates_delegates() { assert_eq!(state.delegates(OWNER()), RECIPIENT()); } -// #[test] -// fn test_delegate_by_sig() { -// cheat_chain_id_global('SN_TEST'); -// start_cheat_block_timestamp_global('ts1'); -// let mut state = setup_erc721_votes(); -// let contract_address = test_address(); -// let key_pair = KeyPairTrait::generate(); -// let account = setup_account(key_pair.public_key); -// let nonce = 0; -// let expiry = 'ts2'; -// let delegator = account; -// let delegatee = RECIPIENT(); -// let delegation = Delegation { delegatee, nonce, expiry }; -// let msg_hash = delegation.get_message_hash(delegator); -// let (r, s) = key_pair.sign(msg_hash).unwrap(); -// let mut spy = spy_events(); -// state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s]); -// spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); -// assert_eq!(state.delegates(account), delegatee); -// } +#[test] +fn test_delegate_by_sig() { + // Set up the state + // start_cheat_chain_id_global('SN_TEST'); + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + start_cheat_block_timestamp_global('ts1'); + + // Generate a key pair and set up an account + let key_pair = StarkCurveKeyPairImpl::generate(); + let account = setup_account(key_pair.public_key); + + // Set up delegation parameters + let nonce = 0; + let expiry = 'ts2'; + let delegator = account; + let delegatee = RECIPIENT(); + + // Create and sign the delegation message + let delegation = Delegation { delegatee, nonce, expiry }; + let msg_hash = delegation.get_message_hash(delegator); + let (r, s) = key_pair.sign(msg_hash).unwrap(); + + // Set up event spy and execute delegation + let mut spy = spy_events(); + state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s]); + + spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); + assert_eq!(state.delegates(account), delegatee); +} #[test] #[should_panic(expected: ('Votes: expired signature',))] @@ -219,16 +233,25 @@ fn test_delegate_by_sig_invalid_nonce() { state.delegate_by_sig(OWNER(), RECIPIENT(), 1, 0, signature); } -// #[test] -// #[should_panic(expected: ('Votes: invalid signature',))] -// fn test_delegate_by_sig_invalid_signature() { -// let mut state = setup_erc721_votes(); -// // For some reason this is panicking before we get to delegate_by_sig -// let account = setup_account(0x123); -// let signature = array![0, 0]; - -// state.delegate_by_sig(account, RECIPIENT(), 0, 0, signature); -// } +#[test] +#[should_panic(expected: ('Votes: invalid signature',))] +fn test_delegate_by_sig_invalid_signature() { + let mut state = setup_erc721_votes(); + let key_pair = StarkCurveKeyPairImpl::generate(); + let account = setup_account(key_pair.public_key); + + let nonce = 0; + let expiry = 'ts2'; + let delegator = account; + let delegatee = RECIPIENT(); + let delegation = Delegation { delegatee, nonce, expiry }; + let msg_hash = delegation.get_message_hash(delegator); + let (r, s) = key_pair.sign(msg_hash).unwrap(); + + start_cheat_block_timestamp_global('ts1'); + // Use an invalid signature + state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r + 1, s]); +} // // Tests specific to ERC721Votes and @@ -238,7 +261,7 @@ fn test_delegate_by_sig_invalid_nonce() { fn test_erc721_get_voting_units() { let state = setup_erc721_votes(); - assert_eq!(state.get_voting_units(OWNER()), 10); + assert_eq!(state.get_voting_units(OWNER()), ERC_721_INITIAL_MINT); assert_eq!(state.get_voting_units(OTHER()), 0); } @@ -303,5 +326,4 @@ impl VotesSpyHelpersImpl of VotesSpyHelpers { self.assert_event_delegate_votes_changed(contract, delegate, previous_votes, new_votes); self.assert_no_events_left_from(contract); } -} - +} \ No newline at end of file From ce72985e6990739f13b05659d39908749a050a8c Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Sat, 5 Oct 2024 20:00:35 -0400 Subject: [PATCH 13/15] fmt --- packages/governance/src/tests/mocks.cairo | 2 +- .../governance/src/tests/test_votes.cairo | 30 +++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/governance/src/tests/mocks.cairo b/packages/governance/src/tests/mocks.cairo index 46c829072..6bc6d5875 100644 --- a/packages/governance/src/tests/mocks.cairo +++ b/packages/governance/src/tests/mocks.cairo @@ -1,4 +1,4 @@ +pub(crate) mod account_mocks; pub(crate) mod non_implementing_mock; pub(crate) mod timelock_mocks; pub(crate) mod votes_mocks; -pub(crate) mod account_mocks; diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo index 5a9fc8c8a..97c3795e3 100644 --- a/packages/governance/src/tests/test_votes.cairo +++ b/packages/governance/src/tests/test_votes.cairo @@ -1,11 +1,11 @@ use openzeppelin_governance::tests::mocks::votes_mocks::ERC721VotesMock::SNIP12MetadataImpl; use openzeppelin_governance::tests::mocks::votes_mocks::{ERC721VotesMock, ERC20VotesMock}; +use openzeppelin_governance::votes::utils::Delegation; use openzeppelin_governance::votes::votes::TokenVotesTrait; use openzeppelin_governance::votes::votes::VotesComponent::{ DelegateChanged, DelegateVotesChanged, VotesImpl, InternalImpl, }; use openzeppelin_governance::votes::votes::VotesComponent; -use openzeppelin_governance::votes::utils::Delegation; use openzeppelin_testing as utils; use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT, OTHER}; use openzeppelin_testing::events::EventSpyExt; @@ -14,8 +14,8 @@ use openzeppelin_token::erc721::ERC721Component::{ ERC721MetadataImpl, InternalImpl as ERC721InternalImpl, }; use openzeppelin_token::erc721::ERC721Component::{ERC721Impl, ERC721CamelOnlyImpl}; -use openzeppelin_utils::structs::checkpoint::TraceTrait; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; +use openzeppelin_utils::structs::checkpoint::TraceTrait; use snforge_std::signature::stark_curve::{StarkCurveKeyPairImpl, StarkCurveSignerImpl}; use snforge_std::{ start_cheat_block_timestamp_global, start_cheat_caller_address, spy_events, test_address @@ -117,7 +117,7 @@ fn test_get_past_votes() { #[should_panic(expected: ('Votes: future Lookup',))] fn test_get_past_votes_future_lookup() { let state = setup_erc721_votes(); - + start_cheat_block_timestamp_global('ts1'); state.get_past_votes(OWNER(), 'ts2'); } @@ -153,7 +153,10 @@ fn test_self_delegate() { state.delegate(OWNER()); spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), OWNER()); - spy.assert_only_event_delegate_votes_changed(contract_address, OWNER(), 0, ERC_721_INITIAL_MINT); + spy + .assert_only_event_delegate_votes_changed( + contract_address, OWNER(), 0, ERC_721_INITIAL_MINT + ); assert_eq!(state.get_votes(OWNER()), ERC_721_INITIAL_MINT); } @@ -166,7 +169,10 @@ fn test_delegate_to_recipient_updates_votes() { state.delegate(RECIPIENT()); spy.assert_event_delegate_changed(contract_address, OWNER(), ZERO(), RECIPIENT()); - spy.assert_only_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, ERC_721_INITIAL_MINT); + spy + .assert_only_event_delegate_votes_changed( + contract_address, RECIPIENT(), 0, ERC_721_INITIAL_MINT + ); assert_eq!(state.get_votes(RECIPIENT()), ERC_721_INITIAL_MINT); assert_eq!(state.get_votes(OWNER()), 0); } @@ -188,26 +194,26 @@ fn test_delegate_by_sig() { let mut state = setup_erc721_votes(); let contract_address = test_address(); start_cheat_block_timestamp_global('ts1'); - + // Generate a key pair and set up an account let key_pair = StarkCurveKeyPairImpl::generate(); let account = setup_account(key_pair.public_key); - + // Set up delegation parameters let nonce = 0; let expiry = 'ts2'; let delegator = account; let delegatee = RECIPIENT(); - + // Create and sign the delegation message let delegation = Delegation { delegatee, nonce, expiry }; let msg_hash = delegation.get_message_hash(delegator); let (r, s) = key_pair.sign(msg_hash).unwrap(); - + // Set up event spy and execute delegation let mut spy = spy_events(); state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s]); - + spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); assert_eq!(state.delegates(account), delegatee); } @@ -247,7 +253,7 @@ fn test_delegate_by_sig_invalid_signature() { let delegation = Delegation { delegatee, nonce, expiry }; let msg_hash = delegation.get_message_hash(delegator); let (r, s) = key_pair.sign(msg_hash).unwrap(); - + start_cheat_block_timestamp_global('ts1'); // Use an invalid signature state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r + 1, s]); @@ -326,4 +332,4 @@ impl VotesSpyHelpersImpl of VotesSpyHelpers { self.assert_event_delegate_votes_changed(contract, delegate, previous_votes, new_votes); self.assert_no_events_left_from(contract); } -} \ No newline at end of file +} From 4ca0ae485582c881637adc6f8f8746e7e0b77907 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Sat, 5 Oct 2024 20:10:13 -0400 Subject: [PATCH 14/15] add changelog entry --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0312236cf..fc57425e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- `VotesComponent` with implementation for ERC721 and ERC20 tokens (#1114) + +### Changed (Breaking) + +- Remove `ERC20Votes` component in favor of `VotesComponent` (#1114) + ### Changed - Bump scarb to v2.8.3 (#1166) From f866b4c2966da16d89aba8f08d07b994d7679376 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Mon, 7 Oct 2024 18:59:54 -0400 Subject: [PATCH 15/15] add burn tests --- .../governance/src/tests/test_votes.cairo | 55 +++++++++++++++++-- packages/governance/src/votes/votes.cairo | 5 +- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo index 97c3795e3..bfc7e74ee 100644 --- a/packages/governance/src/tests/test_votes.cairo +++ b/packages/governance/src/tests/test_votes.cairo @@ -95,9 +95,6 @@ fn test_get_votes() { assert_eq!(state.get_votes(OWNER()), ERC_721_INITIAL_MINT); } -// This test can be improved by using the api of the component -// to add checkpoints and thus verify the internal state of the component -// instead of using the trace directly. #[test] fn test_get_past_votes() { let mut state = setup_erc721_votes(); @@ -260,7 +257,7 @@ fn test_delegate_by_sig_invalid_signature() { } // -// Tests specific to ERC721Votes and +// Tests specific to ERC721Votes and ERC20Votes // #[test] @@ -279,6 +276,55 @@ fn test_erc20_get_voting_units() { assert_eq!(state.get_voting_units(OTHER()), 0); } +#[test] +fn test_erc20_burn_updates_votes() { + let mut state = setup_erc20_votes(); + let mut mock_state = ERC20VOTES_CONTRACT_STATE(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, OWNER()); + start_cheat_block_timestamp_global('ts1'); + + state.delegate(OWNER()); + + // Burnsome tokens + let burn_amount = 1000; + mock_state.erc20.burn(OWNER(), burn_amount); + + // Manually update voting units (this would typically be done in a hook) + state.transfer_voting_units(OWNER(), ZERO(), burn_amount); + + // We need to move the timestamp forward to be able to call get_past_total_supply + start_cheat_block_timestamp_global('ts2'); + assert_eq!(state.get_votes(OWNER()), SUPPLY - burn_amount); + assert_eq!(state.get_past_total_supply('ts1'), SUPPLY - burn_amount); +} + +#[test] +fn test_erc721_burn_updates_votes() { + let mut state = setup_erc721_votes(); + let mut mock_state = ERC721VOTES_CONTRACT_STATE(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, OWNER()); + start_cheat_block_timestamp_global('ts1'); + + state.delegate(OWNER()); + + // Burn some tokens + let burn_amount = 3; + let mut i: u256 = 0; + while i < burn_amount { + mock_state.erc721.burn(i); + // Manually update voting units (this would typically be done in a hook) + state.transfer_voting_units(OWNER(), ZERO(), 1); + i += 1; + }; + + // We need to move the timestamp forward to be able to call get_past_total_supply + start_cheat_block_timestamp_global('ts2'); + assert_eq!(state.get_votes(OWNER()), ERC_721_INITIAL_MINT - burn_amount); + assert_eq!(state.get_past_total_supply('ts1'), ERC_721_INITIAL_MINT - burn_amount); +} + // // Helpers // @@ -333,3 +379,4 @@ impl VotesSpyHelpersImpl of VotesSpyHelpers { self.assert_no_events_left_from(contract); } } + diff --git a/packages/governance/src/votes/votes.cairo b/packages/governance/src/votes/votes.cairo index 0f98eda41..4accb677b 100644 --- a/packages/governance/src/votes/votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -285,13 +285,12 @@ pub mod VotesComponent { to: ContractAddress, amount: u256 ) { - let zero_address = Zero::zero(); let block_timestamp = starknet::get_block_timestamp(); - if (from == zero_address) { + if from.is_zero() { let mut trace = self.Votes_total_checkpoints.read(); trace.push(block_timestamp, trace.latest() + amount); } - if (to == zero_address) { + if to.is_zero() { let mut trace = self.Votes_total_checkpoints.read(); trace.push(block_timestamp, trace.latest() - amount); }