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

Add fees and coin selection #159

Merged
merged 79 commits into from
Oct 27, 2024
Merged

Add fees and coin selection #159

merged 79 commits into from
Oct 27, 2024

Conversation

callebtc
Copy link
Contributor

@callebtc callebtc commented Jul 26, 2024

Introduces support for fees for spending ecash with the mint. Fees are published in the /v1/keysets response (see NUT-02). Also introduces a new input coinselection algorithm to minimize fees when spending coins and an output selection, to maintain a diverse set of amounts in the user's wallet, increasing the chance for being able to make offline transactions in the future.

Changes

Automatic input selection

There is a rather sophisticated coinselection algorithm now that will select with fees in mind. The library will automatically fetch the keyset of the mint in order to determine the fees during selection. The implementor can decide to use includeFees = true in various class methods in order to include fees in the selection so that the selection can be spent. This can be used in situations where the sender pays the fees instead of the receiver (and the receiver can receive a predictable amount). It is necessary to use if we perform coin selection to pay a Lightning invoice afterwards:

const { keep: proofsToKeep, send: proofsToSend } = await wallet.send(amountToSend, proofs, {
	includeFees: true
});
const meltResponse = await wallet.meltProofs(meltQuote, proofsToSend);

Output amounts

AmountPreference is removed, use OutputAmounts now, which can be used to define sendAmounts and keepAmounts which are now both simple arrays of numbers. Previously we could only define amounts to send through AmountPreference.

Automatic output amount selection

Users can use proofsWeHave to specify in various methods which proofs are present in the wallet. This will make the library to choose output amounts to refill the wallet with missing denominations:

const { keep: proofsToKeep, send: proofsToSend } = await wallet.send(amountToSend, proofs, {
            proofsWeHave: proofs
});

The target defaults to DEFAULT_DENOMINATION_TARGET = 3; but can be defined in the constructor of the CashuWallet class.

Renaming

  • in SendResponse, returnChange is now called keep
  • mintTokens is now called mintProofs
  • meltTokens is now called meltProofs
  • CashuMint.split is now called CashuMint.swap

Legacy code

I suggest we get rid of all functions a la PayLNInvoice and force the implementor to understand how to use the library themselves instead of hiding crucial details. This should be done in a separate PR after merging this.

PR Tasks

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

Copy link
Collaborator

@gandlafbtc gandlafbtc left a comment

Choose a reason for hiding this comment

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

Yuge PR! thanks.

There are a lot of new functions. Please make sure that functions in the CashuWallet class that should not be used externally are marked private

Also, please add the minimal docs to the functions that are accessible to the implementoors

I didn't mark all as comments, please go through it (CashuWallet) again and add where required

src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Show resolved Hide resolved
@Egge21M Egge21M linked an issue Oct 24, 2024 that may be closed by this pull request
README.md Show resolved Hide resolved
@gandlafbtc gandlafbtc linked an issue Oct 25, 2024 that may be closed by this pull request
@callebtc
Copy link
Contributor Author

@gandlafbtc you're the best. thank you for doing all that work

@callebtc
Copy link
Contributor Author

callebtc commented Oct 25, 2024

@Egge21M @gandlafbtc @gudnuf all changes applied. Please review again.

Thanks a lot again to @gandlafbtc for the fantastic work.

Copy link
Collaborator

@gandlafbtc gandlafbtc left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work!

src/CashuWallet.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@Egge21M Egge21M left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you again for this @callebtc !

src/CashuWallet.ts Show resolved Hide resolved
@Egge21M Egge21M merged commit 8e6e128 into development Oct 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 This issue will be solved in the v2 major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wallet: mintTokens does not return a full token A set of amount preference fixes
5 participants