Aggregate packages with differing, but similar, locktimes

In the previous commit we started aggregating claims much more
aggressively but only did so when timelocks on HTLC timeout claims
were identical. While there's nothing we can do to change this for
HTLC-Timeout claims (on our commitment transactions), on
counterparty commitment transactions there's no such restirction.
However, we do need to avoid aggregating different claims with
drastically different locktimes to avoid delaying claims too long,
and thus introduce a new constant, `CLTV_DIFFERENCE_BATCH_CLAIM`,
to limit this.

This new batching has security implications as we may delay
claiming an HTLC for up to that many blocks, and thus we make the
constant public, update the `cltv_expiry_delta` docs, and update
some constant security checks.

Batch claim much more aggressively even on pinnable outputs

When we first added batch claiming, we only did so for claims which
we thought were not pinnable, i.e. those for which only we can
claim the output.

This was the conservative choice - any outputs which we think are
potentially pinnable might be pinned, leaving our entire batch
unable to confirm on chain. However, if we assume that pinnable
outputs are, indeed pinnable, and attempts to pin a transaction and
keep it from confirming have highly correlated success rates,
there's no reason we shouldn't also batch claims of pinnable
outputs separately.

Sadly, aggregating other types of inputs is rather nontrivial, as
evidenced by the size of this commit -

HTLC-Timeout claims have locktimes fixed by our counterparty's
signature and thus can only be aggregated with other HTLCs of the
same CLTV, which we have to check for.

Further, our concept of "pinnable" is deliberately somewhat fuzzy -
outputs which we believe are not pinnable will eventually become
pinnable at the height where our counterparty can spend them. Thus,
we treat outputs as pinnable if they're over that height, and if
they're within `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` of
that height we treat them as neither pinnable nor not-pinnable,
aggregating such claims only with other claims which will become
pinnable at the same height.

However, the complexity required is worth it - aggregation can save
our users a lot of money in the case of a force-close, and directly
impacts the amount of UTXOs they need as reserve for anchors, so we
want to aggregate as much as possible.

Fixes #3064

Make ignoring duplicates `PackageTemplate` claims more robust

`update_claims_view_from_requests` assumes that any
`PackageTemplate`s passed to it will not be aggregated (i.e. have
only one input) and uses that assumption when skipping duplicates.

This is currently true, but dropping `PackateTemplate` claims
entirely based on only the first input is somewhat brittle so we
add a debug assertion here and update the logic to not spuriously
drop claims if they happen to come into the `OnChainTxHandler`
pre-aggregated.

Move `PackageTemplate` merging decisions entirely into `package.rs`

Currently our package merging logic is strewn about between
`package.rs` (which decides various flags based on the package
type) and `onchaintx.rs` (which does the actual merging based on
the derived flags as well as its own logic), making the logic hard
to follow.

Instead, here we consolidate the package merging logic entirely
into `package.rs` with a new `PackageTemplate::can_merge_with`
method that decides if merging can happen. We also simplify the
merge pass in `update_claims_view_from_requests` to try to
maximally merge by testing each pair of `PackateTemplate`s we're
given to see if they can be merged.

This is overly complicated (and inefficient) for today's merge
logic, but over the coming commits we'll expand when we can merge
and not having to think about the merge pass' behavior makes that
much simpler (and O(N^2) for <1000 elements done only once when a
commitment transaction confirms is fine).

Make several tests more robust against harmless transaction changes

In the next commit we'll be changing the order some transactions
get spent in packages, causing some tests to spuriously fail. Here
we update a few tests to avoid that by checking sets of inputs
rather than specific ordering.

Rename `soonest_conf_deadline` to `counterparty_spendable_height`

This renames the field in `PackageTemplate` which describes the
height at which a counterparty can make a claim to an output to
match its actual use.

Previously it had been set based on when a counterparty can claim
an output but also used for other purposes. In the previous commit
we cleaned up its use for fee-bumping-rate, so here we can rename
it as it is now only used as the `counteraprty_spendable_height`.

Clean up `PackageTemplate::get_height_timer` to consider type

`PackageTemplate::get_height_timer` is used to decide when to next
bump our feerate on claims which need to make it on chain within
some window. It does so by comparing the current height with some
deadline and increasing the bump rate as the deadline approaches.

However, the deadline used is the `counterparty_spendable_height`,
which is the height at which the counterparty might be able to
spend the same output, irrespective of why. This doesn't make sense
for all output types, for example outbound HTLCs are spendable by
our counteraprty immediately (by revealing the preimage), but we
don't need to get our HTLC timeout claims confirmed immedaitely,
as we actually have `MIN_CLTV_EXPIRY` blocks before the inbound
edge of a forwarded HTLC becomes claimable by our (other)
counterparty.

Thus, here, we adapt `get_height_timer` to look at the type of
output being claimed, and adjust the rate at which we bump the fee
according to the real deadline.

Stop passing current height to `PackageTemplate::build_package`

Now that we don't store the confirmation height of the inputs
being spent, passing the current height to
`PackageTemplate::build_package` is useless - we only use it to set
the height at which we should next bump the fee, but we just want
it to be "next block", so we might as well use `0` and avoid the
extra argument. Further, in one case we were already passing `0`,
so passing the argument is just confusing as we can't rely on it
being set.

Note that this does remove an assertion that we never merge
packages that were crated at different heights, and in the future
we may wish to do that (as there's no specific reason not to), but
we do not currently change the behavior.

Drop unused `PackageTemplate::height_original`

This has never been used, and its set to a fixed value of zero for
HTLCs on local commitment transactions making it impossible to rely
on so might as well remove it.

Rename `PackageTemplate::timelock` `counteraprty_spendable_height`

This function was very confusing - its used to determine by when
we have to stop aggregating this claim with others as it starts to
be at risk of pinning due to the counterparty's ability to spend
the output.

It is not ever used as a timelock for a transaction, and thus its
name is very confusing.

Instead we rename it `counterparty_spendable_height`.

Rename claim cleaning match bool for accuracy

We don't actually care if a confirmed transaction claimed other
outputs, only that it claimed a superset of the outputs in the
pending claim we're looking at. Thus, the variable to detect that
is renamed `is_claim_subset_of_tx` instead of `are_sets_equal`.

Merge pull request #3290 from tnull/2024-09-add-tx-sync-copyright

Add copyright notices to `lightning-transaction-sync` modules

Add copyright notices to `lightning-transaction-sync` modules

Merge pull request #3278 from TheBlueMatt/2024-08-124-relnotes

Add an 0.0.124 CHANGELOG entry

Update versions to 0.0.124 and invoice 0.32 types 0.1 release

Add an 0.0.124 CHANGELOG entry

Merge pull request #3285 from TheBlueMatt/2024-08-tx-too-small

Correct handling of added `OP_RETURN` outputs

Merge pull request #3284 from TheBlueMatt/2024-08-bindings-imports-suck

Correct `ANCHOR_INPUT_WITNESS_WEIGHT` constant

`ANCHOR_INPUT_WITNESS_WEIGHT` is too high by two weight units,
likely it was calculated to include the SegWit marker bytes, but
it is used to describe an `Input::satisfaction_weight`, which does
not expect the marker bytes.

This corrects that oversight, reducing the constant by two and
adding the marker bytes back in our own internal weight
calculations. It also fixes a second issue where the constant was
too low by one when `grind_signatures` is not set, as that may
result in a signature being one byte longer than we expect.

Add a test for the past few commits

This adds a single test for coin selection which exercises the
issues fixed in the past three commits.

Drop bogus debug assertion that we don't overpay on fees

We should always select at least as many coins as is required to
meet the feerate target, but its perfectly fine if we overshoot.
Specifically, we may overshoot deliberately if we choose to burn
change to fee instead.

Handle under-coin-selecting due to an added OP_RETURN output

When we do coin selection for channel close anchor spends, we may
do coin selection targeting exactly the input values we need.
However, if coin selection does not include a change output, we may
add an OP_RETURN output, which may cause us to end up with less
fee than we wanted on the resulting package.

Here we address this issue by running coin selection twice - first
without seeking the extra weight of the OP_RETURN output, and again
if we find that we under-selected.

Ensure we never try to broadcast a transaction <= 64 bytes

While these are consensus-valid, they have been nonstandard for
quite some time and will not relay nor confirm.

Specify imports manually in `types::features::sealed`

There's not a lot of reason to do this, but sadly the bindings
don't currently resolve wildcard imports at all, and I don't want
to deal with implementing it right now.

Merge pull request #3282 from TheBlueMatt/2024-08-fix-bench-logging

Dont output logs when benchmarking

Dont output logs when benchmarking

In 11ab302087622b546d116fc9679f601667d18c4d we accidentally removed
the `not(ldk_bench)` bound before outputting logs to stderr, which
we restore here. Instead of simply ignoring logs in benchmarks,
which we did previously, we instead format logs (in a way that LLVM
will not optimize out).

Remove log level filtering in tests

There's no reason why we should be filtering our logs during tests.
It seems mostly like the `TestLogger` was written more as a sample
logger than a test logger, but these days we have samples, so we
should just drop the filtering and show everything.

Merge pull request #3280 from TheBlueMatt/2024-08-124rc

Bump versions to rc1

Merge pull request #3279 from TheBlueMatt/2024-08-no-pub-usize

Use `u64` for `required_unknown_bits_from` indexes, not `usize`

Bump versions to rc1

Merge pull request #3257 from tnull/2024-08-fix-is-public

Rename instances of `is_public` to `is_announced`

Rename `announced_channel` to `is_announced_for_forwarding`

.. we rename the flag configuring whether we announce a channel or not.

Use `u64` for `required_unknown_bits_from` indexes, not `usize`

While `usize` should be fine, we're multiplying the index by 8 so
if we have a jumbo feature bit fitting in a 32-bit size type may
not quite work. More importantly, this would be the first use of a
`usize` in the public API and dealing with it in bindings is
annoying so we just replace with a `u64`.

Merge pull request #3238 from Mirebella/clippy-checks

Added clippy ignore rules for all errors and warnings

Merge pull request #3275 from TheBlueMatt/2024-08-3259-followups

#3259 followups

Merge pull request #3276 from arik-so/arik/2024/08/apply_monitor_updates_on_archive

Process updates before archiving monitors.

Process updates before archiving monitors.

Previously, `MonitorUpdatingPersister` was disregarding any unapplied
monitor updates when archiving them. This commit ensures that upon
reading monitors, their corresponding updates are also read and
applied prior to archiving.

Store Broadcaster and FeeEstimator on MonitorUpdatingPersister.

`MonitorUpdatingPersister` does not currently correctly archive
monitors because it neglects any unapplied updates. In order to start
applying these updates, the archiving methods will require access to
instances of `BroadcasterInterface` and `FeeEstimator`.

This commit requires that the `MonitorUpdatingPersister` be
instantiated with those instances, obviating the need for passing
them around, and laying the foundation for the following commit.

Test manual funding transaction `Event::DiscardFunding` generation

Correct manual shutdown detection on channel closure

In 5e874c3dc9cf1606a3cbbccab3a0d25089a97280 we'd intended to not
reveal the dummy funding transaction in `Event::DiscardFunding`.
However, instead of looking at the channel that was just closed,
the logic only looks at any other channels which were funded as a
part of the same batch. Because manually-funded transactions
cannot currently be done for batch funding, this was actually dead
code, preventing the new changes from taking effect.

Add additional documentation on `Channel::unbroadcasted_funding`

Merge pull request #3268 from TheBlueMatt/2024-08-moar-feerate-categories

Split up `ConfirmationTarget` even more

Fix CI linting error

Add clippy ignore rules for all errors and warnings

Test new `ConfirmationTarget` selection based on HTLC set

This updates `test_yield_anchors_events` to test both anchor
channels with and without HTLCs, and relies on overriding only the
singular expected `ConfirmationTarget` used, testing the new
`ConfirmationTarget::UrgentOnChainSweep` use.

Don't ignore events in `test_yield_anchors_events`

Our tests should never ignore the events generated as they provide
critical context about what's happening in LDK. Here we fix
`test_yield_anchors_events` to avoid doing so.

Add a constructor and per-target override to `TestFeeEstimator`

This will allow us to test `ConfirmationTarget`s used in functional
tests by setting an override on just the target we expect to be
used.

Add missing pending changelog for downgrade on unsafe funding

Only generate an `Event::DiscardFunding` when we need to

5e874c3dc9cf1606a3cbbccab3a0d25089a97280 changed
`Event::DiscardFunding` to not include a dummy transaction when we
were funded without a full funding tx, but in doing so started
generating `DiscardFunding` events on every channel closure rather
than only when there's actually still a pending funding broadcast.

This restores the previous behavior to only generate the event when
we should actually discard the funding tx.

Merge pull request #3259 from vincenzopalazzo/macros/dummy-tx

[RFC] event: store the outpoint when is_manual_broadcast

event: store the outpoint when is_manual_broadcast

With [1], it's possible to specify `manual_broadcast` for
the channel funding transaction. When `is_manual_broadcast` is
set to true, the transaction in the `DiscardFunding` event is
replaced with a dummy empty transaction.

This commit checks if `is_manual_broadcast` is true and
stores the funding OutPoint in the DiscardFunding event instead.

[1] https://github.com/lightningdevkit/rust-lightning/pull/3024

Link: https://github.com/lightningdevkit/rust-lightning/issues/3164
Suggested-by: TheBlueMatt
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Split `ConfirmationTarget::OnChainSweep` into urgent and non-urgent

When we force-close a channel, occasionally its due to feerate
disagreements or other non-HTLC-related issues. In those cases,
there's no reason to use a very urgent feerate estimate - we don't
have any timers expiring soon.

Instead, we should give users the information they need to be more
economical on fees in this case, which we do here by splitting
`OnChainSweep` into `UrgentOnChainSweep` and
`NonUrgentOnChainSweep` `ConfirmationTarget`s.

Add a new `ConfirmationTarget::MaximumFeeEstimate`

When we broke `ConfirmationTarget` out into task-specific names, we
left `MaxDustHTLCExposure::FeeRateMultiplier` as using the "when we
broadcast feerate" as we were mostly concerned about the dust
thresholds on outbound channels where we pick the fee and drive our
own funds to dust.

In 51bf78d604b28fe189171e1b976fe87cbb2614b7, that changed to
include transaction fees on both inbound and outbound channels in
our dust exposure amount, but we continued to use
`ConfirmationTarget::OnChainSweep` for the fee estimator threshold.

While the `MaxDustHTLCExposure::FeeRateMultiplier` value is quite
conservative and shouldn't lead to force-closures unless feerate
estimates disagree by something like 500 sat/vB (with only one HTLC
active in a channel), this happened on Aug 22 when feerates spiked
from 4 sat/vB to over 1000 sat/vB in one block.

To avoid simple feerate estimate horizons causing this in the
future, here we add a new
`ConfirmationTarget::MaximumFeeEstimate` which is used for dust
calculations. This allows users to split out the estimates they use
for checking counterparty feerates from the estimates used for
actual broadcasting.

Merge pull request #3273 from TheBlueMatt/2024-08-bindings-no-static

Return owned `String`s for onion message message types

Merge pull request #3248 from jkczyz/2024-08-blinded-path-utils-refactor

Refactor `BlindedPath` construction utils

Return owned `String`s for onion message message types

Returning a reference from a trait method is relatively difficult
to map in bindings and is currently handled by storing the object
in the trait instance, returning a reference to the local field.

This is fine when the object we're returning only needs to live as
long as the trait, but when it needs to be `'static` (as is the
case for onion message `msg_type`s), there's not really a good way
to map them at all.

Instead, here, condition on `#[cfg(c_bindings)]` we return a fully
owned `String`. This is obviously relatively less effecient, but
the extra allocation and `memcpy` isn't the end of the world,
especially given it should be released relatively quickly.

Note that this breaks doctests in with `c_bindings`.

Merge pull request #3272 from TheBlueMatt/2024-08-bindings-hidden

Mark new `types` `#[doc(hidden)]` methods as bindings no-export

Mark `io::Cursor` as no-export in bindings

Mark new `types` `#[doc(hidden)]` methods as bindings no-export

Merge pull request #3153 from alecchendev/2024-07-async-closing-signed

Allow sending closing tx signatures asynchronously

Merge pull request #2992 from G8XSU/monitor-update-seq-doc

Doc Clarity: Handling gaps in persisted ChannelMonitorUpdates.

Merge pull request #3269 from TheBlueMatt/2024-08-this-should-work-bindings

Use the actual type, not the associated type, in trait impls

Test async sign_closing_transaction

Allow sending closing tx signatures asynchronously

Use the actual type, not the associated type, in trait impls

This is equivalent and just easier for the bindings to deal with
(though, really, they should handle it).

Merge pull request #3266 from optout21/bech32-tests

Add invoice serialization tests [TestOnly][Minor]

Add invoice serialization tests

Merge pull request #3263 from TheBlueMatt/2024-08-bindings-om

Remove message type bound on `ResponseInstruction`

Add a `MessageSendInstructions::ForReply`

In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes #3178

Change `send_onion_message` to take a `MessageSendInstructions`

This lets callers include a `reply_path` without doing the
path-finding at the callsite, utilizing the built-in path-finding
logic in `OnionMessenger`

3/3 Use `MessageSendInstructions` instead of `PendingOnionMessage`

Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `AsyncPaymentsMessageHandler`. Better, we can also
drop the `c_bindings`-specific message queue variant, unifying the
APIs.

Here we also drop `PendingOnionMessage` entirely.

2/3 Use `MessageSendInstructions` instead of `PendingOnionMessage`

Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `OffersMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

Because `ChannelManager` needs to actually control the reply path
set in individual messages, however, we have to halfway this patch,
adding a new `MessageSendInstructions` variant that allows
specifying the `reply_path` explicitly. Still, because other message
handlers are moving this way, its nice to be consistent.

1/3 Use `MessageSendInstructions` instead of `PendingOnionMessage`

Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the first step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `CustomMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

Give `MessageSendInstructions` any `Destination`, not only Blinded

In the next commit we'll use `MessageSendInstructions` for all
messages, so it needs the ability to take any `Destination`, not
only a `Destination::Blinded`.

Make `ResponseInstruction` `Clone`able

Pull the guts of `ResponseInstruction` into a new enum

In the coming commits we'll use the `ResponseInstruction` enum's
contents for all messages, allowing message senders to rely on the
in-`OnionMessegner` reply path selection logic.

In order to do so and avoid users confusing the new
`MessageSendInstructions` for `ResponseInstruction`, we leave
`ResponseInstruction` as a now-unconstructible struct which wraps a
`MessageSendInstructions`.

Remove message type bound on `ResponseInstruction`

Our onion message handlers generally have one or more methods which
return a `ResponseInstruction` parameterized with the expected
message type (enum) of the message handler.

Sadly, that restriction is impossible to represent in our bindings -
our bindings concretize all LDK structs, enums, and traits into a
single concrete instance with generics set to our concrete trait
instances (which hold a jump table). This prevents us from having
multiple instances of `ResponseInstruction` structs for different
message types.

Our bindings do, however, support different parameterizations of
standard enums, including `Option`s and tuples.

In order to support bindings for the onion message handlers, we
are thus forced into std types bound by expected message types,
which we do here by making `ResponseInstruction` contain only the
instructions and generally using it as a two-tuple of
`(message, ResponseInstruction)`.

Merge pull request #3193 from tnull/2024-07-2995-followups

#2995 followups

Merge pull request #3265 from TheBlueMatt/2024-08-globally-unique-names

Disambiguate blinded path `ForwardNode`s between payment + message

Merge pull request #3250 from TheBlueMatt/2024-08-feature-cleanup

Correct `ConnectionNeeded` docs

Avoid allocating `Vec` for a single result

Fix permissive test

Merge pull request #3260 from TheBlueMatt/2024-08-more-deref

Bound `Persister`'s `WriteableScore` by `Deref`

Disambiguate blinded path `ForwardNode`s between payment + message

We currently have two structs with identical names in our public
API - `blinded_path::message::ForwardNode` and
`blinded_path::payment::ForwardNode`. This makes the API somewhat
awkward to use - users have to try (and fail) to import
`ForwardNode` twice only to then have to change their imports.

More importantly, however, this makes the API very hard to use in
some bindings languages where rename-imports or module imports
aren't available.

Thus, here, we rename both to give them context.

Remove uneccessary `std` bounds on many tests

We never actually build with `#![no_std]` in tests as Rust does
not support it. Thus, many tests have spurious `std` feature gates
when they run just fine without them. Here we remove those gates,
though note that tests that depend on behavior elsewhere in the
codebase which is `std`-gated obviously need to retain their
feature gates.

Reduce `feature = std` flags in `gossip.rs` somewhat

Fewer feature flags makes for more readable code, so we opt for
that over very marginally more effecient code here.

Drop `std::Error` impl for `PeerHandleError`

Not sure why we ever really had this, no one really ever bounds
anything on `std::Error` and its kinda a dead type, so there's no
need for us to `impl` it for our types.

Simplify `Instant` mocking in outbound payments

To handle `std` and `no-std` concepts of time in scoring, we'd
originally written a generic `Time` trait which we could use to
fetch the current time, observe real (not wall-clock) elapsed time,
and serialize the time.

Eventually, scoring stopped using this `Time` trait but outbound
payment retry expiry started using it instead to mock time in
tests.

Since scoring no longer uses the full features which required the
`Time` trait, we can substantially simplify by just having the
mocking option.

Remove note about `std`/`no-std` scorer serialization compat

In 81389dee306d960a8030bec5ffa304004148ce85 we removed a note about
mixing the `std` and `no-std` feature when de/serializing
`ProbabilisticScorer`s but forgot to note that there was a second
copy of that note in the module documentation.

This removes that note.

Bound `Persister`'s `WriteableScore` by `Deref`

This makes the trait marginally more flexible, but more importantly
matches our normal structure which makes the bindings generator a
bit happier.

Rename `ChannelDetails::is_public` to `is_announced`

Referring to announced/unannounced channels as 'public'/'private'
regularly leads to confusion on what they are and when which should be
used. To avoid perpetuating this confusion, we should avoid referring to
announced channels as 'public' in our API.

Here we rename `ChannelDetails::is_public` (which breaks
previously-released API) to align it with the changes in
`OpenChannelRequest`.

Rename `OpenChannelRequest::is_public` to `is_announced`

Referring to announced/unannounced channels as 'public'/'private'
regularly leads to confusion on what they are and when which should be
used. To avoid perpetuating this confusion, we should avoid referring to
announced channels as 'public' in our API.

Here we rename the recently introduced field in `OpenChannelRequest`
(which doesn't break released API), and will align the pre-existing
instances of `is_public` in the following commit (which will break API).

Rename construct_keys_callback functions

Require Destination in construct_keys_callback

Use a single iterator to construct a BlindedPath

Instead of using separate iterators for pubkeys and TLVs, use an
iterator that yields a tuple of them. This allows for holding a mutable
reference to the TLVs such that they can be padded. With two iterators,
the pubkey iterator would have a reference to the ForwardNode slice when
constructing a payment path. However, this would prevent holding mutable
references in the TLVs iterator.

Generalize build_keys_helper

When constructing a blinded path, two iterators are used: one for the
pubkeys and another for Writeable TLVs. The first iterator is used in
the build_keys_helper utility function while the second is used inside
of a callback. Update this utility to work on any type that can be
borrowed as a PublicKey. This allows for using a single iterator of
tuples, which is necessary for padding the hops without additional
allocations and clones.

Add a construct_keys_callback for blinded paths

When constructing a BlindedPath, an Option<Destination> parameter to
construct_keys_callback is never passed. Make a separate utility
function that doesn't take this parameter. This allows for using the
build_keys_helper macro with a Iterator yielding items other than
PublicKey.

Use PublicKey values in construct_keys_callback

Instead of accepting iterators yielding PublicKey by reference in
utils::construct_keys_callback, take iterators yielding values since
these implement Copy and need to be copied anyway. This will help avoid
a situation when padding where both a reference and mutable reference
are needed.

Refactor helper macro from construct_keys_callback

When constructing a BlindedPath, utils::construct_blinded_hops uses two
iterators. However, this makes it difficult to pad blinded hops to equal
sizes without allocating a vector or cloning data. Refactor the
construct_keys_callback utility function so that is can be used with an
Iterator with different Item types. This allows using a single Iterator
of tuples while still supporting its use only with pubkeys.