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

sdk: correct static keypair bytes in test sample transaction #32930

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

t-nelson
Copy link
Contributor

Problem

way back when, we incorrectly converted a pkcs8-formatted ed25519 test keypair to raw bytes while porting ed25519 support from ring to ed25519_dalek. i noticed and yolo'd a "fix" into #32926, but nfc if the raw private key bytes were actually a valid private key.

Summary of Changes

  • use the correct keypair bytes from the original pkcs8
  • verify that our "sample" transaction is actually signed correctly

NOTE: i went to the trouble of tracking this down to ensure #32926 wasn't likely to break any existing filesystem keypairs we may have generated in the past.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #32930 (0dcddd8) into master (9d83bb2) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #32930     +/-   ##
=========================================
- Coverage    82.0%    82.0%   -0.1%     
=========================================
  Files         784      784             
  Lines      212449   212450      +1     
=========================================
- Hits       174270   174238     -32     
- Misses      38179    38212     +33     

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Hehe that was a fun little dive into history. Looks good!

@t-nelson t-nelson merged commit 14d0759 into solana-labs:master Aug 22, 2023
@t-nelson t-nelson deleted the fttk branch August 22, 2023 22:19
jumpsiegel pushed a commit to firedancer-io/solana that referenced this pull request Sep 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.

3 participants