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 data inconsistency between peers #31

Closed
4 of 7 tasks
St333p opened this issue Dec 3, 2020 · 5 comments
Closed
4 of 7 tasks

channel data inconsistency between peers #31

St333p opened this issue Dec 3, 2020 · 5 comments
Labels
bug Something isn't working question Further information is requested

Comments

@St333p
Copy link
Contributor

St333p commented Dec 3, 2020

Playing around with lnp-node, I noticed some inconsistencies between the view of the two peers over a channel they have in common.

My setting is composed of:

  • 2 lnp nodes, version 0.1.0-beta.1, let's call them A and B
  • 1 rgb node, version 0.2.0-beta.4

My goal was to perform a LN transfer for both btc and one rgb asset, with the following workflow:

  1. A listens for connections
  2. B initiates peer connection
  3. B proposes channel
  4. B funds channel (btc)
  5. B performs btc transfer
  6. B refills channel with rgb asset
  7. B performs rgb transfer

Here is a list of things that seem wrong to me:

  • after step 4, B sees his updated local balance, while remote balance for A is still 0
  • In step 5, A accepts the transfer even though the remote balance in her view is 0, so the transfer makes it underflow to 2^64-amt
  • after step 5, B has total_payments: 1, while A has total_payments: 0
  • after step 6, rgb balance appears as 'local' for both nodes
  • transfer --asset expects asset-id in the opposite endianess (big vs little -endian) than the one output in logs and lnp-cli info <temp_channel_id>
  • for A local_keys are the same as remote_keys, while for B they are different.
  • after step 2, A has her own node uri in the list of peers. Same happens later on as key of remote_balances

After having a quick look at the code I think I can fix at least some of the listed problems. The last two will require some debugging and I'm not sure I'll be able to solve them.

Can please anyone confirm these are not expected behaviors before I spend time fixing them?

EDIT: I'm working on this and I managed to solve issues identified by the ticked box.

@St333p
Copy link
Contributor Author

St333p commented Dec 4, 2020

I figured that asset transfer fails because of endianess inconsistency between input and output data, so I'll update it in the issue description.
In fact:

  • After step 6, the cli logs: adding 100 of 883926d7f25d99298a1c7b4fbd019df21d1d24a2bbfb63adacbc115f6b48d627 to balance and the same asset_id is shown in the asset field in the output of lnp-cli info <temp_channel_id>.
  • This call fails: lnp1-cli transfer --asset 883926d7f25d99298a1c7b4fbd019df21d1d24a2bbfb63adacbc115f6b48d627 <temp_channel_id> 10
  • This call succeeds: lnp1-cli transfer --asset 27d6486b5f11bcacad63fbbba2241d1df29d01bd4f7b1c8a29995df2d7263988 <temp_channel_id> 10

@St333p
Copy link
Contributor Author

St333p commented Dec 8, 2020

I'm stuggling a bit with the 7th point. From what i can see, remote_id is never set in src/bin/peerd.rs for the peer that receives the connection and in fact from the pov of A, a lnp-cli info <peer> returns an empty list for remote_id. However, it's still not clear to me how A gets to show its own nodeid in the list of peers, my best bet points at how id is assigned at line 203 in peerd.rs (b901fb0), since it contains local_node as nodeid.
I have the feeling that at that point in the workflow the remote nodeid is not yet available and it should be set later on, but I can't figure out where to get it from nor where it should be set in order to solve the issue. Any help on this is very well appreciated.

EDIT: after another debugging session I figured that initiator's nodeid is available at the end of the noise protocol to the receiver. I still don't understand where precisely in code the noise protocol gets executed between peers and so how to retrieve initiator nodeid from that.

@St333p
Copy link
Contributor Author

St333p commented Dec 9, 2020

As for asset identifiers, I have the feeling that the cleanest possible solution would be to switch everything over to bech32 encoding, also for consistency with rgb-node. This could be obtained by having the rgb-node Asset api return a bech32 encoded string or by implementing a the conversion between hex and bech32 encodings in lnpbp::bp::chain::AssetId. Am I right?

@UkolovaOlga UkolovaOlga added this to Backlog in Development Calls Agenda via automation Dec 9, 2020
@UkolovaOlga UkolovaOlga added bug Something isn't working question Further information is requested labels Dec 9, 2020
@UkolovaOlga UkolovaOlga moved this from Backlog to Agenda in Development Calls Agenda Dec 9, 2020
@St333p
Copy link
Contributor Author

St333p commented Dec 9, 2020

Updates from 20.12.09 call:

  • asset identifiers will be encoded in bech32 from the next lnpbp core library release, so the related issue will be solved rather automatically at that point
  • nodeid is currently not exposed by the Noise-related part of lnpbp core library API, this will be done with the next release (either 0.2.[1+] or 0.3)

As a result, the best approach is probably to close this issue with a PR that addresses the lesser issue that I already solved and open a new one to just keep track of the missing points and point to the right info until lnpbp core library is released.

@St333p
Copy link
Contributor Author

St333p commented Dec 10, 2020

bugs 1-4 were solved by #32
bug 5 is waiting for an update in rust-lnpbp and has a dedicated issue #33
bug 6-7 are linked to each other and are waiting for an update in rust-lnpbp, here is the dedicated issue #34

@St333p St333p closed this as completed Dec 10, 2020
UkolovaOlga added a commit to LNP-BP/devcalls that referenced this issue Dec 11, 2020
Discussed questions:
- how will Taproot affect RGB?
- Bitcoin Pro vs wallet difference
- WASM & bindings of RGB SDK
- LNP-WG/lnp-node#31
- preliminary discussion about invoicing for Bitcoin, LN and RGB
@dr-orlovsky dr-orlovsky moved this from Agenda to Implemented / Done in Development Calls Agenda Dec 17, 2020
@dr-orlovsky dr-orlovsky removed this from Implemented / Done in Development Calls Agenda Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants