The `CWallet::AddToWallet` method had completely divergent
behavior depending upon the value of the fFromLoadWallet
flag, and `pwalletdb` was unused when this flag was set
to `true`, so this is better represented as two distinct
methods on CWallet.
The 13.0.1-1 MSYS2 binaries cause linker errors due to missing `new` and
`delete` symbols. This commit partially reverts the LLVM 13.0.1 upgrade:
Windows cross-compilation still uses `clang 13.0.1`, but is compiled
against `libc++ 13.0.0`.
A prior commit moved the `IsValidMinerAddress()` call before the keypool
exhaustion check, but `IsValidMinerAddress()` internally handles keypool
exhaustion. We now check it explicitly first to retain the prior error
behaviour (as checked in the `keypool` RPC test).
`orchard::Builder` pads to two Actions, but does so using a "no OVK"
policy for dummy outputs, which violates coinbase rules requiring all
shielded outputs to be recoverable. We manually add a dummy output to
sidestep this issue.
The initial value for the provided return value is now `std::nullopt`
instead of the default value of the `MinerAddress` variant. This makes
it easier to distinguish "no miner address was provided" from "invalid
miner address".
By forgetting to pop the tree, we were leaving the view in an
inconsistent state, where it believed the best Orchard anchor was for a
tree that didn't correspond to the rest of the view. This would then
propagate in subsequent block connections, and the chain history
commitments to Orchard tree roots eventually result in inconsistent
`blockcommitments` values in subsequent blocks.
`ConnectBlock` was already checking that the given view's "best block"
was the previous block. However, it then assumed the view was correct on
its claimed best anchors.
For Orchard, we know that `hashFinalOrchardRoot` field of `CBlockIndex`
will only ever be set when a block is (attempted to be) connected to the
main chain, and so we can instead add assertions around its value and
ensure the view is consistent with the previous block.
This allows us to use --gtest_repeat= on the WalletZkeysTest group,
otherwise, each test will reuse its previous iteration's wallet file,
causing assertions to be invalidated.
We put the RemoveDb at the beginning of each test because putting it
at the end of the test means it doesn't get run if the test fails.
This would lead to a single failed test causing every instance of that
test thereafter to fail (since it uses the same temporary directory).
The dummyWallet.LoadWallet() ensures that CDB is initialized, otherwise
bitdb.RemoveDb causes the error:
"BDB1565 DB_ENV->dbremove: method not permitted before handle's open
method"