-
Notifications
You must be signed in to change notification settings - Fork 353
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 Account preset #803
Merged
martriay
merged 36 commits into
OpenZeppelin:main
from
ericnordelo:feat/add-account-preset
Nov 10, 2023
Merged
Add Account preset #803
Changes from 31 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
331844d
fix: link (#545)
ericnordelo 2e7758e
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts
ericnordelo 77cca2d
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts
ericnordelo 4fc9be5
feat: update logic
ericnordelo ce91ed0
feat: add mocks
ericnordelo 4a90358
docs: update
ericnordelo 7f74150
feat: update docs
ericnordelo bff9179
Update src/account/account.cairo
ericnordelo 4415191
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…
ericnordelo 100c483
feat: apply review updates
ericnordelo b3e2106
Merge branch 'feat/migrate-account-to-component' of github.com:ericno…
ericnordelo 0570e44
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…
ericnordelo 52df8b1
feat: update overview
ericnordelo 9705ce7
Update docs/modules/ROOT/pages/api/account.adoc
ericnordelo feeac44
feat: apply review updates
ericnordelo 0f64992
Merge branch 'feat/migrate-account-to-component' of github.com:ericno…
ericnordelo f9ce9eb
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…
ericnordelo fcdcdf7
feat: flatten events
ericnordelo d12a610
feat: add account preset
ericnordelo 66c3fd7
feat: apply review updates
ericnordelo daa9d68
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…
ericnordelo 495ff94
feat: apply review updates
ericnordelo 472b04c
feat: remove preset
ericnordelo ce3ef47
Revert "feat: remove preset"
ericnordelo 2b32a3e
Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…
ericnordelo 5e03f4c
fix: merge unresolved conflicts
ericnordelo e8d6d89
docs: update
ericnordelo 95dd1a6
Update src/presets/account.cairo
ericnordelo b6628ce
feat: apply review updates
ericnordelo 70088ff
Merge branch 'feat/add-account-preset' of github.com:ericnordelo/cair…
ericnordelo 26b7988
feat: add account preset section
ericnordelo 4dc2a22
Update docs/modules/ROOT/pages/api/account.adoc
ericnordelo af26be2
Update src/presets/account.cairo
ericnordelo ba94871
Update src/presets/account.cairo
ericnordelo b36755f
feat: apply review updates
ericnordelo ed93269
Merge branch 'feat/add-account-preset' of github.com:ericnordelo/cair…
ericnordelo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
mod access; | ||
mod account; | ||
mod introspection; | ||
mod presets; | ||
mod security; | ||
#[cfg(test)] | ||
mod tests; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
mod account; | ||
|
||
use account::Account; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// SPDX-License-Identifier: MIT | ||
// OpenZeppelin Contracts for Cairo v0.8.0-beta.0 (account/presets/account.cairo) | ||
ericnordelo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// # Account Preset | ||
/// | ||
/// OpenZeppelin's basic account which can declare or invoke other contracts. | ||
ericnordelo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[starknet::contract] | ||
mod Account { | ||
use openzeppelin::account::AccountComponent; | ||
use openzeppelin::introspection::src5::SRC5Component; | ||
|
||
component!(path: AccountComponent, storage: account, event: AccountEvent); | ||
component!(path: SRC5Component, storage: src5, event: SRC5Event); | ||
|
||
// Account | ||
#[abi(embed_v0)] | ||
impl SRC6Impl = AccountComponent::SRC6Impl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl SRC6CamelOnlyImpl = AccountComponent::SRC6CamelOnlyImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl PublicKeyImpl = AccountComponent::PublicKeyImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl PublicKeyCamelImpl = AccountComponent::PublicKeyCamelImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl DeclarerImpl = AccountComponent::DeclarerImpl<ContractState>; | ||
#[abi(embed_v0)] | ||
impl DeployableImpl = AccountComponent::DeployableImpl<ContractState>; | ||
impl AccountInternalImpl = AccountComponent::InternalImpl<ContractState>; | ||
martriay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// SRC5 | ||
#[abi(embed_v0)] | ||
impl SRC5Impl = SRC5Component::SRC5Impl<ContractState>; | ||
|
||
#[storage] | ||
struct Storage { | ||
#[substorage(v0)] | ||
account: AccountComponent::Storage, | ||
#[substorage(v0)] | ||
src5: SRC5Component::Storage | ||
} | ||
|
||
#[event] | ||
#[derive(Drop, starknet::Event)] | ||
enum Event { | ||
#[flat] | ||
AccountEvent: AccountComponent::Event, | ||
#[flat] | ||
SRC5Event: SRC5Component::Event | ||
} | ||
|
||
#[constructor] | ||
fn constructor(ref self: ContractState, public_key: felt252) { | ||
self.account.initializer(public_key); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
mod test_account; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
don't we want to update the line number? by the way, these links are broken across the API page, in
Account Component
andPreset
.I also realized
IDeclarer
,IDeployer
, as well as camel only traits are not documented, only the implsThere 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 think ew shouldn't, because that's very hard to maintain, when updating a file we would need to look in the doc if a line was affected, and I don't think is worth the benefit. Maybe I'm wrong.
The only link I see broken is the one for the preset, because that file is not in 0.8.0-beta.0, which will be fixed after releasing (maybe I should update to the X.Y.Z until that point? ). Are there other broken links?
true
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.
Do you think it is worth adding them? That would make the API Reference grow bigger without adding much real value IMO (in this particular case).
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.
True, but without the line to me it makes no sense to link in each section. I think the best experience for our docs is to have direct links (i find them very useful), probably they should not be updated manually in each PR, we could:
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'm not sure. I don't think it'll grow much since I expect most of it to be moved from the component and the component ro look similar to the preset. I don't have strong feelings, I just think it's weird to not mention them since they're referenced (e.g. in the impls). I guess it's good for users to know they're available.
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.
All the source code links, they're not broken links, they're just pointing to the wrong lines.