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.

Sign up for GitHub

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

Jump to bottom

add HTLCs to ledger channel update protocol #1815

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

add HTLCs to ledger channel update protocol #1815

wants to merge 20 commits into from

Conversation

NiloCK
Copy link
Contributor

@NiloCK NiloCK commented Oct 9, 2023

This is draft work toward including HTLC transfers as part of the base consensus_channel.

Strongest caveat: there is no on-chain implementation to enforce any of these data types.

@netlify
Copy link

netlify bot commented Oct 9, 2023 •
edited
Loading

👷 Deploy Preview for nitrodocs processing.

Name Link
🔨 Latest commit a2f9a68
🔍 Latest deploy log https://app.netlify.com/sites/nitrodocs/deploys/6526c2e181423b0008cf0ad0

@netlify
Copy link

netlify bot commented Oct 10, 2023 •
edited
Loading

✅ Deploy Preview for nitro-payment-demo canceled.

Name Link
🔨 Latest commit a2f9a68
🔍 Latest deploy log https://app.netlify.com/sites/nitro-payment-demo/deploys/6526c2e12462960008c289b7

@NiloCK NiloCK mentioned this pull request Oct 11, 2023
Open
Colin Kennedy added 10 commits October 10, 2023 23:33
make HTLC fields public...
8336a78 
to permit good default behavior for marshaling
improve error message to identify broken test case
b478780
specify a literal proposal in msg service test...
7f14a4e 
Previous test was relying on a default switch fallthrough returning
a specific "blank" proposal type.
add a nil check for copying htlc map...
581c76f 
avoids clashes in equality checks between a nil map and an empty map
add missing import
ec103d4
add HTLC proposal -> ObjectiveID mapping...
dbc0cd6 
requires further thought - these returned IDs only take the ledger
channel ID as identifier.

Multiple simultaneous objectives would collide.
update string literals for serde tests...
505fe23 
and update jsonProposal helper struct with added fields
comment out empty branch (for staticcheck)
053caf2
add RemoveHTLC implementation
73d2fbd
go mod tidy
a2f9a68
@NiloCK NiloCK marked this pull request as ready for review October 11, 2023 15:49
@NiloCK NiloCK changed the title wip: add HTLCs to ledger channel update protocol add HTLCs to ledger channel update protocol Oct 11, 2023
@geoknee
Copy link
Contributor

geoknee commented Oct 12, 2023 •
edited
Loading

This is looking good so far @NiloCK !

I think we probably want:

  • an ADR to accompany these changes, explaining how LN-compatible multi hop payments can be incorporated into go-nitro. You've probably got the germ of that in Notion already.
  • some unit test for the functionality you built so far
  • eventually an integration test involving any changes to the go-nitro API (etc). We could develop that as a feature branch and merge it all in one go

@NiloCK
Copy link
Contributor Author

NiloCK commented Oct 13, 2023

👍

Will probably shelve this until after the hackathon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NiloCK @geoknee