Conflicts were mostly obvious, except for those in CreateTransactionInternal.
(This function was moved from wallet.cpp to spend.cpp, and substantially
rewritten, between 0.21 and 22.0.) For those I manually applied the changes
from the diff, which wound up taking a slightly different form.
Also had to update the new test because the `addresses` field of the RPC
output was removed.
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.
Changes API for AcceptToMemoryPool to no longer check maximum fee; we remove
this check from interfaces::CChain::testPeginClaimAcceptance. This is fine,
the check never should have been there (this method's job is to notice double
claim attempts and provide a more helpful message to the user; other mempool
failures should be caught later in the `claimrawpegin` logic when we call
`pwallet->commitTransaction` which actually subits to the mempool). It was
only checking fees because the API made it easier to do so than to not.
Also uncommented a bunch of PSBT functional tests (had to add a fee output
to one transaction, update `find_output` to skip CT outputs, and change two
constant checks at the end of the commented-out section).
-BEGIN VERIFY SCRIPT-
sed -i 's/Fee exceeds maximum configured by \-\maxtxfee/Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)/g' src/util/error.cpp test/functional/rpc_fundrawtransaction.py test/functional/rpc_psbt.py test/functional/rpc_rawtransaction.py test/functional/wallet_bumpfee.py test/functional/wallet_create_tx.py
-END VERIFY SCRIPT-
- Move the decision whether to translate an error message to where it is
defined. This simplifies call sites: no more `InitError(Untranslated(...))`.
- Make all functions in `util/error.h` consistently return a
`bilingual_str`. We've decided to use this as error message type so
let's roll with it.
This has no functional changes: no messages are changed, no new
translation messages are defined.
Translated strings should not end up in the debug log, stderr, or
returned by an RPC.
Changing the util methods in util/error to return a bilingual_str paves
the way to achieve this goal in the long term.
-BEGIN VERIFY SCRIPT-
sed -i 's/inline std::string _(const char\* psz)/inline bilingual_str _(const char\* psz)/' src/util/translation.h
sed -i 's/return G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz;/return bilingual_str{psz, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz};/' src/util/translation.h
sed -i 's/\b_("\([^"]\|\\"\)*")/&.translated/g' $(git grep --files-with-matches '\b_("' src)
echo Hard cases - multiline strings.
sed -i 's/"Visit %s for further information about the software.")/&.translated/g' src/init.cpp
sed -i "s/\"Only rebuild the block database if you are sure that your computer's date and time are correct\")/&.translated/g" src/init.cpp
sed -i 's/" restore from a backup.")/&.translated/g' src/wallet/db.cpp
sed -i 's/" or address book entries might be missing or incorrect.")/&.translated/g' src/wallet/wallet.cpp
echo Special case.
sed -i 's/_(COPYRIGHT_HOLDERS)/&.translated/' src/util/system.cpp test/lint/lint-format-strings.py
-END VERIFY SCRIPT-
FundTransaction calls GetMinimumFee which, when the fee rate is absurdly high, quietly reduced the fee to -maxtxfee. Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.
Adds the following util units and adds them to libbitcoin_util:
- `util/url.cpp` takes `urlDecode` from `httpserver.cpp`
- `util/error.cpp` takes `TransactionErrorString` from
`node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from
`ui_interface.cpp`
- `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp`
- `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp`
- 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
