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

channelID received from LND gets corruptedby XUD and can't be used #745

Closed
offerm opened this issue Dec 11, 2018 · 7 comments
Closed

channelID received from LND gets corruptedby XUD and can't be used #745

offerm opened this issue Dec 11, 2018 · 7 comments
Assignees
Labels
bug Something isn't working critical bug
Milestone

Comments

@offerm
Copy link
Contributor

offerm commented Dec 11, 2018

I'm connected to xud1 and try to swap with it. should send BTC to xud1 and get LTC from xud1. LTC route from xud1 to me is xud1->xud2->xud3-me. When I do queryroutes on xud1 I get:

xud@xud-test-1:~/go/src/github.com/lightningnetwork/lnd$ lndltc-lncli queryroutes 03d719aec37c6645eec86429df4d3cda7def0496dfc51591c3dab82a78346334b5 115000000
{
"routes": [
{
"total_time_lock": 144790,
"total_fees": "232",
"total_amt": "115000232",
"hops": [
{
"chan_id": "139024449239580673",
"chan_capacity": "110354793778",
"amt_to_forward": "115000116",
"fee": "116",
"expiry": 144214,
"amt_to_forward_msat": "115000116000",
"fee_msat": "116000"
},
{
"chan_id": "139024449239384064",
"chan_capacity": "225000000000",
"amt_to_forward": "115000000",
"fee": "116",
"expiry": 143638,
"amt_to_forward_msat": "115000000000",
"fee_msat": "116000"
},
{
"chan_id": "139032145820712960",
"chan_capacity": "225000000000",
"amt_to_forward": "115000000",
"fee": "0",
"expiry": 143638,
"amt_to_forward_msat": "115000000000",
"fee_msat": "0"
}
],
"total_fees_msat": "232000",
"total_amt_msat": "115000232000"
}
]
}

But when XUD does it gets :

12/9/2018, 3:58:14 PM [SWAPS] debug: got 1 routes to destination: 145333,2302,1150002302,139024449239580670,110354793778,1150001151,1151,144757,1150001151000,1151001,139024449239384060
,225000000000,1150000000,1151,144181,1150000000000,1151000,139032145820712960,225000000000,1150000000,,144181,1150000000000,2302001,1150002302001

If you look carefully the channelID for the first hop that XUD prints is 139024449239580670 while queryroutes shows 139024449239580673

3 at the end is missing!

As a result the swap fails since LND can't find the first channel.

@offerm
Copy link
Contributor Author

offerm commented Dec 11, 2018

Reason for the problem (most probably can be found here):

protocolbuffers/protobuf#3666
and
https://stackoverflow.com/questions/9643626/does-javascript-support-64-bit-integers

@offerm
Copy link
Contributor Author

offerm commented Dec 11, 2018

May be relevant
agreatfool/grpc_tools_node_protoc_ts#10

@kilrau
Copy link
Contributor

kilrau commented Dec 11, 2018

@sangaman @moshababo can you jump in here, looks indeed like xud has some integer problem...

@kilrau kilrau added this to the 1.0.0-alpha.6 milestone Dec 11, 2018
@sangaman
Copy link
Collaborator

I've been researching this issue for a while, it seems like all 64 bit integers defined in the proto file are being defaulted to numbers instead of strings in the generated javascript code. This causes us to lose precision anytime the number is greater than 2^55.

Adding jstype=JS_STRING to the field definition in the proto file seems to partly fix the issue for a given field - but it's not updating the type definitions strings as per offer's latest link, and it's referred to as a "legacy option" here. Plus it's clunky to have to specify this field on every type, especially for the lnd proto definition.

This is also a relevant issue: protocolbuffers/protobuf#1832

Still looking into what the best solution might be.

@sangaman
Copy link
Collaborator

An option is to use the protobufjs library to generate proto code instead of protoc. They support the long.js package for 64 bit types which supports 64 bit numbers. I've played with it a bit and it seems like it should work. It would require changing around the proto scripts but it might be the cleanest approach. We can discuss tomorrow.

@offerm
Copy link
Contributor Author

offerm commented Dec 12, 2018

@sangaman can you please post a PR for this even if not final.
It is currently blocking me without an easy workaround.

@sangaman
Copy link
Collaborator

Yes, I've been working on it. Expect a PR soon.

sangaman added a commit that referenced this issue Dec 13, 2018
This adds `jstype = JS_STRING` annotations on the channel ids in the
lnd proto definition so that they use `string` types instead of numbers.
Javascript does not natively support 64 bit numbers and precision
can be lost, and the channel ids are known to cause issues because of
this.

This also adds a new package to generate type definitions from the
`.proto` files. This is because of a known bug in the previously used
plugin, agreatfool/grpc_tools_node_protoc_ts#10, which does not update
the generated type definition when `jstype = JS_STRING` is used.

Fixes #745.
@ghost ghost added the in progress label Dec 13, 2018
sangaman added a commit that referenced this issue Dec 17, 2018
This adds `jstype = JS_STRING` annotations on the channel ids in the
lnd proto definition so that they use `string` types instead of numbers.
Javascript does not natively support 64 bit numbers and precision
can be lost, and the channel ids are known to cause issues because of
this.

This also adds a new package to generate type definitions from the
`.proto` files. This is because of a known bug in the previously used
plugin, agreatfool/grpc_tools_node_protoc_ts#10, which does not update
the generated type definition when `jstype = JS_STRING` is used.

Fixes #745.
sangaman added a commit that referenced this issue Dec 18, 2018
This adds `jstype = JS_STRING` annotations on the channel ids in the
lnd proto definition so that they use `string` types instead of numbers.
Javascript does not natively support 64 bit numbers and precision
can be lost, and the channel ids are known to cause issues because of
this.

This also adds a new package to generate type definitions from the
`.proto` files. This is because of a known bug in the previously used
plugin, agreatfool/grpc_tools_node_protoc_ts#10, which does not update
the generated type definition when `jstype = JS_STRING` is used.

Fixes #745.
@ghost ghost removed the in progress label Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical bug
Projects
None yet
Development

No branches or pull requests

3 participants