We use them largely as indexes into a Vec<Transaction> so there's
little reason for them to be u32s. Instead, use them as usize
everywhere.
We also take this opportunity to add range checks before
short_channel_id calculation, as we could otherwise end up with a
bogus short_channel_id due to an output index out of range.
This was just an oversight when route calculation was split up into
parts - it makes no sense for get_route to require that we have a
full route message handler, only a network graph (which can always
be accessed from a NetGraphMsgHandler anyway).
Instead of making the filter_block fn in the ChainWatchInterface
trait return both a list of indexes of transaction positions within
the block and references to the transactions themselves, return
only the list of indexes and then build the reference list at the
callsite.
While this may be slightly less effecient from a memory locality
perspective, it shouldn't be materially different.
This should make it more practical to generate bindings for
filter_block as it no longer needs to reference Rust Transaction
objects that are contained in a Rust Block object (which we'd
otherwise just pass over the FFI in fully-serialized form).
non-mut references to primitives are only excess overhead, so
there's not much reason to ever have them. As a nice bonus, it also
is one less thing to worry about when generating C bindings
This is more consistent with the way we use std::cmp over the
codebase and avoids `use std`, which is only actually needed to
support older rustcs, so feels a bit awkward.
This isn't a big difference in the API, but it avoids needing to
wrap a given NetworkGraph in a RwLock before passing it, which
makes it much easier to generate C bindings for.
In general, we don't need an explicit lifetime when doing something
like:
fn get_thing(&self) -> &Thing { &self.thing }.
This also makes it easier to reason about what's going on in the
bindings generation.
... instead of only the txid.
This is another instance of it not being possible to fully
re-implement SimpleManyChannelMonitor using only public methods. In
this case you couldn't properly register outpoints for monitoring
so that the funding transaction would be matched.
Tests use sources of randomness to produce seeds, preimages, secrets,
and ephemeral data. However, this makes comparing logs between different
test runs difficult. Remove uses of random number generators and the
current time in favor of fixed values in order to make the test output
deterministic.
When we were sending an open_channel messages we were asking the
feerate estimator for a new value instead of using the one we had.
If the feerate estimator gave a different value than the one it did
when we created the Channel struct, we'd start out-of-sync with our
counterparty and blow up on funding_signed. Even worse, the
ConfirmationTarget used was different, so its highly likely they
would disagree.
Also remove newly unused fee estimator parameter from get_open-channel
API.
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
When we receive an inbound HTLC from a peer on an inbound channel,
make sure the funder can still cover the additional on-chain cost
of the HTLC while maintaining their channel reserve.
When we're sending an outbound HTLC, make sure the funder can still
cover the additional on-chain cost of the HTLC while maintaining
their channel reserve.
+ implement fee spike buffer for channel initiators sending payments.
Also add an additional spec-deviating fee spike buffer on the
receiving side (but don't close the channel if this reserve is
violated, just fail the HTLC).
From lightning-rfc PR #740.
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
This also includes adding a closure that creates a new pending HTLC status
as a parameter for Channel's update_add_htlc. This will later be useful
when we add the check for fee spike buffer violations, which will also result
in changing an HTLC's pending status to failing.
Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
This is a response to splitting #585 into smaller components. This extraction should allow the subsequent creation of a trait for all message handling, thereby enabling more flexibility in the state machine, particularly for bindings.
As we drop the requirement that all ChannelKeys expose the private
keys used, we should have a way to access the private keys in use
when using InMemoryChannelKeys.
`to_local` output or remote output on remote commitment transaction
needs a channel keys to be spent. As per-channel keys are derived from
KeysManager seed and per-channel secrets those must be backed up by
any descriptor bookmarking for latter spend. We test that generating
a new KeysManager loaded with such backed-up seed/per-channel secrets
return the correct keys for spending a `to_local` output.
Instead of blindly signing provided witnessScript, signer must derive
channel keys corresponding to the provided per-commitment-point and
regenerate templated witnessScript to ensure its syntax correctness.
Instead of blindly signing provided witnessScript, signer must derive
channel keys corresponding to the provided per-commitment-point and
regenerate templated witnessScript to ensure its syntax correctness.
on_remote_tx_csv is the CSV delay encumbering remote transactions
revokable outputs as required by local.
on_local_tx_csv is the CSV delay encumbering local transactions
revokable outputs as required by remote.
Local/remote is here defined from a code processing viewpoint,
process running this code is "local".
