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

[PoC] Payment aware CoinjoinCoinSelector #12646

Closed
wants to merge 3 commits into from

Conversation

turbolay
Copy link
Collaborator

Fixes #12638

There are 3 ideas here, that only change behavior if all coins are private and there are pending payments:

  • Always try to register a combination of coins that allows you to make the maximum number of pending payments while using the minimum amount of coins and then the minimum amount of change
  • Don't mix if the sum of all our coins is lower than the minimum pending payment (except is StopWhenAllMixed = false)
  • If we have the balance to perform a payment but can't do it with only MaxInputsRegistrableByWallet inputs, mix in ConsolidationMode to try to create bigger outputs to be able to perform the payment in a later coinjoin

If not all our coins are private, nothing changes.
It's a POC because there are optimizations and better factoring possible.

@lontivero can you tell me what you think about the concept please?

Copy link
Collaborator

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

cack
will test later

@@ -80,6 +80,55 @@ public ImmutableList<TCoin> SelectCoinsForRound<TCoin>(IEnumerable<TCoin> coins,
.Where(x => x.IsRedCoin(SemiPrivateThreshold))
.ToArray();

if (WalletPrivateButHasPendingPayments(pendingPaymentsAmount, semiPrivateCoins.Length, redCoins.Length))
{
var maxRegistreableAmountPrivateCoins = privateCoins.OrderByDescending(x => x.Amount).Take(MaxInputsRegistrableByWallet).Sum(x => x.Amount);
Copy link
Collaborator

Choose a reason for hiding this comment

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

some inconsistency in using Registreable or Registrable throughout the PR

Suggested change
var maxRegistreableAmountPrivateCoins = privateCoins.OrderByDescending(x => x.Amount).Take(MaxInputsRegistrableByWallet).Sum(x => x.Amount);
var maxRegistreableAmountPrivateCoins = privateCoins.OrderByDescending(x => x.Amount).Take(MaxInputsRegistrableByWallet).Sum(x => x.Amount);

@turbolay turbolay marked this pull request as draft March 16, 2024 02:13
@turbolay
Copy link
Collaborator Author

turbolay commented May 2, 2024

This PR is still relevant, and should be finished, I'm closing it simply because I will not maintain it in a foreseeable future

@turbolay turbolay closed this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coinjoin only when payincoinjoin will be sent
2 participants