dagurval / electrum-cash-protocol

Electrum Cash protocol reference
MIT License
9 stars 6 forks source link

Protocol implications of double spend proofs #22

Open rnbrady opened 4 years ago

rnbrady commented 4 years ago

Double-Spend Proofs (DSProofs, DSPs) are a great new way for merchants to detect fraudulent transactions. It would be great to agree how merchants and wallets can take advantage of them through Electrum Cash Protocol. I see two immediate requirements for merchants:

Requirement 1: blockchain.address|scripthash.get_balance

A merchant who accepts unconfirmed transactions is likely to be interested in the unconfirmed balance for which no DSP has been received. Two options:

Option 1: UTXOs with DSPs should be excluded from unconfirmed field of the response to get_balance. Option 2: UTXOs with DSPs remain included in unconfirmed field of the response to get_balance but a new field such as unconfirmed_nodsp is added which excludes any UTXOs with DSP.

I would recommend/prefer Option 1.

Requirement 2: blockchain.address.subscribe|scripthash.subscribe

A merchant would also want to subscribe to events relating to DSPs. I propose that the status be made to depend on the DSP in one of the following ways:

Option 1: mempool UTXOs with DSPs should be excluded from the transactions used to construct the string. Option 2: mempool UTXOs with DSPs should be included but height is -2 if the transaction has at least one double-spent input.

I'm agnostic on which option is used here.

Cc @monsterbitar, @dagurval, @cculianu, @rkalis.

rnbrady commented 4 years ago

Requirement 2, Option 3: include all the conflicting transactions in the status string.

cculianu commented 4 years ago

Initial thoughts for both requirements:

For req 2, option 3: Hmm. That will also confuse existing/old software.

That being said a new API to be notified of these things, or at least query what's going on as far as double-spends goes -- might also be in order on top of the above modifications.

monsterbitar commented 4 years ago

For the balance, remember that it's likely common for wallets to "check if you have enough" first, before trying to do UTXO selection. As such, the first option for balance is reasonable, while the 2nd might make these wallets naively ignore the added data and then try to create transactions based on the double-spends.

monsterbitar commented 4 years ago

for the status, I think you want to include all data.

There also is a need for a way to get the actual doublespend proofs themselves.

cculianu commented 4 years ago

for the status, I think you want to include all data.

Hmm. So include the extra double-spend tx in the status history? I think that will break existing installations of EC. They will see this crazy status with extra history and want to show all of it... which will not have the intended UX effect -- users receiving funds will now think they have MORE money than they otherwise have, not less!


Or did you mean to show the DS tx that's in mempool only (and not the conflicting one)?

EC actually doesn't grab UTXOs using "listunspent" -- it builds its own little db of utxos based on its history and based upon iterating through all past transactions to figure out what it spent and what it not spent.

While showing the double-spent tx's in history is status quo now -- and would not break anything -- a clever "hack" would be to intentionally omit them. As was Option (1) above... and it may actually work.

We can add new "complete" calls that don't omit anything but instead flag tx's that are doublespends...

I dunno. Or we can not omit them -- then it's just status quo -- and we just add new calls for new "double-spend aware" wallets.

I'm ok with either approach.

monsterbitar commented 4 years ago

for the status, I think you want to include all data.

Hmm. So include the extra double-spend tx in the status history? I think that will break existing installations of EC. They will see this crazy status with extra history and want to show all of it...

The status is just a hash of some data, right? Not the data itself. It's used to indicate "something has change" or "nothing has changed". I think a DSProof constitutes "a change".

How you'd get the actual data on what changed is less obvious - either a flag on the TX that has a double-spend alternative (thus mutating the TX itself), or a non-TX entry in the get_history that includes a new data type (DSproofs) would work in my opinion.

There might be other reasonable ways as well.

rnbrady commented 4 years ago

If status is X before a new transaction and Y once it's in the mempool, is it acceptable for status to revert to X or does it need to change to new value Z?

monsterbitar commented 4 years ago

In my opinion, it needs to be a new value Z.

Remember, a transaction with a double-spend proof can still be mined.

There is also network latencies, so if you query multiple nodes you can end up in a situation where if it reverts back to X, you can end up never detecting it.

cculianu commented 4 years ago

@monsterbitar What would that new status Z be?

if it reverts back to X, you can end up never detecting it.

So? You will be at "X" then. X is before you were about to get "robbed" by a DS. What's so bad about that?

monsterbitar commented 4 years ago

If status X is a hash of [some data], then Z is a hash of [some data + the DSproof].

If I talk with three servers, and want to cross reference them to make sure at least two of them are in agreement, then when a doublespend proof comes along, if "revert to X" is happening you can get this outcome:

Server A hasn't seen the original transaction yet, will send X. Server B has seen the original transction, but not the double spend, it will send Y. Server C has seen both the original transaction and the double spend, will send X.

Client see "two says X, they seem to be in agreement" and thinks there hasn't been a transaction.

There's likely to be more edgecases with this setup, and I think it's important that the information is propagated rather than "handled"/"interpreted" by the backend.

I don't know what impact this has on existing EC wallets, so to not confuse them this additional behaviour (taking DSproofs into account) should probably be part of a the subscribe/get status calls only from a new version number so that old clients don't automatically get confused by it.

dagurval commented 4 years ago

1) In general, I don’t think it’s a good idea to change behaviour of existing method calls, except perhaps purely additive.

2) I’m not convinced DSP is guaranteed to receive much adoption, so baking it deeply into the protocol may be premature, it will be difficult to take out.

3) We need to keep in mind performance/network implications of BCH supporting long mempool-chains. If BCH supports 500-depth tx chains, then a single double spend at the “root of the chain”, will change the status of 500 transactions in the mempool.

To address (1) and (2), does it make sense for clients to explicitly request double spend proofs via new API methods?

As for (3), how do we help a client efficiently verify that a transaction at the tip of a 500-transaction deep mempool chain is, in fact, at risk of being wiped out due to double spend? It should ideally be efficient for both client and server.

monsterbitar commented 4 years ago

As for 3), don't forget that all 500 transactions are at risk of double-spend, not just the one we have a double-spend proof on.

rnbrady commented 4 years ago

@monsterbitar there's an analogy between the scenario you described:

If I talk with three servers, and want to cross reference them to make sure at least two of them are in agreement, then when a doublespend proof comes along, if "revert to X" is happening you can get this outcome:

Server A hasn't seen the original transaction yet, will send X. Server B has seen the original transction, but not the double spend, it will send Y. Server C has seen both the original transaction and the double spend, will send X.

Client see "two says X, they seem to be in agreement" and thinks there hasn't been a transaction.

and the way double spends are handled today when a conflicting transaction gets mined:

If I talk with three servers, and want to cross reference them to make sure at least two of them are in agreement, then when a block with a double spend comes along, "revert to X" will happen and you can get this outcome:

Server A hasn't seen the original transaction yet, will send X. Server B has seen the original transaction, but not the block, it will send Y. Server C has seen both the original transaction and the block, will send X (as it will have removed the original transaction from the mempool).

Clients see "two says X, they seem to be in agreement" and thinks there hasn't been a transaction.

So I'm not convinced the edge case argument is a strong one.

rnbrady commented 4 years ago

Coping @zander as someone who is working with DSPs, if not with Electrum itself.

@dagurval re adoption, has the fork changed your opinion on this? And in terms of changing the protocol, what are you thoughts on bumping the version?

@monsterbitar am alternative version of your argument is that any change to status incorporating DSPs would break quorum functionality in multi-server clients if the servers used are on difference versions.

monsterbitar commented 4 years ago

If there's no clear way that we an all agree on, perhaps it would be better to reconsider alternative ways to handle this - what if we could subscribe to double-spend proofs in general (so the server wouldn't need to match to any specific address or transaction, just relay every one of them), and it would be up to client side to handle them?

Or the same as above but we tell the server which address(ess) we're interested in seeing DSPs for?

Is there some other way we can allow developers to access the functionality without being backwards incompatible?

rnbrady commented 4 years ago

That's definitely a valid option, which falls under the purely additive category.

Is anyone opposed to adding a field to the get_balance response? Here are some options:

format1 = {
    confirmed: 1000,
    unconfirmed: 100,
    contested: 10
};

format2 = {
    confirmed: 1000,
    unconfirmed: 100,
    uncontested: 1090
};

These formats both represent the same state, i.e. 1000 sats confirmed plus 100 sats in mempool of which 10 sats at risk according to DSP. I'd probably go with format1 and in code I would calculate that my customer is good for uncontested = confirmed + unconfirmed - contested.

For status I think my suggestion of using height = -2 for transactions with a DSP qualifies as purely additive as this adds both transitions and states to the state machine (no new transitions to old states). So you'll have all the old notifications and more.

@monsterbitar this may break quorum mechanisms that use status but I think that's a misuse of status since it's derived from mempool transactions in any order which could differ per implementation or even per instance. status should not be used for confirming two nodes have different sets of transactions.

zander commented 4 years ago

Thanks for copying me, I read through the whole thread and I will start with a bit of higher level point:

the majority of wallets today are crap at handling incoming transactions. We are absolutely not helping our users because we are only giving them a boolean, it arrived or it didn't arrive. Granted, we may add number of confirmations to arrived transactions, but this is not enough.

Any transaction we receive has a lot of variables that become relevant for actually confirming as the network grows. The number of unconfirmed ancestors was already mentioned, this is a gradient where deeper is higher risk.
We also have fees and other such properties that differ between nodes and may stop propagation. Imagine a user sends you a tx that has a fee below 1sat/byte via bip70. It won't get mined, though. Making it easy to double spend later.

As the network grows we will remove the hard-coded 1sat/byte and a market will form for fees and other variables. Wallets need to be able to judge weather a transaction will be mined soon or they, again, open themselves up to a double spend later.

Double Spend Proofs allows us to ignore the complexity for a while as it turns our boolean into a 3-state. But please keep in mind that the protocol will have to evolve more as time goes by to create a much more sturdy risk-profile for any given incoming transaction.

So, from my point of view the electrum-cash protocol will need to slowly move to incorporate such features, and this will require wallets to follow and libraries to follow these changes.

What I'm saying is that while it may seem disruptive to lower the 'unconfirmed' balanace to seprate out the disputed funds, it does sound like the way to go in light of the longer term vision and the wish to keep electrum-cash relevant.

I'm not an expert on this protocol itself, I expect other people to have better ideas on how to use the ideas here and create a smooth path from here to the future.

But I'll comment on the questions based on the view I set out above: get_balance would benefit from a 3rd field that lists only the disputed funds. And unconfirmed is lowered (from todays value) by this same amount.

subscribe: in a full node it is trivial for any code that finds a scripthash to also instantly know there is a DSP. In APIs I have for subscribing to changes for scripthash (address is essntially just a scripthash) we indeed send out a separate message for the DSP being seen.
It is relevant for EC and likely other such software to actually download the DSP and store the full proof in its own history. So on that basis alone you will benefit from some alteration that sends those.

In the subscribe case I'd say that the request may add a boolean to make this sending of the DSPs to a client happen, making the client request this may be easier to avoid breaking clients. But since I've not worked with the subscribe method myself take this as the insane ramblings of a happy mind.

rnbrady commented 4 years ago

Thanks @zander.

@cculianu do you have any thoughts on @dagurval's point (3) about how clients might verify the at-risk status of chained mempool transactions?

Could the proposed mempool.changes notification be of use?

Does anyone know what other wallets or apps out there are using this protocol?

rnbrady commented 3 years ago

@cculianu has implemented some DSP functionality in https://github.com/cculianu/Fulcrum/pull/74 with protocol extensions documented at https://github.com/cculianu/electrum-cash-protocol/commit/005253d10845da4c810c5f2f40aaaef9be454941.

Nice work Calin. Did this lead to any further insights into how we might extend status and get_balance?

cculianu commented 3 years ago

Are you asking if I think get_balance and status should somehow encapsulate dsproof information? I think not at the present time. Perhaps someday. For now I think it's more prudent for dsproof to live in that blockchain.transaction.dsproof.* namespace in the electum cash extensions that were made for Fulcrum.

I think there's enough there for any wallet to offer the information in the UI. Later on, as it gets more usage, the "perfect design" will become obvious to everybody. Right now there is no wallet that i know of that uses the electrum cash protocol and also offers DSProof information in the UI...

rnbrady commented 3 years ago

Yes that's what I'm asking and thanks for the answer, which is reasonable.

The wallet I'm working on is part of the backend to a (not yet working) web app. I have an address per customer and I'm not interested in individual transactions, only balances. I accept unconfirmed transactions so it's currently just a matter of address.get_balance on every address.subscribe notification.

It would be great to be able to keep doing that but looks like I'd have to start tracking UTXOs if I want to use DSPs. A fair trade-off for now.

cculianu commented 3 years ago

Oh, I see. Hmm. So you would want to get from address -> set of DSPs more directly?

rnbrady commented 3 years ago

I want to get from address -> total double-spent value. Then I can allow the customer to spend confirmed balance + unconfirmed balance - double spent amount.

cculianu commented 3 years ago

Interesting. I see, yes right now it is very awkward to get from address -> doublespend balance -- like you said you have to subscribe and/or examine the mempool tx's, and from there ask about double-spends... hmm.

There is work that can be done here to make that more convenient. Yeah I think your above get_balance proposal is a good one. We can always add keys for this. Hmm. I'll look into it in a few weeks hopefully when I get more time, and see if it's easily doable in such a way so as to not be a performance hit (It shouldn't be, really).

zander commented 3 years ago

@rnbrady that makes sense, it was the first API point I added for Flowee the Hub.

https://flowee.org/docs/hub/api/service/addressmonitor/#doublespendfound