-
Notifications
You must be signed in to change notification settings - Fork 58
refactor: create dedicated native functions for the AA object creation #9485
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
base: vm-lang/aa-auth/8116-feature-branch
Are you sure you want to change the base?
refactor: create dedicated native functions for the AA object creation #9485
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
2d4a719 to
067190c
Compare
miker83z
left a comment
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.
LGTM!
| if current_module | ||
| .value | ||
| .is(&IOTA_ADDR_VALUE, TRANSFER_FUNCTION_NAME) | ||
| .is(&IOTA_ADDR_VALUE, TRANSFER_MODULE_NAME) |
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.
was this a "bug"?
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.
It was a typo, here the module name is checked, it worked because:
TRANSFER_FUNCTION_NAME == TRANSFER_MODULE_NAME == transfer
| "migration", | ||
| "beta", | ||
| "development", | ||
| "account", |
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.
We should think about the usage of simply "account" for a module name, might be limiting in the future.
| ident_str!("borrow_auth_info_v1"), | ||
| ident_str!("has_auth_info_v1"), | ||
| ]; | ||
| pub const PRIVATE_ACCOUNT_FUNCTIONS: &[&IdentStr] = &[ |
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.
As for consideration we can rename private to internal to not confuse these private functions with visibility modifiers at least.
| Ok(()) | ||
| } | ||
|
|
||
| fn verify_private_account( |
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.
An adition to my previous comment above.
Is this function verifies some "private" account or "private" functions that account contains?
I would suggest to add a comment that explains what actually happens here despite of possible renaming.
It's about all similar methods.
Dkwcs
left a comment
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.
LGTM, left comments for consideration
[run-ci]
Description of change
The account creation Move API was simplified by removing
AuthenticatorInfoV1CompatibilityProof.Links to any relevant issues
fixes #9459