CIP-0008: Allow for signing with stake key directly #154
Conversation
|
Let me know if the versioning update I put in here is not correct I am not certain on your version conventions. |
|
Tagging @cffls for review when you have a moment. |
There was a problem hiding this comment.
Hi @thaddeusdiamond , thank you so much for your contribution, and apologies for the delayed response. The PR looks good to me. Also, it would be great if you can add a unit test for it. Thank you!
pyproject.toml
Outdated
| @@ -1,6 +1,6 @@ | |||
| [tool.poetry] | |||
| name = "pycardano" | |||
| version = "0.7.2" | |||
| version = "0.7.3" | |||
There was a problem hiding this comment.
We usually update version in a different workflow outside of feature PR. Let's remove this line of change.
pycardano/cip/cip8.py
Outdated
| if isinstance(signing_key, StakeSigningKey) or isinstance(signing_key, StakeExtendedSigningKey): | ||
| address = Address(payment_part=None, staking_part=verification_key.hash(), network=network) | ||
| else: | ||
| address = Address(payment_part=verification_key.hash(), staking_part=None, network=network) |
There was a problem hiding this comment.
This change looks good to me. Could you add a unit test for it?
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #154 +/- ##
==========================================
+ Coverage 86.44% 86.46% +0.01%
==========================================
Files 26 26
Lines 2752 2755 +3
Branches 651 652 +1
==========================================
+ Hits 2379 2382 +3
Misses 281 281
Partials 92 92
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
No worries on delay just checking in! See the new commit for the unit tests and version removal. |
In the browser, if a user specifies the stake key as part of CIP-0030
the message returned in JSON has the COSE key as the staking key. The
verify() method correctly detects this, but the sign() method does not.
This change detects if a stake SigningKey type was passed in and if so,
sign with a stake key.
Sample behavior:
[00:28:29 01/24 thaddeuss-mbp] hacker:~/workspaces/wildtangz/cardano-vending-machine$ python3
Python 3.8.1 (v3.8.1:1b293b6006, Dec 18 2019, 14:08:53)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pycardano
>>> skey = pycardano.key.StakeSigningKey.load('/var/folders/qz/ldnst4y96_z436s0ptw0vyp40000gn/T/cardano-vm-test-eobtvu6g/buyers/buyer.stake.skey')
>>> sign_addr = pycardano.cip.cip8.verify(pycardano.cip.cip8.sign('hello', skey, attach_cose_key=False, network=pycardano.network.Network.TESTNET))['signing_address']
>>> sign_addr
stake_test1uq5xgs974dna05tjc5pr8sfq5cc4hzludaz7z4e4vpx6p0gkrzzmd
The old behavior resulted in output of 'addr_test1vq5xgs974dna05tjc5pr8sfq5cc4hzludaz7z4e4vpx6p0gktu6v8'
[ Documentation: pydoc updates ]
[ Testing: python -m build, manual test in CLI ]
[ Code Review: Python-Cardano#154 ]
|
Didn't realize you used black, that's why tests failed. Ran black and repushed commit. |
CIP-0008: Allow for signing with stake key directly
In the browser, if a user specifies the stake key as part of CIP-0030
the message returned in JSON has the COSE key as the staking key. The
verify() method correctly detects this, but the sign() method does not.
This change detects if a stake SigningKey type was passed in and if so,
sign with a stake key.
Sample behavior:
[00:28:29 01/24 thaddeuss-mbp] hacker:~/workspaces/wildtangz/cardano-vending-machine$ python3
Python 3.8.1 (v3.8.1:1b293b6006, Dec 18 2019, 14:08:53)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
The old behavior resulted in output of 'addr_test1vq5xgs974dna05tjc5pr8sfq5cc4hzludaz7z4e4vpx6p0gktu6v8'
[ Documentation: pydoc updates ]
[ Testing: python -m build, manual test in CLI ]
[ Code Review: https://github.com//pull/154 ]