mirror of
https://github.com/ElementsProject/elements.git
synced 2026-08-16 13:01:19 +02:00
578ee6183f
There are a few layers of bullshit to this PR. First, there is the fact that it adds a functional test gated on a new config flag which is disabled by default, so it actually adds broken code with no tests, waiting to ruin your day 520 PRs later when #21935 enables the broken test. Second, the test appears to be superficially nonsensical because it generates two transactions from different wallets and tries to compare them for byte-for-byte equality, which doesn't make sense (at least) because change outputs are randomly located...so something fishy is going on. Of course, in Elements the transactions are *not* equal half the time because the outputs are permuted, which may have let me quickly figure out the issue, except... Third, there is a red herring of a bug where the two transactions have slightly different feerates. This turns out to be caused by CWallet::CalculateMaximumSignedTxSize using differently sized dummy transactions depending on whether watchonly outputs are included (this fact is conveniently disguised by #17211 slightly changing this logic; this is an unmerged PR in Core that Elements has a backport of an old version of). And the two wallets have different watchonly settings. A sub-red-herring is the fact that this bug results in a discrepancy of 0.25 vbytes, so it does not appear in Core but does appear in Elements (there is a 3/16 probability that we should be so unlucky... we are). But this is all irrelevant, because... Fourth, this test is actually super bullshit. The way it works is by constructing a PSBT legitimately, saving this to disk, then re-"signs" using the external signer interface by using a mock signer that COMPLETELY REPLACES THE TRANSACTION UNDER CONSTRUCTION. So it doesn't matter what the fee output looks like and it doesn't matter what the order of the outputs. Core does not detect this malfeasance and neither does Elements. For some reason, Core has a functional test that explicitly checks that you can do this even though it is insane and it is hard to think of non-malicious reasons to do it. Fifth, while Elements fails to detect that its external signer is actually changing the transaction out from under it, it DOES assume that this won't happen. In CWallet::SignPSBT it blithely un-replaces the transaction, which undermines the functional test. Sixth, the original PR where this test was introduced has comments locked, so anyone who spent six hours reverse-engineering this idiotic broken test, and is still feeling charitable enough to discuss it with the Core developors, can go pound sand. Anyway, just disabled the broken test and move on with our lives. |
||
|---|---|---|
| .. | ||
| __init__.py | ||
| address.py | ||
| authproxy.py | ||
| bdb.py | ||
| bip340_test_vectors.csv | ||
| blocktools.py | ||
| coverage.py | ||
| descriptors.py | ||
| key.py | ||
| liquid_addr.py | ||
| messages.py | ||
| muhash.py | ||
| netutil.py | ||
| p2p.py | ||
| script.py | ||
| script_util.py | ||
| segwit_addr.py | ||
| siphash.py | ||
| socks5.py | ||
| test_framework.py | ||
| test_node.py | ||
| test_shell.py | ||
| util.py | ||
| wallet.py | ||
| wallet_util.py | ||