On master fw_fail_htlc is, especially on the CI, flaky.
We mine 100 blocks, then wait fixed 5 seconds, then check if bob has
failed back the htlcs to alice.
However if the test runs slowly (CI) 5 seconds can be too short
for bob to catch up to the new 100 mined blocks.
Instead we should just use the wait_until_htlcs_settled helper function
which polls Alice local_unsettled_sent with 30 sec timeout, allowing bob
to take a bit longer (or be faster) than 5 s.
```
.***** test_fw_fail_htlc ******
initializing alice
funding alice
a101c8c4c22043ff42029bcab2f0bf6ce5482a60d656294cbec3a4df557e2687
initializing bob
funding bob
d323d572c54817116d185c91f15e449550c651eb4ed76891d3011e0a8eb4ef9a
initializing carol
funding carol
bbf3503663876a4ae00f70c7e58ad49318e83d5cf99d6effe692e113d10910c2
mining 1 blocks
starting daemon (PID 5559)
/tmp/alice/regtest/wallets/default_wallet
true
starting daemon (PID 5577)
/tmp/bob/regtest/wallets/default_wallet
true
starting daemon (PID 5595)
/tmp/carol/regtest/wallets/default_wallet
true
alice and carol open channels with bob
mining 3 blocks
wait until alice sees channel open.
wait until alice sees channel open..
wait until alice sees channel open...
alice pays carol
Daemon stopped
mining 1 blocks
mining 150 blocks
wait until 99ad1d44b9054f5a85c2fb45e9a9b93eb13c785104ed0664be5cf866d79d38fc:2 is spent.
...
wait until 99ad1d44b9054f5a85c2fb45e9a9b93eb13c785104ed0664be5cf866d79d38fc:2 is spent............................
mining 1 blocks
mining 100 blocks
alice htlc was not failed
FDaemon stopped
```
This test was flaky: the mpp_set resolution gets set to SETTLING several asyncio event loop iterations before the hold invoice callback "cb" gets called.
If the 0.1 sec polling triggers just in the middle of that interval, `assert cb_got_called` fails.
```
async def check_mpp_state():
async def wait_for_resolution():
while True:
await asyncio.sleep(0.1)
if payment_key not in bob_w.received_mpp_htlcs:
continue
if not bob_w.received_mpp_htlcs[payment_key].resolution == RecvMPPResolution.SETTLING:
continue
return
await util.wait_for2(wait_for_resolution(), timeout=2)
> assert cb_got_called
E assert False
tests/test_lnpeer.py:1898: AssertionError
```
see https://github.com/spesmilo/electrum/blob/16c8cb50e38c274cce8f9f66f28d8dd453f9f074/electrum/lnpeer.py#L3136-L3137
fixes https://github.com/spesmilo/electrum/issues/10589
-----
diff to reproduce the failure without present patch:
```
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 8669931c24..e15973d68f 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1885,6 +1885,7 @@ class TestPeerDirect(TestPeer):
cb_got_called = False
async def cb(_payment_hash):
self.logger.debug(f"hold invoice callback called. {bob_w.network.get_local_height()=}")
+ await asyncio.sleep(1)
nonlocal cb_got_called
cb_got_called = True
```
It is sufficiently rare that we have to touch this stuff that I always have to re-discover where/how it is done. And it is impractical to grep for "bitcoin:" or "lightning:".
Putting this "master list" comment very close to the BITCOIN_BIP21_URI_SCHEME variable seems like a good spot - at least this is where I would look for it first.
Allows to rename a wallet file from the QML Wallet Details view.
This seems like a feature we should support as the use-case of a wallet can
change or maybe the user didn't think about a proper name when setting
up the wallet. Especially with lightning channels it is not possible to
restore from seed to change the name.
Fixes#4377
The CoinGecko API failed as the Honduran Lempira currency returned
null as value, rendering the API unusable:
```
11.19 | I | exchange_rate.CoinGecko | getting fx quotes for EUR
11.41 | E | exchange_rate.CoinGecko | failed fx quotes: InvalidOperation([<class 'decimal.ConversionSyntax'>])
Traceback (most recent call last):
File "/var/home/user/code/vibecoding_vm/electrum/electrum/exchange_rate.py", line 87, in update_safe
self._quotes = await self.get_rates(ccy)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/var/home/user/code/vibecoding_vm/electrum/electrum/exchange_rate.py", line 449, in get_rates
return dict([(ccy.upper(), to_decimal(d['value']))
~~~~~~~~~~^^^^^^^^^^^^
File "/var/home/user/code/vibecoding_vm/electrum/electrum/util.py", line 243, in to_decimal
return Decimal(str(x))
decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]
```
```
"hnl":{"name":"Honduran Lempira","unit":"L","value":null,"type":"fiat"}
```
Due to how the txid-commitment merkle tree used in the block headers is constructed, we need an extra check to be able to validate the *position* of a txid in a block.
I think this is low severity for us.
See https://bitcointalk.org/?topic=102395 :
> The Merkle hash implementation that Bitcoin uses to calculate the Merkle
> root in a block header is flawed in that one can easily construct multiple
> lists of hashes that map to the same Merkle root.
> For example, merkle_hash([a, b, c]) and merkle_hash([a, b, c, c]) yield
> the same result. This is because, at every iteration, the Merkle hash
> function pads its intermediate list of hashes with the last hash if the
> list is of odd length, in order to make it of even length.
>
> And so, the Merkle root function can be effectively preimaged by
> changing the input so that one of the intermediate lists is of even
> length with the last two elements equal (where originally it was
> of odd length with a last element equal to the earlier mentioned two).
> As was later noted, this extends to any input length that is
> not a power of two:
> merkle_hash([a, b, c, d, e, f]) == merkle_hash([a, b, c, d, e, f, e, f]).
> Note that to maintain the same root hash, the only flexibility that
> exists is duplication of elements.
Ported from https://github.com/Electron-Cash/Electron-Cash/commit/165146362b4cb0ad74770b36aca1f9acb2800195
Co-authored-by: bitcoincashautist <80100588+A60AB5450353F40E@users.noreply.github.com>
release.sh expects signed apks. if a non-releasemanager uses
release.sh to build it will build the apks unsigned and then
rename them to the same name as the signed apks. However
if the apks have already been built separately and are still named
*-unsigned.apk it will not detect them and instead try to build them
again. Instead it should just rename them to *-release.apk as if built
directly through release.sh.
Don't include first hop of the path,
this is the hop from us to the first node and we don't
need a payload for ourselves.
Also adds unittest checking this.
Factor out code from `send_onion_message_to` into a separate
function `_create_route_to_introduction_point` to make it
easier to reason about it and more testable.
This allows restricting blinded paths to channels that have sufficient receive
capacity for payment.
NOTE: this might have privacy issues, as this can be used to probe channel capacity.
Maybe randomize leeway?
@f321x: changed to use scid alias in create_blinded_path
- I noticed we were creating the RPC server unix domain socket with 0o775.
Instead of hunting down each individual line we create files/dirs, we should
just set a restrictive umask by default. We can still use chmod to relax this
for individual files. -- but we should try to be secure by default
- note: bitcoin core does the same
https://github.com/bitcoin/bitcoin/blame/2fe76ed8324af44c985b96455a05c3e8bec0a03e/src/common/system.cpp#L92
- the umask is set in run_electrum as opposed to __init__.py so that we don't side-effect use-cases where electrum is imported as a library
Adds a CI step to the Cirrus CI which will run claude code on the diff
of a Pull Request and fail if it finds critical security vulnerabilities
or serious code issues. Optinally it can be given a GitHub api key to
create a comment in the pull request.
Followup #10541.
Fixes tests.regtest.TestLightningSwapserver.test_swapserver_forceclose.
In the regtest bob would now signal trampoline support due to #10541 and
include Alice into the invoice trampoline as he is connected to Alice.
Alice would then try to add herself onto the trampoline route, causing
the payment to fail.
Check if self.network before trying to access it. This would trigger an
exception when toggling the trampoline checkbox in offline mode:
```
29.13 | E | gui.qt.exception_window.Exception_Hook | exception caught by crash reporter
Traceback (most recent call last):
File "/home/user/Documents/electrum/electrum/gui/qt/settings_dialog.py", line 133, in on_trampoline_checked
self.network.run_from_another_thread(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'run_from_another_thread'
31.00 | E | gui.qt.exception_window.Exception_Hook | exception caught by crash reporter
Traceback (most recent call last):
File "/home/user/Documents/electrum/electrum/gui/qt/settings_dialog.py", line 131, in on_trampoline_checked
self.network.start_gossip()
^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'start_gossip'
```
Even though the NIP-47 specification kind of defines that requests should
always pass a params dict in their request i witnessed way too often
that clients don't include it in some requests where it is technically
not neccessary and we fail on it.
Handling this gracefully improves compatibility without obvious
downsides.
f321x