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

First idea of "confirmation layer" #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

First idea of "confirmation layer" #25

wants to merge 1 commit into from

Conversation

karelbilek
Copy link
Contributor

This is a first idea for #23

The function would get "rich utxos", which include not just input with script+value, but also information about confirmations, and whether it is "own" utxo or not.

The point is - we want to allow the user to make a transaction on unconfirmed utxos, but only when utxos with more confirmations fail.

The API is not yet ideal; but I would like to hear your thoughts now

@dcousens
Copy link
Contributor

dcousens commented Aug 15, 2017

Can this be accomplished by providing a specialized sort function, before handling utxos to an algorithm?

We could then offer the different options to that function, such as DESCENDING_VALUE, HIGH_CONFIRMATIONS_THEN_DESCENDING_VALUE etc.

@karelbilek
Copy link
Contributor Author

The algorithms have no guarantee of choosing utxos in the order given, right?

A-ha, they actually all do. I have confused it with the algorithms in stats/stragegies that randomize the utxos first. Good.

@dcousens
Copy link
Contributor

@Runn1ng they do, maybe that should be documented better than "UTXOs order"?

@karelbilek
Copy link
Contributor Author

karelbilek commented Aug 15, 2017

Related to #24 discussion

Should there be another layer of algorithms, that have type

(Rich Inputs | (Rich inputs -> Inputs) | Outputs | Fee Rate | Maybe Outputs) -> Nothing | Fee | (Fee | Inputs | Outputs)
  • the second argument some ordering + filtering

Or the underlyng algorithm another input (as in the PR)?

The algorigthm in index.js sorts the utxos, but the other modules don't.

@dcousens
Copy link
Contributor

dcousens commented Aug 15, 2017

@Runn1ng I actually played with that function prototype initially, but ruled against it because it didn't easily lend itself to composition, and the method used would only use .sort() anyway, and that was it.

Easier to have users do that themselves using a provided comparator TBH.

The algorigthm in index.js sorts the utxos, but the other modules don't.

The default export was optimized to provide the best results for users, if they were composing the algorithms themselves, the idea was they might know what they were doing.

@karelbilek
Copy link
Contributor Author

I did some changes, combining the ideas from here and from the #24 discussion, don't want to make another PR

https://github.com/runn1ng/coinselect/tree/reorg
https://github.com/runn1ng/coinselect/commit/357f0cb9cecc3d54be000a474e6705c75b3f341f

Adding the various sorting function and utils.algorithmBackup simplified simulation code somewhat, the index.js is shorter and the composition is easier, but the function-returning-functions are slightly harder to read and understand.

You can comment on the commit directly, or I can make it as a PR

@dcousens
Copy link
Contributor

dcousens commented Aug 16, 2017

@Runn1ng I commented on the commit directly. Nice work! Mostly nits.

I think we're going to have 3 user-choices.

  • UTXO priority order
  • UTXO selection algorithm
  • Output selection algorithm (maybe)

Algorithms like BIP126 don't offer a choice, but some algorithms are entirely composable.
I don't want to over-engineer it, and I'd rather not make many copies of utxos, so maybe I'd prefer if left the .sort action to the user (except for the default export).

Example

let coinselect = require('coinselect')
let utils = require('coinselect/utils')
let accumulative = require('coinselect/u_accumulative')
let blackjack = require('coinselect/u_blackjack')
let feeRate = 10

utxos = utxos.sort(utils.BY_DESCENDING_VALUE) // no issue with copies now
let { inputs, outputs } = utils.bestOf(blackjack, accumulative)(utxos, desiredOutputs, feeRate)
if (!inputs) return

...

That is a bit lame, but, it puts it all out on the table.
Now we only have to improve it?

@dcousens
Copy link
Contributor

Maybe also add an example of how BNB fits into this

@karelbilek
Copy link
Contributor Author

Hm, now I realized how just sorting the utxos won't be sufficient for BnB (or blackjack).

BnB and blackjack will try to find an exact match, from any of the inputs.

However, if a non-exact match is possible from 6+ confirmed utxos (or 1+ confirmed utxos for own change utxos), they should be used instead of exact match with little confirmed utxos.

Just sorting the UTXOs won't cut it. I will need some logic as I made in this PR - first try [6,1] - and try first exact match, then accumulative - and then repeatedly loosen the restrictions.

@karelbilek
Copy link
Contributor Author

I have updated the reorg branch - I have added bnb, added a more efficient repeated confirmation filtering as in the PR, and I implemented your comments (simplified the sorting, better names). I still have the src/ dir structure.

Hm. I will update this PR with the new confirmation filtering

@karelbilek
Copy link
Contributor Author

karelbilek commented Aug 17, 2017

...updated

(tests check, but the new file is not covered :))

@karelbilek
Copy link
Contributor Author

What should I do to move this forward.

To be more concrete - do you think the multiple trials, with the confirmation restriction being loosened in every trial, be included in this library? Or should it be in another layer / another library, if the user wants it?

In my case - I am adding it to Trezor Web Wallet and I will want that logic there one way or the other.

@dcousens
Copy link
Contributor

dcousens commented Aug 22, 2017

I simply haven't had time to review/discuss this yet 😦 - apologies @Runn1ng

@karelbilek
Copy link
Contributor Author

No problem, I want to help, not overwhelm. :)

@dcousens dcousens self-assigned this Apr 14, 2018
@dcousens
Copy link
Contributor

@karel-3d reading this again, I still don't know if this is the right space.

Isn't the optimal behaviour around confirmations entirely contextually dependent?
Many users may always want 6+ confirmations, others who have say P2MS scripts etc may be able to work on 0 confirmation.
I don't know if we can do this reliably.

@karelbilek
Copy link
Contributor Author

The fuction is optional (is just another layer above current selection algorithms) and whoever calls the library can set how much confirmations he wants; can be 0 for both "own" utxos and "other" utxos

@dcousens dcousens removed their assignment Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants