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

removing rlp-encoded private key string to use flow.AccountPrivateKey #1525

Merged
merged 1 commit into from Oct 26, 2021

Conversation

j1010001
Copy link
Member

@j1010001 j1010001 commented Oct 25, 2021

Testing performed:

  • running unit tests locally
  • running integration tests locally
  • running integration load test

@j1010001 j1010001 changed the title removing rlp-encoded private key string and using flow.AccountPrivate… removing rlp-encoded private key string to use flow.AccountPrivateKey Oct 25, 2021
@j1010001 j1010001 requested a review from Kay-Zee October 25, 2021 22:42
@codecov-commenter
Copy link

Codecov Report

Merging #1525 (8369c6b) into master (e73e19a) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1525      +/-   ##
==========================================
- Coverage   55.18%   55.13%   -0.05%     
==========================================
  Files         521      521              
  Lines       32527    32527              
==========================================
- Hits        17950    17935      -15     
- Misses      12168    12184      +16     
+ Partials     2409     2408       -1     
Flag Coverage Δ
unittests 55.13% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
model/flow/account_encoder.go 30.95% <0.00%> (-11.91%) ⬇️
module/component/component.go 37.86% <0.00%> (-2.92%) ⬇️
admin/command_runner.go 77.19% <0.00%> (-1.76%) ⬇️
engine/collection/synchronization/engine.go 62.90% <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 e73e19a...8369c6b. Read the comment docs.

Copy link
Member

@Kay-Zee Kay-Zee left a comment

Choose a reason for hiding this comment

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

Nice! this brings us more inline with how more of our other tools represent the private key (i.e. the private key bytes separately from the sig/hash algs)

This feels much better to me inter-operability, with, for example, the CLI.

Copy link
Contributor

@janezpodhostnik janezpodhostnik left a comment

Choose a reason for hiding this comment

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

Nice change!

@j1010001 j1010001 merged commit f943eeb into master Oct 26, 2021
@j1010001 j1010001 deleted the replace_rlp_encoded_private_key branch October 26, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants