Scope payment preimage in do_test_keysend_payments

[TheBlueMatt]

b0d4ab8 fixed a nasty bug where
we'd failed to include the payment preimage in keysend onions at
all. Ultimately, this was a test failure - the existing test suite
should which did keysend payments were not structured in a way that
would fail in this case, instead using the same preimage variable
both for sending and receiving.

Here we improve the main keysend test tweaked by b0d4ab8
to make absolutely sure it cannot work if the preimage doesn't come
from the onion. We make the payment preimage on the sending side a
variable inside a scope which only exists for the send call. Once
that scope completes the payment preimage only exists in the
sending ChannelManager, which must have put it in the onion in
order for the receiving node to have it.

[codecov-commenter]

Codecov Report

Patch coverage: 93.33% and project coverage change: -0.01% ⚠️

Comparison is base (100fdbb) 90.33% compared to head (67e5399) 90.33%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2481      +/-   ##
==========================================
- Coverage   90.33%   90.33%   -0.01%     
==========================================
  Files         106      106              
  Lines       55771    55775       +4     
  Branches    55771    55775       +4     
==========================================
+ Hits        50382    50384       +2     
- Misses       5389     5391       +2     

Files Changed	Coverage Δ
lightning/src/ln/payment_tests.rs	97.66% <93.33%> (-0.05%)	⬇️

... and 2 files with indirect coverage changes

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

[tnull]

FWIW, now also introduced test coverage for spontaneous payments in LDK Node (lightningdevkit/ldk-node#151).

[TheBlueMatt]

Will let @alecchendev take a look here, I don't really care much about this, just wanted to demonstrate what I think makes a good test for this kinda thing and will let him comment.

[alecchendev]

This makes sense to me. I was thinking we should technically hit the "We require payment secrets" onion error in this single-path keysend test (which is how I ended up finding the bug), but now that we accept payment secrets on keysend after adding MPP support, if we sent a secret in this test it would probably pass before this PR (and ofc either way this better tests the actual keysend flow; the fact that we catch the secret case here is more a byproduct of being a better test). So yea this looks good (and maybe should also apply to the MPP keysend tests too)

[TheBlueMatt]

and maybe should also apply to the MPP keysend tests too

It maybe should, but doing it on the MPP tests is annoying since it would mean being much more verbose and doing commitment_signed_dances like 4 times. Because we do check for the preimage in do_pass_along_path (today) I'm gonna just merge as-is and call it a day.