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

Channel splitting #11

Closed
wants to merge 6 commits into from
Closed

Channel splitting #11

wants to merge 6 commits into from

Conversation

luckysori
Copy link
Collaborator

No description provided.

@luckysori
Copy link
Collaborator Author

@Tibo-lg: I've finally created the draft PR I mentioned. I had to update the fork's main to 0.0.116, so that we get a useful diff here.

@Tibo-lg
Copy link

Tibo-lg commented Nov 1, 2023

That's awesome thanks!

@luckysori
Copy link
Collaborator Author

I've squashed the fixup and removed the merge commit in the process since we don't even want it.

I'll open a PR to update rust-dlc to the tip of this branch.

The (de)serialisation for `ChannelTransactionParameters` changed
between 0.0.114 and 0.0.116. This type had already been modified by us
when adding the `original_funding_outpoint` field. When first
introduced, we chose a type value of 14.

After the update, we chose to reduce it down to 12 since the type 12
was no longer being used after changes between 0.0.114 and 0.0.116.
Unfortunately, this unnecessarily broke channels created before the
update.

Eventually we will need to settle on a way to handle these possible
conflicts with upstream. Tibo already suggested a few options:

- Use much larger numbers for `rust-dlc` custom fields.
- Move LDK values around as we rebase on top of new changes.
- Add a subdomain for `rust-dlc` custom fields.
@Tibo-lg
Copy link

Tibo-lg commented Nov 1, 2023

Looks like something's wrong with the build?

@luckysori
Copy link
Collaborator Author

Looks like something's wrong with the build?

I don't think so. We just get warnings because of all the things that are not linted properly in LDK, right?

@Tibo-lg
Copy link

Tibo-lg commented Nov 2, 2023

The build used to pass at least AFAIR

@Tibo-lg
Copy link

Tibo-lg commented Nov 2, 2023

The build used to pass at least AFAIR

Never mind, the build works but not after rebase so it's the same as before sorry.

@luckysori
Copy link
Collaborator Author

Superseded by #13 to use a differently named branch: split-tx-experiment.

@luckysori luckysori closed this Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants