Skip to content
This repository was archived by the owner on Oct 31, 2024. It is now read-only.

fix: private key parsing #324

Merged
merged 2 commits into from
Oct 5, 2023
Merged

fix: private key parsing #324

merged 2 commits into from
Oct 5, 2023

Conversation

Freyskeyd
Copy link
Member

Description

This PR is fixing an issue while trying to parse the private key into a LocalWallet, instead of trying to parse bytes into ut8 str I directly use the bytes to generate the wallet.

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@Freyskeyd Freyskeyd requested review from Nashtare and a team as code owners October 4, 2023 10:32
@Freyskeyd Freyskeyd requested review from dvdplm and atanmarko October 4, 2023 10:32
@Freyskeyd Freyskeyd force-pushed the fix/private-key-parsing branch from 2918250 to 2247f2c Compare October 4, 2023 10:35
@Freyskeyd Freyskeyd changed the title fix: fix private key parsing fix: private key parsing Oct 4, 2023
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

This seems like a different take on #323.

I think this one is better.

@atanmarko
Copy link
Member

Duplicate of #323

@atanmarko atanmarko marked this as a duplicate of #323 Oct 4, 2023
@atanmarko
Copy link
Member

This seems like a different take on #323.

I think this one is better.

Ok lets then take this one

@atanmarko atanmarko mentioned this pull request Oct 4, 2023
4 tasks
@atanmarko
Copy link
Member

This seems like a different take on #323.

I think this one is better.

I think it is cleaner for testing to stay with string representation of keys

@Freyskeyd Freyskeyd force-pushed the fix/private-key-parsing branch 2 times, most recently from b0fd8b9 to fdb1fa9 Compare October 4, 2023 13:04
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (705b97e) 60.52% compared to head (9fe601e) 60.55%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
+ Coverage   60.52%   60.55%   +0.02%     
==========================================
  Files         225      225              
  Lines       12049    12054       +5     
==========================================
+ Hits         7293     7299       +6     
+ Misses       4756     4755       -1     
Files Coverage Δ
crates/topos-crypto/src/messages.rs 93.10% <100.00%> (+0.79%) ⬆️
crates/topos-crypto/tests/messages.rs 100.00% <ø> (ø)
crates/topos-tce-broadcast/src/tests/mod.rs 100.00% <100.00%> (ø)
crates/topos-test-sdk/src/tce/protocol.rs 100.00% <100.00%> (ø)
crates/topos/src/lib.rs 67.83% <ø> (ø)
crates/topos-tce/src/lib.rs 0.00% <0.00%> (ø)
crates/topos/src/components/tce/mod.rs 30.07% <0.00%> (-1.18%) ⬇️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@atanmarko
Copy link
Member

atanmarko commented Oct 4, 2023

@Freyskeyd @dvdplm This PR grows unnecessary complex. Could we switch back to much simpler variant #323?

EDIT: I take back this observation. Lets merge this

Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
@Freyskeyd Freyskeyd force-pushed the fix/private-key-parsing branch from fdb1fa9 to 5640f97 Compare October 4, 2023 13:37
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
@Freyskeyd Freyskeyd force-pushed the fix/private-key-parsing branch from df93320 to 9fe601e Compare October 5, 2023 08:56
@Freyskeyd Freyskeyd merged commit ae3edc9 into main Oct 5, 2023
@Freyskeyd Freyskeyd deleted the fix/private-key-parsing branch October 5, 2023 09:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants