-
Notifications
You must be signed in to change notification settings - Fork 359
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
Add ERC721 preset #811
Add ERC721 preset #811
Changes from 71 commits
103375f
7968708
4e09fb6
8da4c00
5cc8a6d
1f29ac1
8d14e0f
18f5f23
3b2259b
4396dcc
c820965
b42744b
aac3940
816a28a
682887c
4499d24
0c9427b
4e58863
e8b696a
4e459f9
c01445f
04be9e9
d89af43
3f5c77a
ab72343
2a853c4
4893df9
3cf4940
74ce94e
1c106a6
0941a61
cf7b7d6
d5feed7
0cc9420
35f144c
e8f6a3a
50e7f26
cd86147
9ee7511
8d0cd9c
974c942
65ef95e
eb9c55c
f0d7a0f
9baca0d
53c5ac5
dbe5f61
81d7971
d518fcf
8094a96
8efe05e
0f5ae41
aa24838
1f13281
2905387
263defe
f917443
a048b8a
b4591ab
baff1e8
dffc3b9
24460d0
5b65a6e
00eaad7
d5d5592
76ba674
6238c1e
b838c61
bad7662
403cc85
9dc86dc
314ba29
3f3e237
cde00a4
b13e02e
bc45dec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
mod account; | ||
mod erc721; | ||
|
||
use account::Account; | ||
use erc721::ERC721; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
// SPDX-License-Identifier: MIT | ||
// OpenZeppelin Contracts for Cairo v0.8.0-beta.0 (presets/erc721.cairo) | ||
|
||
/// # ERC721 Preset | ||
/// | ||
/// The ERC721 contract offers a batch-mint mechanism that | ||
/// can only be executed once upon contract construction. | ||
#[starknet::contract] | ||
mod ERC721 { | ||
use openzeppelin::introspection::src5::SRC5Component; | ||
use openzeppelin::token::erc721::ERC721Component; | ||
use starknet::ContractAddress; | ||
|
||
component!(path: SRC5Component, storage: src5, event: SRC5Event); | ||
component!(path: ERC721Component, storage: erc721, event: ERC721Event); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should swap the components to make ERC721 appear first (this applies to impls below too). That is consistent with the Account Preset as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree in the sense that it's the "main" component, but I switched it because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmm, got your point, but I still think it makes more sense to have the main component first, instead of letting the imports decide. We still need to define the rule for this, but I don't like the one coming from import orders that much 😢 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fair, and we also already have the account preset using the suggested approach. I'll update and we can maybe revisit in the future There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i actually like +1 to not inheriting order from imports There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Let's say we create an ownable, pausable ERC20 preset. There's no component dependency like with SRC5. If we're not using the main component approach and we're not inheriting order from imports, how else can we organize the component declarations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we don't need a rule for that |
||
|
||
// SRC5 | ||
#[abi(embed_v0)] | ||
impl SRC5Impl = SRC5Component::SRC5Impl<ContractState>; | ||
|
||
// ERC721 | ||
#[abi(embed_v0)] | ||
impl ERC721Impl = ERC721Component::ERC721Impl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl ERC721MetadataImpl = ERC721Component::ERC721MetadataImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl ERC721CamelOnly = ERC721Component::ERC721CamelOnlyImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl ERC721MetadataCamelOnly = | ||
ERC721Component::ERC721MetadataCamelOnlyImpl<ContractState>; | ||
impl ERC721InternalImpl = ERC721Component::InternalImpl<ContractState>; | ||
|
||
#[storage] | ||
struct Storage { | ||
#[substorage(v0)] | ||
src5: SRC5Component::Storage, | ||
#[substorage(v0)] | ||
erc721: ERC721Component::Storage | ||
} | ||
|
||
#[event] | ||
#[derive(Drop, starknet::Event)] | ||
enum Event { | ||
#[flat] | ||
SRC5Event: SRC5Component::Event, | ||
#[flat] | ||
ERC721Event: ERC721Component::Event | ||
} | ||
|
||
mod Errors { | ||
const UNEQUAL_ARRAYS: felt252 = 'Array lengths do not match'; | ||
} | ||
|
||
/// Sets the token `name` and `symbol`. | ||
/// Mints the `token_ids` tokens to `recipient` and sets | ||
/// each token's URI. | ||
#[constructor] | ||
fn constructor( | ||
ref self: ContractState, | ||
name: felt252, | ||
symbol: felt252, | ||
recipient: ContractAddress, | ||
token_ids: Span<u256>, | ||
token_uris: Span<felt252> | ||
) { | ||
self.erc721.initializer(name, symbol); | ||
self._mint_assets(recipient, token_ids, token_uris); | ||
} | ||
|
||
/// Mints `token_ids` to `recipient`. | ||
/// Sets the token URI from `token_uris` to the corresponding | ||
/// token ID of `token_ids`. | ||
/// | ||
/// Requirements: | ||
/// | ||
/// - `token_ids` must be equal in length to `token_uris`. | ||
#[generate_trait] | ||
impl InternalImpl of InternalTrait { | ||
fn _mint_assets( | ||
ref self: ContractState, | ||
recipient: ContractAddress, | ||
mut token_ids: Span<u256>, | ||
mut token_uris: Span<felt252> | ||
) { | ||
assert(token_ids.len() == token_uris.len(), Errors::UNEQUAL_ARRAYS); | ||
|
||
loop { | ||
if token_ids.len() == 0 { | ||
break; | ||
} | ||
let id = *token_ids.pop_front().unwrap(); | ||
let uri = *token_uris.pop_front().unwrap(); | ||
|
||
self.erc721._mint(recipient, id); | ||
self.erc721._set_token_uri(id, uri); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
mod test_account; | ||
mod test_erc721; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that it may be easier to add the license with X.Y.Z version, because before releasing it would be easier to find these new files with a quick search, instead of having to implement other mechanisms for not forgetting some. what do you think cc @martriay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we care about new files only? we want to update them all even if they're
v0.7.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean adding the XYZ instead of removing the license line, because files without the license line are harder to find with Ctrl+Shift+F
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but why do we want to identify new files specifically?