-
Notifications
You must be signed in to change notification settings - Fork 40
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
Restructured blinding + custom messages + keep vectors #218
Conversation
more refactoring more refactoring
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.
Looks great! Just some minor nits and questions.
I don't fully understand the keepFactory
and when it used. Can you elaborate a bit more on the logic behind that? My assumption is that this is used as a way to default how the user wants to store their proofs, but I think my confusion is related to my comment here
if (outputData) { | ||
newOutputData = { send: outputData }; | ||
} else if (this._keepFactory) { | ||
newOutputData = { send: this._keepFactory }; |
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.
why are you using the keepFactory for send
?
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.
Good catch. Unfortunately there is a bug in the current version that requires this: #224
Thank you for taking the time to go through this! Using this, you can create a factory that creates some sort of outputs (like deterministic ones) and make the library use is automatically. A simplistic example of a CashuWallet that creates auto-incrementing, deterministic secrets using a keepFactory can be seen here: https://njump.me/nevent1qqsvzxrfqyt5xk0s7f60mm9rlthkdtv70g3qsl8xykrgcvnfdrfpaecpzemhxue69uhhyetvv9ujumn0wd68ytnzv9hxgqg0waehxw309ahx7um5wghx6mmdqgsdmup6e2z6mcpeue6z6kl08he49hcen5xnrc3tnpvw0mdgtjemh0slwxw3x |
Fix deterministic secrets
src/utils.ts
Outdated
@@ -49,7 +49,7 @@ export function splitAmount( | |||
); | |||
} | |||
split.forEach((amt: number) => { | |||
if (!hasCorrespondingKey(amt, keyset)) { | |||
if (!hasCorrespondingKey(amt, keyset) && amt !== 0) { |
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.
Can amt
be zero?
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.
Check out #240
I think this check was redundant. Removed the check in the refactor
SplitAmount Refactor
Description
This PR addresses a couple of things that were discussed in the past: Sorted outputs when swapping, as well as custom outputs. I always felt like the old
BlindingData
type was not ergonomic enough, so I restructured some of the blinding types and logic, moved them to their own Class and Interface. These changes are so closely related, that they became a single PR (that got a little out of hand, sorry...).Changes
outputData
that implements the OutputDataLike Interface. Using this consumers of the library can opt out of the automatic creation of outputs and supply their own logic.PR Tasks
npm run test
--> no failing unit testsnpm run format