dashpay / dash

Dash - Reinventing Cryptocurrency
https://www.dash.org
MIT License
1.49k stars 1.2k forks source link

Instantsend Bug? #2252

Closed globaltoken closed 6 years ago

globaltoken commented 6 years ago

Hello,

What is the usecase of instantsend? I see if you send a Instantsend Transaction it gets instantly confirmed, but it is useless for daemon wallets.

If you send for example an instantx to an exchange, it will not get the confirmations because listtransactions or listsinceblock shows always 0, but this tx is an instant tx. So it should display 5 / nInstantSendDepth?

Why is this so?

Thanks in advance.

UdjinM6 commented 6 years ago

Hi,

Yes, there is still an inconsistency in RPC calls/results in 12.3 for various commands, see https://github.com/dashpay/dash/blob/master/doc/instantsend.md#rpc for more info about some nuances. Basically, that's a legacy issue. We are going to get rid of dummy confirmations (https://github.com/dashpay/dash/pull/2040) and unify all RPC calls correspondingly (https://github.com/dashpay/dash/blob/develop/doc/instantsend.md#rpc) in 12.4.

globaltoken commented 6 years ago

Hey @UdjinM6

So this means in the upcoming release there will not be any confirmations for IX? It is just verificable by the boolean value of "instantlock" ?

okay, so you could guess the TX is confirmed, if the boolean is true. I think lots of merchants, exchanges and other payment providers just check confirmations by default and not the instantlock arg. So imo it could be a problem.

How if we integrate a new NetmsgType that Broadcasts the required confirmations, or better directly in the Txlock candidates a new var that will be broadcasted to get other nodes known about tx confirm target?

UdjinM6 commented 6 years ago

just check confirmations by default and not the instantlock arg. So imo it could be a problem.

This is actually the other way around imo - these changes are backwards compatible in a non-disruptive way, and it would be a problem if this wasn't the case. They will simply not recognise IS txes on 12.4 (and will see 0 confirmations instead of 5) if they won't update their software to use instantlock. The worst case scenario: no 3rd-party software updates -> no acceptance of IS txes for some time on that specific service. But these txes are still valid 0-confs, so it's like IS is simply disabled by them on their side, no funds loss or any other security consequences.

to get other nodes known about tx confirm target

How would that help and why would other nodes trust this?

globaltoken commented 6 years ago

Okay I understand now. I thought the reason why people pays more tx fees, should be to force Instant tx. Means the merchant will get displayed the current ix confirmations wheter he wants or not. Thats why the people pay more fees.

The tx confirm target is meant:

I can configure on my side -instantsenddepth, but the receiving side does not know how much confirmations I want. So it shows on the other side, the confirmations that are configured locally on the other side.

For example: I configure wallet -instantsenddepth 10 The other side shows "5/6" confirmations, not 10/6 (confirmed).

But my thought was, that the receiving side will be forced to receive instantsend confirmations. But I see now, that this is can be enabled via rpc args.

So my idea was, to force instantsend, if the merchant has -enableinstantsend arg. If the client pays now anything he can configure his confirm target, that will also be broadcasted to the server (receiver).

UdjinM6 commented 6 years ago

The reason IS works is the assumption that if majority of the nodes/miners follows the rules (i.e. respect MN votes), there is no way to reuse the same inputs again in a double-spend tx - not only the tx itself but also a block containing such tx is going to be rejected by everyone even if it's going to be mined by some malicious miner. But you can't force the other node to do what you want in general, it has no obligation to you. As you might see, IS has nothing to do with confirmations or anything like that really, it's basically up to you (and only you) how to display this info locally, no other node would really care or be able to force you to represent it in another way.

globaltoken commented 6 years ago

I see, thanks for the explanation.