Only signal `OPTION_ZEROCONF_OPT` to peers if we either:
1. Have no trusted peer configured (assuming that we are LSP)
2. Have a trusted peer configured, and the peer we are connecting
to is this trusted peer.
Otherwise peers that are LSPs but are not the clients trusted LSP
might try to open a channel to the client but it would get rejected.
Make the just in time channel fees and channel size
configvars, as in practice not every provider would
use the same hardcoded fees or channel sizes.
Add the mining fees required for the funding transaction on top of the
opening fees to prevent opening channels at a loss in a higher
fee environment.
Adds unittests for `LNWallet.open_channel_just_in_time()`,
`LNWallet._cleanup_failed_jit_channel()`,
`LNWallet.can_get_zeroconf_channel()` and
`LNWallet.receive_requires_jit_channel()`.
...so we can have multiple just in time channels with the same lsp.
We already save a remote scid alias in `on_channel_ready` which we
already have received after the new zeroconf channel is in open state.
So setting the alias to the static node id hash is counterproductive
because it doesn't allow to differentiate between channels.
Also extends the regtest (`just_in_time`) to do a second channel
opening, to cover this scenario. This doesn't add much runtime to
the test, so the cost seems reasonable.
On LSP side we were only checking if ACCEPT_ZEROCONF_CHANNELS
is enabled while forwarding a non-trampoline htlc.
During trampoline forwarding the config was ignored.
The ACCEPT_* prefix implied this was only for accepting inbound
zeroconf channels, but it also controls whether we open them when
forwarding HTLCs.
Renames the config var to OPEN_ZEROCONF_CHANNELS
to clarify it enables zeroconf channel opens in both directions,
and add the missing check when forwarding trampoline HTLCs.
Fixes AbstractChannel.update_unfunded_state to stop calling a
non-existent method (unwatch_channel).
Adds unittest to execute the zeroconf path of update_unfunded_state.
Add unittest that verifies we only include r_tags for trampoline nodes
if we signal trampoline support in the invoice_features and only signal
trampoline support if we use trampoline or have only open trampoline
channels.
- could not find a single project that still actually cares about bip70 [0]
- well except maybe BitPay.
- but I cannot test with BitPay:
- they have a testnet3 staging environment on test.bitpay.com
- but the SSL cert they use for bip70 has expired in 2021
- the webUI probably also has not been updated since then...
- they claim to have added LN support in 2022 in a blog post,
but it's not there on test.bitpay.com
- on mainnet, they require KYC before payment
- < ... angry noises >
- their loss then, I don't care.
- this is code that no one wants to maintain
- this does not yet delete the signed bip70 payment data for historical txs
- but it is no longer possible to export it from the GUI
[0]: https://bitcoinops.org/en/topics/bip70-payment-protocol/
If SwapManager.percentage was a 0.2 float, rounding differences would
cause an exception in the fee calculation inverse sanity check when entering 20
000 sats into the SwapDialog. By making self.percentage a decimal we can
prevent this kind of issue.
```
File "/home/user/code/vibecoding_vm/electrum/electrum/gui/qt/swap_dialog.py", line 294, in on_send_edited
recv_amount = self.swap_manager.get_recv_amount(send_amount, is_reverse=self.is_reverse)
File "/home/user/code/vibecoding_vm/electrum/electrum/submarine_swaps.py", line 1320, in get_recv_amount
if abs(send_amount - inverted_send_amount) > 1:
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for -: 'int' and 'NoneType'
```
As there are two htlcs, the `alice_htlc_resolved` Event might get set either once or twice by the time `alice_htlc_resolved.wait()` returns. The previous code was assuming that both htlcs are resolved by then, but it could happen that only one htlc was resolved, due to timing.
"When should we reveal preimages onchain?"
This commit tries to simplify the thinking by making the observation:
- we can reveal preimages (actually in any context) if they are already public
- a preimage is public if any other lightning node knows it besides us
- if we learn the preimage from another LN node, it is public
- if we send update_fulfill_htlc, it becomes public
- if we see a preimage onchain, it is public
- in lnsweep._maybe_reveal_preimage_for_htlc:
- partial mpp check is not relevant if preimage is already public
- let's just always do KeepWatchingTXO, for sanity/safety
Co-authored-by: ThomasV <thomasv@electrum.org>
Test that Abstract_Wallet.bump_fee() raises if the given feerate
of the replacement is equal to the feerate of the tx to bump as this
wouldn't be accepted to the mempool.
Adds unittest to check the fee increase when adding outputs to a base
tx. Supposed to prevent creating transactions that don't get accepted
like in this traceback:
```
broadcast_transaction error [DO NOT TRUST THIS MESSAGE]: "RPCError(1, 'the transaction was rejected by network rules.\\n\\ninsufficient fee, rejecting replacement ceeaef5ac7f82286e42ebd530e965fa4c7a6c11933d6b89d6d6f0ee2c69db839; new feerate 0.00001109 BTC/kvB <= old feerate 0.00001110 BTC/kvB
```
I noticed CLN is sending our own channel update to us on
reestablishment, we then assume it to be the remote nodes
update and try to verify the signature against their pubkey
which fails and throws `InvalidGossipMsg`.
This adds a check preventing us from trying to save our own
channel updates as remote update.
Adds unittest for Abstract_Wallet.export_history_to_file that
compares the output against reference files. This
should help to prevent regressions and ensure the layout
of the export stays static over time.
When gossip is enabled we waste a lot of time trying to connect
to onion peers if we don't have a proxy enabled. We should just skip
them and try to connect to clearnet peers instead.
LNPeerManager.add_peer would only check if self.network.proxy is set,
which it is always as Network is initialized with self.proxy =
ProxySettings(). Instead it should check if proxy is set and enabled.
Don't set the NETWORK_AUTO_CONNECT config if the user checked the custom
server config checkbox and then cancels the wizard, so the next time the
user starts the wizard they again will have the choice instead of
silently being forced into a random server.
Currently if the user cancels the wizard during the network config the
welcome component will already have set the NETWORK_AUTO_CONNECT config
to False (as the user selected the custom server checkbox), preventing
the wizard from starting again on the next startup.
This patch changes the CallbackManager to use WeakMethods (weakrefs) to
break the ref cycle and allow the GC to clean up the wallet objects.
unregister_callbacks() will also get called automatically, from
EventListener.__del__, to clean up the CallbackManager.
I also added a few unit tests for this.
fixes https://github.com/spesmilo/electrum/issues/10427
-----
original problem:
In many subclasses of `EventListener`, such as `Abstract_Wallet`, `LNWatcher`,
`LNPeerManager`, we call `register_callbacks()` in `__init__`.
`unregister_callbacks()` is usually called in the `stop()` method.
Example - consider the wallet object:
- `Abstract_Wallet.__init__()` calls `register_callbacks()`
- there is a `start_network()` method
- there is a `stop()` method, which calls `unregister_callbacks()`
- typically the wallet API user only calls `stop()` if they also called
`start_network()`.
This means the callbacks are often left registered, leading to the wallet
objects not getting GC-ed. The GC won't clean them up as
`util.callback_mgr.callbacks` stores strong refs to instance methods
of `Abstract_Wallet`, hence strong refs to the `Abstract_Wallet` objects.
An annoying example is `daemon.check_password_for_directory`, which
potentially creates wallet objects for all wallet files in the datadir.
It simply constructs the wallets, does not call `start_network()` and
neither does it call `stop()`.
- to gracefully take duplicate calls of register_callbacks(): should be idempotent now
- as a side-effect, the order of the callbacks is changed and not guaranteed
- not like anyone should have been relying on it before though
ec65c53 replaces the usage of `PeerInTest` with `Peer` in
test_lnpeer.py.
PeerInTests sets `Peer.DELAY_INC_MSG_PROCESSING_SLEEP` to 0 so all
incoming messages get processed immediately. Because `Peer` instead of
`TestInPeer` was used the delay caused `test_reestablish_with_old_state`
to fail regularly because bob receives the old channel state and kills
the OldTaskGroup of the unittest with GracefulDisconnect before Alice
processed the answer of Bob and is still in ChannelState.REESTABLISHING.
```
FAILED tests/test_lnpeer.py::TestPeerDirect::test_reestablish_with_old_state - AssertionError: <PeerState.REESTABLISHING: 1> != <PeerState.BAD: 3>
```
f321x