This removes a lot of the craziness that was added during the 0.21
rebase 240f03f586 (Bitcoin PR #17290)
as well as a lot of other upstream craziness. It's a very positive
change but will probably be a bear to review.
There are two places where I (purposely) nontrivially changed the
logic beyond what the upstream PR did:
1. When setting the change output positions, I used a uniform
distribution for all change outputs and also obeyed the user's
`change_position` choice. The old code was non-uniform and
had a bug where it would offset the change position.
For non-policy assets, I do not create zero-sized change
since we would just remove them later. For the policy asset,
I do sometimes create zero-sized change since the upstream
logic expects it to exist.
2. Rather than "reblinding whenever something changes" we blind
once for size/fee estimation purposes and then again at the
end after all potential adjustments have been made. The
resulting code should be closer to upstream.
We should really redo this code though because our use of the
`BlindDetails` structure is very confusing and stateful.
3. We consider whether or not we're using CT/CA when estimating
the size of change outpust. This is because we now use branch
and bound far more often (yay!) which selects coins such that
there will be no change output. This is implemented however
by adding a change output then dumping it to fees ... meaning
that if we dramatically overestimate the size of a change out
then we'll end up dramatically dumping too many coins to fees.
There are another couple small things, which I apologize for .. this
PR took me 2 hours to get compiling and 11 hours to get working(!!)
and some things got away from me.
One fun thing is that I changed CWallet::SelectCoinsMinConf to gate
an addition on coin_selection_params.m_subtract_fee_outputs ... this
is actually an upstream bug that I'll file whenever I get around to
producing a test that triggers it. (A bit hard as I have to gin up
a scenario where branch-and-bound fails to hit this codepath.)
Also, in test/functional/wallet_bumpfee.py I had to change the feerate
required to force selection to add another input from 500 to 800. I
don't know why and I don't care to figure it out.
I hacked up rpc_fundrawtransaction.py a little bit because our `get_address`
method here depends on nobody having called `createwallet` on a particular
node ... the "correct" fix for this would be to redo the Elements changes
to this functional test for the 0.20+ createwallet changes, but because
there will be big future changes to this test (in Elements #900 (PSET))
I did the quick/hacky thing, and will defer cleanups to a separate
post-rebase PR.
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
