search

bitshares / bsips

BitShares Improvement Proposals and Protocols. These technical documents describe the process of updating and improving the BitShares blockchain and technical ecosystem.
https://bitshares.github.io
63 stars 86 forks source link

BSIP 64: Optional HTLC Preimage Length and Add Hash160 Algorithm #163

Closed abitmore closed 4 years ago

abitmore commented 5 years ago 
BSIP: 64
Title: Optional HTLC preimage length and HASH160 addition
Authors: John Jones <jmjatlanta@gmail.com>, abitmore
Status: Draft
Type: Protocol
Created: 2019-07-09
Discussion: https://github.com/bitshares/bsips/issues/163

Abstract

This BSIP proposes two changes to Hashed Time Lock Contracts (HTLC) defined in BSIP 44. The first makes the preimage length parameter optional. The second adds the HASH160 algorithm to the set of allowed hash functions.

Motivation

The original implementation of Hashed Timelock Contracts included a protection to prevent an attack based on the size of the preimage. While it is still believed this is a valuable feature, this makes the implementation incompatible with a strict implementation of the popular BIP199 implemented by Bitcoin.

In addition, BIP199 includes the ability to use HASH160, which is not currently part of the BitShares implementation.

Risks

Note below is a discussion of HTLC risks in general, not risks inherent in implementing this BSIP.

An Oversized Preimage Attack There are limits to the size of a preimage that can be used. That limit is set by the blockchain itself. But when dealing with atomic swaps across blockchains, an attacker could use a preimage that is too large for one side of the 2 sided transaction. That means that an HTLC can be created on both chains, but only redeemed on one chain, and not the other.

This can be mitigated by specifying the preimage size as part of the contract. This feature is available on many bitcoin-based chains among others, but is not standard. Should the preimage size be used with an atomic swap, both sides of the transaction should include the preimage size in their contract.

Timelock Attacks It is standard practice for HTLCs that the timelock should be long enough so that the block that contains the contract can be considered irreversible. This makes it possible for the redeemer to expose the preimage and still be guaranteed that the contract itself will not be reversed.

This becomes even more important with atomic swaps. The shorter duration (a.k.a “inner”) contract should allow time to achieve its own irreversibility. And the longer duration (a.k.a. “outer”) contract must allow time for irreversibility of both.

In addition, with an atomic swap both sides must consider the redemption time necessary. The creator of the inner contract must decide that should the redeemer redeem at the last moment, is there enough time to redeem the outer contract before the timelock expires.

One final consideration of the timelock portion of the contract is the use of capital. Should the other party not accept, the funds in the contract are held until the timelock expires. Long expiration times could result in missed opportunity costs.

Specifications

Technically, the impelentation of these changes are small.

Preimage Size: Permit that an HTLC can have the preimage size set to 0. This removes the size check during validation.

HASH160: Include this additional hashing algorithm to the list of hashes available to be used.

Discussion

https://github.com/bitshares/bsips/issues/163

Copyright

This document is placed in the public domain.

See Also

Bitshares BSIP 44 Bitcoin BIP 199 Preimage Size Attack

syalon commented 5 years ago

In the case of ignoring the extremely low probability of hash collisions, omitting preimage length is much more convenient for the user. I think this improvement is necessary.

jmjatlanta commented 5 years ago

Under the category of "ease of use" I believe we also should include HASH160 as an available hash. Would this require a bsip as well? Is it too different to put it with this bsip?

ryanRfox commented 5 years ago

I do not believe adding HASH160 as an available hash would require a distinct BSIP, as the HTLC BSIP makes clear we intend to support relevant hash algs over time. IMO, same would apply to remove a broken alg. I consider both cases implementation details. Suggest to open an Issue within Core.

ryanRfox commented 5 years ago

I'm open to revising the HTLC BSIP to make preimage_length optional. Beyond the collision protection properties, it also informs the redeeming party what the total fees will be to complete the transaction. Optional, but encouraged works for me. I feel we can author a new BSIP for this change, get it approved, then revise BSIP44 to include the change with references.

pmconrad commented 5 years ago

ISTR we talked about compatibility with other chains when we were discussing recommendations for the committee. IIRC the preimage length was considered an important piece of information because very long preimages might be prohibitive for one chain but not another. Leaving it out would open up a way to scam people. It was also mentioned that the preimage length is supported by other blockchains as well, although not required nor used in standard examples, but that using it is considered "best practice". @jmjatlanta can you confirm, perhaps with a quote? Or am I wrong?

Adding another hash algorithm is a protocol change and would IMO require a BSIP. Of course it makes sense to bundle all planned changes to HTLC into one BSIP instead of writing many little ones.

ryanRfox commented 5 years ago

@pmconrad I'm actually not aware of any HTLC implementations that include hash length. I included in the specification for the reasons you stated above. My desire is for BitShares to improve upon the design bitcoin implemented. I don't want to hinder adoption by making it required. It should be highly recommended (within our documentation) to mitigate risk. I feel it use should become "best practice" going forward.

ryanRfox commented 5 years ago

I feel BSIP44 should be redefined, rather than creating a new BSIP here. It's clear the original design requiring the preimage_length is limiting adoption of our HTLC implementation. As one of the authors on BSIP44, I must admit it contains a design bug. It must be redefined to be an optional field to be compatible with existing HTLC implementations such as BIP199.

pmconrad commented 5 years ago

I feel BSIP44 should be redefined, rather than creating a new BSIP here.

I disagree. A BSIP that has been approved by voters should not be changed anymore.

A modification to the original design is a new change that should be described and approved separately. (Precedent: RFCs)

Content-wise I still think a mandatory preimage length is a good idea because it prevents certain ways to scam people. (Admittedly, the mandatory length open certain other ways to scam careless people. The important difference is that with the mandatory length careful people can protect themselves whereas without they can't.)

Perhaps we should compile an overview

ryanRfox commented 5 years ago

A BSIP that has been approved by voters should not be changed anymore.

I will support this and retract my prior statement. If a new BSIP is approved, status of BSIP44 would become superseded. This seems the best case.

The only site I am aware of tracking HTLC (and PC) compatibility is https://swapready.net/ However, it does not capture preimage length requirements (optional). We may need to make our own.

jmjatlanta commented 5 years ago

If this meets your requirements @abitmore, I can move it to the top of this issue...

BSIP: TBD
Title: HTLC Compatibility Changes
Authors: John Jones <jmjatlanta@gmail.com>, abitmore
Status: Draft
Type: Protocol
Created: 2019-07-09
Discussion: https://github.com/bitshares/bsips/issues/163

Abstract

This BSIP proposes two changes to Hashed Time Lock Contracts (HTLC) defined in BSIP 44. The first makes the preimage length parameter optional. The second adds the HASH160 algorithm to the set of allowed hash functions.

Motivation

The original implementation of Hashed Timelock Contracts included a protection to prevent an attack based on the size of the preimage. While it is still believed this is a valuable feature, this makes the implementation incompatible with a strict implementation of the popular BIP199 implemented by Bitcoin.

In addition, BIP199 includes the ability to use HASH160, which is not currently part of the BitShares implementation.

Risks

Note below is a discussion of HTLC risks in general, not risks inherent in implementing this BSIP.

An Oversized Preimage Attack There are limits to the size of a preimage that can be used. That limit is set by the blockchain itself. But when dealing with atomic swaps across blockchains, an attacker could use a preimage that is too large for one side of the 2 sided transaction. That means that an HTLC can be created on both chains, but only redeemed on one chain, and not the other.

This can be mitigated by specifying the preimage size as part of the contract. This feature is available on many bitcoin-based chains among others, but is not standard. Should the preimage size be used with an atomic swap, both sides of the transaction should include the preimage size in their contract.

Timelock Attacks It is standard practice for HTLCs that the timelock should be long enough so that the block that contains the contract can be considered irreversible. This makes it possible for the redeemer to expose the preimage and still be guaranteed that the contract itself will not be reversed.

This becomes even more important with atomic swaps. The shorter duration (a.k.a “inner”) contract should allow time to achieve its own irreversibility. And the longer duration (a.k.a. “outer”) contract must allow time for irreversibility of both.

In addition, with an atomic swap both sides must consider the redemption time necessary. The creator of the inner contract must decide that should the redeemer redeem at the last moment, is there enough time to redeem the outer contract before the timelock expires.

One final consideration of the timelock portion of the contract is the use of capital. Should the other party not accept, the funds in the contract are held until the timelock expires. Long expiration times could result in missed opportunity costs.

Specifications

Technically, the impelentation of these changes are small.

Preimage Size: Permit that an HTLC can have the preimage size set to 0. This removes the size check during validation.

HASH160: Include this additional hashing algorithm to the list of hashes available to be used.

Discussion

https://github.com/bitshares/bsips/issues/163

Copyright

This document is placed in the public domain.

See Also

Bitshares BSIP 44 Bitcoin BIP 199 Preimage Size Attack

pmconrad commented 5 years ago

This is now duplicated by #174 - which one should we close / where to continue the discussion?

@jmjatlanta please add a "Risks" section describing attack vectors with and without preimage length. Also note that other chains may have implicit limits that are not well-defined, e. g. through maximum transaction length.

jmjatlanta commented 5 years ago

Well, @abitmore won as his issue number is lower. Do you agree @ryanRfox ? I say we merge the two by copying or linking any important comments in the other to this one.

And, yes, I believe a "Risks" session is in order. I will add it.

ryanRfox commented 5 years ago

Yes, let's merge the relevant bits from here, then close.

jmjatlanta commented 5 years ago

Interesting post from merged issue:

Does declaring preimage length reduce collision risk?

jmjatlanta commented 5 years ago

I would like to ask @abitmore as the OP if it would be okay for me to change the title to BSIP64: Hashed Time-Locked Contract and move the above post to the top.

Also, please edit or let me know if someone should be added to the authors list.

abitmore commented 5 years ago

I think it's better (readability) to have the title and top post (OP) expresses what piece we're changing, than to show the whole thing after changed. It would be more friendly to readers.

abitmore commented 5 years ago

@jmjatlanta the above post is fine to updated to OP. Title should be updated to "BSIP64: something" but I think "BSIP64: Hashed Time-Locked Contract " is not a good one.

abitmore commented 5 years ago

"Title: HTLC Compatibility Changes" is not clear enough as a title. We have enough room to add a brief about the changes in the title.

jmjatlanta commented 5 years ago

Modified title and original post. The old OP is below:

HTLC of some (or all?) other blockchains has no preimage length parameter. To be compatible we can effectively let it be optional, e.g. don't check the length when redeeming if the HTLC's preimage_length field is zero.

Related discussion: https://github.com/bitshares/bitshares-core/issues/1713#issuecomment-485272734

@syalon would this work for you?

abitmore commented 5 years ago

@jmjatlanta please update the text in OP, add BSIP number and update the "title" in the text.

Why not create a pull request?

ryanRfox commented 4 years ago

Core Team today discussed removing SHA1 from the list of supported hash functions, as its crypto is considered "broken". The use of SHA1 on bitcoin and other networks is already discouraged. The BitShares project intends to mitigate risks to end users by implementing good crypto best practices. We propose the removal of SHA1 from the implementation by revising the BSIP64 spec.

ioBanker commented 4 years ago

Please add the support of ed25519-sha-256 since it's so important when we later start creating atomic swaps compatible with XRP network.

ioBanker commented 4 years ago

JS code for creating the Condition & Fulfillment on interledger

ryanRfox commented 4 years ago

@ioBanker I too am keen to support additional hash algos to facilitate cross-chain communications. Unfortunately, at this time we must defer this change request to add ed25519-sha-256 into a future BSIP draft, as we desire to place BSIP64 for voting as is to allow us the best opportunity to make the 4.0.0 Protocol Release if approved.

I encourage you to begin that process now by using BSIP64 as a template for a new draft BSIP to add ed25519-sha-256 into our HTLC protocol. Just create a new Issue, paste in the BSIP text to the description and modify the relevant sections. Thanks for your efforts.

abitmore commented 4 years ago

The draft has been written and merged, see #195.