Require WithContext log wrappers on OutboundPayments calls and pass payment hashes

[TheBlueMatt]

In much of LDK we pass around `Logger` objects both to avoid having
to `Clone` `Logger` `Deref`s (soon to only be `Logger`s) and to
allow us to set context with a wrapper such that any log calls on
that wrapper get additional useful metadata in them.

Sadly, when we added a `Logger` type to `OutboundPayments` we broke
the ability to do the second thing - payment information logged
directly or indirectly via logic in the `OutboundPayments` has no
context making log-searching rather challenging.

Here we move to instead using the automated `add_logging`
proc-macro to require that `OutboundPayment` functions receive a
`WithContext` logger, appropriately setting (especially) the
`payment_hash` as we do so.

Fixes #4307

I believe the first three commits can/should be backported to 0.2. Ideally we'd also backports a variant to 0.1 to fix #4307 there but I'm not really sure its quite worth it to write a whole new version just for 0.1.

[ldk-reviews-bot]

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[codecov]

Codecov Report

❌ Patch coverage is 88.26087% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.09%. Comparing base (3fee76b) to head (5e5c54a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/outbound_payment.rs	85.40%	12 Missing and 8 partials ⚠️
lightning/src/ln/channelmanager.rs	86.53%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4342      +/-   ##
==========================================
+ Coverage   86.08%   86.09%   +0.01%     
==========================================
  Files         156      156              
  Lines      102416   102492      +76     
  Branches   102416   102492      +76     
==========================================
+ Hits        88165    88242      +77     
+ Misses      11759    11755       -4     
- Partials     2492     2495       +3     

Flag	Coverage Δ
tests	86.09% <88.26%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

Planning to backport to 0.2 in #4344

[wpaulino]

Code LGTM though I'm not really a fan of the hidden logger argument.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[ldk-reviews-bot]

✅ Added second reviewer: @valentinewallace

[TheBlueMatt]

I'm not really a fan of the hidden logger argument.

I could go either way. It seems like an easy way to ensure we always have the logger which will make @joostjager happy, and in this case its fairly straightforward (because we're not trying to automate passing the logger from ChannelManager).

[valentinewallace]

I'm not really into this juice vs squeeze ratio. I think @joostjager should take a look since it's supposed to make him happy :) If it really does, I guess it's fine.

Mentioned to Matt offline but Claude pointed out that the visit-mut syn feature allows removing ~all the boilerplate.

[joostjager]

Appreciate the attempt, but I don't think this is the right direction. The macro adds a layer of complexity, and the hidden logger argument that gets silently injected into every self.*() call isn't very intuitive. I agree with @wpaulino and @valentinewallace's hesitation.

My preference would still be to keep exploring something similar to what the tracing crate does with the context. For no-std the developer experience might not be optimal, but I still think that is a reasonable trade-off.

If that is unacceptable, I think we're better off sticking with explicit logger parameters and types. At least the code does what it says.

[TheBlueMatt]

Alrighty, explicit loggers it is.