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

dcr: Add send all. #13

Merged
merged 1 commit into from
Jan 18, 2025
Merged

dcr: Add send all. #13

merged 1 commit into from
Jan 18, 2025

Conversation

JoeGruffins
Copy link
Member

No description provided.

Copy link
Member

@itswisdomagain itswisdomagain left a comment

Choose a reason for hiding this comment

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

Untested, but LGTM.

@JoeGruffins
Copy link
Member Author

@jrick
Copy link
Member

jrick commented Jan 14, 2025

does this respect the maximum tx size, or even the mempool policy for max accepted tx size it will relay?

@jrick
Copy link
Member

jrick commented Jan 14, 2025

Also, why not use Wallet.NewUnsignedTransaction with wallet.OutputSelectionAlgorithmAll?

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Jan 15, 2025

Also, why not use Wallet.NewUnsignedTransaction with wallet.OutputSelectionAlgorithmAll?

I wasn't sure how to deal with the fee in this case. We want to send all and not leave anything, so would we need to calculate the fee when using Wallet.NewUnsignedTransaction in order to get the correct amount to send and then that amount would have to agree with wallet internal fee calculations?

@JoeGruffins
Copy link
Member Author

Just rebased.

@JoeGruffins
Copy link
Member Author

@jrick
Copy link
Member

jrick commented Jan 15, 2025

Also, why not use Wallet.NewUnsignedTransaction with wallet.OutputSelectionAlgorithmAll?

I wasn't sure how to deal with the fee in this case. We want to send all and not leave anything, so would we need to calculate the fee when using Wallet.NewUnsignedTransaction in order to get the correct amount to send and then that amount would have to agree with wallet internal fee calculations?

The method takes a fee rate (per kB), not an absolute fee. No matter what fee rate you provide, it shouldn't change the input selection, only the amount that is eventually sent.

@JoeGruffins
Copy link
Member Author

Ok, do we just set the output value to max value? I'll try.

@JoeGruffins
Copy link
Member Author

The wallet method hangs when setting the output value to the max int64. May be something to do with cgo, unsure.

wallet.OutputSelectionAlgorithmAll I assume uses all the inputs, but we still have to calculate the fee in order to know what the output value should be, or am I still missing something? If we are calculating the fee anyway ourselves and already know all inputs I don't know why it is important to pass the values through wallet. Am I missing more checks? We are checking utxo.Spendable already.

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Jan 16, 2025

Is the "OutputSelectionAlgorithm" argument completely ignored if we include an input source? It's convenient for use to choose the inputs because in cake users can choose to "freeze" or use certain inputs. Perhaps it is more correct for us to lock and unlock inputs in wallet? That would require alot of changes presently that I would rather not make at this time. I don't think the OutputSelectionAlgorithm argument is great for us atm.

Maybe I am still missing something though.

@jrick
Copy link
Member

jrick commented Jan 16, 2025

Input source must be nil to use OutputSelectionAlgorithmAll.

If you want to sweep all value, you need to provide only a change address source and no other outputs. This will result in paying the total input value, minus any fee, to the single change address.

@jrick
Copy link
Member

jrick commented Jan 16, 2025

If you want to control which inputs are used, return them all from the first call to the input source you provide, and again, specify only a change address source and no other outputs. This should have the same effect as OutputSelectionAlgorithmAll but lets you select which outputs are used.

@JoeGruffins
Copy link
Member Author

JoeGruffins commented Jan 17, 2025

@JoeGruffins JoeGruffins merged commit a781a66 into decred:master Jan 18, 2025
2 checks passed
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.

3 participants