Add `ERC721ABI` and fix other abis #761

andrew-fleming · 2023-09-29T20:53:02Z

Supersedes #741 and fixes #739.

This PR proposes to normalize the interface/impl function order. Furthermore, this PR also proposes to reorder the internal functions to better match the order of their external counterparts

ericnordelo

Small comment, but besides I think it looks good!

ericnordelo · 2023-10-03T11:16:52Z

src/account/account.cairo

    //
    // Internal
    //

    #[generate_trait]
    impl InternalImpl of InternalTrait {
        fn initializer(ref self: ContractState, _public_key: felt252) {
-            let mut unsafe_state = src5_state();
+            let mut unsafe_state = SRC5::unsafe_new_contract_state();


Should we keep this? It should be removed when migrating to component anyway, buy I think src5_state looks better.

Agreed! I think this got mixed up in the merge/release. Good catch

andrew-fleming · 2023-10-03T17:28:58Z

@ericnordelo I included the camelCase methods in the Accounts api (as mentioned here). There's a few camel impls in the Account mod. Listing each camel impl seems a bit noisy IMO. I propose we group them as CAMELCASE-SUPPORT in the api. Wdyt?

ericnordelo · 2023-10-05T21:53:27Z

It looks way better to me than adding each impl. I agree.

ericnordelo

LGTM! Left a really small suggestion, and we need to add the camelCase support to other modules as well, but we can leave that for another PR.

docs/modules/ROOT/pages/api/account.adoc

Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com>

andrew-fleming · 2023-10-10T02:45:31Z

@ericnordelo I updated the Account module so it's exposing external impls (not non-standard fns). I didn't update the ERC20 Non-Standard name because I think this is enough for this PR and we'll have to change some things around anyway with the component migration. We could also just apply these changes in the migration PRs instead. Let me know what you think

martriay

need to fix conflicts and update to latest, other than that it looks good to me!

martriay · 2023-11-25T05:38:02Z

src/token/erc20/interface.cairo

-    fn symbol(self: @TState) -> felt252;
-    fn decimals(self: @TState) -> u8;
+    // Camel case compatibility
+    // See https://docs.openzeppelin.com/contracts-cairo/0.7.0/interfaces#dual_interfaces


Suggested change

// See https://docs.openzeppelin.com/contracts-cairo/0.7.0/interfaces#dual_interfaces

// See https://docs.openzeppelin.com/contracts-cairo/0.8.0-beta.0/interfaces#dual_interfaces

martriay · 2023-11-25T05:41:44Z

src/token/erc721/interface.cairo

+    fn token_uri(self: @TState, token_id: u256) -> felt252;
+
+    // Camel case compatibility
+    // See https://docs.openzeppelin.com/contracts-cairo/0.7.0/interfaces#dual_interfaces


Suggested change

// See https://docs.openzeppelin.com/contracts-cairo/0.7.0/interfaces#dual_interfaces

// See https://docs.openzeppelin.com/contracts-cairo/0.8.0-beta.0/interfaces#dual_interfaces

* bump to 0.8.0 (#834) * Update PULL_REQUEST_TEMPLATE.md (#827) * Update PULL_REQUEST_TEMPLATE.md * Update PULL_REQUEST_TEMPLATE.md * Update PULL_REQUEST_TEMPLATE.md * Update RELEASING and CONTRIBUTING (#828) * update RELEASING * fix linting * update CONTRIBUTING * fix emphasis * add changelog to releasing.md * improve live testing guidelines * Update CONTRIBUTING.md Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com> --------- Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com> * Add `ERC721ABI` and fix other abis (#761) * fix account abi * fix erc20 abi * fix erc721 abi * reorder internal fns * reorder internal fns * fix comment * reorder fns * fix casing * fix casing * reorder api fns * add dual interfaces link * add camel methods to api * remove imports * re-add src5_state * Apply suggestions from code review Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * add deployer trait/impl * change non-standard to impls in fn list * fix in-code doc style * fix comments * fix interface order * remove unused impl --------- Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * Update utility docs (#825) * fix: link (#545) * feat: add utility docs * Update docs/modules/ROOT/pages/utilities.adoc Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update docs/modules/ROOT/pages/utilities.adoc Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update docs/modules/ROOT/pages/utilities.adoc Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates * Update docs/modules/ROOT/pages/utilities.adoc Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates * fix: remove drop events * feat: update CHANGELOG * feat: apply review udpates --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Add in-code docs (#822) * add in-code comment * normalize in-code comments * fix comment * fix comments * fix comments * add double back ticks for comment possessives * add missing reqs * add missing reqs, fix _burn description * fix __execute__ reqs * remove double backticks * remove unused imports * fix: implementation name (#840) * Improve component tests with ComponentState (#836) * refactor: component state in tests * feat: update CHANGELOG * Update CHANGELOG.md Co-authored-by: Martín Triay <martriay@gmail.com> * feat: apply review updates * fix: CHANGELOG --------- Co-authored-by: Martín Triay <martriay@gmail.com> * Document SRC5 migration (#821) * start migration guide * add links, add register_interfaces example * add supports_interfaces * add supports_interfaces to docs * fix formatting * revert change * Apply suggestions from code review Co-authored-by: Martín Triay <martriay@gmail.com> * remove upgrade fn from code block * remove supports_interfaces * add deregister_erc165_interface * change src5 migration title * fix src5 migration doc * fix how-to section * fix formatting * add deregister_erc165_interface * update interface registration section * add link to register_interface * update CHANGELOG * Apply suggestions from code review Co-authored-by: Martín Triay <martriay@gmail.com> * remove register_erc165_interface * remove deregister_erc165_interface from utilities * remove deregister from guide * fix import * remove register_interfaces fn from API * remove unused var * add initializable warning --------- Co-authored-by: Martín Triay <martriay@gmail.com> * Document Class Hashes (#832) * fix: link (#545) * feat: add page * feat: finish UI * Update docs/modules/ROOT/pages/api/erc721.adoc Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates * fix: remove trailing space * feat: update CHANGELOG * Update CHANGELOG.md Co-authored-by: Martín Triay <martriay@gmail.com> * feat: apply review updates * feat: apply review updates --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Co-authored-by: Martín Triay <martriay@gmail.com> * Update extensibility docs (#823) * add extensibility docs * start setup guide * change file name * Apply suggestions from code review Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * fix name * change title * change title to component impl * update changelog * add comp storage section * finish new structure * finish edits, add customization section * simplify titles * finish custom impl section * Apply suggestions from code review Co-authored-by: Martín Triay <martriay@gmail.com> * fix comp storage section * change section title to setup, minor edits * add cmp storage link, clean up impl section * add cairo book link * add api design tip * Apply suggestions from code review --------- Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> Co-authored-by: Martín Triay <martriay@gmail.com> * Update docs navbar (#838) * update docs navbar * complete navbar * fix navbar * fix navbar * update changelog * update changelog * fix changelog * update wizard and rename usage -> components * fix broken links * reordered changelog * fix default collapsed sections --------- Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com> Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>

andrew-fleming added 12 commits September 29, 2023 16:36

fix account abi

3b6a08c

fix erc20 abi

e663320

fix erc721 abi

5471a7d

reorder internal fns

79b044d

reorder internal fns

fc3872b

fix comment

2d279a0

reorder fns

300723f

fix casing

605e454

fix casing

9c2c16d

reorder api fns

71b2bea

add dual interfaces link

b00c017

add camel methods to api

278ccc4

ericnordelo reviewed Oct 3, 2023

View reviewed changes

andrew-fleming added 3 commits October 3, 2023 12:37

remove imports

74d053c

re-add src5_state

e11811a

fix conflicts

9de416c

ericnordelo approved these changes Oct 5, 2023

View reviewed changes

docs/modules/ROOT/pages/api/account.adoc Outdated Show resolved Hide resolved

Apply suggestions from code review

25b2dd8

Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com>

andrew-fleming requested a review from martriay October 6, 2023 17:29

andrew-fleming mentioned this pull request Oct 6, 2023

Migrate Ownable to component #768

Merged

3 tasks

andrew-fleming added 4 commits October 9, 2023 18:50

add deployer trait/impl

05c7936

change non-standard to impls in fn list

e154354

fix in-code doc style

f0a1673

fix comments

50038b4

andrew-fleming mentioned this pull request Oct 17, 2023

Migrate Account to component #783

Merged

3 tasks

martriay reviewed Nov 25, 2023

View reviewed changes

fix conflicts

0e2199a

andrew-fleming added 2 commits November 26, 2023 20:38

fix interface order

f3cf6af

remove unused impl

f9bb313

martriay approved these changes Nov 27, 2023

View reviewed changes

martriay merged commit cd0ce89 into OpenZeppelin:main Nov 27, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ERC721ABI` and fix other abis #761

Add `ERC721ABI` and fix other abis #761

andrew-fleming commented Sep 29, 2023

ericnordelo left a comment

ericnordelo Oct 3, 2023

andrew-fleming Oct 3, 2023

andrew-fleming commented Oct 3, 2023

ericnordelo commented Oct 5, 2023

ericnordelo left a comment

andrew-fleming commented Oct 10, 2023

martriay left a comment

martriay Nov 25, 2023

martriay Nov 25, 2023

	// See https://docs.openzeppelin.com/contracts-cairo/0.7.0/interfaces#dual_interfaces
	// See https://docs.openzeppelin.com/contracts-cairo/0.8.0-beta.0/interfaces#dual_interfaces

Add ERC721ABI and fix other abis #761

Add ERC721ABI and fix other abis #761

Conversation

andrew-fleming commented Sep 29, 2023

ericnordelo left a comment

Choose a reason for hiding this comment

ericnordelo Oct 3, 2023

Choose a reason for hiding this comment

andrew-fleming Oct 3, 2023

Choose a reason for hiding this comment

andrew-fleming commented Oct 3, 2023

ericnordelo commented Oct 5, 2023

ericnordelo left a comment

Choose a reason for hiding this comment

andrew-fleming commented Oct 10, 2023

martriay left a comment

Choose a reason for hiding this comment

martriay Nov 25, 2023

Choose a reason for hiding this comment

martriay Nov 25, 2023

Choose a reason for hiding this comment

Add `ERC721ABI` and fix other abis #761

Add `ERC721ABI` and fix other abis #761