-
Notifications
You must be signed in to change notification settings - Fork 86
feat: Allow PublicKey for TokenCreateTransaction keys #736
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: main
Are you sure you want to change the base?
feat: Allow PublicKey for TokenCreateTransaction keys #736
Conversation
exploreriii
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.
I think you need to unit and integration test the new functionality you've added - not just the existing functionality
For example
test if you can pass a public key and this will generate the token and convert to proto correctly
No tests for edge cases
I think we can expect ed2559 keys to have a problem with this method
65ab0d7 to
3af0f20
Compare
|
missing edge cases please |
Signed-off-by: Adityarya11 <[email protected]>
3af0f20 to
a7f0fa4
Compare
|
@skurzyp-arianelabs requesting your opinion please |
|
@exploreriii @Adityarya11 The implementation looks good to me, but I would recommend adding some testcases for
for setting the keys for the token. Also, just to unify across the codebase I would recommend aligning the |
|
Thanks for your ideas. I'm working at the moment maybe @nadineloepfe or @manishdait are available, else i can create it likely in a few hours. |
|
@manishdait created the issue, thank you so much |
|
Hi @Adityarya11 I'll be releasing a new python sdk release soon - |
Description:
This PR updates
TokenCreateTransactionto acceptPublicKeyobjects for all key fields, in addition to the existingPrivateKey. This is a crucial feature to enable non-custodial transaction building.With this change, an application (like an agent) can construct a transaction using a user's
PublicKey, serialize it, and then return the bytes to the user for them to sign with theirPrivateKey.This implementation follows the exact pattern already established in
TopicCreateTransaction.py, as suggested by the maintainer.Changes:
Key = Union[PrivateKey, PublicKey]type alias.TokenKeysdataclass to use the newKeytype.set_*_keymethods to accept theKeytype._to_proto_keyhelper to correctly handle bothPrivateKey(by getting its public key) andPublicKey(by using it directly).Related issue(s):
Fixes #735
also fixes
#104
Checklist