Skip to content

Introduce Trampoline Packet deserialization for Inbound::Receive - #2806

Closed
shaavan wants to merge 6 commits into
lightningdevkit:mainfrom
shaavan:#2299-inbound-deserial
Closed

Introduce Trampoline Packet deserialization for Inbound::Receive#2806
shaavan wants to merge 6 commits into
lightningdevkit:mainfrom
shaavan:#2299-inbound-deserial

Conversation

@shaavan

@shaavanshaavan commented Dec 28, 2023

Copy link
Copy Markdown
Member

This PR builds over #2756.
And addresses Inbound Deserialization in #2299

  • This PR introduces Trampoline Packets to Inbound::Receive
  • Implement Readable for VariableLengthOnionPacket struct to allow
    deserialization for Trampoline Packets.
  • Make appropriate changes in the rest of the code, to allow proper compiling
    with the new change.

TODO:

  • Introduce Trampoline Packets for Inbound::Receive.
  • Write tests for the introduced change.

arik-soand others added 6 commits December 4, 2023 10:56
- This PR introduces Trampoline Packets to Inbound::Receive
- Implement Readable for VariableLengthOnionPacket struct to allow
deserialization for Trampoline Packets.
- Make appropriate changes in rest of code, to allow proper compiling
with the new change.
r.read_exact(&mut buf)?;
PublicKey::from_slice(&buf)
},
hop_data: Readable::read(r)?,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you add a unit test to make sure this works? The current spec doesn't encode the length of the hop_data, so I think there's a chance this might break without using a reader with an a priori specified fixed length.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Thanks for working on this! Sadly, @arik-so came up with a potential new version of trampoline that we may want to do instead of the existing one. He's gonna present more about it in the lightning spec meeting on Monday and we'll decide where to go from there, but if other implementations prefer the alternative approach this may end up being a dead-end. Sorry about that.

@shaavan

Copy link
Copy Markdown
MemberAuthor

Putting this PR on hold. Since the original PR's being revamped. comment

I will be closing this PR within the next couple days in favor of a more flexible implementation that does not rely on nesting onions.

@shaavanshaavan closed this Jan 12, 2024
Sign up for freeto 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

@shaavan@TheBlueMatt@arik-so