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

Account enhancement #62

Merged
merged 14 commits into from
Feb 24, 2022
Merged

Account enhancement #62

merged 14 commits into from
Feb 24, 2022

Conversation

dribeiro-ShardLabs
Copy link
Contributor

@dribeiro-ShardLabs dribeiro-ShardLabs commented Feb 21, 2022

Add Account class, which serves as a proxy for contract interaction

  • Removed open-zeppelin-cairo-contracts test, which is no longer needed

Copy link

@clacladev clacladev left a comment

Choose a reason for hiding this comment

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

It looks like a real step forward in the account management!

src/account.ts Outdated Show resolved Hide resolved
src/account.ts Outdated Show resolved Hide resolved
src/account.ts Outdated
* @param selector function in the contract to be called
* @param calldata calldata to use as input for the contract call
*/
abstract adaptArgs(toAddress: string, selector: string, calldata: StringMap): any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think adaptArgs doesn't have to be in the abstract class, it's an implementation detail.
Either that, or call/invoke can be template methods already implemented in the abstract class, relying on methods yet to be overriden in child classes (methods like adaptArgs or getExecuteFunction.

src/utils.ts Outdated Show resolved Hide resolved
src/account.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/type-extensions.ts Show resolved Hide resolved
src/extend-utils.ts Show resolved Hide resolved
src/account.ts Show resolved Hide resolved
src/account.ts Outdated Show resolved Hide resolved
src/account.ts Outdated Show resolved Hide resolved
src/constants.ts Outdated Show resolved Hide resolved
@FabijanC FabijanC self-requested a review February 24, 2022 15:46
Copy link
Collaborator

@FabijanC FabijanC left a comment

Choose a reason for hiding this comment

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

Readme corrections can be done with the release of a new version. Can be merged.

@dribeiro-ShardLabs dribeiro-ShardLabs merged commit 95627a5 into master Feb 24, 2022
@FabijanC FabijanC deleted the account-enhancement branch March 7, 2022 15:09
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.

None yet

3 participants