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

Atomic Swaps #83

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

GeneFerneau
Copy link

@GeneFerneau GeneFerneau commented Jul 16, 2021

RFC detailing an atomic swap protocol for Grin, based on earlier designs specified in Grin documentation.

The protocol allows for atomically swapping Grin with other cryptocurrencies, facilitating decentralised, permissionless exchange.

Link to the rendered text

@GeneFerneau GeneFerneau force-pushed the atomic-rfc branch 2 times, most recently from 06b8857 to 95cec85 Compare July 21, 2021 01:00
@tromp
Copy link
Contributor

tromp commented Sep 10, 2021

The link to "draft multisignature output specification" in https://github.com/mimblewimble/grin-rfcs/pulls/85 is broken.

RFC detailing an atomic swap protocol for Grin, based on earlier designs
specified in Grin documentation. The protocol allows for atomically
swapping Grin with other cryptocurrencies, facilitating decentralised,
permissionless exchange.
@GeneFerneau
Copy link
Author

The link to "draft multisignature output specification" in https://github.com/mimblewimble/grin-rfcs/pulls/85 is broken.

Thank you, fixed

@tromp
Copy link
Contributor

tromp commented Oct 7, 2021

I find this document poorly structured. It dives into all kinds of details before giving a protocol overview.
It should follow the excellent example set by reference [2], Somsen's S.A.S., who starts out by clearly detailing the protocol steps.

In this document, a section "Protocol rounds" appears only in the latter half, but still doesn't tell me in what order the transactions shown in the illustration are constructed/pre-signed, and published.
Instead it starts with "In Grin, let kB be Bob's random kernel nonce".
This fails to mention which of the multiple Grin transactions this is about.
The next paragraph "Alice and Bob must perform an atomic swap refund transaction before the success transaction is accepted into the blockchain." makes no sense, since performing requires posting. And as the next sentence says, only 1 can be posted. Perhaps, the author means pre-sign instead of "perform" , but the pre-signing must happen before Alice can even post the funding tx.

Trying to see what is the first protocol step, as is trivial in Somsen's description, I come across
"In the first round, Alice selects and verifies the multisignature output from the Grin Fund transaction,"

Huh? The grin fund tx has not even been constructed. How can one verify its output? What does selecting it mean?

Reading this document feels like a bit of a struggle.
Can you rewrite it to start with a clear protocol description in the style of Somsen's document?
Without going into premature details about nonces and derivations? And with clear and consistent naming of outputs and transactions (e.g. this document talks about "the main transaction" early on,
which is not a transaction named in the illustration. I guess it means the succes_tx).

After the reader has a clear understanding of the protocol at a high level, then you go down into details later.

@GeneFerneau
Copy link
Author

I find this document poorly structured. It dives into all kinds of details before giving a protocol overview.
...

Thanks for the constructive criticism. Got lost in the details during implementation, thank you for drawing attention to the need for better high-level explanation of the protocol.

I've added a fixup commit for easier review, and will squash after approval.

@tromp
Copy link
Contributor

tromp commented Oct 30, 2021

You added table doesn't make sense to me. The signing of the BitcoinFund tx only appears at the end, which is too late since Bob could already have claimed Alice's grin.
It also shows up in the Alice column.

You didn't address every problem I pointed out, and didn't change the overall structure.
Please consider a larger rewrite.

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.

2 participants