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 the command line option of "input-addrs" #1112

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

Conversation

BitcoinWukong
Copy link
Contributor

@BitcoinWukong BitcoinWukong commented Dec 12, 2021

With this new command line option, user can spend from specified addresses via the following command:

python sendpayment.py wallet.jmdat amount target_address -i spend_from_address_1 -i spend_from_address_2 -i spend_from_address_3 -m mix_depth_of_from_address

This is the first step of implementing #1010

cc @openoms FYI

@AdamISZ
Copy link
Member

AdamISZ commented Jan 6, 2022

Apologies for the long delay in starting to review this. It's become impossible for me to keep up with all the things I want to on the project.

So first points on first code read-through:

There is an issue with the requiring of an auth-suitable address, that is seen in select_utxos, that would have to be fixed up

(I'll also want to double check the formatting of putting the input address list at the end of the schedule, and double check there is no snafu based on "what's at the end of the schedule item list" logic.)

The general idea makes sense, but I'm wondering where it's coming from, like: if I decide to use exact coin control, why wouldn't I select by utxo, not by address? Obviously we want it to be the case that there's a 1-1 match, but in cases where there isn't, isn't it even better to make sure you're doing a utxo based check, not an address based check?

@BitcoinWukong
Copy link
Contributor Author

BitcoinWukong commented Jan 13, 2022

There is an issue with the requiring of an auth-suitable address, that is seen in select_utxos, that would have to be fixed up

Could you elaborate on the issue? I'm not really understand what the problem is here. Thanks!

The general idea makes sense, but I'm wondering where it's coming from, like: if I decide to use exact coin control, why wouldn't I select by utxo, not by address

  1. It's easier for the user to use address instead of UTXO, as addresses are much shorter than the UTXO, and users interact with addresses much more frequently than with UTXOs directly.
  2. I would like to support selecting by UTXOs as well, in addition to selecting by address. That could be implemented in a future PR.

@AdamISZ
Copy link
Member

AdamISZ commented Jan 13, 2022

Edit: ignore this first point, it's wrong, see here

Could you elaborate on the issue? I'm not really understand what the problem is here. Thanks!

I meant this:

require_auth_address: if True, output utxos must include a
is required for the taker usage (not for the direct send usage of course).

Re: your points as to why address-based may be better, I can see your point, but if anything I would tend to lean the other way, still. Users working at this level (full coin control) should be encouraged to precisely choose the coin they want to spend; doing it by address is an imprecision which could catch them with an unpleasant surprise. So I'm more anti- than for- doing it by address, though your arguments are perfectly reasonable.

@kristapsk
Copy link
Member

I would argue for using addresses, as, from privacy perspective, if you have multiple UTXOs sent to the same address in your wallet, you migh want to always spend them all. But some warning should be printed in case some of them are frozen, some not.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 19, 2022

Coming back to this again, I can see that this comment I made, is just wrong:

I meant this:

require_auth_address: if True, output utxos must include a

is required for the taker usage (not for the direct send usage of course).

Sorry! No wonder you were confused :) This is wrong because the auth_address, which is used to authenticate the counterparty (to prevent a MitM), is only provided by the maker, in the !ioauth message, not by the taker. So this kind of restriction does not exist on taker utxos.

So that's one thing cleared up :) On the second point that was discussed:

There's an old phrase in Bitcoin going back to the early days "there is no such thing as a from address". This is technically true, which is the best kind of true :)
I have two reasons I don't want to use addresses but prefer utxos:

  • This gives users full control of the history of the inputs of a transaction; yes, it's true that coins at the same address are intrinsically connected, but that's still not quite the same, if we use addresses then users cannot choose to spend a single utxo, as they might, for reasons we don't know
  • It also keeps the correct mental model for users of coins/utxos not accounts

I don't think the counterargument that addresses are easier UX wise, is significant (both are horrible UX wise, in plain text form).

@BitcoinWukong
Copy link
Contributor Author

BitcoinWukong commented Jun 21, 2022

This is wrong because the auth_address, which is used to authenticate the counterparty (to prevent a MitM), is only provided by the maker, in the !ioauth message, not by the taker. So this kind of restriction does not exist on taker utxos.

To be clear, does it mean that the problem doesn't really exist and this PR should be working fine?

I don't think the counterargument that addresses are easier UX wise, is significant (both are horrible UX wise, in plain text form).

I agree. And my plan is to add this feature to the QT gui once this PR is merged. The idea is that, you can simply right click an address in the "JM Wallet" main tab, and select a new context menu option, "Spend from this address".

And in the "Coinjoins" tab, there will be an optional field, "spend from addresses". If you did that "Spend from this address" thing, you'll be redirected to the "Coinjoins" tab, with the "spend from addresses" field prefilled for you.

@FeatureSpitter
Copy link

This is wrong because the auth_address, which is used to authenticate the counterparty (to prevent a MitM), is only provided by the maker, in the !ioauth message, not by the taker. So this kind of restriction does not exist on taker utxos.

To be clear, does it mean that the problem doesn't really exist and this PR should be working fine?

I don't think the counterargument that addresses are easier UX wise, is significant (both are horrible UX wise, in plain text form).

I agree. And my plan is to add this feature to the QT gui once this PR is merged. The idea is that, you can simply right click an address in the "JM Wallet" main tab, and select a new context menu option, "Spend from this address".

And in the "Coinjoins" tab, there will be an optional field, "spend from addresses". If you did that "Spend from this address" thing, you'll be redirected to the "Coinjoins" tab, with the "spend from addresses" field prefilled for you.

Hello,

could you make this feature use UTXOs instead of addresses?
UTXOs have much more meaning (actually, they have a direct meaning on the Bitcoin protocol), addresses are not as a granular selection, and some addresses are just the result of a tx change, which might be confusing for the user.

It would be much easier just to select from a list of UTXOs, the ones we want to include in a direct-send or a coinjoin.

@FeatureSpitter
Copy link

I would argue for using addresses, as, from privacy perspective, if you have multiple UTXOs sent to the same address in your wallet, you migh want to always spend them all. But some warning should be printed in case some of them are frozen, some not.

exactly!

@kristapsk
Copy link
Member

It looks this got stuck in discussion of using addresses vs UTXOs. I have my arguments for using addresses, but going UTXO way is also ok with me, better to have something, one way or another, than nothing.

@BitcoinWukong Could you rebase?

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.

4 participants