Since we no longer delete the wallet directory, there's no need to vacate it
The moving only served to risk errors by crossing filesystem boundaries (which fs::rename can't handle)
If any other files exist in the directory, we cannot assume the sharable files are exclusively for this wallet.
But if they are, this also cleans up other log.* files
b834447fb2f2073e25164a80ba197a3120610b92 [doc] generate manpages 29.3rc1 (glozow)
e9c978391ff74e99724fbda9fb50f0c45fb13008 [build] bump version to 29.3rc1 (glozow)
e973b61dbb431141e23846d95a86221b01587900 [doc] update release notes for 29.3rc1 (glozow)
f4b78c42e557aec29f5ed5e570fb55bf70d2b3b4 test: Add a test for anchor outputs in the wallet (Ava Chow)
c6e7765c0a03c124fcc86b452d6870b6d2797130 wallet: Throw an error in sendall if the tx size cannot be calculated (Ava Chow)
bab1ac827b4fdd4984661f32f6b899d56261da5d wallet: Determine IsFromMe by checking for TXOs of inputs (Ava Chow)
71633a9b5c10f0d6a1a1e31bcbf51de2e27649d9 test: Test wallet 'from me' status change (Ava Chow)
daef5852f02513521654e15d62748648765acf92 guix: Fix `osslsigncode` tests (Hennadii Stepanov)
7a71850a6d1d2eaf09e19d9d0af574a90487ec2b Remove unreliable seed from chainparams.cpp, and the associated README (SatsAndSports)
2e4688618ba6a68df0936df0cc86b657ec35b4ef miner: fix `addPackageTxs` unsigned integer overflow (ismaelsadeeq)
Pull request description:
Backports:
- #34227
- #33723
- #33475
- #33268
And final changes for 29.3rc1
ACKs for top commit:
achow101:
ACK b834447fb2f2073e25164a80ba197a3120610b92
janb84:
ACK b834447fb2f2073e25164a80ba197a3120610b92
sedited:
ACK b834447fb2f2073e25164a80ba197a3120610b92
Tree-SHA512: 68e02fbde7162f728229f4bfc803bedda6d269e54593ebe40da607f6bd25b2b10bc4297bfa0bc977ce2dc6b558efe6571a7f875090e0f916fc09e5b67432ba30
Instead of checking whether the total amount of inputs known by the
wallet is greater than 0, we should be checking for whether the input is
known by the wallet. This enables us to determine whether a transaction
spends an of output with an amount of 0, which is necessary for marking
0-value dust outputs as spent.
Github-Pull: #33268
Rebased-From: 39a7dbdd277d1dea9a70314d8cc5ae057999ee88
If something is imported into the wallet, it can change the 'from me'
status of a transaction. This status is only visible through
gettransaction's "fee" field which is only shown for transactions that
are 'from me'.
Github-Pull: #33268
Rebased-From: e76c2f7a4111f87080e31539f83c21390fcd8f3b
76cdeb7b06232050c7d20ffa1395697cc4e53295 wallet: test: Failed migration cleanup (David Gumberg)
9405e915e79d86d262779ea38104624d37add2a3 test: coverage for migration failure when last sync is beyond prune height (furszy)
5e8ad98163af9749e7a3c44a9107cc241c5bd7ab wallet: migration, fix watch-only and solvables wallets names (furszy)
a7e2d106db8f193259420bacbccec80ba3beebf1 wallet: improve post-migration logging (furszy)
9ea84c08d7e24ffefa6f18d6bd2af28ec38cfd98 test: restorewallet, coverage for existing dirs, unnamed wallet and prune failure (furszy)
833848e9b8eab430629da116f753f8d4433f51e2 test: add coverage for unnamed wallet migration failure (furszy)
a074d36254ab0c666f0438fe071cd213715f82de wallet: fix unnamed wallet migration failure (furszy)
d91f56e1e3f1aee99b0c09c23db70622ad6ed1b4 wallet: RestoreWallet failure, erase only what was created (furszy)
cc324aa2bed30afa713625dfb9cf83c438dd15c1 wallettool: do not use fs::remove_all in createfromdump cleanup (Ava Chow)
01c04d32aa3e1c323b304b1c6a573dd933b8b598 wallet: introduce method to return all db created files (furszy)
abaf1e37a79bdf7481cef1cd5ae5e102fdce09be refactor: remove sqlite dir path back-and-forth conversion (furszy)
Pull request description:
Backports:
* #34215
* #34156
* #34226
* 2 required commits from #31423
Note that this backport is unclean and several changes have to be made to most commits to accommodate BDB and the differences in migration cleanup behavior.
ACKs for top commit:
furszy:
Code review ACK 76cdeb7b06232050c7d20ffa1395697cc4e53295
brunoerg:
light code review ACK 76cdeb7b06232050c7d20ffa1395697cc4e53295 + backported the functional tests without the fixes and all of them failed accordingly.
glozow:
light review ACK 76cdeb7b06232050c7d20ffa1395697cc4e53295.
Tree-SHA512: 432268117783fc9a221d895a6f6601b6a2a5031c76d1443cf804cc1d486b40fcded982409d548acd1c01a13c7b378b840fcc3fbe823d6ba5ffc4ebe017d4e13c
Refactor a common way to perform the failed migration test that exists
for default wallets, and add relative-path wallets and absolute-path
wallets.
Github-Pull: 34226
Rebased-From: eeaf28dbe0e09819ab0e95bb7762b29536bdeef6
Because the default wallet has no name, the watch-only and solvables
wallets created during migration end up having no name either.
This fixes it by applying the same prefix name we use for the backup
file for an unnamed default wallet.
Before: watch-only wallet named "_watchonly"
After: watch-only wallet named "default_wallet_watchonly"
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: 82caa8193a3e36f248dcc949e0cd41def191efac
Right now, after migration the last message users see is "migration completed",
but the migration isn't actually finished yet. We still need to load the new wallets
to ensure consistency, and if that fails, the migration will be rolled back. This
can be confusing for users.
This change logs the post-migration loading step and if a wallet fails to load and
the migration will be rolled back.
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: d70b159c42008ac3b63d1c43d99d4f1316d2f1ef
The first test verifies that restoring into an existing empty directory
or a directory with no .dat db files succeeds, while restoring into a
dir with a .dat file fails.
The second test covers restoring into the default unnamed wallet
(wallet.dat), which also implicitly exercises the recovery path used
after a failed migration.
The third test covers failure during restore on a prune node. When
the wallet last sync was beyond the pruning height.
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: f011e0f0680a8c39988ae57dae57eb86e92dd449
Verifies that a failed migration of the unnamed (default) wallet
does not erase the main /wallets/ directory, and also that the
backup file exists.
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: 36093bde63286e19821a9e62cdff1712b6245dc7
When migrating any legacy unnamed wallet, a failed migration would
cause the cleanup logic to remove its parent directory. Since this
type of legacy wallet lives directly in the main '/wallets/' folder,
this resulted in unintentionally erasing all wallets, including the
backup file.
To be fully safe, we will no longer call `fs::remove_all`. Instead,
we only erase the individual db files we have created, leaving
everything else intact. The created wallets parent directories are
erased only if they are empty.
As part of this last change, `RestoreWallet` was modified to allow
an existing directory as the destination, since we no longer remove
the original wallet directory (we only remove the files we created
inside it). This also fixes the restore of top-level default wallets
during failures, which were failing due to the directory existence
check that always returns true for the /wallets/ directory.
This bug started after:
f6ee59b6e2
Previously, the `fs::copy_file` call was failing for top-level wallets,
which prevented the `fs::remove_all` call from being reached.
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: f4c7e28e80bf9af50b03a770b641fd309a801589
Track what RestoreWallet creates so only those files and directories
are removed during a failure and nothing else. Preexisting paths
must be left untouched.
Note:
Using fs::remove_all() instead of fs::remove() in RestoreWallet does
not cause any problems currently, but the change is necessary for the
next commit which extends RestoreWallet to work with existing directories,
which may contain files that must not be deleted.
Github-Pull: bitcoin/bitcoin#34156
Rebased-From: 4ed0693a3f2a427ef9e7ad016930ec29fa244995
2cf352fd8e6a77003e38d954b6c879b20d4b960a doc: document capnproto and libmultiprocess deps (will)
Pull request description:
Closes: #33576
These dependencies are both undocumented, and libmultiprocess has a relatively special requirement in that v6.0 and later are known to not work with v29.x of Bitcoin Core due to https://github.com/bitcoin-core/libmultiprocess/pull/160
ACKs for top commit:
ryanofsky:
Code review ACK 2cf352fd8e6a77003e38d954b6c879b20d4b960a. Thanks for making all these changes and for opening the fix originally.
Tree-SHA512: 3839bb7b0bbb23c3fe358960e93f7919953cac315eaed2b214491dd4f6c80ed500c09a618d11408836bddf6d11fc98b1ca3989f552104c49713ebd8859706ac5
These dependencies are both undocumented, and libmultiprocess has a
relatively special requirement in that v6.0 and later are known to not
work with v29.x of Bitcoin Core due to https://github.com/bitcoin-core/libmultiprocess/pull/160
Previously, we would check failing input scripts twice when considering
a transaction for the mempool, in order to distinguish policy failures
from consensus failures. This allowed us both to provide a different
error message and to discourage peers for consensus failures. Because we
are no longer discouraging peers for consensus failures during tx relay,
and because checking a script can be expensive, only do this once.
Also renames non-mandatory-script-verify-flag error to
mempool-script-verify-flag-failed.
NOTE: Backport required additional adjustment in test/functional/feature_block
Github-Pull: #33050
Rebased-From: b29ae9efdfeeff774e32ee433ce67d8ed8ecd49f
Do not discourage nodes even when they send us consensus invalid
transactions.
Because we do not discourage nodes for transactions we consider
non-standard, we don't get any DoS protection from this check in
adversarial scenarios, so remove the check entirely both to simplify the
code and reduce the risk of splitting the network due to changes in tx
relay policy.
NOTE: Backport required additional adjustment in test/functional/p2p_invalid_tx
Github-Pull: #33050
Rebased-From: 266dd0e10d08c0bfde63205db15d6c210a021b90
Since it was introduced in 4eb515574e1012bc8ea5dafc3042dcdf4c766f26 (#18044), the detection of a
stripped witness relies on running the Script checks 3 times. In the worst case, this consists in
running Script validation 3 times for every single input.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's
wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with
txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so
doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot,
P2WPKH, P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus).
For undefined program types, Script validation is always going to fail regardless of the witness (by
standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation.
However this might lead to more "false positives" (cases where we return witness stripping for an
otherwise invalid transaction) than the existing implementation. For instance a transaction with one
P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing
implementation would treat it as consensus invalid while the implementation in this commit would
always consider it witness stripped.
Github-Pull: #33105
Rebased-From: 27aefac42505e9c083fa131d3d7edbec7803f3c0
We will use this helper in later commits to detect witness stripping without having
to execute every input Script three times in a row.
Github-Pull: #33105
Rebased-From: 2907b58834ab011f7dd0c42d323e440abd227c25
A stripped witness is detected as a special case in mempool acceptance to make sure we do not add
the wtxid (which is =txid since witness is stripped) to the reject filter. This is because it may
interfere with 1p1c parent relay which currently uses orphan reconciliation (and originally it was
until wtxid-relay was widely adopted on the network.
This commit adds a test for this special case in the p2p_segwit function test, both when spending
a native segwit output and when spending a p2sh-wrapped segwit output.
Thanks to Eugene Siegel for pointing out the p2sh-wrapped detection did not have test coverage by
finding a bug in a related patch of mine.
Github-Pull: #33105
Rebased-From: eb073209db9efdbc2c94bc1f535a27ec6b20d954
When using `docker buildx build` in conjunction with the `gha` backend
cache type, it's important to specify the URL and TOKEN needed to
authenticate.
On Cirrus runners this is working with only `ACTIONS_CACHE_URL` and
`ACTIONS_RUNTIME_TOKEN`, but this is not enough for the GitHub backend.
Fix this by exporting all `ACTIONS_*` variables.
This fixes cache restore/save on forks or where GH-hosted runners are
being used.
Github-Pull: #33508
Rebased-From: bc706955d740f8a59bec78e44d33e80d1cca373b
$FILE_ENV has a full relative path already, prepending with ci/test/
results in a non-existent path which means that DEPENDS_HASH was not
actually committing to the test's environment file.
Github-Pull: #33581
Rebased-From: ceeb53adcd0a6a87a65c8ebbb20472c15c502dfd
