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

Update instructions on how to build Bitcoin HTLC#90

Merged
thomaseizinger merged 1 commit into
masterfrom
thomaseizinger-patch-1
Jul 5, 2019
Merged

Update instructions on how to build Bitcoin HTLC#90
thomaseizinger merged 1 commit into
masterfrom
thomaseizinger-patch-1

Conversation

@thomaseizinger
Copy link
Copy Markdown
Contributor

@thomaseizinger thomaseizinger commented Jul 4, 2019

Instead of a table that instructs to concatenate hex strings, we provide a table with replacement offsets. This is much easier to implement than less error-prone.

Resolves #82.

I have also noticed a mismatch in the naming. We sometimes use expiry (in the SWAP REQUEST message) but the rest of the RFC uses refund_timestamp. To not block this PR, I created an issue to fix this: #89

Instead of a table that instructs to concatenate hex strings, we provide a table with replacement offsets. This is much easier to implement than less error-prone.

Resolves #82.
@thomaseizinger thomaseizinger force-pushed the thomaseizinger-patch-1 branch 2 times, most recently from 6164e59 to 129cc15 Compare July 4, 2019 08:31
Copy link
Copy Markdown
Member

@da-kami da-kami left a comment

Choose a reason for hiding this comment

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

I approve these changes in general, but we should discuss if we really want to rename expiry to refund_timestamp.

expiry describes the HTLC on a more abstract level, refund_timestamp for our specific use-case.

From my point of view the higher level protocol always defines the lower level, and https://github.com/comit-network/RFCs/blob/master/RFC-003-SWAP-Basic.md defines it as expiry - refund_timestamp is not mentioned there.

Copy link
Copy Markdown
Member

@da-kami da-kami left a comment

Choose a reason for hiding this comment

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

#89 documents the name changes clearly, approved.

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.

Use offset to specify Bitcoin Basic HTLC Swap

4 participants