Skip to content
This repository has been archived by the owner on Jul 14, 2020. It is now read-only.

Initial draft of RFC-003 #13

Merged
merged 38 commits into from Jan 18, 2019
Merged

Initial draft of RFC-003 #13

merged 38 commits into from Jan 18, 2019

Conversation

LLFourn
Copy link
Contributor

@LLFourn LLFourn commented Jan 15, 2019

Please review.

Fixes #7

In this RFC, the expiry activation returns the asset to the original owner while a secret activation transfers it to the other party.
Therefore this document will refer to these paths as *refund* and *redeem* respectively.

The HTLCs defined in this specification use *absolute time locks* where the expiry is set at a specific time in the future.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ the motivation for this is that it's an attack vector if we use relative timelocks on the beta ledger. If Alice has the capability to delay Bob's HTLC from getting confirmed she essentially changes beta_expiry to a later point such that she may be able to redeem and then refund.

What do you guys think? Do you think this is worth explaining in this RFC?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely!

I think stuff like that is actually pretty important. Documenting the downsides and weaknesses of protocols is a sign of maturity IMO :)

The identity on β that **B** will be transferred to when the β-HTLC is activated with the correct secret.

#### `secret_hash`
Type: `HexString` <!-- Where to define this. I guess it should be RawBytes and in the JSON encoding it's Hex -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where to hang these definitions? In the registry also? Or can we add some primitive types to RFC001 and how to encode them in JSON.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a need to "normalize" or "DRY" too much here. Just say it is a hex-string.


This RFC describes a basic atomic swap protocol and the related parameters required to use it as a COMIT [RFC002](./RFC-002-SWAP.md#protocol) SWAP protocol.
It uses Hash Time Locked Contracts to swap ownership of two assets on different ledgers between two parties.
It is a simplified version of the protocol originally described by TierNolan[^fn1].
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This citation thing didn't work at all XD

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work! I like the way you wrote this RFC.


### Identity

This RFC introduces the notion of a "identity".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo?

Suggested change
This RFC introduces the notion of a "identity".
This RFC introduces the notion of an "identity".

The protocol begins with a SWAP request with the `protocol` header's value set to `COMIT-RFC-003` and the following header parameters:

#### `hash_function`
Type: [Hash Function](./COMIT-registry.md#Hash Functions)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ This link doesn't render correctly.

### SWAP Request Body
When `COMIT-RFC-003` is used as the value for `protocol` for a `SWAP REQUEST` message the body must have the following fields:

#### `alpha_expiry`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 I think these would be more readable in the form of a table.

This indicates to the sender that the difference between `alpha_expiry` and `beta_expiry` is too small and the receiver may accept the swap if they are given more time.

parameters:
| Name | JSON Type | Description |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ The table doesn't render correctly.

RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
The protocol is described below as if both parties had immediate access to the most recent state of the ledger and are able to effect persistent actions to it immediately.

For Ledgers with a probabilistic notion of the latest state such as the Bitcoin and Etheruem networks, parties must wait until they have confidence that their counterparty's transactions will not be reverted before they take any action.
Parties must also take this into account when choosing or accepting the `alpha_expiry` and `beta_expiry` parameters (see [Security Considerations](#Security Considerations)).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Link doesn't render correctly.


The HTLC definitions for particular ledgers will be included in subsequent RFCs

### Alice deploys α-HTLC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Maybe have a 1. in front of this?

RFC-003-SWAP-basic.md Show resolved Hide resolved
An identity belongs to a user and represents the minimal information required to transfer an asset to that user on a particular ledger.
On blockchain based ledgers a user's identity is usually some function of a public key where the private key is only known to the user.

This RFC extends the [COMIT-registry](./COMIT-registry.md) to include a section specifying the identity for each supported ledger and any subsequent RFCs adding support for a ledger must include an identity specification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ I think all these extensions should go in a combined section (# Registry extensions) at the end. This would help to not forget anything.

RFC-003-SWAP-basic.md Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor

Pretty good work! I like it :)

COMIT-registry.md Outdated Show resolved Hide resolved
COMIT-registry.md Outdated Show resolved Hide resolved
COMIT-registry.md Outdated Show resolved Hide resolved
COMIT-registry.md Outdated Show resolved Hide resolved
COMIT-registry.md Outdated Show resolved Hide resolved
| `beta_expiry` | `u32` | The UNIX timestamp of the time-lock of the beta HTLC |
| `alpha_refund_identity` | `α::Identity` | The identity on α that **A** can be transferred to after `alpha_expiry` |
| `beta_redeem_identity` | `β::Identity` | The identity on β that **B** will be transferred to when the β-HTLC is activated with the correct secret. |
| `secret_hash` | `hex-encoded-bytes` | The output by calling `hash_function` with the secret as input |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd clarify whether it's prefixed with 0x

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could have an example column?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make an examples section at the end.

We should definitely clarify what exactly hex-encoded-bytes means somewhere but doing it everywhere we use it will be a pain. I still think RFC1 which defines the JSON encoding is the right place to define these primitive types we can reuse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LLFourn can you correct in RFC-001 now or open an issue then?

RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
D4nte and others added 3 commits January 17, 2019 14:45
Co-Authored-By: LLFourn <LLFourn@users.noreply.github.com>
And s/must/MUST/ everywhere
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
Copy link
Contributor

@D4nte D4nte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vendu!

Copy link
Collaborator

@luckysori luckysori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nitpicks, I like it a lot 🥇

RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
The Hash Time Locked Contract (HTLC) is the primary construct used in this protocol. An HTLC locks an asset until one of two possible paths are activated:

- **Activation with secret**: The contract is activated with a *secret*. The hash of the secret MUST match the hash in the contract.
- **Activation after expiry**: The contract is activated after a time fixed in the contract.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙃 May just be me, but this could be misinterpreted as the contract activating the refund path on its own after the time lock has expired.

RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
RFC-003-SWAP-basic.md Show resolved Hide resolved
RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
This protocol offers an application the following functionality:

- **Up for Sale**: Alice puts an asset **A** up for sale until `alpha_expiry`
- **Give Option**: Bob can give Alice an *option* to exchange for **A** for his asset **B** until `beta_expiry`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **Give Option**: Bob can give Alice an *option* to exchange for **A** for his asset **B** until `beta_expiry`
- **Give Option**: Bob can give Alice an *option* to exchange **A** for his asset **B** until `beta_expiry`.

RFC-003-SWAP-basic.md Outdated Show resolved Hide resolved
Lucas Soriano and others added 5 commits January 18, 2019 11:29
Co-Authored-By: LLFourn <LLFourn@users.noreply.github.com>
- Clear up that *someone* must activate the HTLC
- Clear up section about absolute time locks
Co-Authored-By: LLFourn <LLFourn@users.noreply.github.com>

This RFC describes a basic atomic swap protocol and the related parameters required to use it as a COMIT [RFC002](./RFC-002-SWAP.md#protocol) SWAP protocol.
It uses Hash Time Locked Contracts to swap ownership of two assets on different ledgers between two parties.
It is a simplified version of the protocol originally described by TierNolan [\[1\]][1].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
It is a simplified version of the protocol originally described by TierNolan [\[1\]][1].
It is a simplified version of the protocol originally described by TierNolan [\[1\]](1).

@LLFourn LLFourn merged commit 9589c48 into master Jan 18, 2019
@LLFourn LLFourn deleted the RFC-003 branch January 18, 2019 03:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants