What an awful PR. This introduces Python-like runtime failures into the
RPC interface in case there are type mismatches between the documentation
and every possible return value from every RPC call. I fixed all the errors
that are triggered by the existing tests but there will be a long tail of
errors to come, I'm sure.
This PR adds a new functional test which unfortunately we need to disable
because it triggers an unrelated Elements bug (assertation failure when
attempting a large fundrawtransaction).
See https://github.com/ElementsProject/elements/issues/880
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.
This is a really non-trivial wallet refactor "effective value" which has been
in the Core queue for a pretty long time. There were a ton of merge conflicts
but they were mostly mechanical (all the benchmarks) or "take the Core side
of the diff" (most of wallet.cpp).
The two nontrivial things were:
* In `SelectCoinsMinConf` in wallet.cpp I had to slightly move the asset
filtering logic (but I didn't change it)
* In `OutputGroup::Insert' in coinselection.cpp I changed the effective
value computation to only consider policyAsset values
Made some edits to rpc/client.cpp on the advice of test/functional/rpc_help.py
to make the "RPC conversion tables" consistent, but I have no idea what these
tables are or what they're for. I assume, given that nobody has noticed these
inconsistencies ever, that they're fine.
Had to change a test vector to match our test framework's different deterministic
keys (when I produced these during the 0.21 rebase I made new random keys rather
than converting the existing ones, it appears).
