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

Pure-python implementation of the Sphinx onion protocol #4056

Merged
merged 4 commits into from
Sep 24, 2020

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Sep 16, 2020

This is a simple (insecure) implementation of the onion creation and processing functionality. It is mainly intended to be used in lnprototest and experimental code, but is not guaranteed to have constant-time execution, and should not be used in production.

The PR builds on top of #3811, so the real content is in the last 4 commits.

Depends #3811

Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

Looks pretty snazzy.

(Missed the comment about this being a "builds on top of PR" whoops.)

@@ -67,4 +69,75 @@ def __str__(self):
return "{self.block}x{self.txnum}x{self.outnum}".format(self=self)

def __eq__(self, other):
return self.block == other.block and self.txnum == other.txnum and self.outnum == other.outnum
return (
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems like something a bit different from a type annotations update. should be in a separate commit. also seems to be missing type annotation information?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well it's just reformatting some code, but you're right I forgot to add the annotations here (mypy seemed to infer them correctly).

contrib/pyln-proto/pyln/proto/primitives.py Outdated Show resolved Hide resolved
contrib/pyln-proto/pyln/proto/onion.py Show resolved Hide resolved
contrib/pyln-proto/tests/test_onion.py Outdated Show resolved Hide resolved
contrib/pyln-proto/tests/test_onion.py Outdated Show resolved Hide resolved
contrib/pyln-proto/pyln/proto/onion.py Show resolved Hide resolved
contrib/pyln-proto/tests/test_onion.py Outdated Show resolved Hide resolved
@@ -739,7 +739,9 @@ def restart(self, timeout=10, clean=True):
self.start()

def fund_channel(self, l2, amount, wait_for_active=True, announce_channel=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a way to mark this as deprecated?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's the warnings package which is a bit heavy handed (runtime warnings). I added a warning since this is mostly used in testing anyway.

@cdecker cdecker force-pushed the pyln-onion branch 3 times, most recently from fd9044a to cac7c42 Compare September 18, 2020 13:41
@cdecker
Copy link
Member Author

cdecker commented Sep 18, 2020

I moved the deprecation of fund_channel to #3811 since it was a bit extraneous here, and fixed up the reviews 👍

@cdecker cdecker force-pushed the pyln-onion branch 2 times, most recently from e3e1a71 to a46b8df Compare September 18, 2020 14:00
@cdecker cdecker changed the title Pyln onion Pure-python implementation of the Sphinx onion protocol Sep 18, 2020
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ah, I see you already moved away from hexlify. Will rebase now I've merged dep, and roll in fixup!


@classmethod
def from_hex(cls, s: str):
return cls.from_bin(unhexlify(s))
Copy link
Contributor

Choose a reason for hiding this comment

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

we should really wean off unhexlify and hexlify in favor of bytes.fromhex() and bytes.hex()...


return struct.pack("b", self.version) + \
ephkey + \
self.payloads + \
Copy link
Contributor

Choose a reason for hiding this comment

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

The backslashes here are kinda weird?

Suggested-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <@cdecker>
Changelog-Added: pyln-proto: Added pure python implementation of the sphinx onion creation and processing functionality.
@rustyrussell
Copy link
Contributor

Ack 18b6939

@rustyrussell rustyrussell added the Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! label Sep 24, 2020
@rustyrussell rustyrussell merged commit a351b9b into ElementsProject:master Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants