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.

Sign up for GitHub

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

Jump to bottom

CIP-0105 | Add missing test vectors #840

Merged
merged 5 commits into from
Jul 5, 2024
Merged

CIP-0105 | Add missing test vectors #840

merged 5 commits into from
Jul 5, 2024
+630 −0

Conversation

paweljakubas
Copy link
Contributor

Work remaining:

  1. To be added : x_sk
  2. To verify if the keys/hashes are correct

@Ryun1 Ryun1 changed the title add test vector to CIP-0105 CIP-0105 | Add test vectors Jun 13, 2024
@rphair rphair added the Category: Wallets Proposals belonging to the 'Wallets' category. label Jun 13, 2024
@paweljakubas paweljakubas changed the title CIP-0105 | Add test vectors CIP-0105 | Add missing test vectors Jun 13, 2024
@paweljakubas paweljakubas mentioned this pull request Jun 13, 2024
Merged
@paweljakubas
add test vector
471ebd3 
spelling fix
@paweljakubas paweljakubas force-pushed the paweljakubas/cip-0105-test-vectors branch from ac989d3 to 471ebd3 Compare June 13, 2024 13:48
@Ryun1
Copy link
Collaborator

Ryun1 commented Jun 13, 2024

Hey @paweljakubas

Thanks for this,
I will go through each test vector and verify and also add missing keys.

Ryun1
Ryun1 reviewed Jun 13, 2024
View reviewed changes
CIP-0105/README.md Outdated Show resolved Hide resolved
rphair
rphair reviewed Jun 13, 2024
View reviewed changes
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Are these valid words in the the mnemonic word set, or maybe introduced from a copy/paste & line-break problem? wearocean turtlepalm stumblemonth

CIP-0105/README.md Outdated Show resolved Hide resolved
CIP-0105/README.md Outdated Show resolved Hide resolved
@paweljakubas paweljakubas requested review from rphair and Ryun1 June 13, 2024 16:12
rphair
rphair reviewed Jun 13, 2024
View reviewed changes
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@paweljakubas looks fine from my own technical level and will ✅ following @Ryun1's endorsing it.

Ryun1
Ryun1 reviewed Jul 5, 2024
View reviewed changes
CIP-0105/README.md Outdated Show resolved Hide resolved
Ryun1
Ryun1 approved these changes Jul 5, 2024
View reviewed changes
Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

This is a great addition to CIP-105
thank you for kicking this off @paweljakubas ✅

Crypto2099
Crypto2099 approved these changes Jul 5, 2024
View reviewed changes
Copy link
Collaborator

@Crypto2099 Crypto2099 left a comment

Choose a reason for hiding this comment

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

Given the feedback and review from @Ryun1 I feel comfortable marking this one as approved and getting changes merged.

rphair
rphair approved these changes Jul 5, 2024
View reviewed changes
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Good all around; let's check it in...

@rphair rphair merged commit 3f6f52e into cardano-foundation:master Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Crypto2099 Crypto2099 Crypto2099 approved these changes

@Ryun1 Ryun1 Ryun1 approved these changes

@rphair rphair rphair approved these changes

Assignees
No one assigned
Labels
Category: Wallets Proposals belonging to the 'Wallets' category.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@paweljakubas @Ryun1 @rphair @Crypto2099