Clarify docs on provide_channel_parameters

[TheBlueMatt]

Its very confusing to say that LDK will call
provide_channel_parameters more than once - its true for a channel, but not for a given instance. Instead, phrase the docs with reference to a specific instance, which is much clearer.

[codecov-commenter]

Codecov Report

Base: 90.77% // Head: 90.76% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (2059190) compared to base (d4dc05b).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1903      +/-   ##
==========================================
- Coverage   90.77%   90.76%   -0.02%     
==========================================
  Files          94       94              
  Lines       49603    49603              
  Branches    49603    49603              
==========================================
- Hits        45025    45020       -5     
- Misses       4578     4583       +5     

Impacted Files	Coverage Δ
lightning/src/chain/keysinterface.rs	83.14% <ø> (ø)
lightning/src/chain/onchaintx.rs	95.17% <0.00%> (-0.21%)	⬇️
lightning/src/ln/functional_tests.rs	97.07% <0.00%> (-0.07%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[wpaulino]

I had something similar in a previous iteration and @devrandom thought otherwise #1867 (comment).

[TheBlueMatt]

Added a note that the "instance" here includes calling again if the object was created by read_chan_signer. I don't buy that we should just say "LDK calls this more than once", because it precisely does not, and saying that it can call this more than once but users need to somehow handle that and ignore the new state is even more confusing - if there's multiple instances of a given signer now you have to have some shared state across them.

[wpaulino]

We don't ever call it immediately after read_chan_signer, it either must have been called in the past and was serialized, or it will be called once funding occurs.

[TheBlueMatt]

Oops, lol, right.

[wpaulino]

LGTM after squash.

[devrandom]

I'm OK with the doc in this PR.

however, I have some thoughts about the BaseSign docs in general, which may affect the thinking here.

when a dev is looking at the BaseSign docs, it seems likely they are actually trying to implement their own signer. if so, the docs should guide the dev towards best practices, which includes making the signer properly enforcing. as an example, one consequence of the signer being enforcing is the need for signer persistence.

so if that's the most likely use case that the docs should focus on, then the docs should probably be adjusted to address it, rather than implicitly focusing on how to use InMemorySigner.

[TheBlueMatt]

I don't think the wording here is wrong given that, either, though? I suppose, indeed, we should include a little more guidance on enforcement (or, honestly, just link to VLS, cause its a ton of work lol), but I also think someone may implement the sign trait with a goal of using different keys, learning when a transaction is being signed, or just doing naive policy checks (like closing transaction pays to the right place).

[TheBlueMatt]

Will address #1892 (comment) here

[TheBlueMatt]

Rebased and did so.