-
Notifications
You must be signed in to change notification settings - Fork 13
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
Rollup anchor for ink! smart contracts #30
Conversation
Merge from main repo
Thanks for the contribution! The Phala team is at ETHcc event. So the review and communication may take longer than expected. I will start reviewing this PR from Monday. |
- All methods in a 'Openbrush Trait' are callable (#[ink::message]) - All inner methods are in a Trait named Internal and are not callable
…bute macro `openbrush::trait_definition`
…sion for ink! smart contract)
Sorry for the very late reply. Was too busy in preparing hackathon, etc. I'm reviewing it right now. |
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.
Not finished. Just push some comments I've already made.
ink/crates/phat_rollup_anchor_ink/src/traits/meta_transaction.rs
Outdated
Show resolved
Hide resolved
…ay protection. Remove unnecessary "public key registering".
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. High quality and very responsive delivery!
} | ||
|
||
/// Converts a compressed public key to SS58 format | ||
fn pub_key_to_ss58(pub_key: &[u8; 33]) -> AccountId { |
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.
Nit: AccountId
is just [u8; 32]
under the hood. SS58 only kicks in when it's displayed on the frontend. So I suggest to name it get_ecdsa_account_id()
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 renamed the function
@@ -337,12 +339,21 @@ impl<'a> SubmittableRollupTx<'a> { | |||
} | |||
} | |||
|
|||
/// Hashing function for bytes | |||
fn pub_key_to_ss58(input: &[u8]) -> [u8; 32] { |
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.
Same
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 renamed the function
rollup anchor for ink! smart contracts
simple phat contract for the tests
simple ink smart contract for the tests