Look-up functions for ReadOnlyNetworkGraph

[jkczyz]

ReadOnlyNetworkGraph uses BTreeMap to store its nodes and channels, but these data structures are not supported by C bindings. Expose look-up functions on these maps in lieu of such support.

[TheBlueMatt]

Hmmmm, yuckkkk. That's a ton of copying, so much so I'd bet we'll see some nontrivial issues with anyone trying to use this on mobile. I'd much rather expose some method to query for individual entries rather than expose the full set.

[jkczyz]

Hmmmm, yuckkkk. That's a ton of copying, so much so I'd bet we'll see some nontrivial issues with anyone trying to use this on mobile. I'd much rather expose some method to query for individual entries rather than expose the full set.

Yeah, wasn't really happy about the cloning. But to confirm my understanding of bindings, (1) could we use references instead and (2) is HashMap unsupported?

[TheBlueMatt]

(1) could we use references instead

References to what? In theory references are supported, we could implement a getter that returns a reference, but most of the language bindings would just clone on the return, which is fine if we're just returning info for one channel.

(2) is HashMap unsupported?

In general none of the std things are supported, we don't have a manual mapping for maps (unlike vecs).

[jkczyz]

(1) could we use references instead

References to what? In theory references are supported, we could implement a getter that returns a reference, but most of the language bindings would just clone on the return, which is fine if we're just returning info for one channel.

Was thinking either Vec<&(NodeId, NodeInfo)> or Vec<(NodeId, &NodeInfo)>. But a look-up function may be preferable, as you noted.

[codecov-commenter]

Codecov Report

Merging #1543 (2d50c17) into main (22dc964) will increase coverage by 0.65%.
The diff coverage is n/a.

❗ Current head 2d50c17 differs from pull request most recent head 3024b07. Consider uploading reports for the commit 3024b07 to get more accurate results

@@            Coverage Diff             @@
##             main    #1543      +/-   ##
==========================================
+ Coverage   90.94%   91.60%   +0.65%     
==========================================
  Files          80       80              
  Lines       43469    47658    +4189     
  Branches    43469    47658    +4189     
==========================================
+ Hits        39533    43657    +4124     
- Misses       3936     4001      +65     

Impacted Files	Coverage Δ
lightning/src/routing/gossip.rs	91.69% <ø> (ø)
lightning/src/ln/features.rs	99.28% <0.00%> (-0.19%)	⬇️
lightning/src/ln/functional_tests.rs	97.06% <0.00%> (-0.11%)	⬇️
lightning/src/ln/payment_tests.rs	99.31% <0.00%> (+0.07%)	⬆️
lightning/src/routing/router.rs	93.27% <0.00%> (+0.80%)	⬆️
lightning/src/ln/functional_test_utils.rs	96.71% <0.00%> (+1.48%)	⬆️
lightning/src/ln/channelmanager.rs	87.79% <0.00%> (+3.46%)	⬆️
lightning/src/ln/channel.rs	92.06% <0.00%> (+3.48%)	⬆️
lightning/src/util/config.rs	57.50% <0.00%> (+14.36%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22dc964...3024b07. Read the comment docs.

[TheBlueMatt]

Tagging 110 due to user request, plus its trivial

[tnull]

LGTM, mod two style nits.