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

Waive new key's signature when a privileged payer updates a system account #1164

Merged
merged 8 commits into from Mar 12, 2021

Conversation

anighanta
Copy link
Contributor

@anighanta anighanta commented Mar 11, 2021

Signed-off-by: anighanta anirudh.ghanta@hedera.com

Related issue(s):
Closes #1148

Summary of the change:
wave signing with the new key from cryptoUpdate if payer a special account and updating a system account

External impacts:
System administrators will now be able to update the key on a system account without signing the CryptoUpdate transaction with the new key.

Applicable documentation

Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
@codecov
Copy link

codecov bot commented Mar 11, 2021

Codecov Report

Merging #1164 (08a90d0) into master (f8c8966) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1164      +/-   ##
============================================
- Coverage     87.45%   87.45%   -0.01%     
  Complexity     5277     5277              
============================================
  Files           436      436              
  Lines         15608    15607       -1     
  Branches       1594     1594              
============================================
- Hits          13650    13649       -1     
  Misses         1515     1515              
  Partials        443      443              
Impacted Files Coverage Δ Complexity Δ
...hedera/services/sigs/order/HederaSigningOrder.java 89.77% <100.00%> (-0.03%) 166.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8c8966...08a90d0. Read the comment docs.

Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
@anighanta anighanta marked this pull request as ready for review March 12, 2021 16:30
@anighanta anighanta self-assigned this Mar 12, 2021
@anighanta anighanta added this to the Hedera 0.14.0 milestone Mar 12, 2021
@anighanta anighanta added the Bug An error that causes the feature to behave differently than what was expected based on design. label Mar 12, 2021
Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

Nice work!! There's one redundant line in HederaSigningOrder, and I had some suggestions to make the EET spec more clearly target this feature's impact.

Unit test is great, and this will definitely give the desired functional change.

@tinker-michaelj tinker-michaelj changed the title wave signing if its a special account and updating a system account Waive new key's signature when a privileged payer updates a system account Mar 12, 2021
Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
Signed-off-by: anighanta <anirudh.ghanta@hedera.com>
Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM!

@anighanta anighanta merged commit 9b16ea2 into master Mar 12, 2021
@anighanta anighanta deleted the 01148-M-cryptoUpdate-waive-signing branch March 12, 2021 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An error that causes the feature to behave differently than what was expected based on design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When updating a system account's key, if the account's signature is waived, also waive signing with new key
4 participants