Conversation
We call it once at the end, but calling on each allocation is excessive, and it shows when dealing with large PSBTS. Testing a 700-input PSBT was unusably slow without this: after this the entire test ran in 9 seconds. Changelog-Fixed: JSON-RPC: Dealing with giant PSBTs (700 inputs!) is now much faster. Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/giant-tx-fix
branch
2 times, most recently
from
6c0eb3e to
1c84919
Compare
tests/test_plugin.py
Outdated
| """Test that a giant tx doesn't crash bcli""" | ||
| l1 = node_factory.get_node(start=False) | ||
| # With memleak we spend far too much time gathering backtraces. | ||
| del l1.daemon.env["LIGHTNINGD_DEV_MEMLEAK"] |
There was a problem hiding this comment.
I think this line is causing your test to fail. It seems like the LIGHTNINGD_DEV_MEMLEAK key does not exist under valgrind? You can use .pop() instead of del, that way the key will be removed only if it exists.
There was a problem hiding this comment.
I have no idea why the liquid test fails like this though:
FAILED tests/test_plugin.py::test_bitcoin_backend_gianttx - pyln.client.lightning.RpcError: RPC call failed: method: withdraw, payload: {'destination': 'el1qqwawntx90mlylxle6q3vwjcm9r5fc7te6nf58gq469ejxym6fuv5sl6fhxnuat264hf86vmh6h0qupv99flgsy8hhudn3t32f', 'satoshi': 'all'}, error: {'code': -1, 'message': 'Could not parse destination address, destination should be a valid address'}
It seems like l1 is a liquid node, and we're trying to withdraw to a correct liquid address?
There was a problem hiding this comment.
I think this line is causing your test to fail. It seems like the LIGHTNINGD_DEV_MEMLEAK key does not exist under valgrind? You can use .pop() instead of del, that way the key will be removed only if it exists.
pop still raises on missing (according to the docs), so I just tested before delete.
There was a problem hiding this comment.
I have no idea why the liquid test fails like this though:
FAILED tests/test_plugin.py::test_bitcoin_backend_gianttx - pyln.client.lightning.RpcError: RPC call failed: method: withdraw, payload: {'destination': 'el1qqwawntx90mlylxle6q3vwjcm9r5fc7te6nf58gq469ejxym6fuv5sl6fhxnuat264hf86vmh6h0qupv99flgsy8hhudn3t32f', 'satoshi': 'all'}, error: {'code': -1, 'message': 'Could not parse destination address, destination should be a valid address'}It seems like l1 is a liquid node, and we're trying to withdraw to a correct liquid address?
Ah, we have a getnewaddress() wrapper which gives us an unconfidential address: need to use that for elements!
There was a problem hiding this comment.
I think this line is causing your test to fail. It seems like the LIGHTNINGD_DEV_MEMLEAK key does not exist under valgrind? You can use .pop() instead of del, that way the key will be removed only if it exists.
pop still raises on missing (according to the docs), so I just tested before delete.
Sorry, I probably should have elaborated on what I meant when I said to use .pop(), you can use .pop() with a default argument which is returned when the key isn't found in the dictionary. So you can call it like this:
l1.daemon.env.pop("LIGHTNINGD_DEV_MEMLEAK", None)
But a test works too!
Signed-off-by: Rusty Russell <[email protected]>
``` lightningd-1 2025-10-27T11:26:04.285Z **BROKEN** plugin-bcli: bitcoin-cli exec failed: Argument list too long ``` Use -stdin to bitcoin-cli: we can then handle arguments of arbitrary length. Fixes: ElementsProject#8634 Changelog-Fixed: plugins: `bcli` would fail with "Argument list too long" when sending a giant tx.
rustyrussell
force-pushed
the
guilt/giant-tx-fix
branch
from
1c84919 to
7ee7407
Compare
3 participants
Fixes: #8634