protocol 1.7: outpoint subs#2
Conversation
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
SomberNight
force-pushed
the
202502_protocol_next
branch
2 times, most recently
from
d32a69e to
c10f63f
Compare
SomberNight
force-pushed
the
202502_protocol_next
branch
2 times, most recently
from
e74e0f1 to
9e02865
Compare
SomberNight
force-pushed
the
202502_protocol_next
branch
from
ff53c91 to
1c30e19
Compare
SomberNight
force-pushed
the
202502_protocol_next
branch
from
1c30e19 to
337b006
Compare
SomberNight
force-pushed
the
202502_protocol_next
branch
from
f17061d to
7911396
Compare
This is a minimal changeset for a new protocol version. The ideas for the less trivial changes are deferred a bit, just so we get something out. ref spesmilo#2 ref spesmilo/electrumx#90
SomberNight
force-pushed
the
202502_protocol_next
branch
2 times, most recently
from
7efc355 to
7eb1d59
Compare
SomberNight
force-pushed
the
202502_protocol_next
branch
2 times, most recently
from
e4a702a to
e3fd26e
Compare
|
|
||
| If the history of the scriptPubKey is too long (busy address, touched by thousands of txs), | ||
| and so the server refuses to serve it, it SHOULD send a JSON-RPC error with integer code `10_001` | ||
| to communicate this to the client. |
There was a problem hiding this comment.
Edge case: if the history is not too long when you subscribe, but then it becomes too long.
Should the server send a notification in that case?
Error codes in jsonrpc only exist in responses, not in notifications.
There was a problem hiding this comment.
Error codes in jsonrpc only exist in responses, not in notifications.
Right, I also looked this up exactly for this reason a couple days ago :D
Well the way ElectrumX has always worked in this case is that the server silently drops the subscription at that point. Not ideal but I would leave that as-is.
The protocol 2.0 history pagination and incremental recursive status would be the true fix.
There was a problem hiding this comment.
We could return zeroes as the hash in the notification.
The client will request history if he receives a hash that does not match its own.
The protocol 2.0 history pagination and incremental recursive status would be the true fix.
indeed
SomberNight
force-pushed
the
202502_protocol_next
branch
from
acc7d16 to
e8a379c
Compare
|
Reviewed to e8a379c, looks reasonable to me |
|
@cculianu @craigraw @Davidson-Souza @ecdsa @evoskuil @f321x @Overtorment @romanz @shesek @t-bast @tnull If you are interested, please see if what is proposed here looks reasonable. We want to require some of the changes here soon in the Electrum Wallet, and would want to also bump the minimum protocol version the client requires to 1.7 (from the |
|
Reviewed. I think everything looks reasonable barring one major concern: The new
As I noted previously, my measurements between ElectrumX 1.16 and ElectrumX 1.18 (the latter having implementation of timing delays) indicated that the time spent loading a small wallet doubled with this change. With ElectrumX 1.18 each call took a little over a second to complete, unless it was batched. This performance hit occurs regardless of whether it is necessary, for clients connecting to servers over a local LAN or using Tor, and regardless of whether they have other mitigations in place. My recommendation would be to hold the timing delay recommendation, or downgrade it to COULD, until #7 lands the response-side Either way, I believe the recommendation of timing delays must be balanced with the ability to control them from the client side. |
| Traffic analysis | ||
| ---------------- | ||
|
|
||
| The goal is to defend against a passive network Man-in-the-Middle, such as an ISP | ||
| or a Tor exit node, observing the encrypted TLS stream, and making educated guesses | ||
| of the message contents based on TCP packet flow: timing, direction, and sizes of TCP packets. | ||
|
|
||
| .. note:: When using raw cleartext TCP as transport for the JSON-RPC payloads, without encryption, | ||
| a passive network observer can see all the plaintext messages. | ||
|
|
||
| As a generic mitigation, implementations (both client and server) SHOULD | ||
|
|
||
| - pad messages to bucketed lengths (e.g. powers of 2, with a min size), and | ||
|
|
||
| - introduce small timing delays, ideally by buffering messages. |
There was a problem hiding this comment.
one major concern: The new
Traffic analysissection bundles two different things under one SHOULD:
- Padding: reasonably cheap and backwards-compatible, needs no handshake. Fine as a SHOULD in 1.7.
- Buffering / timing delay: costs interactive latency and the client can't refuse it. Should be COULD, or gated behind feature negotiation.
[...]
My recommendation would be to hold the timing delay recommendation, or downgrade it to COULD, until #7 lands the response-sideoptionsfield and adelaysfeature definition.
[...]
Either way, I believe the recommendation of timing delays must be balanced with the ability to control them from the client side.
Changed in 1a58f6c to explicitly say SHOULD for padding and COULD for timing delays.
I agree that it would be desirable for the client to be able to influence the server's behaviour here via an opt-out, e.g. by negotiating a max timing delay. Still, I would postpone that for later.
|
We had recently added everything that was listed here and didn't encounter any problems. It appears that two mempool methods ( https://github.com/libbitcoin/libbitcoin-server/tree/master/src/protocols/electrum |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks, mostly LGTM
Just a few comments regarding the padding. IMO it could make sense to just do it at the TCP level rather than for each individual message. And in general it might be good to prescribe one way to do it rather than leaving too many degrees of freedom, as all of them increase confusion and the risk of implementers end up doing something that makes them leak information to an observer.
|
|
||
| As a generic mitigation, implementations (both client and server) | ||
|
|
||
| - SHOULD pad messages to bucketed lengths (e.g. powers of 2, with a min size), and |
There was a problem hiding this comment.
Very much appreciate the push to preserve more privacy!
Note that padding on the transport layer could be more efficient. I.e. rather than padding each individual message, implementations could make sure they send out TCP frames of uniform lengths. Then, the only way missing would simply be a new variable-length padding message types (e.g. ping with a variable length payload) that allows to fill up remaining space whenever a message is to be sent that doesn't exactly fit into a multiple of the designated length value. We are now implementing this approach in Lightning land.
There was a problem hiding this comment.
rather than padding each individual message, implementations could make sure they send out TCP frames of uniform lengths
The protocol supports multiple underlying transports: TCP, SSL, WS, WSS.
(The "TCP" and the "WS" transports are not encrypted so out-of-scope here.)
The most popular transport is probably SSL.
How much control does a userspace application have over the TCP frame when using e.g. the OpenSSL library?
At least when interacting with OpenSSL from the python stdlib, in our case, we don't have a lot of control.
What we have control over is what we feed into the SSL stream.
In the websocket case, it's probably even less control.
We are now implementing this approach in Lightning land.
We currently use SSL instead of BOLT-08 :(
With BOLT-08, indeed you have ~direct control of the TCP stream, but the situation seems to be more complicated here.
There was a problem hiding this comment.
How much control does a userspace application have over the TCP frame when using e.g. the OpenSSL library?
(...)
In the websocket case, it's probably even less control.
Hmm, granted. I think you could always implement your own buffered message queue, disable nagle, and at least push out constant-sized chunks. But yea, probably you have little control over what happens after in another layer.
|
|
||
| - COULD introduce small timing delays, ideally by buffering messages. | ||
|
|
||
| (A future protocol version will allow the client to opt-in/opt-out of this server-behaviour, |
There was a problem hiding this comment.
Tbh, this is a bit odd. If we want to make it optional going forward, maybe padding shouldn't be part of the spec until everything is ready?
There was a problem hiding this comment.
Tbh, this is a bit odd. If we want to make it optional going forward, maybe padding shouldn't be part of the spec until everything is ready?
Fair enough, I see your point.
However, in this version, we want to extend "server.ping" so that it can be used to mimic traffic patterns in a generic way. That's the only thing that is new in 1.7.
All the rest of the traffic analysis text is basically independent of protocol versions and is just describing what Electrum Wallet and ElectrumX have already been doing for the past year or so. It describes one specific way of doing things and gives some recommendations. It is bundled as part of this PR because I felt that rationale was needed for why "server.ping" is being extended.
The opt-in/opt-out would be a new thing that's not there yet. It is mentioned as it would be useful and is somewhat important, however I would not hold up the rest of the changes for it. We need to do smallish incremental changes as otherwise - experience suggests - all progress is stalled.
| they send and not the server (or the other way around), | ||
| that just limits the effectiveness of the defense against traffic analysis. | ||
|
|
||
| Note when the JSON-RPC messages are sent in the TLS stream, they are sometimes batched together. |
There was a problem hiding this comment.
Right, but why not make this the default way of doing padding? The more ambiguity the higher the chances will be that implementations will mess up or specific clients will stand out, i.e., still end up leaking data to observers.
There was a problem hiding this comment.
Right, but why not make this the default way of doing padding?
Do you mean that instead of injecting whitespaces, why not send json-rpc "Batches" with one of the requests in the batch being a "server.ping" notification with a custom data length? Yes that works too. (note that the "server.ping" notification is a new addition in protocol 1.7. In older protocol versions there is ~nothing the server could send.)
It is trivial to implement padding by injecting extra whitespaces. It also has high granularity: you can add any number of bytes of padding, even just one byte.
Whitespace-injection can be done at any protocol version as json parsers just ignore it. It can even be done at a lower level, abstracted away from the electrum session logic. I like this approach the most.
Conceptually even "batching" could be done in two different ways: using the JSON-RPC "Batch" concept, or just using a buffer and sending out multiple JSON-RPC messages at the same time. The main difference is that using the JSON-RPC "Batch" concept forces the responder to similarly batch the responses.
So, yes you are right there is a high degree of freedom.
In any approach one might want to do timing delays because it allows adding more padding by amortising the cost of the padding bytes over more payload data. So batching (in whatever form) needs to be mentioned here.
The more ambiguity the higher the chances will be that implementations will mess up or specific clients will stand out, i.e., still end up leaking data to observers.
I think it's probably not realistic to make sessions indistinguishable across implementations. Already the choice of server a client communicates with typically gives a good clue regarding which client implementation is being used. The choice of server can be inferred either based on the IP address or from the hostname in the TLS Client Hello. If the sniffing is done close to the client, the number of simultaneous server connections is another fingerprinting vector: if it's more than one, it's probably Electrum Wallet.
There was a problem hiding this comment.
Do you mean that instead of injecting whitespaces, why not send json-rpc "Batches" with one of the requests in the batch being a "server.ping" notification with a custom data length? Yes that works too.
Yes. Reading the current version I felt that it describes too many ways to achieve the same thing, which usually either means something is underspecified or the given examples can be reduced/reformulated in an abstract way. Adding too much verbosity usually just leads to confusion and devs either messing up or not doing anything at all.
So, not necessarily saying that batching is a waaay preferable approach, but it could be preferable to reduce the guidance to one suggested/preferred way of introducing padding. If you think whitespace padding is the preferable approach, well, then maybe that should be the only approach described, if only to keep the spec leaner?
| Similarly, the server MAY ignore new subscription requests if the spending tx is already | ||
| mined at a reorg-safe height but it still MUST send at least one full response. | ||
|
|
||
| **Result** |
There was a problem hiding this comment.
It might be useful if this (as well as similar APIs like get_balance, etc) would include a field with the current tip hash. When we make multiple subsequent requests it's otherwise always hard to tell whether the chain moved while we're at it.
There was a problem hiding this comment.
similar APIs like get_balance, etc
I guess you have these methods in mind:
blockchain.scriptpubkey.get_balance- dict- I guess tip could make sense
- though this method cannot be SPV-ed, so it is fully trusting the server. Not sure how widely it is used in the wild.
blockchain.scriptpubkey.get_history- list => cannot easily add tip- should be used with sub??
blockchain.scriptpubkey.get_mempool- list => cannot easily add tip- not sure how people use this method...?
blockchain.scriptpubkey.listunspent- list => cannot easily add tip- I guess tip could make sense
blockchain.outpoint.get_status- dict- I guess tip could make sense
blockchain.outpoint.subscribe- dict- if tip raced, client will just get a new notification
- note btw current text says (so would have to be tweaked a bit):
"any event that changes any field of the
statusdictionary results in a notification"
For both of bc.outpoint.get_status and bc.outpoint.subscribe, if you then subsequently SPV the txids, that mostly solves the issue.
Though if you called bc.outpoint.get_status just when the tip also moved, and a relevant tx just moved from mempool to the new block, then you would not know you received stale data (still mentioning the mempool) - but indeed you would if we included the chaintip in the response.
In our use case, we use the subscriptions, and SPV the txid, so there is no ambiguity. What kind of client-usage pattern do you have in mind?
(feedback from ThomasV)
|
LGTM |
|
Concept ACK; thank you for adding the |
|
This is the first I'm seeing of this, but as the maintainer of Fulcrum I'll chime in with a brief set of points:
|
|
I agree that script-hash is good to retain. Since we support all versions it’s available, so I wasn’t really thinking of it as gone. But it creates a complexity to have to negotiate different versions. |
yeah but what if you want some stuff from v1.7 but still want to use the scripthashes. makes no sense to remove them it’s actually superior for many use cases mistake to remove… |
Version negotiation is not only about which features are available to the client, it is also about what the server can expect from the client. Light servers (such as EPS) do not index scripthashes, they can only serve requests using SPKs. Therefore it makes sense to require SPKs from the client. |
|
Given that this is a proposed change, how could such servers been operational to date? And out of curiosity, what makes it challenging to provide an index of the hash? |
| * The `blockchain.scripthash.*` methods are all replaced with `blockchain.scriptpubkey.*` | ||
| methods. Note: `sha256(scriptPubKey) == scripthash`. |
There was a problem hiding this comment.
- removing
blockchain.scripthash.*and replacing them withblockchain.scriptpubkey.*is a mistake. The scripthash thing is clever for a variety of reasons. Support both. Internally the spk one can just map to scripthash impl. anyway.
For a full index server it is indeed trivial and cheap to support both. As the notifications still use scripthashes, the server would need to keep some scripthash logic, also the existing servers already have DBs that reference scripthashes. So yes, the idea is that a server impl would internally map spks to scripthashes anyway.
makes no sense to remove them it’s actually superior for many use cases
When are they superior?
I see the following advantages for scripthashes, all of them weak:
- they are constant-length, which is good against traffic analysis
- but then you also said for server.ping:
I realize this is for "privacy" but I think it's a waste of time to even worry about.
- instead we should just pad the traffic, as also described in this changeset
- but then you also said for server.ping:
- they use less bandwidth compared to long scriptpubkeys
- but note that typical popular scriptpubkey types are of similar length:
(from top of my head, ignoring hex, p2wpkh is 22 bytes, p2wsh is 34 bytes, p2tr 34 bytes, vs scripthashes are 32 bytes)
- but note that typical popular scriptpubkey types are of similar length:
- I remember kyuupichan's argument in the past was that they are good for privacy against low-skill would-be-chainalysis server operators, as if the client directly sent over their addresses, the server op would have all their addresses linked together already
- in contrast, if the client only sends a list of scripthashes, many of which are unused, the server op needs to store them in a database for a long time and only slowly over time would the addresses get revealed when they get used
- also if a server was blanket saving all sets of subbed unused scripthashes together, they could be spammed (tricky clients could intentionally bloat their DB)
- while an interesting thought experiment, against a high-tech adversary such as the actual Chainalysis and the like, I believe the argument to be moot
These were the arguments we already went through internally and disregarded them. Though indeed I had not mentioned them publicly in this issue before.
Given that this is a proposed change, how could such servers been operational to date? And out of curiosity, what makes it challenging to provide an index of the hash?
See discussion linked from the OP:
- the
blockchain.scripthash.*methods are all replaced withblockchain.scriptpubkey.*methods
Basically light personal servers such as EPS, bwt, and Floresta can be used if you already have your own bitcoind but want to use an electrum-protocol-based wallet, and connect your wallet somehow to your bitcoind.
The way they work is that the user needs to configure the middleware with their xpub (or output script descriptors), and then
- either the middleware RPC-calls into bitcoind wallet to create a wallet using a list of imported addresses, and it is the middleware that pre-calculates the addresses and rolls ahead the gap limit
- or the middleware RPC-calls into bitcoind wallet (usually Bitcoin Core) to create a wallet using the output script descriptor
- or Floresta uses BIP-158 neutrino-style compact block filters to scan for the addresses, and those filters are indexed based on scriptpubkeys
(note that EPS and bwt are unmaintained but Floresta is very much alive)
The issue with keeping support for scripthashes is that then basically no client will migrate and Floresta and similar approaches will not be able to take advantage of the scriptpubkey subs in practice.
on another note, please try to keep top-level comments to a minimum: organise topics into threads, to keep it easier to follow.
There was a problem hiding this comment.
note: there is an in-development EPS plugin: https://github.com/svanstaa/electrum-eps-plugin
There was a problem hiding this comment.
These were the arguments we already went through internally and disregarded them. Though indeed I had not mentioned them publicly in this issue before.
Yes, you tend to do that. Just unilaterally make decisions.
There is no harm in continuing to support scripthashes. Aside from the advantages you mentioned, here is another that you neglect:
- Why impose a cost on client software to update to support scriptPubKeys if it already has code written to think/talk in terms of scriptHashes?
I know it's a small update (after all to get to a scriptHash you need an spk first) -- but it's an update nonetheless! Why impose this cost at all? You can easily support both and you should.
The issue with keeping support for scripthashes is that then basically no client will migrate and Floresta and similar approaches will not be able to take advantage of the scriptpubkey subs in practice.
Huh? I said support both.
There was a problem hiding this comment.
The issue with keeping support for scripthashes is that then basically no client will migrate and Floresta and similar approaches will not be able to take advantage of the scriptpubkey subs in practice.
Huh? I said support both.
Floresta is a server. I am arguing that by supporting both, clients will never migrate. This is also what you are saying by the way, you are arguing clients should not need to be changed. Except then servers like Floresta will never get a chance to be adopted, because they will never work with lazy clients.
(by lazy client I mean clients that implement the new protocol but only make the most minimal changes so e.g. would not change from sh to spk)
Clients that don't implement changes will have to keep using old protocol versions.
That provides a clear incentive structure: if a client wants to take advantage of new features, they also have to adapt their existing code for other changes.
Years from now, servers might not support old protocol versions anymore, and at that point, old clients that still will not have updated, will get broken. I think that is fine. We don't want to or need to keep accumulating technical debt.
These were the arguments we already went through internally and disregarded them. Though indeed I had not mentioned them publicly in this issue before.
Yes, you tend to do that. Just unilaterally make decisions.
As soon as someone asked, I listed the arguments.
So will you list the "actually superior for many use cases" use cases you had in mind? :)
"weak advantage" does not mean "superior"
The argument that we should keep supporting scripthashes because then clients do not have to be changed is extremely weak. By using a new protocol version, the client impl already needs to make a change! They need to signal they support it. Might as well implement some changes then. :) Or don't change anything in the client impl and keep negotiating the existing protocol versions.
Technical feedback is very welcome, that's why I tagged people. I had also tagged you. Subjective opinions are also welcome. The tradeoffs need to be weighed, but ultimately someone needs to make decisions, or there will never be any progress.
This is not like Bitcoin consensus, forking away is much cheaper. We don't need a full democracy and don't need to argue over soft forks for 5 years. Unlike in consensus rules, we can also undo changes later if they turned out to be bad decisions. I don't want the protocol to ossify, I want to be able to change things. To deprecate and remove stuff. To try new things. For example, when scripthash subs were added, soon after address subs were removed. I think that was a good change, very much worth it. I don't want the server to have to know about address encoding/decoding at all. But AFAIU you still support address subs in Fulcrum. I strongly think it's not worth it to keep supporting that.
In my eyes, one of the main points of having version negotiation is to be able to remove functionality.
In general your feedback is appreciated on these matters, as you have implemented both server and client code using the protocol, so clearly you are a subject matter expert.
11 participants
This defines a new Electrum Protocol version: 1.7.
changes compared to 1.6:
blockchain.scripthash.*methods are all replaced withblockchain.scriptpubkey.*methodsblockchain.scriptpubkey.*endpoints #3blockchain.outpoint.subscribeRPC (and related RPCs) to subscribe to a transactionoutpoint, and get a notification when it gets spent.
server.pingcan now also be sent as an unrequested notification, by both the clientand the server. The request/response signature also changed.
blockchain.scriptpubkey.*methodsblockchain.transaction.testmempoolaccepttestmempoolaccept#11mempool.recentto list a few of the latest txs just added to the mempool.mempool.recentprotocol method #13This proposal is less ambitious than previous drafts of 1.7:
method_flavoursfield for theserver.featuresresponse is also postponed,as I realised we are not clear re how to signal/negotiate optional features
server.versionfor feature negotiation #7 ideablockchain.outpoint.subscriberequiresspk_hintas an input param, to help server implsWe have a client implementation in spesmilo/electrum#10627,
and a server implementation in spesmilo/electrumx#303.