Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash Time Lock Contracts #168

Merged
merged 34 commits into from
Mar 18, 2024
Merged

Hash Time Lock Contracts #168

merged 34 commits into from
Mar 18, 2024

Conversation

JoshOrndorff
Copy link
Contributor

@JoshOrndorff JoshOrndorff commented Feb 5, 2024

This PR is a first step towards more interesting verifiers and cross-chain atomic swaps. Already there are enough verifiers to support swaps between two Tuxedo chains with the HTLC verifier 🚀

Before we can realize the goal of serving on the arbitrating side of the Farcaser protocol we would need Either a verifier that supports bitcoin script, or dedicated farcaster verifiers, both of which are feasible and put off until a future PR.

Some references I followed:

New verifier implementations

  • Simple hash lock. Can be spent simply by supplying a preimage.
  • Simple time lock. Can be spent by anyone after a given block height.
  • P2PKH. Private ownership just like a simple sig check but doesn't leak the pubkey. Inspired by bitcoin.
  • HTLC. Has a spend path that requires a signature of the recipient and a preimage. Also has a refund path which requires a time delay and a signature by the refunder.

@JoshOrndorff JoshOrndorff marked this pull request as ready for review February 10, 2024 22:27
@@ -0,0 +1,380 @@
//! This module contains `Verifier` implementations related to Hash Time Lock Contracts.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For review purposes, this file is the main additions.

The rest of the diff is mostly moving existing verifiers to dedicated files.

muraca
muraca previously requested changes Feb 11, 2024
Copy link
Collaborator

@muraca muraca left a comment

Choose a reason for hiding this comment

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

I would have preferred to see the changes to the Verifier trait in a separate PR, then make HTLC depend on it. Although being small, I think it's a better approach for such a breaking change, but that's up to you.

Anyway the Redeemer is a really nice and clever addition, great job with that!
And the HTLC implementation looks good to me.

PS please take a look to the failing test 😿

tuxedo-core/src/verifier/htlc.rs Show resolved Hide resolved
Comment on lines 80 to 84
/// interesting to use public key hash, or better yet, simply abstract this over some opaque
/// inner verifier for maximum composability.
///
/// @lederstrumpf when the time expires, is the primary "happy" path supposed to remain open?
/// Or is it that if the swap hasn't started on time, the refund is the only option.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these kinds of comments should be kept out of the docs

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. This one needs to be resolved before this is merged. Based Andreas's point in the video (link to exact moment) I believe that that the happy path should remain enabled which is how I have it now.

Choose a reason for hiding this comment

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

/// @Lederstrumpf when the time expires, is the primary "happy" path supposed to remain open?
/// Or is it that if the swap hasn't started on time, the refund is the only option.

Strong question you raise.
In the Farcaster protocol, the happy path closes once one of the parties initiates the cancel path.

HTLC atomic swap protocols on Bitcoin keep the happy path open even after timeout expires - see e.g. the COMIT spec: https://comit.network/docs/0.13.0/core-concepts/atomic-swap-htlc/#htlcs-on-different-ledgers (interestingly, they've done the same for the ETH protocol: https://github.com/comit-network/RFCs/blob/master/RFC-007-SWAP-Basic-Ether.adoc)
or the original proposal:
https://bitcointalk.org/index.php?topic=193281.msg2224949#msg2224949
or BIP 119:
https://github.com/bitcoin/bips/blob/master/bip-0199.mediawiki#summary

The reason for this is indeed that there's deliberately no OP_CHECKSEQUENCE nor OP_CHECKLOCKTIME - only their guardian variants OP_CHECKSEQUENCEVERIFY & OP_CHECKLOCKTIMEVERIFY.
Most discussion around this is probably in the annals of the bitcoin wizards IRC, if you wanna investigate deeper.
The arguments that have bubbled to my surface:
Satoshi's stance in 2010 (i.e. The Prophet's unassailable proclamation in Bitcoin Land, for better or worse): https://bitcointalk.org/index.php?topic=1786.msg22119#msg22119
Jeremy Rubin's AMA: https://www.reddit.com/r/Bitcoin/comments/gwjr8p/comment/ft80amd/?utm_source=share&utm_medium=web2x&context=3
Another disgruntled (& highly ignored) seeker of The Truth's summary: https://bitcoin.stackexchange.com/questions/96366/what-are-the-reasons-to-avoid-spend-paths-that-become-invalid-over-time-without

Now, while I have reservations about the reorg arguments even in Bitcoin's context, they are not applicable to chains with deterministic finality, i.e. all substrate based chains (aside from Kulupu).
i.e. with deterministic finality, you could have the equivalent of OP_CHECK{SEQUENCE,LOCKTIME}, and it would be safe as long as the output being spent has been finalized.

I reckon it might be preferable to have mutually exclusive success & refund paths for HTLC based swaps too - with a more general definition of HTLC, i.e. going beyond what Bitcoin Script allows.
It would still not ensure atomicity (since the chains run on different clocks), but improves safety of the swap for Bob assuming chain A's clock doesn't outrace chain B's clock.
Breakable assumption if crosschain: t1 < t2:
Let's take chain A to be the chain where Alice funds TX1 (with a timelock t2 for her refund TX2) and chain B as the chain where Bob funds TX3 (with a timelock t1 for his refund TX4)

Assuming both funding txs have been published, but Alice has not claimed yet, then the possible spending conditions are:

TX1 (Chain A): (x ∧ Bob) ∨ (t2 ∧ Alice)
TX3 (Chain B): (x ∧ Alice) ∨ (t1 ∧ Bob)

In detail:

  • !t1 ∧ !t2: Alice can claim. Bob can't do jack and has to wait for t1 or Alice to claim.
  • t1 ∧ !t2 (t1 has passed): Alice can claim (unsafe since Bob would learn x and his refund might frontrun her claim tx) or wait for t2 to refund. Bob can (and should) refund or (unsafe!) wait for Alice to claim.
  • t1 ∧ t2 (both t1 and t2 have passed): Alice can claim or refund. Bob can refund and should already have done so before t2 passed, else non-atomic.
  • !t1 ∧ t2 (t2 has passed): Alice can claim or refund. Bob has to wait for t1 or for Alice to claim: non-atomic scenario.

If you now impose that the happy path is unavailable after t1 or t2 respectively, then this changes to:
Chain A: (!t2 ∧ x ∧ Bob) ∨ (t2 ∧ Alice)
Chain B: (!t1 ∧ x ∧ Alice) ∨ (t1 ∧ Bob)

  • !t1 ∧ !t2: Alice can claim. Bob can't do jack and has to wait for t1 or for Alice to claim.
  • t1 ∧ !t2 (t1 has passed): Alice can't claim (which would anyway be unsafe), so must wait for t2 to refund. Bob can refund.
  • t1 ∧ t2 (both t1 and t2 have passed): Alice can only refund. Bob can only refund.
  • !t1 ∧ t2 (t2 has passed): Alice can claim or refund. Bob has to wait for t1 or for Alice to claim: non-atomic scenario.

The primary advantage is that Alice can't steal from Bob unless she publishes TX2 prior to t1.
Note you'd get the same if you'd just make the paths on TX1 temporally mutually exclusive, assuming Alice never leaks x to Bob (e.g. by hopelessly trying to spend TX1, or by getting hacked).
So if this was a crosschain swap of Tuxedo with e.g. Bitcoin, you could retain this advantage as long as Tuxedo = Chain A.

Choose a reason for hiding this comment

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

TLDR: to keep compatibility with Bitcoin Script (not in terms of only supporting its features, but also the assumptions about what can happen), I'd keep the happy path open.

But keeping compatibility with BS's assumptions means restricting Verifier rules to what BS would permit, so this is a pretty fundamental decision. The other alternative is going with whatever makes most sense for a particular protocol and breaking compatibility with BS's assumptions. And that's also a pretty fundamental decision ;)

Copy link
Contributor Author

@JoshOrndorff JoshOrndorff Feb 13, 2024

Choose a reason for hiding this comment

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

Because Tuxedo is a framework, we don't have to make any decision once and for all here. The framework can support any old verifier whether it is compatible with bitcoin or not.

When it comes time to launch a Tuxedo-based chain then we have to decide what verifiers are concretely allowed on that particular chain. I don't think we should enforce bitcoin compatibility to the entire Tuxedo ecosystem.

tuxedo-core/src/verifier/simple_signature.rs Outdated Show resolved Hide resolved
Copy link

@Lederstrumpf Lederstrumpf left a comment

Choose a reason for hiding this comment

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

lgtm so far :)

Left comments on the HTLC implementation where I reckon some decisions should be made.

tuxedo-core/src/verifier/htlc.rs Outdated Show resolved Hide resolved
Comment on lines 80 to 84
/// interesting to use public key hash, or better yet, simply abstract this over some opaque
/// inner verifier for maximum composability.
///
/// @lederstrumpf when the time expires, is the primary "happy" path supposed to remain open?
/// Or is it that if the swap hasn't started on time, the refund is the only option.

This comment was marked as resolved.

Comment on lines 80 to 84
/// interesting to use public key hash, or better yet, simply abstract this over some opaque
/// inner verifier for maximum composability.
///
/// @lederstrumpf when the time expires, is the primary "happy" path supposed to remain open?
/// Or is it that if the swap hasn't started on time, the refund is the only option.

Choose a reason for hiding this comment

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

/// @Lederstrumpf when the time expires, is the primary "happy" path supposed to remain open?
/// Or is it that if the swap hasn't started on time, the refund is the only option.

Strong question you raise.
In the Farcaster protocol, the happy path closes once one of the parties initiates the cancel path.

HTLC atomic swap protocols on Bitcoin keep the happy path open even after timeout expires - see e.g. the COMIT spec: https://comit.network/docs/0.13.0/core-concepts/atomic-swap-htlc/#htlcs-on-different-ledgers (interestingly, they've done the same for the ETH protocol: https://github.com/comit-network/RFCs/blob/master/RFC-007-SWAP-Basic-Ether.adoc)
or the original proposal:
https://bitcointalk.org/index.php?topic=193281.msg2224949#msg2224949
or BIP 119:
https://github.com/bitcoin/bips/blob/master/bip-0199.mediawiki#summary

The reason for this is indeed that there's deliberately no OP_CHECKSEQUENCE nor OP_CHECKLOCKTIME - only their guardian variants OP_CHECKSEQUENCEVERIFY & OP_CHECKLOCKTIMEVERIFY.
Most discussion around this is probably in the annals of the bitcoin wizards IRC, if you wanna investigate deeper.
The arguments that have bubbled to my surface:
Satoshi's stance in 2010 (i.e. The Prophet's unassailable proclamation in Bitcoin Land, for better or worse): https://bitcointalk.org/index.php?topic=1786.msg22119#msg22119
Jeremy Rubin's AMA: https://www.reddit.com/r/Bitcoin/comments/gwjr8p/comment/ft80amd/?utm_source=share&utm_medium=web2x&context=3
Another disgruntled (& highly ignored) seeker of The Truth's summary: https://bitcoin.stackexchange.com/questions/96366/what-are-the-reasons-to-avoid-spend-paths-that-become-invalid-over-time-without

Now, while I have reservations about the reorg arguments even in Bitcoin's context, they are not applicable to chains with deterministic finality, i.e. all substrate based chains (aside from Kulupu).
i.e. with deterministic finality, you could have the equivalent of OP_CHECK{SEQUENCE,LOCKTIME}, and it would be safe as long as the output being spent has been finalized.

I reckon it might be preferable to have mutually exclusive success & refund paths for HTLC based swaps too - with a more general definition of HTLC, i.e. going beyond what Bitcoin Script allows.
It would still not ensure atomicity (since the chains run on different clocks), but improves safety of the swap for Bob assuming chain A's clock doesn't outrace chain B's clock.
Breakable assumption if crosschain: t1 < t2:
Let's take chain A to be the chain where Alice funds TX1 (with a timelock t2 for her refund TX2) and chain B as the chain where Bob funds TX3 (with a timelock t1 for his refund TX4)

Assuming both funding txs have been published, but Alice has not claimed yet, then the possible spending conditions are:

TX1 (Chain A): (x ∧ Bob) ∨ (t2 ∧ Alice)
TX3 (Chain B): (x ∧ Alice) ∨ (t1 ∧ Bob)

In detail:

  • !t1 ∧ !t2: Alice can claim. Bob can't do jack and has to wait for t1 or Alice to claim.
  • t1 ∧ !t2 (t1 has passed): Alice can claim (unsafe since Bob would learn x and his refund might frontrun her claim tx) or wait for t2 to refund. Bob can (and should) refund or (unsafe!) wait for Alice to claim.
  • t1 ∧ t2 (both t1 and t2 have passed): Alice can claim or refund. Bob can refund and should already have done so before t2 passed, else non-atomic.
  • !t1 ∧ t2 (t2 has passed): Alice can claim or refund. Bob has to wait for t1 or for Alice to claim: non-atomic scenario.

If you now impose that the happy path is unavailable after t1 or t2 respectively, then this changes to:
Chain A: (!t2 ∧ x ∧ Bob) ∨ (t2 ∧ Alice)
Chain B: (!t1 ∧ x ∧ Alice) ∨ (t1 ∧ Bob)

  • !t1 ∧ !t2: Alice can claim. Bob can't do jack and has to wait for t1 or for Alice to claim.
  • t1 ∧ !t2 (t1 has passed): Alice can't claim (which would anyway be unsafe), so must wait for t2 to refund. Bob can refund.
  • t1 ∧ t2 (both t1 and t2 have passed): Alice can only refund. Bob can only refund.
  • !t1 ∧ t2 (t2 has passed): Alice can claim or refund. Bob has to wait for t1 or for Alice to claim: non-atomic scenario.

The primary advantage is that Alice can't steal from Bob unless she publishes TX2 prior to t1.
Note you'd get the same if you'd just make the paths on TX1 temporally mutually exclusive, assuming Alice never leaks x to Bob (e.g. by hopelessly trying to spend TX1, or by getting hacked).
So if this was a crosschain swap of Tuxedo with e.g. Bitcoin, you could retain this advantage as long as Tuxedo = Chain A.

Comment on lines +109 to +138
impl Verifier for HashTimeLockContract {
type Redeemer = HtlcSpendPath;

fn verify(&self, simplified_tx: &[u8], block_height: u32, spend_path: &HtlcSpendPath) -> bool {
match spend_path {
HtlcSpendPath::Claim { secret, signature } => {
// Claims are valid as long as the secret is correct and the receiver signature is correct.
BlakeTwo256::hash(secret) == self.hash_lock
&& sp_io::crypto::sr25519_verify(
signature,
simplified_tx,
&self.recipient_pubkey,
)
}
HtlcSpendPath::Refund { signature } => {
// Check that the time has elapsed
block_height >= self.claim_period_end

&&

// Check that the refunder has signed properly
sp_io::crypto::sr25519_verify(
signature,
simplified_tx,
&self.refunder_pubkey,
)
}
}
}
}

Choose a reason for hiding this comment

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

What's your stance on composable Verifier?

I'm otherwise undecided whether you are deliberately not composing HashLock & TimeLock implementations to construct HashTimeLockContract, or are leaving this for a later stage.
I think there are pros (e.g. leaner interface & codebase) but also cons (more moving parts beneath a Verifier, so requires more careful versioning) to composing.
See 29a20fe1 for a candidate implementation that reuses Verifier implementations from HashLock & TimeLock.

Copy link
Contributor Author

@JoshOrndorff JoshOrndorff Feb 13, 2024

Choose a reason for hiding this comment

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

Thanks for the detailed writeup and citations regarding expiring transactions. I'm not all that convinced by the Prophet's argument TBH. As others explained in the forum, there are already double spend scenarios that make a transaction invalid from one fork to another, so I don't see expiry being that much worse. The state boat from expired transactions is more convincing to me, but in the case of swaps, the transaction never truly expires, only one of its paths does, so I'm not too concerned. I think for now we let the happy path remain open simply for compatability with the chains where farcaster currently runs. Anyone could write their own verifier that leaves it open or adds a happy_path_expires: bool field if they want it later.

I think composing the verifiers is a good idea. I was already starting to consider it when I was writing this PR. Even looking at the simple HashLock, for example, even it could be composed with an inner Sr25519Signature rather than repeating the sig check logic. And by making them composable, we can also make them generic. I could imagine some verifier combinators for scenarios like And Or Not etc. I'll experiment with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I love the composed Verifier. Although it's not really more convenient in this particular case, it is actually a great example of what I believe should be a good practice in action.

@JoshOrndorff

This comment was marked as resolved.

Copy link

Coverage after merging joshy-hash-time-lock into main will be

61.99%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
tuxedo-core/aggregator/src
   lib.rs99.16%100%100%99.13%186, 22, 84
tuxedo-core/no_bound/src
   clone_no_bound.rs36.63%100%30%37.36%24, 32–39, 56, 59–72, 74–87, 89, 91–95, 98–99
   debug_no_bound.rs32.73%100%30%33%101–105, 108–109, 24, 32–42, 58, 60–78, 80–95, 97–99
   default_no_bound.rs19.11%100%16.67%19.31%100–122, 124–131, 133, 136–147, 151–157, 32–39, 51, 54–80, 83–88, 91–99
   lib.rs100%100%100%100%
tuxedo-core/src
   constraint_checker.rs54.02%100%50%55.22%71–91
   dynamic_typing.rs83.33%100%70.59%88.37%
   executive.rs92.15%100%93.55%91.98%118, 144, 177, 209, 238, 249, 300, 327, 378–383, 385, 395–396, 401–402, 404, 407–408, 411, 413–414, 417–418, 420, 424–445, 447, 451–452, 454–455, 457, 460–477, 54
   genesis.rs43.69%100%42.86%43.75%102–103, 106, 158–160, 169, 36–49, 60, 62–65, 67–68, 70–72, 75–79, 81–83, 85–96, 98–99
   inherents.rs7.02%100%11.76%6.19%121–123, 159, 165–179, 181–207, 216–221, 223–232, 234, 55–57, 64–69, 71–79, 82, 84
   types.rs65.99%100%48%72.11%82–86, 88–91, 93–99
   utxo_set.rs90.91%100%100%88.89%39–40
   verifier.rs55.10%100%43.24%62.30%57, 64–66
tuxedo-core/src/verifier
   htlc.rs79.27%100%45.59%87.20%101, 103, 109, 40, 42, 55, 57, 85
   multi_signature.rs89.96%100%70%93.78%45, 71
   simple_signature.rs79.56%100%58.82%86.41%61, 63
tuxedo-parachain-core/register_validate_block/src
   lib.rs0%100%0%0%100–119, 12, 120–125, 128, 13, 130–131, 14–16, 18, 29–35, 38–46, 50, 54–56, 59–61, 64–66, 84–99
tuxedo-parachain-core/src
   collation_api.rs0%100%0%0%19–40
   lib.rs48.65%100%53.85%45.83%85–90, 94–96
   relay_state_snapshot.rs0%100%0%0%112, 128–140, 147–155, 157, 173–188, 193–202, 204–211, 223, 225, 227–233, 235–240, 242, 245–250, 252–257, 259–271, 274–286, 292–298, 303–310, 316–323, 330–337, 346–354, 362–370, 378–383, 389–394, 41, 54, 82
   tests.rs100%100%100%100%
tuxedo-template-runtime/src
   genesis.rs84.62%100%88.89%84.35%23–45
   lib.rs5.38%100%13.41%3.36%100–103, 126, 129, 134–136, 140–142, 204, 206, 208, 210, 212, 214, 216, 218, 221, 223, 226, 232, 241–248, 252, 261–282, 285–312, 315–434, 62–67, 98–99
wallet/src
   amoeba.rs0%100%0%0%100–106, 109–110, 112–118, 120–127, 19–48, 51–52, 54–99
   cli.rs0%100%0%0%100, 115, 119, 125, 128, 133, 144, 150, 17, 20, 22, 31, 36, 41, 48, 65, 77
   keystore.rs0%100%0%0%30–33, 38–45, 47–48, 51, 53–59, 65–73, 76–78, 80–82, 85–91, 93–94
   main.rs0%100%0%0%100–193, 196–203, 206–209, 211–213, 216–218, 220, 222, 225–231, 234–247, 250–253, 255–266, 268, 27–99
   money.rs0%100%0%0%100, 106–113, 115, 119–124, 128, 131, 133, 136–139, 141–142, 146, 150–153, 155, 160–166, 168–172, 175–176, 180–185, 187–188, 191–202, 204, 206, 22–23, 25–40, 43, 47–60, 63–69, 72–90, 94–99
   output_filter.rs100%100%100%100%
   rpc.rs0%100%0%0%16–21, 24–26, 28–44, 47–53, 55–58, 60–61
   sync.rs0%100%0%0%103–106, 113–114, 116, 119–122, 127–129, 132–133, 136–141, 143–145, 148, 150–151, 156–159, 162–163, 170–174, 176–182, 185–187, 189–190, 193–194, 199–202, 205, 207–208, 213–216, 219, 221–222, 225–231, 233–234, 237–238, 241–242, 245–246, 250–256, 259–263, 266, 268, 271, 273, 277, 279–280, 283–284, 287–294, 296–297, 300–301, 303, 305–306, 310–312, 314–315, 317–318, 320–321, 324–326, 328–329, 331–332, 334–335, 339, 341–342, 346, 348–353, 356–357, 360–362, 365, 368–371, 373, 376–379, 382, 385–386, 389–390, 395–400, 402, 404, 409–412, 415–416, 419–424, 426, 429–430, 434–435, 437, 439–441, 443–446, 449–450, 46–50, 54, 57–58, 61, 63–77, 82–86, 88–89, 94–99
   timestamp.rs0%100%0%0%12–17, 20–27
wardrobe/amoeba/src
   lib.rs59.33%100%26.83%71.56%121, 139, 178, 81
   tests.rs100%100%100%100%
wardrobe/kitties/src
   lib.rs47.76%100%26.72%57.14%100, 102–103, 107–109, 117–119, 121–123, 127–129, 133–134, 137–139, 143–145, 150–152, 154–155, 159–161, 172–192, 211–212, 215–222, 225, 228, 230, 232, 234, 236, 238, 240, 242, 244, 246, 248,

@JoshOrndorff JoshOrndorff dismissed muraca’s stale review March 18, 2024 16:23

I've addressed the relevant concerns.

@JoshOrndorff JoshOrndorff merged commit b93090b into main Mar 18, 2024
6 checks passed
@JoshOrndorff JoshOrndorff deleted the joshy-hash-time-lock branch March 18, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants