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

Parse keystore address without 0x prefix, fix parse error logging #2759

Merged
merged 4 commits into from
Feb 17, 2023

Conversation

eliteprox
Copy link
Contributor

@eliteprox eliteprox commented Feb 17, 2023

What does this pull request do? Explain your changes. (required)
This resolves a bug from #2713 when parsing keystore addresses without the 0x prefix. Errors from parseEthKeystorePath were also not being logged, because err was only available within the scope of if err == nil.

Specific updates (required)

  • Updated logic flow in starter.go.
  • Changed ethcommon.HexToAddress to ethcommon.BytesToAddress(ethcommon.FromHex(address)) in parseEthKeystorePath().
    • ethcommon.FromHex considers the 0x prefix optional.

How did you test each of these updates (required)

  • Validated error logging by performing a functional test with valid/invalid JSON, file not found, and bad JSON format
  • Validated address parsing with/without 0x prefix.
  • Updated unit test TestParse_ParseEthKeystorePathValidFile

Does this pull request close any open issues?
Fixes #2757

Checklist:

@eliteprox eliteprox changed the title Log errors from parseEthKeystorePath Log any errors from parseEthKeystorePath Feb 17, 2023
@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Merging #2759 (5f0344d) into master (a8f3f4e) will decrease coverage by 0.00294%.
The diff coverage is 25.00000%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #2759         +/-   ##
===================================================
- Coverage   56.30182%   56.29888%   -0.00294%     
===================================================
  Files             88          88                 
  Lines          19177       19178          +1     
===================================================
  Hits           10797       10797                 
- Misses          7788        7789          +1     
  Partials         592         592                 
Impacted Files Coverage Δ
cmd/livepeer/starter/starter.go 4.28954% <25.00000%> (-0.00384%) ⬇️

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 2f467db...5f0344d. Read the comment docs.

Impacted Files Coverage Δ
cmd/livepeer/starter/starter.go 4.28954% <25.00000%> (-0.00384%) ⬇️

@eliteprox eliteprox changed the title Log any errors from parseEthKeystorePath Parse keystore address without 0x prefix, fix parsing error logging Feb 17, 2023
@eliteprox eliteprox changed the title Parse keystore address without 0x prefix, fix parsing error logging Parse keystore address without 0x prefix, fix parse error logging Feb 17, 2023
@eliteprox eliteprox marked this pull request as ready for review February 17, 2023 07:05
Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @eliteprox

@leszko leszko merged commit 8ac6002 into livepeer:master Feb 17, 2023
This was referenced Aug 8, 2023
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.

EthKeystorePath fails if pointed to a file instead of a directory in 0.5.38
2 participants