Cairo: Use Contracts for Cairo v0.12.0, add ERC20Votes

[ericglau]

• Use Contracts for Cairo v0.12.0
• Use Hooks with ERC20
• Add Votes option to ERC20
• Sort implemented traits for consistency

[ericnordelo]

Looking good! I Left a couple of comments.

[ericnordelo]

Users should be able to set this and DAPP_VERSION as inputs.

[ericglau]

Changed it to use the contract name as the name, and added a separate version input.

Let me know if you think there is a need to allow a different name (i.e. a separate name input specifically for SNIP12) than the contract name.

[ericnordelo]

The order of the embedded implementations changes when the Mintable option is toggled. Not a big deal, but it would be nice to maintain the consistency when using the same components.

[ericglau]

Can you share an example of which embedded impls you are referring to?

[ericglau]

Added sorting of implemented traits, first by number of tags, then by name.

[socket-security]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@polka/url@1.0.0-next.25	None	0	4.48 kB	lukeed
npm/@popperjs/core@2.11.8	None	0	1.46 MB	fezvrasta
npm/@trysound/sax@0.2.0	None	0	48.8 kB	trysound
npm/@types/estree@1.0.5	None	0	25.7 kB	types
npm/@types/semver@7.5.8	None	0	23.3 kB	types
npm/camelcase-css@2.0.1	None	0	4.05 kB	stevenvachon
npm/colord@2.9.3	None	0	114 kB	omgovich
npm/commander@7.2.0	filesystem, shell	0	145 kB	abetomo
npm/css-select@5.1.0	None	+2	268 kB	feedic
npm/css-tree@2.3.1	unsafe	0	1.19 MB	lahmatiy
npm/css-what@6.1.0	None	0	66 kB	feedic
npm/didyoumean@1.2.2	None	0	17.3 kB	dcporter
npm/dlv@1.1.3	None	0	5.8 kB	developit
npm/dom-serializer@2.0.0	None	0	28.8 kB	feedic
npm/domelementtype@2.3.0	None	0	11.4 kB	feedic
npm/domhandler@5.0.3	None	0	75.3 kB	feedic
npm/domutils@3.1.0	network	0	162 kB	feedic
npm/entities@4.5.0	None	0	413 kB	feedic
npm/eventemitter3@4.0.7	None	0	38 kB	lpinca
npm/mdn-data@2.0.30	None	0	602 kB	schalkneethling
npm/postcss-selector-parser@6.0.16	None	+1	203 kB	evilebottnawi
npm/postcss-value-parser@4.2.0	None	0	27.2 kB	evilebottnawi
npm/postcss@8.4.38	environment, filesystem	+2	228 kB	ai
npm/resolve@1.22.8	environment, filesystem	+2	185 kB	ljharb
npm/source-map-js@1.2.0	None	0	140 kB	7rulnik
npm/typescript@5.4.5	None	0	32.4 MB	typescript-bot
npm/yaml@1.10.2	environment	0	448 kB	eemeli

🚮 Removed packages: npm/@alloc/quick-lru@5.2.0, npm/@isaacs/cliui@8.0.2, npm/@jridgewell/gen-mapping@0.3.5, npm/@jridgewell/source-map@0.3.6, npm/@mapbox/node-pre-gyp@1.0.11, npm/@types/istanbul-lib-coverage@2.0.6, npm/@types/node@18.19.31, npm/camelcase@5.3.1, npm/cliui@5.0.0, npm/console-control-strings@1.1.0, npm/cross-spawn@6.0.5, npm/decamelize@1.2.0, npm/find-up@3.0.0, npm/glob@7.2.3, npm/minipass@5.0.0, npm/path-key@2.0.1, npm/string-width@3.1.0, npm/strip-ansi@5.2.0, npm/throat@4.1.0, npm/typescript@4.9.5, npm/wsrun@5.2.4, npm/yargs-parser@13.1.2

View full report↗︎

[andrew-fleming]

Looking good, Eric! I have a suggestion 😅

Now that we have hooks, I'm thinking that we can embed ERC20MixinImpl with pausable enabled and add assert_not_paused in the before_update hook.

This will change the behavior from past Cairo wizard iterations because approvals will be able to go through when paused. Do note that the extension in Solidity also allows approvals when paused. As an added bonus if we agree on the change, the code will be much cleaner. What do you guys think? @ericglau @ericnordelo

[ericglau]

@andrew-fleming That sounds good to me! However, I would prefer for that to be in a separate PR, and perhaps (depending on timing) it may make sense to do this when hooks are available for the other contract kinds. I think the current PR is an improvement over the current state (and enables 0.12.0) and therefore makes sense on its own.

[andrew-fleming]

I would prefer for that to be in a separate PR, and perhaps (depending on timing) it may make sense to do this when hooks are available for the other contract kinds. I think the current PR is an improvement over the current state (and enables 0.12.0) and therefore makes sense on its own.

Sounds good on all accounts!

[ericglau]

@ericnordelo @andrew-fleming I've added SNIP12-specific options as "Application Name" and "Application Version", and added checks to ensure these are not empty string. This PR is ready for re-review.

[ericnordelo]

Looking good Eric! One small detail, I think we could move SNIP12MetadataImpl above the ERC20HooksTraitImpl just for visibility.