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

Restructured blinding + custom messages + keep vectors #218

Merged
merged 54 commits into from
Feb 1, 2025
Merged

Conversation

Egge21M
Copy link
Collaborator

@Egge21M Egge21M commented Nov 30, 2024

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

  • BlindingData is now called OutputData. It holds all the data required to construct an Output from a BlindedMessage and a Signature
  • OutputData (form. BlindingData) is no longer an Array, but now represents a single encapsulated future Proof
  • Wallet methods now accept a new option 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.
  • createSwapPayload now sorts the requested outputs by amount. It returns an additional keepVector (Array) that can be used to determine which outputs are meant to be kept. It also returns a sortedIndices array that can be used to reorder the sorted outputs in their original order.
  • Removed some of the old blinding code. Was internal code, so no breaking changes.

PR Tasks

  • Open PR
  • run npm run test --> no failing unit tests
  • run npm run format

@Egge21M Egge21M changed the title [WIP] Restructured bliding [WIP] Restructured blinding + custom messages + keep vectors Nov 30, 2024
more refactoring

more refactoring
@Egge21M Egge21M changed the title [WIP] Restructured blinding + custom messages + keep vectors Restructured blinding + custom messages + keep vectors Dec 4, 2024
@Egge21M Egge21M marked this pull request as ready for review December 4, 2024 12:58
Copy link
Collaborator

@gudnuf gudnuf left a 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 };
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

@Egge21M
Copy link
Collaborator Author

Egge21M commented Dec 30, 2024

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

Thank you for taking the time to go through this!
The idea of the keepFactory is to provide a standard "fallback" factory to the CashuWallet instance that should be used whenever we create proofs that are supposed to be "kept" (e.g. mint, melt-change, send-change, receive, etc.).

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

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can amt be zero?

Copy link
Collaborator Author

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

@Egge21M Egge21M merged commit 7cb1e80 into development Feb 1, 2025
4 checks passed
@Egge21M Egge21M deleted the blinding branch February 1, 2025 20:01
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.

5 participants