-
Notifications
You must be signed in to change notification settings - Fork 66
Keyset ID V2 #182
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
Open
davidcaseria
wants to merge
56
commits into
cashubtc:main
Choose a base branch
from
davidcaseria:keyset-id-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+544
−50
Open
Keyset ID V2 #182
Changes from all commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
f3f0280
Define Keyset ID V2
davidcaseria 61af756
Address PR feedback
davidcaseria d3d01d6
Fixups
davidcaseria ae8f6d1
Fix error and typo
davidcaseria abae74b
Clarify keyset ID usage in NUT-13
davidcaseria 4668545
Clarify abbreviated keyset id length
davidcaseria 51473dc
keyset final expiration + commit final expiration
a1denvalu3 e936d02
remove note about expiry best practice
a1denvalu3 dbc8042
Merge pull request #1 from lollerfirst/keyset-final-expiry
davidcaseria 9c774ff
Clarify the role of s_id
davidcaseria 07d296f
02-tests
a1denvalu3 2f19d3b
02-test-vectors
a1denvalu3 33e2ff0
13-test-vector
a1denvalu3 06e2522
Merge pull request #3 from lollerfirst/add-v2-test-vectors
davidcaseria eba95c5
Update 13.md
davidcaseria 0f3cef6
Apply suggestions from code review
davidcaseria cf03fc8
new secret derivation
a1denvalu3 bf468d6
critical -> caution
a1denvalu3 336660e
add code examples for new generation
a1denvalu3 42f5ae2
Merge pull request #4 from lollerfirst/new-secret-derivation
davidcaseria 3478261
Merge remote-tracking branch 'upstream/main' into keyset-id-v2
davidcaseria 12dd688
Merge remote-tracking branch 'origin/keyset-id-v2' into keyset-id-v2
davidcaseria fb02a9b
Remove utf-8 encoding from KDF
davidcaseria 57b9db8
Remove VSCode settings
davidcaseria 1bd8c32
Fix prettier
davidcaseria 56fb2ae
Require wallets to retain counter when upgrading from legacy derivation
davidcaseria 69d3fd4
Update NUT-13 to version secret derivation by keyset ID
davidcaseria f05902f
Fix prettier
davidcaseria 419d21d
update secret derivation (again)
a1denvalu3 6d9b255
Merge pull request #5 from lollerfirst/derivation-type-byte
davidcaseria 9118f4b
update 13 test vectors
a1denvalu3 e427448
Update 13.md
davidcaseria 7e15c44
Merge pull request #6 from lollerfirst/update-13-test-vectors
davidcaseria d87f4b2
Fix prettier
davidcaseria 8295a9b
consensus update
a1denvalu3 b25abaa
Apply suggestions from code review
davidcaseria b5b33fa
Merge branch 'keyset-id-v2' into consensus-update
davidcaseria 3789b4f
Merge pull request #7 from lollerfirst/consensus-update
davidcaseria ed78be4
Fix prettier
davidcaseria e1da7b1
Merge remote-tracking branch 'upstream/main' into keyset-id-v2
davidcaseria eed0ae2
update test vectors
a1denvalu3 caca29f
Apply suggestions from code review
davidcaseria 1b6a372
Merge pull request #8 from lollerfirst/update-test-vectors
davidcaseria 6075d35
add final_expiry to GetKeysResponse
d4rp4t 1d1d39f
Merge pull request #9 from d4rp4t/keyset-id-v2
davidcaseria 25e7c35
Update test vectors
d4rp4t 6e785b2
Merge pull request #10 from d4rp4t/keyset-id-v2
davidcaseria ba1626a
revert separators in ID derivation
a1denvalu3 1efb5a4
Merge pull request #11 from lollerfirst/revert-separators
davidcaseria 869d76e
Merge remote-tracking branch 'upstream/main' into keyset-id-v2
davidcaseria 9f5a747
Remove redundant comments
davidcaseria 282ea77
edits + modulo for derivation
callebtc 720e5a5
prettier
callebtc da9fa4d
move short ID to 00.md
callebtc aca39cd
wording
callebtc 97b9573
restore shorter
callebtc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keyset ID derivation doesn't protect/guarantee the amounts have not been tampered with, only that the amounts are still ordered the same.
What stops a mint from changing the amounts bound to each key to devalue and "inflate" away their liabilities?
eg: 1, 2, 4, 8, 16, 32, 64
could become: 1, 2, 3, 4, 8, 16, 32
or maybe even: 1, 1, 1, 1, 1, 1, 1
Perhaps we should concat all the amounts in order too... or maybe just adding the sum of key amounts would be enough to reduce the risk of serious damage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good point. It would change the derivation algorithm and all test vectors etc.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what is the state of implementation and mainly usage of Keyset ID v2, but I think it is worth the change if the usage is non-existent or minimal.
If we will be changing this, I would suggest to include my suggestion which was dismissed earlier: #182 (comment)