• The hash-ID for the double spend proof is a double sha256 over the entire serialized content of the proof, as defined next.

    Is there a need to use this as ID instead of just the outpoint? Do we envision ever needing to store or relay more than one proof per outpoint?

  • Yes, there is a need.

    While there is no need for there to be more than one proof per outpoint, there can be an infinite amount of such proofs. And we want to be able to differentiate before we download the proof which one it is.

    As such, the hash-ID is not just an ID for the outpoint. It is an ID for the entire proof.

    The ID is a hash of the entire content to protect from altering the contents. Much like this is the case in most of the protocol (tx, block, etc). For instance you see a transaction being advertised by INV and if after download the transaction doesn't validate its txid is remembered and we don't download it again later. I have a patch for Flowee doing the same for proofs, that a proof is only downloaded once. Even if its tossed as invalid.

    If you were to use your suggestion of using an outpoint, the following attack is possible;

    • Person intents to double spend outpoint O.
    • Person sends an invalid (not even validating) proof with ID based on 'O'
    • Node downloads, rejects and blacklists hash.
    • Person double spends and is certain no node will learn about it.

    If nodes do not black-list already validated but rejected proofs then sending INVs will cause a lot of bandwidth to be burned to send the same thing again and again.

    Hope that's clear. Let me know if its not.

  • That attack seems to be mitigated by simply blacklisting the peer instead of the proof.

  • Possibly, I went with what bitcoin already does in these situations.

    Did you see a benefit to use the outpoint-hash as an ID?

  • It just seems like useless work to inspect both the whole-proof hash and the outpoint, the latter of which you'll have to inspect anyway. The former does not offer any additional defense against DoS - since it's trivial to "generate" invalid proof with a different hash - and seems to be additional hashing and inspection for no benefit.

    At the end of the day you'll likely need to ban the peer who forward you invalid proof anyway. Using outpoint saves you a hash, makes redundancy detection slightly easier among honest peers, and simplifies things.

  • I think you have a misconception of what the hash is used for. It is not inspected or checked. Any more than a block or a transaction have their txid/blockhash inspected. Instead, we just use it as a identifier that is generated on the fly should we need it.

    The usage of the outpoint for ID would in the end have zero effect on how much checking we do.

    And, yes, on a non-valid proof the peer that forwarded gets banned. On the other hand, low-effort Sybill is avoided by remembering the hash (generated from the actual proof they sent us).

  • If it's used as an identifier, then in the case two or more different proofs exist for the same outpoint, both will need to be downloaded. Using outpoint as identifier, the second one can be not downloaded at all.

  • Someone asked me to followup;

    If it's used as an identifier, then in the case two or more different proofs exist for the same outpoint,

    that usecase means that we are talking about triple spends. As the spec is designed such that a double spend can only lead to one unique proof.

    both will need to be downloaded. Using outpoint as identifier, the second one can be not downloaded at all.

    This is true. In case of a triple spend you could avoid downloading the second proof.

    The problem, as I described in my previous answer, is that the Bitcoin design is based on a unique ID for a unique primitive. Going away from this is a huge undertaking and I personally don't see the benefit.

    Yes, for every additional transaction spending a specific outpoint there exists a new proof, and INV. But as nodes very quickly will have that outpoint already marked with a DSP the additional ones will stop propagating and thus you won't see them clogging up the network. Just like triple spends don't clog up the network because of the first seen principle a double spend proof won't be propagated when that outpoint is already notified as having a DSP.

    Bottom line; the p2p layer as we inherited doesn't like your optimization as its different from what everything else does.

  • I've read through this a couple times and I think I get the overall concept, but the technical details are a bit too deep for me to comfortable provide meaningful feedback. I asked some clarifying questions on twitter that might be of some insight to how others like me might look at this.

    There are some minor things that might make sense to update, such as clarify that this really isn't about proving someone spent exactly twice, but rather more-than-once, but that is mostly semantics.

    Some of the properties of the protocol might not be immidiately obvious to a layman or someone who is new to the underlying structures, so it might be valuable to explain at more depth how or where the important properties come from.

  • Thank you Jonathan, the main goal of the spec is to let other implementations support this from a specification first. The concept that if there are differences in code, then the spec is the one that wins. This to counter the problem of "other chains" that specify that there is a reference implementation which is right.

    I will take a look at clarifying some of the points you highlighted, you made some good observations here and on Twitter.

    1. What are we doing with the protocol version?

    2. Are we sending reject messages if the dsproof is rejected?

  • What are we doing with the protocol version?

    The message uses the existing inv/getdata messages which have been around forever. Only nodes that recognize and explicitly request the inv to be for a DSP will get the new message. Added to this that the majority of the node implementations aim to have this implemented soon, there should be no need for SPV clients to do work in order to find DSP capable peers.

    As such the protocol version doesn't really seem to be relevant to me.

    Are we sending reject messages if the dsproof is rejected?

    A DSProof can be "rejected" based on two grounds. One is that there is no transaction in the mempool to complete the data with in order to verify it. My implementation waits some time and then tosses the DSP giving one (out of 100) ban point to the peer.

    The second ground a DSProof can be rejected is because it does not validate. And in that case I suggest simply disconnecting the peer as they are violating the protocol.

    I don't see any other cases where a rejection message is useful at this time.

  • Feeding some review comments upstream from our downstream review in BCHN:

    I linked to our review comments so you can find which content those comments relate to exactly.

    Edited by freetrader
  • Thank you! I made all the relevant changes, it shows the age of this doc that I can remove the bip62 note :)

  • Why do the spender fields (FirstSpender, DoubleSpender) have no sizes?

    Also, you may want to update this:

    Last Revised: 2020-08-11

  • Why do the spender fields (FirstSpender, DoubleSpender) have no sizes?

    What would you put there?

  • It's clearly variable (due at least to the push-data), so I would put something like "variable, see below" (since the tables don't have proper names).

    I checked on the implementation provided to BCHN, and did not find any of the list-size fields in the data structure or its serialization.

    So right now I'm uncertain why there is a discrepancy here between the structure laid out in the spec above, and the implementation.

    Edited by freetrader
  • I would put something like "variable, see below"

    That would mess up the table structure a bit and since the lines between the two tables explain thi, i'll skip this suggestion.

    Thanks for your review!

  • Ok, after reading the code it's clear now that pushData is a vector of byte vectors, and each vector is serialized with a preceding list-size giving its number of elements.

    The first one ("Number of push-data's") seems always one according to current code), which is why there is only one push-data listed in the table, and it all results in two consecutive list-size items.

    I would suggest renaming the description "Number of items in the push-data list" to "Number of bytes in the push-data list" since the push-data itself is a vector of bytes, and there should be no question about the length of an item in such a vector - it is precisely 8 bits.

  • The question has been raised: what is the use case for multiple push-data's in one Spender?

    ie. what do we need the list-size field "Number of push-data's" for? It seems to contain a single signature always, and the spec states

    The details required to validate one input are provided in the spender field

    There doesn't seem to be a reason to store multiple signatures in one Spender.

  • I would suggest renaming the description...

    Sure, I can adjust it for someone with your eye.

    what is the use case for multiple push-data's in one Spender?

    Future extensibility. Multisig comes to mind. Maybe it will be removed in the next iteration. The goal is to get this on mainnet now so we can get the rest of the infrastructure to start using this imperfect solution. Running code beating perfect code.

    Edited by Tom Zander
  • While reviewing this spec w.r.t. our implementation and tests, I stumbled across the statement in 2020-09-20 spec version):

    Transactions that spend all, confirmed, P2PKH outputs with all inputs signed SIGHASH_ALL without ANYONECANPAY, are double-spend-proof's "protected transactions".

    That would mean txs that spend unconfirmed outputs are not protected.

    Is that really supposed to be like that? Because apparently, that's not how it is implemented, at least in BCHN right now. I raised this question on our Slack, and @matricz kindly implemented a test which confirms that it's not implemented like that.

    https://gitlab.com/bitcoin-cash-node/bitcoin-cash-node/-/merge_requests/700/diffs?commit_id=ff00e0b9f7375417af2265c05067a8c3797f76c8

    It the "only confirmed input" aspect is supposed to be like that, I would like to know the rationale, as it seems it would make DSProof less useful. So I'm hoping this is just an obsolete part of the spec, and we can remove the word 'confirmed' from the spec there.

  • Hey Freetrader.

    Thats a really good question and hints at a rather subtle detail. Good catch!

    This is about a wallet receiving a transaction with unconfirmed parents.
    If your unconfirmed parent transaction is double spent, a bloom-filter / p2p using client won't be guarenteed to get the DSProof because there may be no match in the bloom filter.

    There are several solutions for this, and each solution has some up and down sides. More importantly, each of those solutions can be applied to a different layer of the stack. Some work in the wallet (it tracks all parents), some in the middleware, and maybe something in a full node.

    AFAIK no perfect solution as of yet exists. More importantly, due to the complete absense of pure p2p wallets (i.e. ones that don't use any middleware), it would be unneeded overhead to add such a feature at this layer of the stack.

  • My question here is only about what a full node client is supposed to do.

    I'm assuming any other wallets talk to a full node which is supposed to implement the DSProof spec.

    So in order to accept a transaction T to the mempool, a full node would need to know about all its ancestors. That much is given. So let's say T is accepted (not yet confirmed).

    Now we just consider the case where at least one of these ancestors is unconfirmed (it could be more).

    If transaction T is valid (according to mempool acceptance criteria), and has unconfirmed ancestors, and the full node sees a double spend of T, it could notify, technically, with proof.

    That seems to be, in fact, what the DSProof code in BCHN is doing at this time. It also seems logical. If I'm the (merchant) recipient of the spend T, I'd like to know about double spends of it. For simplicity, we can even assume the merchant (me) is running his full node, if that's the way to be reasonably sure at this time to get the DSProof notification.

    So... I assume the code is correct?

  • So... I assume the code is correct?

    AFAIK, I wrote the code. It is correct.

  • If the code is correct (and I do agree the code is sensible), I'd see a need to fix the spec to agree with the code. Suggestion:

    Transactions that spend only all, confirmed, P2PKH outputs with all inputs signed SIGHASH_ALL without ANYONECANPAY, are double-spend-proof's "protected transactions".

  • I have given this argument some thought and I think the spec deals only with confirmed transaction because of the difficulties of dealing with unconfirmed transactions. Say we have a transaction chain, and say A is confirmed

    A -> B -> C -> D
               \
                -> E

    We can easily detect and prove a double spend between D and E

    But say that my node is aware of if the chain from A to D and a double spend attempt is made like this

    A -> B -> C -> D
          \
           -> E

    In this case I've validated C and D, and sent them to SPV nodes, which sent those to wallets.

    C is easily flagged as being doublespent, but how do I flag the risk for D or any other following transaction?


    Now if I check for double spending only transactions that are confirmed and everybody knows that, I have a scenario like this:

    A -> B -> C 
     \
      -> D

    In which I can detect a double spend between B and D. C is still not covered.

    So what changed wrt the second case above? If everybody knows that unconfirmed transactions are not covered by DSPs, they attach a different risk to transactions coming from unconfirmed inputs in general.

    C is not covered, but nobody expects it to be.

    Edited by matricz
  • Thanks matricz, that is exactly right.

    And to be clear; this is the spec for the Proof only. A network message. Which is a building block in the full solution. Making clear what we don't solve here means that others that build on top of this know what to be aware of and what do solve. That is why the 'limitations and risks' part is in the spec.

  • All of the inputs in the relevant transaction, and its mempool ancestor chain (Optional, requires BIP62), must be signed SIGHASH_ALL without ANYONECANPAY.

    What's the rationale for the exclusion of ANYONECANPAY hashtype?

    How can the relevant transaction have an optional mempool ancestor chain if only confirmed inputs are protected?

    Edited by freetrader
  • What's the rationale for the exclusion of ANYONECANPAY hashtype?

    The two concepts are mutually exclusive.

    If you don't what anyonecanpay (or hashtypes) do, here is some docs:

    https://gitlab.com/FloweeTheHub/thehub/-/blob/master/libs/utils/TransactionBuilder.h#L67-112

    This is the relevant quote that should make it clear why they are mutually exclusive.

    This option allows for full combining of this input with any other inputs, even after signing.

    You also asked:

    How can the relevant transaction have an optional mempool ancestor chain if only confirmed inputs are protected?

    The two are not mutually exclusive. You question implies you see a conflict where none exists.

    And to be clear; this is the spec for the Proof only. A network message. Which is a building block in the full solution.

  • Thanks matricz, that is exactly right.

    Happy to have understood it well. I have given it some more thought, and I'm leaning toward leaving DSPs enabled for unconfirmed transactions too, as is in the current implementation in BCHN (and conflicting with the spec). To the best of my understanding BU also works like BCHN.

    To the best of my understanding BU has also implemented a mechanism to flag all descendants of a double-spent transaction as belonging to a double-spent chain (and sending that through an INV to SPV server), as described by Mr Tschipper in BU slack.

    How is this implemented in flowee?

  • Please look at this comment which explains this fully: https://gitlab.com/-/snippets/1883331#note_475994460

  • I just want to write a quick comment about matricz writing:

    and conflicting with the spec

    Why do you think that sending a valid DSProof is in conflict with the spec?

  • I'm leaning toward leaving DSPs enabled for unconfirmed transactions

    Just noting here that it's about the parents having to be confirmed or not, that was my original request for clarification.

    To me, clarity on which events are covered by DSProofs (ie. a DSP will be generated and broadcast) is an important part of this specification, if we wish users to consider the feature reliable.

    This is already hinted at in the abstract, which also mentions unconfirmed transactions, which necessarily leads to consideration of the possibility of chains of such, and what to do about them w.r.t. DSPs.

    The ability to get informed of such an event can assist greatly in the confident acceptance of unconfirmed transactions as payment. We expect this will help with countering undetected attempts of double spends.

    Edited by freetrader
  • To me, clarity on which events are covered by DSProofs (ie. a DSP will be generated and broadcast) is an important part of this specification

    I might not have said this yet, but this is the spec for the Proof only. A network message. Which is a building block in the full solution.

    More info; https://bitcoincashresearch.org/t/double-spend-proof-roadmap/151

    Edit;

    I already answered your question about 3 times... You don't seem to react to any of the answers I give, so I don't know if you understand them or not, or just don't like the answer that I give.

    The basic point is that this spec is about 1 message. As you can see in the bitcoincashresearch article this is the start. When (finally!) you merge this, we can start talking to middleware and other services to start to add support for things that this basic message itself does not (and can not) support. There are no relevant wallets on the market that will actually use this message directly, support needs to go through several systems (called layers) before we have a full feature.

    Thank you for asking questions, but you seem to misunderstand that the BCHN PR (700) is blocking us from actually STARTING to get support across the entire industry. You asking how things look after the entire industry added support is misguided. Start the merge and we can live that life together and influence it together.

    Edited by Tom Zander
  • I'll try to explain myself better. Sorry for insisting, but I think there is ambiguity in our language that may be in the way of understanding. I won't stress this any further after this comment.


    Say you have 2 scenarios, both start with A being a confirmed transactions, while all others are unconfirmed

    A -> B -> C -> D
     \
      -> Z

    In Scenario 1, Z is a double spend attempt of B, from the common confirmed parent A.

    C and D are on a double-spent-chain: they are descendants of double-spent transaction.

    A -> B -> C -> D
          \
           -> Z

    In Scenario 2, Z is s a double spend attempt of C, from the common unconfirmed parent B. D is the double-spent-chain here.


    The spec says that the Double Spend Proofs should support Scenario 1 only, but the current implementation in BCHN does support scenario 2 too (as verified by ft through reading the code, and by my by implementing a functional test). Mr Tschipper of BU has also said that BU supports scenario 2 too.


    What is the difference between the two scenarios? Why Scenario 2 should not be supported?

    I want to claim and demonstrate that there is no difference.

    In both cases, we create a DSP for the double-spent transactions (B and Z in Scenario 1, and C and Z in Scenario 2)

    In both cases, we create no warning for the transactions in the double-spent-chain (C and D in Scenario 1, and D in Scenario 2)

    In both cases, we rely only on what's happening in my mempool. No considerations on ancestors or descendands are done.


    Initially I had thought that it was better to apply the DSP spec to confirmed transaction only, as claimed by the spec, but later was convinced that there is no cost to apply them to unconfirmed transactions also. There is no difference in confidence we can give to transactions in the double-spent-chain from a confirmed transaction (C and D in Scenario 1) and transactions in the double-spent-chain from an unconfirmed transaction (D in Scenario 2).

    On the other hand, we increase the utility of DSPs by applying them to unconfirmed transactions also. I think that this should probably be part of the spec from the start.


    Edit to add: it looks to me that Flowee does explicitly support constructing Double Spend Proofs for transactions spending unconfirmed outputs, as described for example here.

    Since at this point Flowee, BU, and soon BCHN all support creating DSPs from unconfirmed outputs, and if we find no drawbacks to doing so, I think it would be ok to drop the requirement of constructing DSPs only from confirmed outputs, as described in "Limitations and risks" Transactions that spend all, **confirmed**, P2PKH outputs with all inputs signed SIGHASH\_ALL without ANYONECANPAY, are double-spend-proof's "protected transactions".

    Edited by matricz
  • Forwarding another comment from review:

    The bitcoincash.org site whose repository islinked in the References may be an unreliable source of information about Bitcoin Cash (as seen by events in 2020 with the ABC split).

    You might want to find a more dependable link for that spec. I can offer https://upgradespecs.bitcoincashnode.org/replay-protected-sighash/ .

  • The spec says that the Double Spend Proofs should support Scenario 1 only

    What? No, it doesn't.

    Are you making an assumption that only DSPs are sent for what the "Limitation and risks" chapter calls "protected transaction"? I don't see the basis for such an assumption. I don't see any line in the spec making such a suggestion.

    DSPs are sent for all conflicting transctions. Regardless of parents being confirmed.

    The "limitations and risks" part is in the spec is targetted at consumers of the network message. I don't think this is very confusing as this is quite common in specifications.

  • Forwarding a suggestion from BCHN review to bullet-list this information in the 'Merchant Considerations':

    The transaction must contain all P2PKH.

    The transaction must either be spending only from confirmed UTXOs, or all of its ancestors in mempool must also be all-P2PKH transactions (Optional, requires BIP62).

    All of the inputs in the relevant transaction, and its mempool ancestor chain (Optional, requires BIP62), must be signed SIGHASH_ALL without ANYONECANPAY.

  • @freetrader

    Forwarding another comment from review

    The best solution is to simply integrate the spec into the shared BCH spec and link to the same repo. This would have happened some months ago, but we are waiting for BCHN to merge this before we reach the point where we can integrate this spec in the common protocol spec.

  • Are you making an assumption that only DSPs are sent for what the "Limitation and risks" chapter calls "protected transaction"?

    Yes, I did made that assumption and based my understanding of the spec on it. The language in that chapter confused me to think that, by the spec, only transactions spending confirmed outputs were to be covered by DPSs.

    The "protected transaction" wording is confusing. It exists in that chapter only, and has no other definition.

    I'm happy to recognize that we agree on the usefulness on creating DPSs for transactions spending unconfirmed outputs, and that this was a misunderstanding.

    I think I would suggest to change the language of that chapter slightly, or make a clarification, if possible, to reduce future misunderstandings.

    Cheers!

  • Forwarding a suggestion from BCHN review to bullet-list this

    Thank you so much for this suggestion. I think this is identical to the list already discussed some months ago, it didn't make the cut then.

  • Thanks matricz for writing and I think it is now resolved.

  • The best solution is to simply integrate the spec into the shared BCH spec and link to the same repo. This would have happened some months ago, but we are waiting for BCHN to merge this before we reach the point where we can integrate this spec in the common protocol spec.

    The referenced document should be part of BCH-UAHF (BUIP-55) which is currently not yet imported into the common BCH spec (I used reference.cash to check) although some later upgrade docs are.

    This merge process is unrelated to the BCHN merge of DSProofs. But indeed, once the replay-protected-sighash doc becomes part of the common BCH spec set, that would be a good place to link to.

    Edited by freetrader
  • This merge process is unrelated to the BCHN merge of DSProofs.

    No, its not. As I just wrote. The integration into the reference spec is blocked by the DSProof message being merged into BCHN. This is a decision that is outside your and my decision making capability. You, however, can decide to merge DSProofs in BCHN in order to allow us to move on with various projects.

  • You're talking about the integration of this DSProofs spec, I was talking about the integration of the UAHF spec, part of which your doc links to.

    My comments over the last few days have been made precisely because BCHN is in the process of rounding off the DSProof merge request. Trust me that this is high up on our list of priorities for our next release, and we have several people actively working to complete things.

    I'm satisfied that as a result of our review, we (and BU) already managed to avoid a potential source of DoS in the implementations, and clarified several ambiguities in the spec.

    I think we all look forward to moving on with this.

    Edited by freetrader
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment