Our hash tables allow duplicate keys, and we use that in a few places.
However, the get() function only returns the first, so it's not a good
idea with such hash tables.
Another patch fixes this at a deeper level (using different hash table
types depending on whether this table can have duplicates), but this
is the minimal fix for existing code.
This may be the cause behind us occasionally missing onchain events:
Fixes: https://github.com/ElementsProject/lightning/issues/7460
Fixes: https://github.com/ElementsProject/lightning/issues/7377
Fixes: https://github.com/ElementsProject/lightning/issues/7118
Fixes: https://github.com/ElementsProject/lightning/issues/6951
This fixes them in future: fixing them now will require something else.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: lightningd: occasionally we could miss transaction outputs (not telling gossipd, or even onchaind)
Found by very slow CI: the two io_breaks() can combine into one, so we
block waiting for the second initialization which never happens.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
checkchain_timer is run by chaintopology, so why have the pointer in
bitcoind?
And since we free the timers, we don't need them to self-disable by
checking the stopped flag.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handling half in main() and half here was a mess. And the name
"max_blockheight" was poor: it was the max in the db, or UINT32_MAX,
but then we changed it depending on what height we wanted to start at.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/6924
Changelog-Changed: lightningd: we wait for bitcoind if it has somehow gone backwards (as long as header height is still ok).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current code is confusing: there are polling loops, but we wait for
them to run once.
Be explicit: make the calls once, then start the loops in begin_topology.
This removes various chain_topology struct members which only exist for
startup.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These were removed from the spec.
We still support existing ones, though we were the only implementation
which ever did, and only in experimental mode, so we should be able to
upgrade them and avoid a forced close, with a bit of engineering...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the convention everywhere else: allocation ctx comes first, any
other context comes second.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The pattern of making tal-allocated copies of wally data to pass around
was made redundant after these calls were added by the use of
tal_wally_start/tal_wally_end to parent wally allocations. We can thus
just pass the data directly and avoid the allocations.
Removes redundant allocations when checking tx filters and computing fees.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
Standardizes the is_xxx script function all take a script length, and changes
their first-level callers to pass it. This has several knock on benefits:
- We remove the repeated tal_count/tal_bytelen calls on the script, in
particular the redundant calls that result when we must check for multiple
types of script - which is almost all cases.
- We remove the dependency on the memory being tal-allocated (It is, in
all cases, but theres no reason we need to require that).
- We remove all cases where we create a copy of the script just to id it.
- We remove all allocations for non-interesting scripts while iterating block
txs in process_getfilteredblock_step1().
- We remove all allocations *including for potentially interesting scripts* in
topo_add_utxos().
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
We were extracting the output script for all outputs, and discarding
them immediately again if they were not P2WSH outputs which are the
ones of interest to us. This patch move the extraction until after we
have determined it is useful, and so we should save a couple thousand
`tal()` and `tal_free()` calls.
Changelog-Changed: lightningd: Speed up blocksync by not parsing unused parts of the transactions
I did some CHANGELOG and git digging to see when these were deprecated, and
some were very old (v0.8.2!). But since they didn't warn users loudly, I
chose to do so this release only.
I renamed ld's `deprecated_apis` to `deprecated_ok` to make sure I
caught them all.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Removes the tal_count checking overhead when iterating constant arrays.
Separated from the previous commit to make review easier.
Changelog-None
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
- Avoid overhead from tal checks when iterating block txs
- Skip pegin txs as well as coinbase txs while iterating
- Early-exit if the txout cannot possibly be p2wsh
- Don't re-calculate the txid when we already have it
- Don't allocate a script for non-policy asset outputs
- Don't copy txids for non-interesting UTXOs
Note the below -Changed line covers the previous wally and PSBT commits
which also provide general block processing speedups.
Changelog-Changed: core: Processing blocks should now be faster
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
This means refactoring out some of the generic anchor info, from the
per-commitment-tx info (we can have at least two, perhaps more with
splicing!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have to work quite hard to do this, since we don't want to call
finish if the broadcast has been freed in the meantime.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, every broadcast was attached to a channel, but we can
make it explicit, so when the context is freed, the re-broadcast stops
(if rebroadcast is set).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the context is freed, the callback isn't called. This doesn't matter
yet, since our callbacks tend to be such that the callback itself is
required to free things, but it's clearer this way and allows more
flexible usage in following patches.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`struct log` becomes `struct logger`, and the member which points to the
`struct log_book` becomes `->log_book` not `->lr`.
Also, we don't need to keep the log_book in struct plugin, since it has
access to ld's log_book.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is just housekeeping that allows up
to do not spam the logs of people with not
useful information.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
ignore the min fee as specified by the user setting.
We explicitly allow the user to ignore the fee limits, although this comes with inherent risks.
By enabling this option, users are explicitly
I was aware of the potential dangers.
There are situations, such as the one described in [1], where it
becomes necessary to bypass the fee limits to resolve issues like a stuck channel.
BTW experimental-anchors should fix this.
[1] https://github.com/ElementsProject/lightning/issues/6362
Reported-by: @pabpas
Fixes: 64b1ddd761
Link: https://github.com/ElementsProject/lightning/issues/6362
Changelog-Fixes: do not ignore the ignore-fee-limit option during
update_fee
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We usually have access to `ld`, so avoid the global.
The only place generic code needs it is for the json command struct,
and that already has accessors: add one for libplugin and lightningd
to tell it if deprecated apis are OK.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In most cases, it's the same as option_anchor_outputs, but for
fees it's different. This transformation is the simplest:
pass it as a pair, and test it explicitly.
In future we could rationalize some paths, but this was nice
and mechanical.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we can CPFP, we don't have to track the feerate as closely. But
it still needs to get in the mempool, so we use 10 sat/byte, or the
100 block estimate if that is higher.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `feerates` has new fields `unilateral_anchor_close` to show the feerate used for anchor channels (currently experimental), and `unilateral_close_nonanchor_satoshis`.
Changelog-Changed: JSON-RPC: `feerates` `unilateral_close_satoshis` now assumes anchor channels if enabled (currently experimental).
We don't actually use it anywhere, but we actually want to now for
CPFP. So give it more parameters and make it return bool so it can
be set without necessarily suppressing rexmit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It depends on whether we negotiated anchors: just document that this
field doesn't apply for anchors (it becomes zero by the end of this
patch series, which is weird).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When core lightning is asking the information about
the blockchain with `getchaininfo` command lightningd
know already the information about the min and max block height.
the problem is when we have a smarter Bitcoin backend that is able
to switch between different clients in some cases is helpful
give the information about current known height by lightningd and
pass it down to the plugin.
In this way, the plugin knows what is the correct known height from lightnind, and can
try to fix some problems if any exit.
This is particularly useful when you are syncing a new backend from scratch
like https://github.com/cloudhead/nakamoto and we avoid returning the
lower height from the known, and avoid the crash of core lightning.
With this information, the plugin can start to sync the chain and return
the answer back only when the chain is in sync with the current status of
lightningd.
Another reason to add this field and not wait the correct block in core
lightning itself is because Bitcoin Core is extremely slow to sync up,
so the question here is, how long should we wait? The time depends
on various factors.
With this approach of informing the plugin about the height, in some cases,
you can start the syncing but move the execution to another backend until
the previous one is ready.
The problem I want to solve is that I don't want to be left in the dark when
we run `getchaininfo`, and I want to have the opportunity to wait for
the blockchain sync or decide to dispatch the request elsewhere.
Changelog-Added: Pass the current known block height down to the getchaininfo call.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We also do it on every block, but since bitcoind can't always be counted
to rebroadcast for us, we might as well be aggressive!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would only set it the first time, which was OK for how we were using it
before. Now we want to also set it for rebroadcast.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
