Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to multiple packages (workspace) #1065

Merged
merged 51 commits into from
Aug 2, 2024

Conversation

ericnordelo
Copy link
Member

@ericnordelo ericnordelo commented Jul 26, 2024

Fixes #734

PR Checklist

  • Tests
  • Documentation
  • Added entry to CHANGELOG.md
  • Tried the feature on a public network

ericnordelo and others added 24 commits June 28, 2024 11:33
* feat: migrate event tests

* feat: update workflow

* feat: add foundry utils

* feat: finish erc20 and erc20votes migration

* feat: remove dual case tests

* refactor: format files

* refactor: variable name

* Update src/tests/token.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/utils/foundry.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20/test_erc20_votes.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* feat: apply review updates

---------

Co-authored-by: Andrew Fleming <[email protected]>
* Update gitignore with snfoundry cache dir

* Delete foundry utils file

* Add snfoundry-related test utils, comment out previous utils

* Update ERC20 event helpers

* Update ERC20 tests

* Update ERC20Votes tests

* Run linter

* Add util function to drop single event

* Fix review issues

* Make Github workflow run on every PR

* Fix review issues

* Bump snforge version

* Support snforge 0.26 in test utils

* Update ERC20 tests to support snforge upgrade

* Run linter
* feat: migrate event tests

* feat: update workflow

* feat: add foundry utils

* feat: finish erc20 and erc20votes migration

* feat: remove dual case tests

* refactor: format files

* refactor: variable name

* Update src/tests/token.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/utils/foundry.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20/test_erc20_votes.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* feat: apply review updates

* feat: readd dual20 tests

* refactor: remove unnecessary import

* feat: add dual20 import

---------

Co-authored-by: Andrew Fleming <[email protected]>
* migrate pausable and initializable tests

* migrate reentrancy guard tests

* clean up tests

* fix fmt

* add event trait to pausable, fix tests

* Apply suggestions from code review

Co-authored-by: Eric Nordelo <[email protected]>

* fix fmt

---------

Co-authored-by: Eric Nordelo <[email protected]>
* feat: update common module

* feat: migrate dual modules

* feat: finish component test migration

* refactor: remove unnecessary imports

* feat: apply review updates
* feat: migrate event tests

* feat: update workflow

* feat: add foundry utils

* feat: finish erc20 and erc20votes migration

* feat: remove dual case tests

* refactor: format files

* refactor: variable name

* Update src/tests/token.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/utils/foundry.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/token/erc20/test_erc20_votes.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* feat: apply review updates

* feat: migrate erc721 components tests

* refactor: remove extra line

* feat: apply review updates

* feat: update utilities

* feat: apply review updates

* fix: ignore tests

---------

Co-authored-by: Andrew Fleming <[email protected]>
* test: check ignored tests and add reason

* feat: apply review updates
* Update ERC20Votes tests

* Run linter

* Add test helpers for Ownable tests

* Migrate Ownable tests

* Migrate OwnableTwoStep tests

* Migrate Ownable Dual Dispatcher tests

* Support event changes in snforge 0.26

* Resolve review issues

* Address review comments
* Migrate Upgrades tests to Foundry

* Update src/tests/upgrades/test_upgradeable.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* Update src/tests/upgrades/test_upgradeable.cairo

Co-authored-by: Andrew Fleming <[email protected]>

---------

Co-authored-by: Eric Nordelo <[email protected]>
Co-authored-by: Andrew Fleming <[email protected]>
* feat: include erc20 preset tests

* feat: tests updated

* feat: remove unused imports

* feat: add deployments

* Update src/tests/utils/common.cairo

Co-authored-by: Andrew Fleming <[email protected]>

* feat: apply review updates

---------

Co-authored-by: Andrew Fleming <[email protected]>
* feat: update tests

* refactor: remove unnecessary import
* fix import name

* migrate cryptography tests

* migrate udc tests

* fix fmt

* fix impl name

* fix event assertions

* re-add class hash helper
* feat: update dual_eth_account tests

* feat: update eth_account tests

* refactor: remove unused helpers

* refactor: some inconsistencies

* Update src/tests/account/ethereum/test_eth_account.cairo

Co-authored-by: immrsd <[email protected]>

* feat: apply review updates

* refactor: format files

* Update src/tests/account/ethereum/test_eth_account.cairo

Co-authored-by: Andrew Fleming <[email protected]>

---------

Co-authored-by: immrsd <[email protected]>
Co-authored-by: Andrew Fleming <[email protected]>
* feat: update dual_eth_account tests

* feat: update eth_account tests

* refactor: remove unused helpers

* feat: update eth account preset tests

* Update src/tests/account/ethereum/test_eth_account.cairo

Co-authored-by: immrsd <[email protected]>

* Update src/tests/presets/test_eth_account.cairo

Co-authored-by: immrsd <[email protected]>

* feat: apply review updates

---------

Co-authored-by: immrsd <[email protected]>
* Migrate AccessControl tests

* Migrate AccessControl Dual Dispatcher tests

* Fix imports

* Address review comments

* Update error messages

* Bring back separator line

* Fix ignore reasons in access module

* Fix review issues
* Update test utils and helpers

* Update signature tests

* Update account tests

* Update dual account tests

* Run linter

* Run linter

* Fix review issues

* Update ignore reason messages

* Run linter

* Support eth account tests changes

* Run linter

* Improve setup functions, remove unused imports

* Remove unnecessary accept_ownership step, make use of serialized_sign fn
@ericnordelo ericnordelo marked this pull request as ready for review July 29, 2024 11:52
@ericnordelo
Copy link
Member Author

  • The temporary Universal Sierra Compiler installation in the workflow should be removed before merging.
  • Depends on the snforge-migration branch.
  • Documentation is still missing.

@ericnordelo ericnordelo mentioned this pull request Jul 29, 2024
4 tasks
* Update test utils and helpers

* Update signature tests

* Update account tests

* Update dual account tests

* Run linter

* Run linter

* Fix review issues

* Update ignore reason messages

* Run linter

* Support eth account tests changes

* Run linter

* Improve setup functions, remove unused imports

* Remove unnecessary accept_ownership step, make use of serialized_sign fn

* Migrate Starknet Account Preset tests, make some code improvements

* Run linter

* Remove unnecessary clone

* Update the comment for assert_entrypoint_not_found_error function

* Minor review fixes
@ericnordelo ericnordelo mentioned this pull request Jul 31, 2024
4 tasks
…n#1076)

* Remove unnecessary `mut`, minor fixes (OpenZeppelin#1032)

* clean up code

* change array syntax

* fix fmt

* Improve SNIP12 examples (OpenZeppelin#1036)

* docs: improve examples

* refactor: remove empty line

* Update docs/modules/ROOT/pages/guides/snip12.adoc

Co-authored-by: Andrew Fleming <[email protected]>

---------

Co-authored-by: Andrew Fleming <[email protected]>

* Release v0.15.0-rc.0 (OpenZeppelin#1039)

* feat: update CHANGELOG

* Bump version to 0.15.0-rc.0

* docs: add missing header

* feat: apply review suggestions

---------

Co-authored-by: ericnordelo <[email protected]>

* Bump cairo to 2.7.0-rc.1 (OpenZeppelin#1025)

* feat: migrate modules

* feat: delete cairo_project.toml

* feat: update tests

* feat: apply review updates

* fix: remove extra line

* feat: add CHANGELOG entry

* fix: CHANGELOG

* Bump Scarb to 2.7.0-rc.2 (OpenZeppelin#1052)

* bump scarb to 2.7.0-rc.2

* add changelog entry

* update changelog entry

* bump scarb in installation page

* Add timelock component (OpenZeppelin#996)

* start timelock comp draft

* move timelock to own dir, finish drafting component

* tmp: add utility impls

* add timelock mock

* add test mod for timelock

* fix commnet

* add constructor to timelock mock

* set min_delay in initializer

* start tests

* fix fmt

* fix schedule assertion

* add schedule tests

* fix fmt

* remove unused import

* fix errs, _before_call

* start execute tests

* fix fmt

* fix after_call

* add execute tests

* add abi interface

* add reentrancy mock for timelock

* add tests for cancel and update_delay

* fix hash_op test

* add execute with predecessor test

* add timelock utils, add operation_state debug impl

* fix fmt

* improve imports

* improve _execute

* add basic mock for tests

* add tmp call struct

* add batch fns to interface

* add batch fns

* refactor tests to use dedicated mock, add batch tests

* fix fmt

* remove use clause

* add timelock mixin

* fix interface name

* improve event assertions

* fix execute and schedule events

* fix fmt

* add safe token transfer tests

* fix fmt

* tidy up code

* add descriptions to events

* clean up code

* inline CallPartialEq fns

* start fn descriptions

* remove comments

* remove comment

* fix fmt

* add changelog entries

* improve spacing

* add line break to hash test

* clean up tests

* clean up tests

* fix constants in attacker impl

* add initializer helper, register access control support

* add _before_call and _after_call tests

* fix reentrant batch mock call

* add _schedule and _execute tests

* add timelock description

* fix formatting

* fix comments

* fix comment

* fmt

* tidy up tests

* remove batch helper fn

* remove event from mocks

* Apply suggestions from code review

Co-authored-by: Eric Nordelo <[email protected]>

* update spdx

* remove token receiver support

* add additional cancel tests

* Apply suggestions from code review

Co-authored-by: Eric Nordelo <[email protected]>

* initializer: remove mut, use while loop

* fix fmt

* add assert_only_self fn

* fix getter comments re: pending/waiting

* add assert_only_role

* add specific op errors

* make event names consistent

* fix test

* remove serialization from HashCallImpl

* remove unused components from mock

* clean up code

* update to 2.7.0-rc.1

* fix fmt

* import Call from corelib

* update spdx

* fix fmt

* move OperationState, derive debug

* fix fmt

* fix hash impls

* add for loops

* fix PartialEq, add tests

* fix fmt

* simplify mixin fns

* switch Poseidon to Pedersen

* make admin a req in initializer

* update tests with admin

* fix fmt

* fix comment

* undo changes

* add no admin initializer test

---------

Co-authored-by: Eric Nordelo <[email protected]>

* bump scarb to 2.7.0-rc.4 (OpenZeppelin#1064)

* bump scarb to 2.7.0-rc.4

* add changelog

* bump scarb in installation page

* feat: update workflow and fix warning (OpenZeppelin#1066)

* Add actionlint (OpenZeppelin#1067)

* add actionlint for workflows

* remove codecov and gitmodules

* bump checkout to v4, add double quotes, fmt

* bump md lint

* re-add cairo version

* group redirects

* fix link in security

* add local actionlint matcher json

* re-add changelog entry

* add tmp usc install to ci

* fix ci

---------

Co-authored-by: Eric Nordelo <[email protected]>
Co-authored-by: ericnordelo <[email protected]>
Co-authored-by: JChoy <[email protected]>
Copy link
Collaborator

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work on this! Structure-wise, I think this is good to go. I didn't take a deep look at the tests as I assume they match the snforge migration branch. Something tells me that the old testing pattern with common/ might not be the best approach since we have multiple packages now, but that can be reassessed in the future. Aside from that, I left some comments :)

README.md Show resolved Hide resolved
docs/modules/ROOT/pages/components.adoc Outdated Show resolved Hide resolved
docs/modules/ROOT/pages/components.adoc Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitting the issues in this doc because we're handling utils documentation in another PR

packages/access/src/accesscontrol/dual_accesscontrol.cairo Outdated Show resolved Hide resolved
packages/account/src/tests.cairo Outdated Show resolved Hide resolved
packages/governance/src/tests.cairo Outdated Show resolved Hide resolved
packages/token/src/erc1155/erc1155.cairo Outdated Show resolved Hide resolved
pub(crate) mod common;
pub mod common;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth rethinking if we should have a public common/ specifically for tests given the new structure. Not a pressing issue ATM though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, let's just not delay merging this PR because of it, and we can revisit it in the future.

andrew-fleming and others added 9 commits July 31, 2024 21:56
* Remove unnecessary `mut`, minor fixes (OpenZeppelin#1032)

* clean up code

* change array syntax

* fix fmt

* Improve SNIP12 examples (OpenZeppelin#1036)

* docs: improve examples

* refactor: remove empty line

* Update docs/modules/ROOT/pages/guides/snip12.adoc

Co-authored-by: Andrew Fleming <[email protected]>

---------

Co-authored-by: Andrew Fleming <[email protected]>

* Release v0.15.0-rc.0 (OpenZeppelin#1039)

* feat: update CHANGELOG

* Bump version to 0.15.0-rc.0

* docs: add missing header

* feat: apply review suggestions

---------

Co-authored-by: ericnordelo <[email protected]>

* Bump cairo to 2.7.0-rc.1 (OpenZeppelin#1025)

* feat: migrate modules

* feat: delete cairo_project.toml

* feat: update tests

* feat: apply review updates

* fix: remove extra line

* feat: add CHANGELOG entry

* fix: CHANGELOG

* Bump Scarb to 2.7.0-rc.2 (OpenZeppelin#1052)

* bump scarb to 2.7.0-rc.2

* add changelog entry

* update changelog entry

* bump scarb in installation page

* Add timelock component (OpenZeppelin#996)

* start timelock comp draft

* move timelock to own dir, finish drafting component

* tmp: add utility impls

* add timelock mock

* add test mod for timelock

* fix commnet

* add constructor to timelock mock

* set min_delay in initializer

* start tests

* fix fmt

* fix schedule assertion

* add schedule tests

* fix fmt

* remove unused import

* fix errs, _before_call

* start execute tests

* fix fmt

* fix after_call

* add execute tests

* add abi interface

* add reentrancy mock for timelock

* add tests for cancel and update_delay

* fix hash_op test

* add execute with predecessor test

* add timelock utils, add operation_state debug impl

* fix fmt

* improve imports

* improve _execute

* add basic mock for tests

* add tmp call struct

* add batch fns to interface

* add batch fns

* refactor tests to use dedicated mock, add batch tests

* fix fmt

* remove use clause

* add timelock mixin

* fix interface name

* improve event assertions

* fix execute and schedule events

* fix fmt

* add safe token transfer tests

* fix fmt

* tidy up code

* add descriptions to events

* clean up code

* inline CallPartialEq fns

* start fn descriptions

* remove comments

* remove comment

* fix fmt

* add changelog entries

* improve spacing

* add line break to hash test

* clean up tests

* clean up tests

* fix constants in attacker impl

* add initializer helper, register access control support

* add _before_call and _after_call tests

* fix reentrant batch mock call

* add _schedule and _execute tests

* add timelock description

* fix formatting

* fix comments

* fix comment

* fmt

* tidy up tests

* remove batch helper fn

* remove event from mocks

* Apply suggestions from code review

Co-authored-by: Eric Nordelo <[email protected]>

* update spdx

* remove token receiver support

* add additional cancel tests

* Apply suggestions from code review

Co-authored-by: Eric Nordelo <[email protected]>

* initializer: remove mut, use while loop

* fix fmt

* add assert_only_self fn

* fix getter comments re: pending/waiting

* add assert_only_role

* add specific op errors

* make event names consistent

* fix test

* remove serialization from HashCallImpl

* remove unused components from mock

* clean up code

* update to 2.7.0-rc.1

* fix fmt

* import Call from corelib

* update spdx

* fix fmt

* move OperationState, derive debug

* fix fmt

* fix hash impls

* add for loops

* fix PartialEq, add tests

* fix fmt

* simplify mixin fns

* switch Poseidon to Pedersen

* make admin a req in initializer

* update tests with admin

* fix fmt

* fix comment

* undo changes

* add no admin initializer test

---------

Co-authored-by: Eric Nordelo <[email protected]>

* fix fmt

* bump scarb to 2.7.0-rc.4 (OpenZeppelin#1064)

* bump scarb to 2.7.0-rc.4

* add changelog

* bump scarb in installation page

* fix fmt

* feat: update workflow and fix warning (OpenZeppelin#1066)

* bump scarb

* add tmp usc install in ci

* fix changelog

---------

Co-authored-by: Eric Nordelo <[email protected]>
Co-authored-by: ericnordelo <[email protected]>
Co-authored-by: JChoy <[email protected]>
Copy link
Collaborator

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I just left a small question

Comment on lines 5 to 6
branches:
- main
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put this back?

@andrew-fleming andrew-fleming mentioned this pull request Aug 1, 2024
4 tasks
@ericnordelo ericnordelo merged commit 4f1d539 into OpenZeppelin:main Aug 2, 2024
7 checks passed
@ericnordelo ericnordelo deleted the workspace-conversion branch August 2, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: use Scarb workspaces
4 participants