From 6e60ba9310fa7953f045d0c30b343b0ffc168c14 Mon Sep 17 00:00:00 2001 From: Gustavo Gonzalez Date: Tue, 15 Oct 2024 19:26:45 -0400 Subject: [PATCH] Erc721 votes and general Votes component (#1114) * votes first iteration * consolidate votes and delegation to governace module * fmt * add ERC20Votes impl, code docs and remove interface for internal iml * add basic tests and mocks * refactor, remove unused usings * cleanup mocks, remove public impl * refactor * export VotesComponent for easier use * remove erc20votes from token module * fmt * add more tests * fmt * add changelog entry * add burn tests * use hooks in mocks * use crate for local imports * refactor account names in tests * improve docs and remove ERC20Votes * + * use vec in trace and remove storage array * add tests for trace & checkpoint * refactor tests * fixes * fmt * improve docs * typos * Update CHANGELOG.md Co-authored-by: Eric Nordelo * + * + * ++ * add more tests and refactor * add missing internal functions and tests * add VotesABI and format * docs: trait implementations * remove debug and partialeq from checkpoint * refactor hook and improve docs * + * + * improve tests * fix test * fmt * Update CHANGELOG.md Co-authored-by: Eric Nordelo * move _delegate --------- Co-authored-by: Eric Nordelo --- CHANGELOG.md | 5 + Scarb.lock | 4 +- docs/modules/ROOT/pages/api/erc20.adoc | 196 +---- docs/modules/ROOT/pages/api/finance.adoc | 2 +- docs/modules/ROOT/pages/api/governance.adoc | 280 +++++++- docs/modules/ROOT/pages/governance.adoc | 228 ++++++ packages/governance/README.md | 1 + packages/governance/Scarb.toml | 5 + packages/governance/src/lib.cairo | 2 +- packages/governance/src/tests.cairo | 1 + .../governance/src/tests/test_votes.cairo | 679 ++++++++++++++++++ packages/governance/src/utils.cairo | 1 - .../governance/src/utils/interfaces.cairo | 3 - packages/governance/src/votes.cairo | 5 + .../src/votes/delegation.cairo} | 7 +- .../votes.cairo => votes/interface.cairo} | 27 +- .../src/votes/votes.cairo} | 239 +++--- packages/test_common/Scarb.toml | 1 + packages/test_common/src/mocks.cairo | 3 + .../test_common/src/mocks/checkpoint.cairo | 36 + packages/test_common/src/mocks/erc20.cairo | 95 --- packages/test_common/src/mocks/votes.cairo | 162 +++++ packages/testing/src/constants.cairo | 8 + packages/token/README.md | 1 - packages/token/Scarb.toml | 1 - packages/token/src/erc20.cairo | 1 - packages/token/src/erc20/extensions.cairo | 3 - packages/token/src/erc20/interface.cairo | 40 -- packages/token/src/erc20/snip12_utils.cairo | 1 - packages/token/src/tests/erc20.cairo | 1 - .../src/tests/erc20/test_erc20_votes.cairo | 404 ----------- packages/utils/src/structs.cairo | 1 - packages/utils/src/structs/checkpoint.cairo | 108 +-- .../utils/src/structs/storage_array.cairo | 107 --- packages/utils/src/tests.cairo | 1 + packages/utils/src/tests/mocks.cairo | 1 - .../utils/src/tests/test_checkpoint.cairo | 107 +++ 37 files changed, 1738 insertions(+), 1029 deletions(-) create mode 100644 packages/governance/src/tests/test_votes.cairo delete mode 100644 packages/governance/src/utils.cairo delete mode 100644 packages/governance/src/utils/interfaces.cairo create mode 100644 packages/governance/src/votes.cairo rename packages/{token/src/erc20/snip12_utils/votes.cairo => governance/src/votes/delegation.cairo} (72%) rename packages/governance/src/{utils/interfaces/votes.cairo => votes/interface.cairo} (59%) rename packages/{token/src/erc20/extensions/erc20_votes.cairo => governance/src/votes/votes.cairo} (53%) create mode 100644 packages/test_common/src/mocks/checkpoint.cairo create mode 100644 packages/test_common/src/mocks/votes.cairo delete mode 100644 packages/token/src/erc20/extensions.cairo delete mode 100644 packages/token/src/tests/erc20/test_erc20_votes.cairo delete mode 100644 packages/utils/src/structs/storage_array.cairo delete mode 100644 packages/utils/src/tests/mocks.cairo create mode 100644 packages/utils/src/tests/test_checkpoint.cairo diff --git a/CHANGELOG.md b/CHANGELOG.md index 23d77e734..d3e4bdc60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `VotesComponent` with implementation for ERC721 and ERC20 tokens (#1114) - `IUpgradeAndCall` interface (#1148) - `upgrade_and_call` function in UpgradeableComponent's InternalImpl (#1148) - `ERC20Permit` impl for `ERC20Component` facilitating token approvals via off-chain signatures (#1140) @@ -23,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed (Breaking) +- Remove `ERC20Votes` component in favor of `VotesComponent` (#1114) + - `Trace` is now declared as a `storage_node` and now uses `Vec` instead of `StorageArray`. + - `delegate_by_sig` `signature` param in the `IVotes` interface updated from `Array` to `Span`. +- Remove `StorageArray` from `openzeppelin_utils` (#1114) - Bump snforge to 0.31.0 - Remove openzeppelin_utils::selectors (#1163) - Remove `DualCase dispatchers` (#1163) diff --git a/Scarb.lock b/Scarb.lock index fabb1d34d..d1a995644 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -59,9 +59,11 @@ name = "openzeppelin_governance" version = "0.17.0" dependencies = [ "openzeppelin_access", + "openzeppelin_account", "openzeppelin_introspection", "openzeppelin_test_common", "openzeppelin_testing", + "openzeppelin_token", "openzeppelin_utils", "snforge_std", ] @@ -113,6 +115,7 @@ dependencies = [ "openzeppelin_access", "openzeppelin_account", "openzeppelin_finance", + "openzeppelin_governance", "openzeppelin_introspection", "openzeppelin_security", "openzeppelin_testing", @@ -134,7 +137,6 @@ name = "openzeppelin_token" version = "0.17.0" dependencies = [ "openzeppelin_account", - "openzeppelin_governance", "openzeppelin_introspection", "openzeppelin_test_common", "openzeppelin_testing", diff --git a/docs/modules/ROOT/pages/api/erc20.adoc b/docs/modules/ROOT/pages/api/erc20.adoc index ba4328868..daefb5e90 100644 --- a/docs/modules/ROOT/pages/api/erc20.adoc +++ b/docs/modules/ROOT/pages/api/erc20.adoc @@ -456,200 +456,6 @@ See <>. See <>. -== Extensions - -[.contract] -[[ERC20VotesComponent]] -=== `++ERC20VotesComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.17.0/packages/token/src/erc20/extensions/erc20_votes.cairo[{github-icon},role=heading-link] - -```cairo -use openzeppelin_token::extensions::ERC20VotesComponent; -``` - -:DelegateChanged: xref:ERC20VotesComponent-DelegateChanged[DelegateChanged] -:DelegateVotesChanged: xref:ERC20VotesComponent-DelegateVotesChanged[DelegateVotesChanged] - -Extension of ERC20 to support voting and delegation. - -NOTE: Implementing xref:#ERC20Component[ERC20Component] is a requirement for this component to be implemented. - -WARNING: To track voting units, this extension requires that the -xref:#ERC20VotesComponent-transfer_voting_units[transfer_voting_units] function is called after every transfer, -mint, or burn operation. For this, the xref:ERC20Component-ERC20HooksTrait[ERC20HooksTrait] must be used. - -This extension keeps a history (checkpoints) of each account’s vote power. Vote power can be delegated either by calling -the xref:#ERC20VotesComponent-delegate[delegate] function directly, or by providing a signature to be used with -xref:#ERC20VotesComponent-delegate_by_sig[delegate_by_sig]. Voting power can be queried through the public accessors -xref:#ERC20VotesComponent-get_votes[get_votes] and xref:#ERC20VotesComponent-get_past_votes[get_past_votes]. - -By default, token balance does not account for voting power. This makes transfers cheaper. The downside is that it requires users to delegate to themselves in order to activate checkpoints and have their voting power tracked. - -[.contract-index#ERC20VotesComponent-Embeddable-Impls] -.Embeddable Implementations --- -[.sub-index#ERC20VotesComponent-Embeddable-Impls-ERC20VotesImpl] -.ERC20VotesImpl -* xref:#ERC20VotesComponent-get_votes[`++get_votes(self, account)++`] -* xref:#ERC20VotesComponent-get_past_votes[`++get_past_votes(self, account, timepoint)++`] -* xref:#ERC20VotesComponent-get_past_total_supply[`++get_past_total_supply(self, timepoint)++`] -* xref:#ERC20VotesComponent-delegates[`++delegates(self, account)++`] -* xref:#ERC20VotesComponent-delegate[`++delegate(self, delegatee)++`] -* xref:#ERC20VotesComponent-delegate_by_sig[`++delegate_by_sig(self, delegator, delegatee, nonce, expiry, signature)++`] --- - -[.contract-index] -.Internal implementations --- -.InternalImpl -* xref:#ERC20VotesComponent-get_total_supply[`++get_total_supply(self)++`] -* xref:#ERC20VotesComponent-_delegate[`++_delegate(self, account, delegatee)++`] -* xref:#ERC20VotesComponent-move_delegate_votes[`++move_delegate_votes(self, from, to, amount)++`] -* xref:#ERC20VotesComponent-transfer_voting_units[`++transfer_voting_units(self, from, to, amount)++`] -* xref:#ERC20VotesComponent-num_checkpoints[`++num_checkpoints(self, account)++`] -* xref:#ERC20VotesComponent-checkpoints[`++checkpoints(self, account, pos)++`] -* xref:#ERC20VotesComponent-get_voting_units[`++get_voting_units(self, account)++`] --- - -[.contract-index] -.Events --- -* xref:#ERC20VotesComponent-DelegateChanged[`++DelegateChanged(delegator, from_delegate, to_delegate)++`] -* xref:#ERC20VotesComponent-DelegateVotesChanged[`++DelegateVotesChanged(delegate, previous_votes, new_votes)++`] --- - -[#ERC20VotesComponent-Embeddable-functions] -==== Embeddable functions - -[.contract-item] -[[ERC20VotesComponent-get_votes]] -==== `[.contract-item-name]#++get_votes++#++(self: @ContractState, account: ContractAddress) → u256++` [.item-kind]#external# - -Returns the current amount of votes that `account` has. - -[.contract-item] -[[ERC20VotesComponent-get_past_votes]] -==== `[.contract-item-name]#++get_past_votes++#++(self: @ContractState, account: ContractAddress, timepoint: u64) → u256++` [.item-kind]#external# - -Returns the amount of votes that `account` had at a specific moment in the past. - -Requirements: - -- `timepoint` must be in the past. - -[.contract-item] -[[ERC20VotesComponent-get_past_total_supply]] -==== `[.contract-item-name]#++get_past_total_supply++#++(self: @ContractState, timepoint: u64) → u256++` [.item-kind]#external# - -Returns the total supply of votes available at a specific moment 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. - -[.contract-item] -[[ERC20VotesComponent-delegates]] -==== `[.contract-item-name]#++delegates++#++(self: @ContractState, account: ContractAddress) → ContractAddress++` [.item-kind]#external# - -Returns the delegate that `account` has chosen. - -[.contract-item] -[[ERC20VotesComponent-delegate]] -==== `[.contract-item-name]#++delegate++#++(ref self: ContractState, delegatee: ContractAddress)++` [.item-kind]#external# - -Delegates votes from the caller to `delegatee`. - -Emits a {DelegateChanged} event. - -May emit one or two {DelegateVotesChanged} events. - -[.contract-item] -[[ERC20VotesComponent-delegate_by_sig]] -==== `[.contract-item-name]#++delegate_by_sig++#++(ref self: ContractState, delegator: ContractAddress, delegatee: ContractAddress, nonce: felt252, expiry: u64, signature: Array)++` [.item-kind]#external# - -Delegates votes from `delegator` 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. - -[#ERC20VotesComponent-Internal-functions] -==== Internal functions - -[.contract-item] -[[ERC20VotesComponent-get_total_supply]] -==== `[.contract-item-name]#++get_total_supply++#++(self: @ContractState) → u256++` [.item-kind]#internal# - -Returns the current total supply of votes. - -[.contract-item] -[[ERC20VotesComponent-_delegate]] -==== `[.contract-item-name]#++_delegate++#++(ref self: ContractState, account: ContractAddress, delegatee: ContractAddress)++` [.item-kind]#internal# - -Delegates all of ``account``'s voting units to `delegatee`. - -Emits a {DelegateChanged} event. - -May emit one or two {DelegateVotesChanged} events. - -[.contract-item] -[[ERC20VotesComponent-move_delegate_votes]] -==== `[.contract-item-name]#++move_delegate_votes++#++(ref self: ContractState, from: ContractAddress, to: ContractAddress, amount: u256)++` [.item-kind]#internal# - -Moves `amount` of delegated votes from `from` to `to`. - -May emit one or two {DelegateVotesChanged} events. - -[.contract-item] -[[ERC20VotesComponent-transfer_voting_units]] -==== `[.contract-item-name]#++transfer_voting_units++#++(ref self: ContractState, from: ContractAddress, to: ContractAddress, amount: u256)++` [.item-kind]#internal# - -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. - -[.contract-item] -[[ERC20VotesComponent-num_checkpoints]] -==== `[.contract-item-name]#++num_checkpoints++#++(self: @ContractState, account: ContractAddress) → u32++` [.item-kind]#internal# - -Returns the number of checkpoints for `account`. - -[.contract-item] -[[ERC20VotesComponent-checkpoints]] -==== `[.contract-item-name]#++checkpoints++#++(self: @ContractState, account: ContractAddress, pos: u32) → Checkpoint++` [.item-kind]#internal# - -Returns the `pos`-th checkpoint for `account`. - -[.contract-item] -[[ERC20VotesComponent-get_voting_units]] -==== `[.contract-item-name]#++get_voting_units++#++(self: @ContractState, account: ContractAddress) → u256++` [.item-kind]#internal# - -Returns the voting units of an `account`. - -[#ERC20VotesComponent-Events] -==== Events - -[.contract-item] -[[ERC20VotesComponent-DelegateChanged]] -==== `[.contract-item-name]#++DelegateChanged++#++(delegator: ContractAddress, from_delegate: ContractAddress, to_delegate: ContractAddress)++` [.item-kind]#event# - -Emitted when `delegator` delegates their votes from `from_delegate` to `to_delegate`. - -[.contract-item] -[[ERC20VotesComponent-DelegateVotesChanged]] -==== `[.contract-item-name]#++DelegateVotesChanged++#++(delegate: ContractAddress, previous_votes: u256, new_votes: u256)++` [.item-kind]#event# - -Emitted when `delegate` votes are updated from `previous_votes` to `new_votes`. - == Presets [.contract] @@ -716,4 +522,4 @@ Upgrades the contract to a new implementation given by `new_class_hash`. Requirements: - The caller is the contract owner. -- `new_class_hash` cannot be zero. +- `new_class_hash` cannot be zero. \ No newline at end of file diff --git a/docs/modules/ROOT/pages/api/finance.adoc b/docs/modules/ROOT/pages/api/finance.adoc index 60479e47f..f8c5761ac 100755 --- a/docs/modules/ROOT/pages/api/finance.adoc +++ b/docs/modules/ROOT/pages/api/finance.adoc @@ -114,7 +114,7 @@ use openzeppelin_finance::vesting::VestingComponent; Vesting component implementing the xref:IVesting[`IVesting`] interface. [.contract-index] -.Vesting Schedule Trait +.Vesting Schedule Trait Implementations -- .functions * xref:#VestingComponent-calculate_vested_amount[`++calculate_vested_amount(self, token, total_allocation, diff --git a/docs/modules/ROOT/pages/api/governance.adoc b/docs/modules/ROOT/pages/api/governance.adoc index 032b1b5c4..69415a386 100644 --- a/docs/modules/ROOT/pages/api/governance.adoc +++ b/docs/modules/ROOT/pages/api/governance.adoc @@ -5,6 +5,11 @@ :CallCancelled: xref:ITimelock-CallCancelled[CallCancelled] :MinDelayChanged: xref:ITimelock-MinDelayChanged[MinDelayChanged] :RoleGranted: xref:api/access.adoc#IAccessControl-RoleGranted[IAccessControl::RoleGranted] +:DelegateChanged: xref:VotesComponent-DelegateChanged[DelegateChanged] +:DelegateVotesChanged: xref:VotesComponent-DelegateVotesChanged[DelegateVotesChanged] +:VotingUnitsTrait: xref:VotingUnitsTrait[VotingUnitsTrait] +:VotesComponent: xref:VotesComponent[VotesComponent] +:IVotes: xref:IVotes[IVotes] = Governance @@ -600,19 +605,20 @@ Emitted when operation `id` is cancelled. Emitted when the minimum delay for future operations is modified. -== Utils +== Votes + +The `VotesComponent` provides a flexible system for tracking and delegating voting power. This system allows users to delegate their voting power to other addresses, enabling more active participation in governance. [.contract] [[IVotes]] -=== `++IVotes++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.17.0/packages/governance/src/utils/interfaces/votes.cairo[{github-icon},role=heading-link] +=== `++IVotes++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.17.0/packages/governance/src/votes/interface.cairo[{github-icon},role=heading-link] [.hljs-theme-dark] ```cairo -use openzeppelin_governance::utils::interfaces::IVotes; +use openzeppelin_governance::votes::interface::IVotes; ``` -Common interface for Votes-enabled contracts. For an implementation example see -xref:/api/erc20.adoc#ERC20VotesComponent[ERC20VotesComponent]. +Common interface for Votes-enabled contracts. [.contract-index] .Functions @@ -664,6 +670,266 @@ Delegates votes from the sender to `delegatee`. [.contract-item] [[IVotes-delegate_by_sig]] -==== `[.contract-item-name]#++delegate_by_sig++#++(delegator: ContractAddress, delegatee: ContractAddress, nonce: felt252, expiry: u64, signature: Array)++` [.item-kind]#external# +==== `[.contract-item-name]#++delegate_by_sig++#++(delegator: ContractAddress, delegatee: ContractAddress, nonce: felt252, expiry: u64, signature: Span)++` [.item-kind]#external# + +Delegates votes from `delegator` to `delegatee` through a SNIP12 message signature validation. + +[.contract] +[[VotesComponent]] +=== `++VotesComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.17.0/packages/governance/src/votes/votes.cairo[{github-icon},role=heading-link] + +[.hljs-theme-dark] +```cairo +use openzeppelin_governance::votes::VotesComponent; +``` +Component that implements the {IVotes} interface and provides a flexible system for tracking and delegating voting power. + +By default, token balance does not account for voting power. This makes transfers cheaper. The downside is that it requires users to delegate to themselves in order to activate checkpoints and have their voting power tracked. + +NOTE: When using this module, your contract must implement the {VotingUnitsTrait}. For convenience, this is done automatically for `ERC20` and `ERC721` tokens. + +[.contract-index] +.Voting Units Trait Implementations +-- +.ERC20VotesImpl +* xref:#VotesComponent-ERC20VotesImpl-get_voting_units[`++get_voting_units(self, account)++`] + +.ERC721VotesImpl +* xref:#VotesComponent-ERC721VotesImpl-get_voting_units[`++get_voting_units(self, account)++`] +-- + +[.contract-index#VotesComponent-Embeddable-Impls] +.Embeddable Implementations +-- +[.sub-index#VotesComponent-Embeddable-Impls-VotesImpl] +.VotesImpl +* xref:#VotesComponent-get_votes[`++get_votes(self, account)++`] +* xref:#VotesComponent-get_past_votes[`++get_past_votes(self, account, timepoint)++`] +* xref:#VotesComponent-get_past_total_supply[`++get_past_total_supply(self, timepoint)++`] +* xref:#VotesComponent-delegates[`++delegates(self, account)++`] +* xref:#VotesComponent-delegate[`++delegate(self, delegatee)++`] +* xref:#VotesComponent-delegate_by_sig[`++delegate_by_sig(self, delegator, delegatee, nonce, expiry, signature)++`] +-- + +[.contract-index] +.Internal implementations +-- +.InternalImpl +* xref:#VotesComponent-get_total_supply[`++get_total_supply(self)++`] +* xref:#VotesComponent-move_delegate_votes[`++move_delegate_votes(self, from, to, amount)++`] +* xref:#VotesComponent-transfer_voting_units[`++transfer_voting_units(self, from, to, amount)++`] +* xref:#VotesComponent-num_checkpoints[`++num_checkpoints(self, account)++`] +* xref:#VotesComponent-checkpoints[`++checkpoints(self, account, pos)++`] +* xref:#VotesComponent-_delegate[`++_delegate(self, account, delegatee)++`] +-- + +[.contract-index] +.Events +-- +* xref:#VotesComponent-DelegateChanged[`++DelegateChanged(delegator, from_delegate, to_delegate)++`] +* xref:#VotesComponent-DelegateVotesChanged[`++DelegateVotesChanged(delegate, previous_votes, new_votes)++`] +-- + +[#VotesComponent-ERC20VotesImpl] +==== ERC20VotesImpl + +[.contract-item] +[[VotesComponent-ERC20VotesImpl-get_voting_units]] +==== `[.contract-item-name]#++get_voting_units++#++(self: @ComponentState, account: ContractAddress) → u256++` [.item-kind]#internal# + +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. + +NOTE: This implementation will work out of the box if the ERC20 component +is implemented in the final contract. + +WARNING: This implementation assumes tokens map to voting units 1:1. +Any deviation from this formula when transferring voting units (e.g. by using hooks) +may compromise the internal vote accounting. + +[#VotesComponent-ERC721VotesImpl] +==== ERC721VotesImpl + +[.contract-item] +[[VotesComponent-ERC721VotesImpl-get_voting_units]] +==== `[.contract-item-name]#++get_voting_units++#++(self: @ComponentState, account: ContractAddress) → u256++` [.item-kind]#internal# + +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. + +NOTE: This implementation will work out of the box if the ERC721 component +is implemented in the final contract. + +WARNING: This implementation assumes tokens map to voting units 1:1. +Any deviation from this formula when transferring voting units (e.g. by using hooks) +may compromise the internal vote accounting. + +[#VotesComponent-Functions] +==== Functions + +[.contract-item] +[[VotesComponent-get_votes]] +==== `[.contract-item-name]#++get_votes++#++(self: @ComponentState, account: ContractAddress) → u256++` [.item-kind]#external# + +Returns the current amount of votes that `account` has. + +[.contract-item] +[[VotesComponent-get_past_votes]] +==== `[.contract-item-name]#++get_past_votes++#++(self: @ComponentState, account: ContractAddress, timepoint: u64) → u256++` [.item-kind]#external# + +Returns the amount of votes that `account` had at a specific moment in the past. + +Requirements: + +- `timepoint` must be in the past. + +[.contract-item] +[[VotesComponent-get_past_total_supply]] +==== `[.contract-item-name]#++get_past_total_supply++#++(self: @ComponentState, timepoint: u64) → u256++` [.item-kind]#external# + +Returns the total supply of votes available at a specific moment 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. + +Requirements: + +- `timepoint` must be in the past. + +[.contract-item] +[[VotesComponent-delegates]] +==== `[.contract-item-name]#++delegates++#++(self: @ComponentState, account: ContractAddress) → ContractAddress++` [.item-kind]#external# + +Returns the delegate that `account` has chosen. + +[.contract-item] +[[VotesComponent-delegate]] +==== `[.contract-item-name]#++delegate++#++(ref self: ComponentState, delegatee: ContractAddress)++` [.item-kind]#external# + +Delegates votes from the sender to `delegatee`. + +Emits a {DelegateChanged} event. + +May emit one or two {DelegateVotesChanged} events. + +[.contract-item] +[[VotesComponent-delegate_by_sig]] +==== `[.contract-item-name]#++delegate_by_sig++#++(ref self: ComponentState, delegator: ContractAddress, delegatee: ContractAddress, nonce: felt252, expiry: u64, signature: Span)++` [.item-kind]#external# + +Delegates votes from `delegator` 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. + +[#VotesComponent-Internal-functions] +==== Internal functions + +[.contract-item] +[[VotesComponent-get_total_supply]] +==== `[.contract-item-name]#++get_total_supply++#++(self: @ComponentState) → u256++` [.item-kind]#internal# + +Returns the current total supply of votes. + +[.contract-item] +[[VotesComponent-move_delegate_votes]] +==== `[.contract-item-name]#++move_delegate_votes++#++(ref self: ComponentState, from: ContractAddress, to: ContractAddress, amount: u256)++` [.item-kind]#internal# + +Moves delegated votes from one delegate to another. + +May emit one or two {DelegateVotesChanged} events. + +[.contract-item] +[[VotesComponent-transfer_voting_units]] +==== `[.contract-item-name]#++transfer_voting_units++#++(ref self: ComponentState, from: ContractAddress, to: ContractAddress, amount: u256)++` [.item-kind]#internal# + +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. + +WARNING: If voting units are based on an underlying transferable asset (like a token), you must call this function every time the asset is transferred to keep the internal voting power accounting in sync. For ERC20 and ERC721 tokens, this is typically handled using hooks. + +May emit one or two {DelegateVotesChanged} events. + +[.contract-item] +[[VotesComponent-num_checkpoints]] +==== `[.contract-item-name]#++num_checkpoints++#++(self: @ComponentState, account: ContractAddress) → u64++` [.item-kind]#internal# + +Returns the number of checkpoints for `account`. + +[.contract-item] +[[VotesComponent-checkpoints]] +==== `[.contract-item-name]#++checkpoints++#++(self: @ComponentState, account: ContractAddress, pos: u64) → Checkpoint++` [.item-kind]#internal# + +Returns the `pos`-th checkpoint for `account`. + +[.contract-item] +[[VotesComponent-_delegate]] +==== `[.contract-item-name]#++_delegate++#++(ref self: ComponentState, account: ContractAddress, delegatee: ContractAddress)++` [.item-kind]#internal# + +Delegates all of ``account``'s voting units to `delegatee`. + +Emits a {DelegateChanged} event. + +May emit one or two {DelegateVotesChanged} events. + +[#VotesComponent-Events] +==== Events + +[.contract-item] +[[VotesComponent-DelegateChanged]] +==== `[.contract-item-name]#++DelegateChanged++#++(delegator: ContractAddress, from_delegate: ContractAddress, to_delegate: ContractAddress)++` [.item-kind]#event# + +Emitted when an account changes their delegate. + +[.contract-item] +[[VotesComponent-DelegateVotesChanged]] +==== `[.contract-item-name]#++DelegateVotesChanged++#++(delegate: ContractAddress, previous_votes: u256, new_votes: u256)++` [.item-kind]#event# + +Emitted when a token transfer or delegate change results in changes to a delegate's number of votes. + +[.contract] +[[VotingUnitsTrait]] +=== `++VotingUnitsTrait++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.17.0/packages/governance/src/votes/votes.cairo[{github-icon},role=heading-link] + +```cairo +pub trait VotingUnitsTrait { + fn get_voting_units(self: @TState, account: ContractAddress) -> u256; +} +``` + +A trait that must be implemented when integrating {VotesComponent} into a contract. It offers a mechanism to retrieve the number of voting units for a given account at the current time. + +[.contract-index] +.Functions +-- +* xref:#VotingUnitsTrait-get_voting_units[`++get_voting_units(self, account)++`] +-- + +[#VotingUnitsTrait-Functions] +==== Functions + +[.contract-item] +[[VotingUnitsTrait-get_voting_units]] +==== `[.contract-item-name]#++get_voting_units++#++(self: @TState, account: ContractAddress) → u256++` [.item-kind]#external# + +Returns the number of voting units for a given account. For ERC20, this is typically the token balance. For ERC721, this is typically the number of tokens owned. -Delegates votes from `delegator` to `delegatee`. +WARNING: While any formula can be used as a measure of voting units, the internal vote accounting of the contract may be +compromised if voting units are transferred in any external flow by following a different formula. + +For example, when implementing the hook for ERC20, the number of voting units transferred should match the formula given by the +`get_voting_units` implementation. \ No newline at end of file diff --git a/docs/modules/ROOT/pages/governance.adoc b/docs/modules/ROOT/pages/governance.adoc index bdd1e8ce3..90c23b916 100644 --- a/docs/modules/ROOT/pages/governance.adoc +++ b/docs/modules/ROOT/pages/governance.adoc @@ -1,8 +1,15 @@ = Governance :timelock-component: xref:api/governance.adoc#TimelockControllerComponent[TimelockControllerComponent] +:votes-component: xref:api/governance.adoc#VotesComponent[VotesComponent] :accesscontrol-component: xref:api/access.adoc#AccessControlComponent[AccessControlComponent] :src5-component: xref:api/introspection.adoc#SRC5Component[SRC5Component] +:delegate: xref:api/governance.adoc#VotesComponent-delegate[delegate] +:delegate_by_sig: xref:api/governance.adoc#VotesComponent-delegate_by_sig[delegate_by_sig] +:voting_units_trait: xref:api/governance.adoc#VotingUnitsTrait[VotingUnitsTrait] +:votes-usage: xref:../governance.adoc#usage_2[usage] +:nonces-component: xref:api/utilities.adoc#NoncesComponent[NoncesComponent] +:snip12-metadata: xref:api/utilities.adoc#snip12[SNIP12Metadata] Decentralized protocols are in constant evolution from the moment they are publicly released. Often, the initial team retains control of this evolution in the first stages, but eventually delegates it to a community of stakeholders. @@ -197,3 +204,224 @@ pub trait TimelockABI { fn renounceRole(ref self: TState, role: felt252, account: ContractAddress); } ---- + +== Votes + +The {votes-component} provides a flexible system for tracking and delegating voting power. This system allows users to delegate their voting power to other addresses, enabling more active participation in governance. + +NOTE: By default, token balance does not account for voting power. This makes transfers cheaper. The downside is that it requires users to delegate to themselves in order to activate checkpoints and have their voting power tracked. + +IMPORTANT: The transferring of voting units must be handled by the implementing contract. In the case of `ERC20` and `ERC721` this is usually done via the hooks. You can check the {votes-usage} section for examples of how to implement this. + +=== Key Features + +1. *Delegation*: Users can delegate their voting power to any address, including themselves. Vote power can be delegated either by calling the {delegate} function directly, or by providing a signature to be used with {delegate_by_sig}. +2. *Historical lookups*: The system keeps track of historical snapshots for each account, which allows the voting power of an account to be queried at a specific timestamp. + +This can be used for example to determine the voting power of an account when a proposal was created, rather than using the current balance. + +=== Usage + +When integrating the `VotesComponent`, the {voting_units_trait} must be implemented to get the voting units for a given account as a function of the implementing contract. + +For simplicity, this module already provides two implementations for `ERC20` and `ERC721` tokens, which will work out of the box if the respective components are integrated. + +Additionally, you must implement the {nonces-component} and the {snip12-metadata} trait to enable delegation by signatures. + +Here's an example of how to structure a simple ERC20Votes contract: + + +[source,cairo] +---- +#[starknet::contract] +mod ERC20VotesContract { + use openzeppelin_governance::votes::VotesComponent; + use openzeppelin_token::erc20::ERC20Component; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + use starknet::ContractAddress; + + component!(path: VotesComponent, storage: erc20_votes, event: ERC20VotesEvent); + component!(path: ERC20Component, storage: erc20, event: ERC20Event); + component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); + + // Votes + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl VotesInternalImpl = VotesComponent::InternalImpl; + + // ERC20 + #[abi(embed_v0)] + impl ERC20MixinImpl = ERC20Component::ERC20MixinImpl; + impl ERC20InternalImpl = ERC20Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub erc20_votes: VotesComponent::Storage, + #[substorage(v0)] + pub erc20: ERC20Component::Storage, + #[substorage(v0)] + pub nonces: NoncesComponent::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + ERC20VotesEvent: VotesComponent::Event, + #[flat] + ERC20Event: ERC20Component::Event, + #[flat] + NoncesEvent: NoncesComponent::Event + } + + // Required for hash computation. + pub impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + // We need to call the `transfer_voting_units` function after + // every mint, burn and transfer. + // For this, we use the `after_update` hook of the `ERC20Component::ERC20HooksTrait`. + impl ERC20VotesHooksImpl of ERC20Component::ERC20HooksTrait { + fn after_update( + ref self: ERC20Component::ComponentState, + from: ContractAddress, + recipient: ContractAddress, + amount: u256 + ) { + let mut contract_state = ERC20Component::HasComponent::get_contract_mut(ref self); + contract_state.erc20_votes.transfer_voting_units(from, recipient, amount); + } + } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc20.initializer("MyToken", "MTK"); + } +} +---- + +And here's an example of how to structure a simple ERC721Votes contract: + + +[source,cairo] +---- +#[starknet::contract] +pub mod ERC721VotesContract { + use openzeppelin_governance::votes::VotesComponent; + use openzeppelin_introspection::src5::SRC5Component; + use openzeppelin_token::erc721::ERC721Component; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + use starknet::ContractAddress; + + 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 + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl VotesInternalImpl = VotesComponent::InternalImpl; + + // ERC721 + #[abi(embed_v0)] + impl ERC721MixinImpl = ERC721Component::ERC721MixinImpl; + impl ERC721InternalImpl = ERC721Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub erc721_votes: VotesComponent::Storage, + #[substorage(v0)] + pub erc721: ERC721Component::Storage, + #[substorage(v0)] + pub src5: SRC5Component::Storage, + #[substorage(v0)] + pub 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 impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + // We need to call the `transfer_voting_units` function after + // every mint, burn and transfer. + // For this, we use the `before_update` hook of the + //`ERC721Component::ERC721HooksTrait`. + // This hook is called before the transfer is executed. + // This gives us access to the previous owner. + impl ERC721VotesHooksImpl of ERC721Component::ERC721HooksTrait { + fn before_update( + ref self: ERC721Component::ComponentState, + to: ContractAddress, + token_id: u256, + auth: ContractAddress + ) { + let mut contract_state = ERC721Component::HasComponent::get_contract_mut(ref self); + + // We use the internal function here since it does not check if the token + // id exists which is necessary for mints + let previous_owner = self._owner_of(token_id); + contract_state.erc721_votes.transfer_voting_units(previous_owner, to, 1); + } + } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc721.initializer("MyToken", "MTK", ""); + } +} +---- + +=== Interface + +This is the full interface of the `VotesImpl` implementation: +[source,cairo] +---- +#[starknet::interface] +pub trait VotesABI { + // 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: Span); + + // INonces + fn nonces(self: @TState, owner: ContractAddress) -> felt252; +} +---- diff --git a/packages/governance/README.md b/packages/governance/README.md index 89296ae46..9dc2004ac 100644 --- a/packages/governance/README.md +++ b/packages/governance/README.md @@ -12,3 +12,4 @@ This crate includes primitives for on-chain governance. ### Components - [`TimelockControllerComponent`](https://docs.openzeppelin.com/contracts-cairo/0.17.0/api/governance#TimelockControllerComponent) +- [`VotesComponent`](https://docs.openzeppelin.com/contracts-cairo/0.17.0/api/governance#VotesComponent) diff --git a/packages/governance/Scarb.toml b/packages/governance/Scarb.toml index c35180b44..5ab2fc5d2 100644 --- a/packages/governance/Scarb.toml +++ b/packages/governance/Scarb.toml @@ -25,6 +25,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] assert_macros.workspace = true @@ -43,7 +45,10 @@ casm = false [[test]] name = "openzeppelin_governance_unittest" build-external-contracts = [ + "openzeppelin_test_common::mocks::account::SnakeAccountMock", "openzeppelin_test_common::mocks::timelock::TimelockControllerMock", "openzeppelin_test_common::mocks::timelock::MockContract", "openzeppelin_test_common::mocks::timelock::TimelockAttackerMock", + "openzeppelin_test_common::mocks::votes::ERC721VotesMock", + "openzeppelin_test_common::mocks::votes::ERC20VotesMock" ] diff --git a/packages/governance/src/lib.cairo b/packages/governance/src/lib.cairo index 1320484fb..0b56faddc 100644 --- a/packages/governance/src/lib.cairo +++ b/packages/governance/src/lib.cairo @@ -2,4 +2,4 @@ mod tests; pub mod timelock; -pub mod utils; +pub mod votes; diff --git a/packages/governance/src/tests.cairo b/packages/governance/src/tests.cairo index 3aa8297b3..c71f711c4 100644 --- a/packages/governance/src/tests.cairo +++ b/packages/governance/src/tests.cairo @@ -1,2 +1,3 @@ mod test_timelock; mod test_utils; +mod test_votes; diff --git a/packages/governance/src/tests/test_votes.cairo b/packages/governance/src/tests/test_votes.cairo new file mode 100644 index 000000000..6e413df25 --- /dev/null +++ b/packages/governance/src/tests/test_votes.cairo @@ -0,0 +1,679 @@ +use crate::votes::delegation::Delegation; +use crate::votes::votes::VotesComponent::{ + DelegateChanged, DelegateVotesChanged, VotesImpl, InternalImpl, +}; +use crate::votes::votes::VotesComponent; +use crate::votes::votes::VotingUnitsTrait; +use openzeppelin_test_common::mocks::votes::ERC721VotesMock::SNIP12MetadataImpl; +use openzeppelin_test_common::mocks::votes::{ERC721VotesMock, ERC20VotesMock}; +use openzeppelin_testing as utils; +use openzeppelin_testing::constants::{SUPPLY, ZERO, DELEGATOR, DELEGATEE, OTHER, RECIPIENT}; +use openzeppelin_testing::events::EventSpyExt; +use openzeppelin_token::erc20::ERC20Component::InternalTrait; +use openzeppelin_token::erc20::interface::IERC20; +use openzeppelin_token::erc721::ERC721Component::{ + ERC721MetadataImpl, InternalImpl as ERC721InternalImpl, +}; +use openzeppelin_token::erc721::ERC721Component::{ERC721Impl, ERC721CamelOnlyImpl}; +use openzeppelin_token::erc721::interface::IERC721; +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, + start_cheat_chain_id_global +}; +use snforge_std::{EventSpy}; +use starknet::storage::StoragePathEntry; +use starknet::{ContractAddress, contract_address_const}; + +const ERC721_INITIAL_MINT: u256 = 10; + +// +// 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 ERC721_INITIAL_MINT NFTs to DELEGATOR + for i in 0..ERC721_INITIAL_MINT { + mock_state.erc721.mint(DELEGATOR(), i); + }; + state +} + +fn setup_erc20_votes() -> ERC20ComponentState { + let mut state = ERC20_COMPONENT_STATE(); + let mut mock_state = ERC20VOTES_CONTRACT_STATE(); + + // Mint SUPPLY tokens to DELEGATOR + mock_state.erc20.mint(DELEGATOR(), SUPPLY); + state +} + +fn setup_account(public_key: felt252) -> ContractAddress { + let mut calldata = array![public_key]; + utils::declare_and_deploy("SnakeAccountMock", calldata) +} + +// +// Common tests for Votes +// + +// +// get_votes +// + +#[test] +fn test_get_votes() { + let mut state = setup_erc721_votes(); + start_cheat_caller_address(test_address(), DELEGATOR()); + // Before delegating, the DELEGATOR has 0 votes + assert_eq!(state.get_votes(DELEGATOR()), 0); + state.delegate(DELEGATOR()); + + assert_eq!(state.get_votes(DELEGATOR()), ERC721_INITIAL_MINT); +} + +// +// get_past_votes +// + +#[test] +fn test_get_past_votes() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_delegate_checkpoints.entry(DELEGATOR()); + + start_cheat_block_timestamp_global('ts10'); + + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + assert_eq!(state.get_past_votes(DELEGATOR(), 'ts1'), 3); + assert_eq!(state.get_past_votes(DELEGATOR(), 'ts2'), 5); + assert_eq!(state.get_past_votes(DELEGATOR(), 'ts5'), 7); + // This is because we had not delegated at 'ts0' + assert_eq!(state.get_past_votes(DELEGATOR(), 'ts0'), 0); +} + +#[test] +#[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(DELEGATOR(), 'ts2'); +} + +// +// get_past_total_supply +// + +#[test] +fn test_get_past_total_supply() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_total_checkpoints.deref(); + + start_cheat_block_timestamp_global('ts10'); + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + // At ts 'ts0', the total supply is the initial mint + assert_eq!(state.get_past_total_supply('ts0'), ERC721_INITIAL_MINT); + assert_eq!(state.get_past_total_supply('ts1'), 3); + assert_eq!(state.get_past_total_supply('ts2'), 5); + assert_eq!(state.get_past_total_supply('ts5'), 7); +} + +#[test] +fn test_get_past_total_supply_before_checkpoints() { + start_cheat_block_timestamp_global('ts1'); + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_total_checkpoints.deref(); + + start_cheat_block_timestamp_global('ts10'); + trace.push('ts1', 3); + trace.push('ts2', 5); + + assert_eq!(state.get_past_total_supply('ts0'), 0); +} + +#[test] +#[should_panic(expected: ('Votes: future Lookup',))] +fn test_get_past_total_supply_future_lookup() { + let state = setup_erc721_votes(); + start_cheat_block_timestamp_global('ts1'); + state.get_past_total_supply('ts2'); +} + +// +// delegates +// + +#[test] +fn test_delegates() { + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, DELEGATOR()); + + state.delegate(DELEGATOR()); + assert_eq!(state.delegates(DELEGATOR()), DELEGATOR()); +} + +// +// delegate +// + +#[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, DELEGATOR()); + + state.delegate(DELEGATOR()); + spy.assert_event_delegate_changed(contract_address, DELEGATOR(), ZERO(), DELEGATOR()); + spy + .assert_only_event_delegate_votes_changed( + contract_address, DELEGATOR(), 0, ERC721_INITIAL_MINT + ); + assert_eq!(state.get_votes(DELEGATOR()), ERC721_INITIAL_MINT); +} + +#[test] +fn test_delegate_to_delegatee_updates_votes() { + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, DELEGATOR()); + + state.delegate(DELEGATEE()); + spy.assert_event_delegate_changed(contract_address, DELEGATOR(), ZERO(), DELEGATEE()); + spy + .assert_only_event_delegate_votes_changed( + contract_address, DELEGATEE(), 0, ERC721_INITIAL_MINT + ); + assert_eq!(state.get_votes(DELEGATEE()), ERC721_INITIAL_MINT); + assert_eq!(state.get_votes(DELEGATOR()), 0); +} + +#[test] +fn test_delegate_to_delegatee_updates_delegates() { + let mut state = setup_erc721_votes(); + start_cheat_caller_address(test_address(), DELEGATOR()); + state.delegate(DELEGATOR()); + assert_eq!(state.delegates(DELEGATOR()), DELEGATOR()); + state.delegate(DELEGATEE()); + assert_eq!(state.delegates(DELEGATOR()), DELEGATEE()); +} + +#[test] +fn test_delegate_with_no_balance() { + let mut state = setup_erc721_votes(); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, OTHER()); + + // OTHER has no balance, so delegating should not change any votes + state.delegate(DELEGATEE()); + + spy.assert_event_delegate_changed(contract_address, OTHER(), ZERO(), DELEGATEE()); + // No DelegateVotesChanged event should be emitted + spy.assert_no_events_left_from(contract_address); + + assert_eq!(state.get_votes(DELEGATEE()), 0); + assert_eq!(state.get_votes(OTHER()), 0); + assert_eq!(state.delegates(OTHER()), DELEGATEE()); +} + +// +// delegate_by_sig +// + +#[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 = DELEGATEE(); + + // 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].span()); + + spy.assert_only_event_delegate_changed(contract_address, delegator, ZERO(), delegatee); + assert_eq!(state.delegates(account), delegatee); +} + +#[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] +#[should_panic(expected: ('Votes: expired signature',))] +fn test_delegate_by_sig_past_expiry() { + start_cheat_block_timestamp_global('ts5'); + + let mut state = setup_erc721_votes(); + let expiry = 'ts4'; + let signature = array![0, 0]; + + state.delegate_by_sig(DELEGATOR(), DELEGATEE(), 0, expiry, signature.span()); +} + +#[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(DELEGATOR(), DELEGATEE(), 1, 0, signature.span()); +} + +#[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 = DELEGATEE(); + 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].span()); +} + +#[test] +#[should_panic(expected: ('Votes: invalid signature',))] +fn test_delegate_by_sig_bad_delegatee() { + 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 = DELEGATEE(); + let bad_delegatee = contract_address_const::<0x1234>(); + 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 a different delegatee than the one signed for + state.delegate_by_sig(delegator, bad_delegatee, nonce, expiry, array![r, s].span()); +} + +#[test] +#[should_panic(expected: ('Nonces: invalid nonce',))] +fn test_delegate_by_sig_reused_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 = DELEGATEE(); + 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'); + // First delegation (should succeed) + state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s].span()); + + // Attempt to reuse the same signature (should fail) + state.delegate_by_sig(delegator, delegatee, nonce, expiry, array![r, s].span()); +} + +// +// num_checkpoints +// + +#[test] +fn test_num_checkpoints() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_delegate_checkpoints.entry(DELEGATOR()); + + start_cheat_block_timestamp_global('ts10'); + + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + assert_eq!(state.num_checkpoints(DELEGATOR()), 3); + assert_eq!(state.num_checkpoints(OTHER()), 0); +} + +// +// checkpoints +// + +#[test] +fn test_checkpoints() { + let mut state = setup_erc721_votes(); + let mut trace = state.Votes_delegate_checkpoints.entry(DELEGATOR()); + + start_cheat_block_timestamp_global('ts10'); + + trace.push('ts1', 3); + trace.push('ts2', 5); + trace.push('ts3', 7); + + let checkpoint0 = state.checkpoints(DELEGATOR(), 0); + assert_eq!(checkpoint0.key, 'ts1'); + assert_eq!(checkpoint0.value, 3); + + let checkpoint1 = state.checkpoints(DELEGATOR(), 1); + assert_eq!(checkpoint1.key, 'ts2'); + assert_eq!(checkpoint1.value, 5); + + let checkpoint2 = state.checkpoints(DELEGATOR(), 2); + assert_eq!(checkpoint2.key, 'ts3'); + assert_eq!(checkpoint2.value, 7); +} + +// +// Tests specific to ERC721Votes and ERC20Votes +// + +#[test] +fn test_erc721_get_voting_units() { + let state = setup_erc721_votes(); + + assert_eq!(state.get_voting_units(DELEGATOR()), ERC721_INITIAL_MINT); + assert_eq!(state.get_voting_units(OTHER()), 0); +} + +#[test] +fn test_erc20_get_voting_units() { + let mut state = setup_erc20_votes(); + + assert_eq!(state.get_voting_units(DELEGATOR()), SUPPLY); + 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, DELEGATOR()); + start_cheat_block_timestamp_global('ts1'); + + state.delegate(DELEGATOR()); + + // Set spy and burn some tokens + let mut spy = spy_events(); + let burn_amount = 1000; + mock_state.erc20.burn(DELEGATOR(), burn_amount); + + // We need to move the timestamp forward to be able to call get_past_total_supply + start_cheat_block_timestamp_global('ts2'); + spy + .assert_event_delegate_votes_changed( + contract_address, DELEGATOR(), SUPPLY, SUPPLY - burn_amount + ); + assert_eq!(state.get_votes(DELEGATOR()), 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, DELEGATOR()); + start_cheat_block_timestamp_global('ts1'); + + state.delegate(DELEGATOR()); + + // Set spy and burn some tokens + let mut spy = spy_events(); + let burn_amount = 3; + for i in 0 + ..burn_amount { + mock_state.erc721.burn(i); + spy + .assert_event_delegate_votes_changed( + contract_address, + DELEGATOR(), + ERC721_INITIAL_MINT - i, + ERC721_INITIAL_MINT - 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(DELEGATOR()), ERC721_INITIAL_MINT - burn_amount); + assert_eq!(state.get_past_total_supply('ts1'), ERC721_INITIAL_MINT - burn_amount); +} + +#[test] +fn test_erc_721_get_total_supply() { + let state = setup_erc721_votes(); + assert_eq!(state.get_total_supply(), ERC721_INITIAL_MINT); +} + +#[test] +fn test_erc_20_get_total_supply() { + let state = setup_erc20_votes(); + assert_eq!(state.get_total_supply(), SUPPLY); +} + +#[test] +fn test_erc_20_voting_units_update_with_full_balance_transfer() { + let mut state = setup_erc20_votes(); + let mut mock_state = ERC20VOTES_CONTRACT_STATE(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, DELEGATOR()); + + // DELEGATOR self-delegates + state.delegate(DELEGATOR()); + assert_eq!(state.get_votes(DELEGATOR()), SUPPLY); + + let mut spy = spy_events(); + + // Full balance transfer + mock_state.erc20.transfer(RECIPIENT(), SUPPLY); + + spy.assert_event_delegate_votes_changed(contract_address, DELEGATOR(), SUPPLY, 0); + assert_eq!(state.get_votes(DELEGATOR()), 0); + assert_eq!(state.get_votes(RECIPIENT()), 0); // RECIPIENT hasn't delegated yet + + // RECIPIENT delegates to themselves + start_cheat_caller_address(contract_address, RECIPIENT()); + state.delegate(RECIPIENT()); + + spy.assert_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, SUPPLY); + assert_eq!(state.get_votes(RECIPIENT()), SUPPLY); +} + +#[test] +fn test_erc_20_voting_units_update_with_partial_balance_transfer() { + let mut state = setup_erc20_votes(); + let mut mock_state = ERC20VOTES_CONTRACT_STATE(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, DELEGATOR()); + + // DELEGATOR self-delegates + state.delegate(DELEGATOR()); + assert_eq!(state.get_votes(DELEGATOR()), SUPPLY); + + let mut spy = spy_events(); + + // Partial transfer + let partial_amount = SUPPLY / 2; + mock_state.erc20.transfer(RECIPIENT(), partial_amount); + + spy + .assert_event_delegate_votes_changed( + contract_address, DELEGATOR(), SUPPLY, SUPPLY - partial_amount + ); + assert_eq!(state.get_votes(DELEGATOR()), SUPPLY - partial_amount); + assert_eq!(state.get_votes(RECIPIENT()), 0); // RECIPIENT hasn't delegated yet + + // RECIPIENT delegates to themselves + start_cheat_caller_address(contract_address, RECIPIENT()); + state.delegate(RECIPIENT()); + + spy.assert_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, partial_amount); + assert_eq!(state.get_votes(RECIPIENT()), partial_amount); +} + +#[test] +fn test_erc721_voting_units_update_with_single_token_transfer() { + let mut state = setup_erc721_votes(); + let mut mock_state = ERC721VOTES_CONTRACT_STATE(); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, DELEGATOR()); + + // DELEGATOR self-delegates + state.delegate(DELEGATOR()); + assert_eq!(state.get_votes(DELEGATOR()), ERC721_INITIAL_MINT); + + let mut spy = spy_events(); + + // Transfer a single token + let token_id = 0; + mock_state.erc721.transfer_from(DELEGATOR(), RECIPIENT(), token_id); + + spy + .assert_event_delegate_votes_changed( + contract_address, DELEGATOR(), ERC721_INITIAL_MINT, ERC721_INITIAL_MINT - 1 + ); + + assert_eq!(state.get_votes(DELEGATOR()), ERC721_INITIAL_MINT - 1); + assert_eq!(state.get_votes(RECIPIENT()), 0); // RECIPIENT hasn't delegated yet + + // RECIPIENT delegates to themselves + start_cheat_caller_address(contract_address, RECIPIENT()); + state.delegate(RECIPIENT()); + + spy.assert_event_delegate_votes_changed(contract_address, RECIPIENT(), 0, 1); + assert_eq!(state.get_votes(RECIPIENT()), 1); +} + +// +// 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); + } +} 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 new file mode 100644 index 000000000..1225881f9 --- /dev/null +++ b/packages/governance/src/votes.cairo @@ -0,0 +1,5 @@ +pub mod delegation; +pub mod interface; +pub mod votes; + +pub use votes::VotesComponent; diff --git a/packages/token/src/erc20/snip12_utils/votes.cairo b/packages/governance/src/votes/delegation.cairo similarity index 72% rename from packages/token/src/erc20/snip12_utils/votes.cairo rename to packages/governance/src/votes/delegation.cairo index f611bdaaa..e0814d647 100644 --- a/packages/token/src/erc20/snip12_utils/votes.cairo +++ b/packages/governance/src/votes/delegation.cairo @@ -1,9 +1,9 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.17.0 (token/erc20/snip12_utils/votes.cairo) +// OpenZeppelin Contracts for Cairo v0.17.0 (governance/votes/delegation.cairo) use core::hash::{HashStateTrait, HashStateExTrait}; use core::poseidon::PoseidonTrait; -use openzeppelin_utils::cryptography::snip12::StructHash; +use openzeppelin_utils::cryptography::snip12::{StructHash}; use starknet::ContractAddress; // sn_keccak("\"Delegation\"(\"delegatee\":\"ContractAddress\",\"nonce\":\"felt\",\"expiry\":\"u128\")") @@ -21,6 +21,7 @@ pub struct Delegation { impl StructHashImpl of StructHash { fn hash_struct(self: @Delegation) -> felt252 { - PoseidonTrait::new().update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() + let hash_state = PoseidonTrait::new(); + hash_state.update_with(DELEGATION_TYPE_HASH).update_with(*self).finalize() } } diff --git a/packages/governance/src/utils/interfaces/votes.cairo b/packages/governance/src/votes/interface.cairo similarity index 59% rename from packages/governance/src/utils/interfaces/votes.cairo rename to packages/governance/src/votes/interface.cairo index d55c481c2..cd48494c2 100644 --- a/packages/governance/src/utils/interfaces/votes.cairo +++ b/packages/governance/src/votes/interface.cairo @@ -1,3 +1,6 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.17.0 (governance/votes/interface.cairo) + use starknet::ContractAddress; /// Common interface for Votes-enabled contracts. @@ -30,6 +33,28 @@ pub trait IVotes { delegatee: ContractAddress, nonce: felt252, expiry: u64, - signature: Array + signature: Span ); } + +/// Common interface to interact with the `Votes` component. +#[starknet::interface] +pub trait VotesABI { + // Votes + 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: Span + ); + + // Nonces + fn nonces(self: @TState, owner: ContractAddress) -> felt252; +} diff --git a/packages/token/src/erc20/extensions/erc20_votes.cairo b/packages/governance/src/votes/votes.cairo similarity index 53% rename from packages/token/src/erc20/extensions/erc20_votes.cairo rename to packages/governance/src/votes/votes.cairo index 3851a8b6e..93e2e3ced 100644 --- a/packages/token/src/erc20/extensions/erc20_votes.cairo +++ b/packages/governance/src/votes/votes.cairo @@ -1,36 +1,52 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.17.0 (token/erc20/extensions/erc20_votes.cairo) +// OpenZeppelin Contracts for Cairo v0.17.0 (governance/votes/votes.cairo) -/// # ERC20Votes Component +use starknet::ContractAddress; + +/// # Votes Component +/// +/// The Votes component provides a flexible system for tracking and delegating voting power. +/// It is currently implemented for ERC20 and ERC721 tokens. An account can delegate +/// their voting power to a representative, that will pool delegated voting units from different +/// delegators and can then use it to vote in decisions. Voting power must be delegated to be +/// counted, and an account must delegate to itself if it wishes to vote directly without a trusted +/// representative. +/// +/// When integrating the Votes component, the ´VotingUnitsTrait´ must be implemented to get the +/// voting units for a given account as a function of the implementing contract. For simplicity, +/// this module already provides two implementations for ERC20 and ERC721 tokens, which will work +/// out of the box if the respective components are integrated. /// -/// 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. +/// NOTE: ERC20 and ERC721 tokens implementing this component must call ´transfer_voting_units´ +/// whenever a transfer, mint, or burn operation is performed. Hooks can be leveraged for this +/// purpose, as shown in the following ERC20 example: +/// +/// See [the documentation] +/// (https://docs.openzeppelin.com/contracts-cairo/0.17.0/governance.html#usage_2) +/// for examples and more details. #[starknet::component] -pub mod ERC20VotesComponent { +pub mod VotesComponent { use core::num::traits::Zero; - use crate::erc20::ERC20Component; - use crate::erc20::interface::IERC20; - use crate::erc20::snip12_utils::votes::Delegation; + use crate::votes::delegation::Delegation; + use crate::votes::interface::IVotes; use openzeppelin_account::interface::{ISRC6Dispatcher, ISRC6DispatcherTrait}; - use openzeppelin_governance::utils::interfaces::IVotes; + 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::{Checkpoint, Trace, TraceTrait}; - use starknet::ContractAddress; - use starknet::storage::{ - Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess - }; + use starknet::storage::{Map, StoragePathEntry, StorageMapReadAccess, StorageMapWriteAccess}; + use super::{VotingUnitsTrait, ContractAddress}; #[storage] pub struct Storage { - pub ERC20Votes_delegatee: Map, - pub ERC20Votes_delegate_checkpoints: Map, - pub ERC20Votes_total_checkpoints: Trace + pub Votes_delegatee: Map, + pub Votes_delegate_checkpoints: Map, + pub Votes_total_checkpoints: Trace, } #[event] @@ -40,8 +56,8 @@ pub mod ERC20VotesComponent { DelegateVotesChanged: DelegateVotesChanged, } - /// Emitted when `delegator` delegates their votes from `from_delegate` to `to_delegate`. #[derive(Drop, PartialEq, starknet::Event)] + /// Emitted when `delegator` delegates their votes from `from_delegate` to `to_delegate`. pub struct DelegateChanged { #[key] pub delegator: ContractAddress, @@ -66,19 +82,18 @@ pub mod ERC20VotesComponent { pub const INVALID_SIGNATURE: felt252 = 'Votes: invalid signature'; } - #[embeddable_as(ERC20VotesImpl)] - impl ERC20Votes< + #[embeddable_as(VotesImpl)] + impl Votes< TContractState, +HasComponent, - +ERC20Component::HasComponent, - +ERC20Component::ERC20HooksTrait, impl Nonces: NoncesComponent::HasComponent, + +VotingUnitsTrait>, +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() + self.Votes_delegate_checkpoints.entry(account).latest() } /// Returns the amount of votes that `account` had at a specific moment in the past. @@ -91,8 +106,7 @@ pub mod ERC20VotesComponent { ) -> 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) + self.Votes_delegate_checkpoints.entry(account).upper_lookup_recent(timepoint) } /// Returns the total supply of votes available at a specific moment in the past. @@ -100,23 +114,17 @@ pub mod ERC20VotesComponent { /// 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) + self.Votes_total_checkpoints.deref().upper_lookup_recent(timepoint) } /// Returns the delegate that `account` has chosen. fn delegates( self: @ComponentState, account: ContractAddress ) -> ContractAddress { - self.ERC20Votes_delegatee.read(account) + self.Votes_delegatee.read(account) } /// Delegates votes from the sender to `delegatee`. @@ -146,7 +154,7 @@ pub mod ERC20VotesComponent { delegatee: ContractAddress, nonce: felt252, expiry: u64, - signature: Array + signature: Span ) { assert(starknet::get_block_timestamp() <= expiry, Errors::EXPIRED_SIGNATURE); @@ -159,7 +167,7 @@ pub mod ERC20VotesComponent { let hash = delegation.get_message_hash(delegator); let is_valid_signature_felt = ISRC6Dispatcher { contract_address: delegator } - .is_valid_signature(hash, signature); + .is_valid_signature(hash, signature.into()); // Check either 'VALID' or true for backwards compatibility. let is_valid_signature = is_valid_signature_felt == starknet::VALIDATED @@ -176,38 +184,71 @@ pub mod ERC20VotesComponent { // Internal // + impl ERC20VotesImpl< + TContractState, + +HasComponent, + impl ERC20: ERC20Component::HasComponent, + +ERC20Component::ERC20HooksTrait + > of VotingUnitsTrait> { + /// 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. + /// + /// NOTE: This implementation will work out of the box if the ERC20 component + /// is implemented in the final contract. + /// + /// WARNING: This implementation assumes tokens map to voting units 1:1. + /// Any deviation from this formula when transferring voting units (e.g. by using hooks) + /// may compromise the internal vote accounting. + fn get_voting_units( + self: @ComponentState, account: ContractAddress + ) -> u256 { + let erc20_component = get_dep_component!(self, ERC20); + erc20_component.balance_of(account) + } + } + + impl ERC721VotesImpl< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + impl ERC721: ERC721Component::HasComponent, + +ERC721Component::ERC721HooksTrait, + +Drop + > of VotingUnitsTrait> { + /// 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. + /// + /// NOTE: This implementation will work out of the box if the ERC721 component + /// is implemented in the final contract. + /// + /// WARNING: This implementation assumes tokens map to voting units 1:1. + /// Any deviation from this formula when transferring voting units (e.g. by using hooks) + /// may compromise the internal vote accounting. + fn get_voting_units( + self: @ComponentState, account: ContractAddress + ) -> u256 { + let erc721_component = get_dep_component!(self, ERC721); + erc721_component.balance_of(account).into() + } + } + #[generate_trait] pub impl InternalImpl< TContractState, +HasComponent, - impl ERC20: ERC20Component::HasComponent, - +ERC20Component::ERC20HooksTrait, + +VotingUnitsTrait>, +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)); + self.Votes_total_checkpoints.deref().latest() } /// Moves delegated votes from one delegate to another. @@ -219,19 +260,18 @@ pub mod ERC20VotesComponent { 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); + if from != to && amount > 0 { + if from.is_non_zero() { + let mut trace = self.Votes_delegate_checkpoints.entry(from); let (previous_votes, new_votes) = trace - .push(block_timestamp, trace.latest() - amount); + .push(block_timestamp, trace.into().latest() - amount); self.emit(DelegateVotesChanged { delegate: from, previous_votes, new_votes }); } - if (to != zero_address) { - let mut trace = self.ERC20Votes_delegate_checkpoints.read(to); + if to.is_non_zero() { + let mut trace = self.Votes_delegate_checkpoints.entry(to); let (previous_votes, new_votes) = trace - .push(block_timestamp, trace.latest() + amount); + .push(block_timestamp, trace.into().latest() + amount); self.emit(DelegateVotesChanged { delegate: to, previous_votes, new_votes }); } } @@ -242,6 +282,11 @@ pub mod ERC20VotesComponent { /// 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. /// + /// WARNING: If voting units are based on an underlying transferable asset (like a token), + /// you must call this function every time the asset is transferred to keep the internal + /// voting power accounting in sync. For ERC20 and ERC721 tokens, this is typically handled + /// using hooks. + /// /// May emit one or two `DelegateVotesChanged` events. fn transfer_voting_units( ref self: ComponentState, @@ -249,37 +294,61 @@ pub mod ERC20VotesComponent { 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 from.is_zero() { + let mut trace = self.Votes_total_checkpoints.deref(); + trace.push(block_timestamp, trace.into().latest() + amount); } - if (to == zero_address) { - let mut trace = self.ERC20Votes_total_checkpoints.read(); - trace.push(block_timestamp, trace.latest() - amount); + if to.is_zero() { + let mut trace = self.Votes_total_checkpoints.deref(); + trace.push(block_timestamp, trace.into().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() + fn num_checkpoints(self: @ComponentState, account: ContractAddress) -> u64 { + self.Votes_delegate_checkpoints.entry(account).length() } /// Returns the `pos`-th checkpoint for `account`. fn checkpoints( - self: @ComponentState, account: ContractAddress, pos: u32 + self: @ComponentState, account: ContractAddress, pos: u64 ) -> Checkpoint { - self.ERC20Votes_delegate_checkpoints.read(account).at(pos) + self.Votes_delegate_checkpoints.entry(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) + /// 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.Votes_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)); } } } + +/// A trait that must be implemented when integrating {VotesComponent} into a contract. It +/// offers a mechanism to retrieve the number of voting units for a given account at the current +/// time. +pub trait VotingUnitsTrait { + /// Returns the number of voting units for a given account. For ERC20, this is typically the + /// token balance. For ERC721, this is typically the number of tokens owned. + /// + /// WARNING: While any formula can be used as a measure of voting units, the internal vote + /// accounting of the contract may be compromised if voting units are transferred in any + /// external flow by following a different formula. For example, when implementing the hook for + /// ERC20, the number of voting units transferred should match the formula given by the + /// `get_voting_units` implementation. + fn get_voting_units(self: @TState, account: ContractAddress) -> u256; +} diff --git a/packages/test_common/Scarb.toml b/packages/test_common/Scarb.toml index 1d488b357..10ad734d5 100644 --- a/packages/test_common/Scarb.toml +++ b/packages/test_common/Scarb.toml @@ -27,6 +27,7 @@ openzeppelin_security = { path = "../security" } openzeppelin_token = { path = "../token" } openzeppelin_testing = { path = "../testing" } openzeppelin_utils = { path = "../utils" } +openzeppelin_governance = { path = "../governance" } [lib] diff --git a/packages/test_common/src/mocks.cairo b/packages/test_common/src/mocks.cairo index 5cd4a9a96..027f99625 100644 --- a/packages/test_common/src/mocks.cairo +++ b/packages/test_common/src/mocks.cairo @@ -1,5 +1,6 @@ pub mod access; pub mod account; +pub mod checkpoint; pub mod erc1155; pub mod erc20; pub mod erc2981; @@ -13,3 +14,5 @@ pub mod src9; pub mod timelock; pub mod upgrades; pub mod vesting; +pub mod votes; + diff --git a/packages/test_common/src/mocks/checkpoint.cairo b/packages/test_common/src/mocks/checkpoint.cairo new file mode 100644 index 000000000..857c28eb7 --- /dev/null +++ b/packages/test_common/src/mocks/checkpoint.cairo @@ -0,0 +1,36 @@ +#[starknet::interface] +pub trait IMockTrace { + fn push_checkpoint(ref self: TContractState, key: u64, value: u256) -> (u256, u256); + fn get_latest(self: @TContractState) -> u256; + fn get_at_key(self: @TContractState, key: u64) -> u256; + fn get_length(self: @TContractState) -> u64; +} + +#[starknet::contract] +pub mod MockTrace { + use openzeppelin_utils::structs::checkpoint::{Trace, TraceTrait}; + + #[storage] + struct Storage { + trace: Trace, + } + + #[abi(embed_v0)] + impl MockTraceImpl of super::IMockTrace { + fn push_checkpoint(ref self: ContractState, key: u64, value: u256) -> (u256, u256) { + self.trace.deref().push(key, value) + } + + fn get_latest(self: @ContractState) -> u256 { + self.trace.deref().latest() + } + + fn get_at_key(self: @ContractState, key: u64) -> u256 { + self.trace.deref().upper_lookup(key) + } + + fn get_length(self: @ContractState) -> u64 { + self.trace.deref().length() + } + } +} diff --git a/packages/test_common/src/mocks/erc20.cairo b/packages/test_common/src/mocks/erc20.cairo index d2e76b8d0..a77d3275c 100644 --- a/packages/test_common/src/mocks/erc20.cairo +++ b/packages/test_common/src/mocks/erc20.cairo @@ -78,101 +78,6 @@ pub mod SnakeERC20Mock { } } -#[starknet::contract] -pub mod DualCaseERC20VotesMock { - use openzeppelin_token::erc20::ERC20Component; - use openzeppelin_token::erc20::extensions::ERC20VotesComponent::InternalTrait as ERC20VotesInternalTrait; - use openzeppelin_token::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 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); - } -} - #[starknet::contract] pub mod DualCaseERC20PermitMock { use openzeppelin_token::erc20::{ERC20Component, ERC20HooksEmptyImpl}; diff --git a/packages/test_common/src/mocks/votes.cairo b/packages/test_common/src/mocks/votes.cairo new file mode 100644 index 000000000..b8aa1959f --- /dev/null +++ b/packages/test_common/src/mocks/votes.cairo @@ -0,0 +1,162 @@ +#[starknet::contract] +pub mod ERC721VotesMock { + use openzeppelin_governance::votes::VotesComponent; + use openzeppelin_introspection::src5::SRC5Component; + use openzeppelin_token::erc721::ERC721Component; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + use starknet::ContractAddress; + + 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 + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl VotesInternalImpl = VotesComponent::InternalImpl; + + // ERC721 + #[abi(embed_v0)] + impl ERC721MixinImpl = ERC721Component::ERC721MixinImpl; + impl ERC721InternalImpl = ERC721Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub erc721_votes: VotesComponent::Storage, + #[substorage(v0)] + pub erc721: ERC721Component::Storage, + #[substorage(v0)] + pub src5: SRC5Component::Storage, + #[substorage(v0)] + pub 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 impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + impl ERC721VotesHooksImpl of ERC721Component::ERC721HooksTrait { + // We need to use the `before_update` hook to check the previous owner + // before the transfer is executed. + fn before_update( + ref self: ERC721Component::ComponentState, + to: ContractAddress, + token_id: u256, + auth: ContractAddress + ) { + let mut contract_state = ERC721Component::HasComponent::get_contract_mut(ref self); + + // We use the internal function here since it does not check if the token id exists + // which is necessary for mints + let previous_owner = self._owner_of(token_id); + contract_state.erc721_votes.transfer_voting_units(previous_owner, to, 1); + } + } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc721.initializer("MyToken", "MTK", ""); + } +} + +#[starknet::contract] +pub mod ERC20VotesMock { + use openzeppelin_governance::votes::VotesComponent; + use openzeppelin_token::erc20::ERC20Component; + use openzeppelin_utils::cryptography::nonces::NoncesComponent; + use openzeppelin_utils::cryptography::snip12::SNIP12Metadata; + use starknet::ContractAddress; + + component!(path: VotesComponent, storage: erc20_votes, event: ERC20VotesEvent); + component!(path: ERC20Component, storage: erc20, event: ERC20Event); + component!(path: NoncesComponent, storage: nonces, event: NoncesEvent); + + // Votes and ERC20Votes + #[abi(embed_v0)] + impl VotesImpl = VotesComponent::VotesImpl; + impl VotesInternalImpl = VotesComponent::InternalImpl; + + // ERC20 + #[abi(embed_v0)] + impl ERC20MixinImpl = ERC20Component::ERC20MixinImpl; + impl ERC20InternalImpl = ERC20Component::InternalImpl; + + // Nonces + #[abi(embed_v0)] + impl NoncesImpl = NoncesComponent::NoncesImpl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub erc20_votes: VotesComponent::Storage, + #[substorage(v0)] + pub erc20: ERC20Component::Storage, + #[substorage(v0)] + pub nonces: NoncesComponent::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + ERC20VotesEvent: VotesComponent::Event, + #[flat] + ERC20Event: ERC20Component::Event, + #[flat] + NoncesEvent: NoncesComponent::Event + } + + /// Required for hash computation. + pub impl SNIP12MetadataImpl of SNIP12Metadata { + fn name() -> felt252 { + 'DAPP_NAME' + } + fn version() -> felt252 { + 'DAPP_VERSION' + } + } + + impl ERC20VotesHooksImpl of ERC20Component::ERC20HooksTrait { + fn after_update( + ref self: ERC20Component::ComponentState, + from: ContractAddress, + recipient: ContractAddress, + amount: u256 + ) { + let mut contract_state = ERC20Component::HasComponent::get_contract_mut(ref self); + contract_state.erc20_votes.transfer_voting_units(from, recipient, amount); + } + } + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc20.initializer("MyToken", "MTK"); + } +} + diff --git a/packages/testing/src/constants.cairo b/packages/testing/src/constants.cairo index 6875d1fa9..33226fd6b 100644 --- a/packages/testing/src/constants.cairo +++ b/packages/testing/src/constants.cairo @@ -93,6 +93,14 @@ pub fn OPERATOR() -> ContractAddress { contract_address_const::<'OPERATOR'>() } +pub fn DELEGATOR() -> ContractAddress { + contract_address_const::<'DELEGATOR'>() +} + +pub fn DELEGATEE() -> ContractAddress { + contract_address_const::<'DELEGATEE'>() +} + pub fn DATA(success: bool) -> Span { let value = if success { SUCCESS 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 92e74c40b..06138f343 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 2f9f94b58..e3a8368f7 100644 --- a/packages/token/src/erc20.cairo +++ b/packages/token/src/erc20.cairo @@ -1,5 +1,4 @@ pub mod erc20; -pub mod extensions; pub mod interface; pub mod snip12_utils; 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/interface.cairo b/packages/token/src/erc20/interface.cairo index 53b9d73eb..d35c2b75d 100644 --- a/packages/token/src/erc20/interface.cairo +++ b/packages/token/src/erc20/interface.cairo @@ -121,43 +121,3 @@ pub trait ERC20ABI { // ISNIP12Metadata fn snip12_metadata(self: @TState) -> (felt252, felt252); } - -#[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/erc20/snip12_utils.cairo b/packages/token/src/erc20/snip12_utils.cairo index 9b562ed1d..e3540a5d5 100644 --- a/packages/token/src/erc20/snip12_utils.cairo +++ b/packages/token/src/erc20/snip12_utils.cairo @@ -1,2 +1 @@ pub mod permit; -pub mod votes; diff --git a/packages/token/src/tests/erc20.cairo b/packages/token/src/tests/erc20.cairo index 47b8a1b5e..841c19428 100644 --- a/packages/token/src/tests/erc20.cairo +++ b/packages/token/src/tests/erc20.cairo @@ -1,3 +1,2 @@ mod test_erc20; mod test_erc20_permit; -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 3d3fdc918..000000000 --- a/packages/token/src/tests/erc20/test_erc20_votes.cairo +++ /dev/null @@ -1,404 +0,0 @@ -use core::num::traits::Bounded; -use core::num::traits::Zero; -use crate::erc20::ERC20Component::InternalImpl as ERC20Impl; -use crate::erc20::extensions::ERC20VotesComponent::{DelegateChanged, DelegateVotesChanged}; -use crate::erc20::extensions::ERC20VotesComponent::{ERC20VotesImpl, InternalImpl}; -use crate::erc20::extensions::ERC20VotesComponent; -use crate::erc20::snip12_utils::votes::Delegation; -use openzeppelin_test_common::mocks::erc20::DualCaseERC20VotesMock::SNIP12MetadataImpl; -use openzeppelin_test_common::mocks::erc20::DualCaseERC20VotesMock; -use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{SUPPLY, ZERO, OWNER, RECIPIENT}; -use openzeppelin_testing::events::EventSpyExt; -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/utils/src/structs.cairo b/packages/utils/src/structs.cairo index 3b3d8eaa6..4c0eada6d 100644 --- a/packages/utils/src/structs.cairo +++ b/packages/utils/src/structs.cairo @@ -1,2 +1 @@ pub mod checkpoint; -pub mod storage_array; diff --git a/packages/utils/src/structs/checkpoint.cairo b/packages/utils/src/structs/checkpoint.cairo index f4e2e1b55..b0fa3b5bd 100644 --- a/packages/utils/src/structs/checkpoint.cairo +++ b/packages/utils/src/structs/checkpoint.cairo @@ -3,14 +3,17 @@ use core::num::traits::Sqrt; use crate::math; +use starknet::storage::{StoragePath, StorageAsPath, Vec, VecTrait, Mutable, MutableVecTrait}; +use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; use starknet::storage_access::StorePacking; -use super::storage_array::{StorageArray, StorageArrayTrait}; + +const _2_POW_184: felt252 = 0x10000000000000000000000000000000000000000000000; /// `Trace` struct, for checkpointing values as they change at different points in /// time, and later looking up past values by block timestamp. -#[derive(Copy, Drop, starknet::Store)] +#[starknet::storage_node] pub struct Trace { - pub checkpoints: StorageArray + pub checkpoints: Vec } /// Generic checkpoint representation. @@ -24,21 +27,21 @@ pub struct Checkpoint { pub impl TraceImpl of TraceTrait { /// Pushes a (`key`, `value`) pair into a Trace so that it is stored as the checkpoint /// and returns both the previous and the new value. - fn push(ref self: Trace, key: u64, value: u256) -> (u256, u256) { - self.checkpoints._insert(key, value) + fn push(self: StoragePath>, key: u64, value: u256) -> (u256, u256) { + self.checkpoints.as_path()._insert(key, value) } /// Returns the value in the last (most recent) checkpoint with the key lower than or equal to /// the search key, or zero if there is none. - fn upper_lookup(self: @Trace, key: u64) -> u256 { - let checkpoints = self.checkpoints; + fn upper_lookup(self: StoragePath, key: u64) -> u256 { + let checkpoints = self.checkpoints.as_path(); let len = checkpoints.len(); - let pos = checkpoints._upper_binary_lookup(key, 0, len); + let pos = checkpoints._upper_binary_lookup(key, 0, len).into(); if pos == 0 { 0 } else { - checkpoints.read_at(pos - 1).value + checkpoints[pos - 1].read().value } } @@ -47,8 +50,8 @@ pub impl TraceImpl of TraceTrait { /// /// NOTE: This is a variant of `upper_lookup` that is optimised to /// find "recent" checkpoints (checkpoints with high keys). - fn upper_lookup_recent(self: @Trace, key: u64) -> u256 { - let checkpoints = self.checkpoints; + fn upper_lookup_recent(self: StoragePath, key: u64) -> u256 { + let checkpoints = self.checkpoints.as_path(); let len = checkpoints.len(); let mut low = 0; @@ -56,57 +59,55 @@ pub impl TraceImpl of TraceTrait { if (len > 5) { let mid = len - len.sqrt().into(); - if (key < checkpoints.read_at(mid).key) { + if (key < checkpoints[mid].read().key) { high = mid; } else { low = mid + 1; } } - let pos = checkpoints._upper_binary_lookup(key, low, high); - if pos == 0 { 0 } else { - checkpoints.read_at(pos - 1).value + checkpoints[pos - 1].read().value } } /// Returns the value in the most recent checkpoint, or zero if there are no checkpoints. - fn latest(self: @Trace) -> u256 { + fn latest(self: StoragePath) -> u256 { let checkpoints = self.checkpoints; let pos = checkpoints.len(); if pos == 0 { 0 } else { - checkpoints.read_at(pos - 1).value + checkpoints[pos - 1].read().value } } /// Returns whether there is a checkpoint in the structure (i.e. it is not empty), /// and if so the key and value in the most recent checkpoint. - fn latest_checkpoint(self: @Trace) -> (bool, u64, u256) { + fn latest_checkpoint(self: StoragePath) -> (bool, u64, u256) { let checkpoints = self.checkpoints; let pos = checkpoints.len(); if (pos == 0) { (false, 0, 0) } else { - let checkpoint: Checkpoint = checkpoints.read_at(pos - 1); + let checkpoint = checkpoints[pos - 1].read(); (true, checkpoint.key, checkpoint.value) } } /// Returns the number of checkpoints. - fn length(self: @Trace) -> u32 { + fn length(self: StoragePath) -> u64 { self.checkpoints.len() } /// Returns the checkpoint at given position. - fn at(self: @Trace, pos: u32) -> Checkpoint { - assert(pos < self.length(), 'Array overflow'); - self.checkpoints.read_at(pos) + fn at(self: StoragePath, pos: u64) -> Checkpoint { + assert(pos < self.length(), 'Vec overflow'); + self.checkpoints[pos].read() } } @@ -114,26 +115,25 @@ pub impl TraceImpl of TraceTrait { impl CheckpointImpl of CheckpointTrait { /// Pushes a (`key`, `value`) pair into an ordered list of checkpoints, either by inserting a /// new checkpoint, or by updating the last one. - fn _insert(ref self: StorageArray, key: u64, value: u256) -> (u256, u256) { + fn _insert(self: StoragePath>>, key: u64, value: u256) -> (u256, u256) { let pos = self.len(); if (pos > 0) { - let mut last: Checkpoint = self.read_at(pos - 1); + let mut last = self[pos - 1].read(); // Checkpoint keys must be non-decreasing assert(last.key <= key, 'Unordered insertion'); - // Update or append new checkpoint let prev = last.value; if (last.key == key) { last.value = value; - self.write_at(pos - 1, last); + self[pos - 1].write(last); } else { - self.append(Checkpoint { key: key, value: value }); + self.append().write(Checkpoint { key, value }); } (prev, value) } else { - self.append(Checkpoint { key: key, value: value }); + self.append().write(Checkpoint { key, value }); (0, value) } } @@ -141,7 +141,9 @@ impl CheckpointImpl of CheckpointTrait { /// Returns the index of the last (most recent) checkpoint with the key lower than or equal to /// the search key, or `high` if there is none. `low` and `high` define a section where to do /// the search, with inclusive `low` and exclusive `high`. - fn _upper_binary_lookup(self: @StorageArray, key: u64, low: u32, high: u32) -> u32 { + fn _upper_binary_lookup( + self: StoragePath>, key: u64, low: u64, high: u64 + ) -> u64 { let mut _low = low; let mut _high = high; loop { @@ -149,7 +151,7 @@ impl CheckpointImpl of CheckpointTrait { break; } let mid = math::average(_low, _high); - if (self.read_at(mid).key > key) { + if (self[mid].read().key > key) { _high = mid; } else { _low = mid + 1; @@ -159,8 +161,6 @@ impl CheckpointImpl of CheckpointTrait { } } -const _2_POW_184: felt252 = 0x10000000000000000000000000000000000000000000000; - /// Packs a Checkpoint into a (felt252, felt252). /// /// The packing is done as follows: @@ -195,45 +195,3 @@ impl CheckpointStorePacking of StorePacking { } } } - -#[cfg(test)] -mod test { - use core::num::traits::Bounded; - use super::Checkpoint; - use super::CheckpointStorePacking; - use super::_2_POW_184; - - const KEY_MASK: u256 = 0xffffffffffffffff; - const LOW_MASK: u256 = 0xffffffffffffffffffffffffffffffff; - - #[test] - fn test_pack_big_key_and_value() { - let key = Bounded::MAX; - let value = Bounded::MAX; - let checkpoint = Checkpoint { key, value }; - - let (key_and_low, high) = CheckpointStorePacking::pack(checkpoint); - - let expected_key: u256 = (key_and_low.into() / _2_POW_184.into()) & KEY_MASK; - let expected_low: u256 = key_and_low.into() & LOW_MASK; - let expected_high: felt252 = Bounded::::MAX.into(); - - assert_eq!(key.into(), expected_key); - assert_eq!(value.low.into(), expected_low); - assert_eq!(high, expected_high); - } - - #[test] - fn test_unpack_big_key_and_value() { - let key_and_low = Bounded::::MAX.into() * _2_POW_184 + Bounded::::MAX.into(); - let high = Bounded::::MAX.into(); - - let checkpoint = CheckpointStorePacking::unpack((key_and_low, high)); - - let expected_key: u64 = Bounded::MAX; - let expected_value: u256 = Bounded::MAX; - - assert_eq!(checkpoint.key, expected_key); - assert_eq!(checkpoint.value, expected_value); - } -} diff --git a/packages/utils/src/structs/storage_array.cairo b/packages/utils/src/structs/storage_array.cairo deleted file mode 100644 index ee960cb8a..000000000 --- a/packages/utils/src/structs/storage_array.cairo +++ /dev/null @@ -1,107 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.17.0 (utils/structs/storage_array.cairo) - -use core::hash::{HashStateExTrait, HashStateTrait}; -use core::poseidon::PoseidonTrait; -use starknet::storage_access::{ - StorageBaseAddress, storage_address_from_base, storage_base_address_from_felt252 -}; -use starknet::syscalls::{storage_read_syscall, storage_write_syscall}; -use starknet::{Store, SyscallResultTrait, SyscallResult}; - -const NOT_IMPLEMENTED: felt252 = 'Not implemented'; - -/// Represents an Array that can be stored in storage. -#[derive(Copy, Drop)] -pub struct StorageArray { - address_domain: u32, - base: StorageBaseAddress -} - -impl StoreStorageArray, impl TStore: Store> of Store> { - #[inline(always)] - fn read(address_domain: u32, base: StorageBaseAddress) -> SyscallResult> { - SyscallResult::Ok(StorageArray { address_domain, base }) - } - #[inline(always)] - fn write( - address_domain: u32, base: StorageBaseAddress, value: StorageArray - ) -> SyscallResult<()> { - SyscallResult::Err(array![NOT_IMPLEMENTED]) - } - #[inline(always)] - fn read_at_offset( - address_domain: u32, base: StorageBaseAddress, offset: u8 - ) -> SyscallResult> { - SyscallResult::Err(array![NOT_IMPLEMENTED]) - } - #[inline(always)] - fn write_at_offset( - address_domain: u32, base: StorageBaseAddress, offset: u8, value: StorageArray - ) -> SyscallResult<()> { - SyscallResult::Err(array![NOT_IMPLEMENTED]) - } - #[inline(always)] - fn size() -> u8 { - // 0 was selected because the read method doesn't actually read from storage - 0_u8 - } -} - -/// Trait for accessing a storage array. -/// -/// `read_at` and `write_at` don't check the length of the array, caution must be exercised. -/// The current length of the array is stored at the base StorageBaseAddress as felt. -pub trait StorageArrayTrait { - fn read_at(self: @StorageArray, index: usize) -> T; - fn write_at(ref self: StorageArray, index: usize, value: T) -> (); - fn append(ref self: StorageArray, value: T) -> (); - fn len(self: @StorageArray) -> u32; -} - -impl StorageArrayImpl, impl TStore: Store> of StorageArrayTrait { - fn read_at(self: @StorageArray, index: usize) -> T { - // Get the storage address of the element - let storage_address_felt: felt252 = storage_address_from_base(*self.base).into(); - - let mut state = PoseidonTrait::new(); - let element_address = state.update_with(storage_address_felt + index.into()).finalize(); - - // Read the element from storage - TStore::read(*self.address_domain, storage_base_address_from_felt252(element_address)) - .unwrap_syscall() - } - - fn write_at(ref self: StorageArray, index: usize, value: T) { - // Get the storage address of the element - let storage_address_felt: felt252 = storage_address_from_base(self.base).into(); - - let mut state = PoseidonTrait::new(); - let element_address = state.update_with(storage_address_felt + index.into()).finalize(); - - // Write the element to storage - TStore::write( - self.address_domain, storage_base_address_from_felt252(element_address), value - ) - .unwrap_syscall() - } - - fn append(ref self: StorageArray, value: T) { - let len = self.len(); - - // Write the element to storage - self.write_at(len, value); - - // Update the len - let new_len: felt252 = (len + 1).into(); - storage_write_syscall(self.address_domain, storage_address_from_base(self.base), new_len) - .unwrap_syscall(); - } - - fn len(self: @StorageArray) -> u32 { - storage_read_syscall(*self.address_domain, storage_address_from_base(*self.base)) - .unwrap_syscall() - .try_into() - .unwrap() - } -} diff --git a/packages/utils/src/tests.cairo b/packages/utils/src/tests.cairo index fedcc380c..74bd036b1 100644 --- a/packages/utils/src/tests.cairo +++ b/packages/utils/src/tests.cairo @@ -1,2 +1,3 @@ +mod test_checkpoint; mod test_nonces; mod test_snip12; diff --git a/packages/utils/src/tests/mocks.cairo b/packages/utils/src/tests/mocks.cairo deleted file mode 100644 index 7a238265a..000000000 --- a/packages/utils/src/tests/mocks.cairo +++ /dev/null @@ -1 +0,0 @@ -pub(crate) mod nonces_mocks; diff --git a/packages/utils/src/tests/test_checkpoint.cairo b/packages/utils/src/tests/test_checkpoint.cairo new file mode 100644 index 000000000..3f3a45d02 --- /dev/null +++ b/packages/utils/src/tests/test_checkpoint.cairo @@ -0,0 +1,107 @@ +use core::num::traits::Bounded; +use crate::structs::checkpoint::Checkpoint; +use openzeppelin_test_common::mocks::checkpoint::{IMockTrace, MockTrace}; +use starknet::storage_access::StorePacking; + +const _2_POW_184: felt252 = 0x10000000000000000000000000000000000000000000000; +const KEY_MASK: u256 = 0xffffffffffffffff; +const LOW_MASK: u256 = 0xffffffffffffffffffffffffffffffff; + +fn CONTRACT_STATE() -> MockTrace::ContractState { + MockTrace::contract_state_for_testing() +} + +#[test] +fn test_push_checkpoint() { + let mut mock_trace = CONTRACT_STATE(); + + let (prev, new) = mock_trace.push_checkpoint(100, 1000); + assert_eq!(prev, 0); + assert_eq!(new, 1000); + + let (prev, new) = mock_trace.push_checkpoint(200, 2000); + assert_eq!(prev, 1000); + assert_eq!(new, 2000); +} + +#[test] +fn test_get_latest() { + let mut mock_trace = CONTRACT_STATE(); + + mock_trace.push_checkpoint(100, 1000); + mock_trace.push_checkpoint(200, 2000); + + let latest = mock_trace.get_latest(); + assert_eq!(latest, 2000); +} + +#[test] +fn test_get_at_key() { + let mut mock_trace = CONTRACT_STATE(); + + mock_trace.push_checkpoint(100, 1000); + mock_trace.push_checkpoint(200, 2000); + mock_trace.push_checkpoint(300, 3000); + + let value_at_150 = mock_trace.get_at_key(150); + assert_eq!(value_at_150, 1000); + + let value_at_250 = mock_trace.get_at_key(250); + assert_eq!(value_at_250, 2000); + + let value_at_350 = mock_trace.get_at_key(350); + assert_eq!(value_at_350, 3000); +} + +#[test] +fn test_get_length() { + let mut mock_trace = CONTRACT_STATE(); + + assert_eq!(mock_trace.get_length(), 0); + + mock_trace.push_checkpoint(100, 1000); + assert_eq!(mock_trace.get_length(), 1); + + mock_trace.push_checkpoint(200, 2000); + assert_eq!(mock_trace.get_length(), 2); +} + +#[test] +#[should_panic(expected: ('Unordered insertion',))] +fn test_unordered_insertion() { + let mut mock_trace = CONTRACT_STATE(); + + mock_trace.push_checkpoint(200, 2000); + mock_trace.push_checkpoint(100, 1000); // This should panic +} + +#[test] +fn test_pack_big_key_and_value() { + let key = Bounded::MAX; + let value = Bounded::MAX; + let checkpoint = Checkpoint { key, value }; + + let (key_and_low, high) = StorePacking::pack(checkpoint); + + let expected_key: u256 = (key_and_low.into() / _2_POW_184.into()) & KEY_MASK; + let expected_low: u256 = key_and_low.into() & LOW_MASK; + let expected_high: felt252 = Bounded::::MAX.into(); + + assert_eq!(key.into(), expected_key); + assert_eq!(value.low.into(), expected_low); + assert_eq!(high, expected_high); +} + +#[test] +fn test_unpack_big_key_and_value() { + let key_and_low = Bounded::::MAX.into() * _2_POW_184 + Bounded::::MAX.into(); + let high = Bounded::::MAX.into(); + + let checkpoint: Checkpoint = StorePacking::unpack((key_and_low, high)); + + let expected_key: u64 = Bounded::MAX; + let expected_value: u256 = Bounded::MAX; + + assert_eq!(checkpoint.key, expected_key); + assert_eq!(checkpoint.value, expected_value); +}