Age | Commit message (Collapse) | Author |
|
Github-Pull: #26595
Rebased-From: aaf02b5721a8b5d3d9280dc3146fa5e44ea671b6
|
|
Since migration reloads the wallet, the wallet will always be locked
unless the passphrase is given. migratewallet can now take the
passphrase in order to unlock the wallet for migration.
Github-Pull: #26595
Rebased-From: 7fd125b27d48e410509f3009e2eb9fa5cd6729dd
|
|
Github-Pull: #27053
Rebased-From: 14b4921a91920df25b19ff420bfe2bff8c56f71e
|
|
To be eligible for fee-bumping, a transaction must not have any
of its outputs (eg - change) spent in other unconfirmed transactions
in the wallet. However, this check should not apply to abandoned
transactions.
A new test case is added to cover this case.
Github-Pull: #26675
Rebased-From: f9ce0eadf4eb58d1e2207c27fabe69a5642482e7
|
|
Github-Pull: #26761
Rebased-From: 730e14a317ae45fe871c8d6f44a51936756bbbea
|
|
This exercises the bug inside CoinsResult::Erase that
ends up on (1) a wallet crash or (2) a created and
broadcasted tx that contains a reduced recipient's amount.
This is covered by making the wallet selects the preset
inputs twice during the coin selection process.
Making the wallet think that the selection process result covers
the entire tx target when it does not. It's actually creating
a tx that sends more coins than what inputs are covering for.
Which, combined with the SFFO option, makes the wallet
incorrectly reduce the recipient's amount by the difference
between the original target and the wrongly counted inputs.
Which means, a created and relayed tx sending less coins to
the destination than what the user inputted.
Github-Pull: #26560
Rebased-From: cf793846978a8783c23b66ba6b4f3f30e83ff3eb
|
|
Github-Pull: #26569
Rebased-From: 8f2dac54096c20afd8fd12c21a2ee5465fea085e
|
|
Github-Pull: #26594
Rebased-From: 5e65a216d1fd00c447757736d4f2899d235e731a
|
|
Due to an oversight, we cannot currently migrate encrypted wallets,
regardless of whether they are unlocked. Migrating such wallets will
trigger an error, and result in the cleanup being run. This conveniently
allows us to check some parts of the cleanup code.
Github-Pull: #26594
Rebased-From: 88afc73ae0c67a4482ecd3d77eb2a8fd2673f82d
|
|
fad1c55301b9f2d091d3b0d8a75ff522ce8dae5a lint: Skip COMMIT_RANGE if no CIRRUS_PR (MarcoFalke)
Pull request description:
Identical commit from https://github.com/bitcoin/bitcoin/pull/26588
Untested, but this may fix the red-ness in https://cirrus-ci.com/github/bitcoin/bitcoin/24.x, e.g. https://cirrus-ci.com/task/4697153604419584:
Backport requested in https://github.com/bitcoin/bitcoin/pull/26588#issuecomment-1328916876
ACKs for top commit:
hebasto:
ACK fad1c55301b9f2d091d3b0d8a75ff522ce8dae5a, the same commit as in #26588.
Tree-SHA512: 8d8a735e25bc1f774f8cbf058b95b7019941138ab78fb7819852755c7a416a783ee116457b97031d447639002353d7bf12ee8445275b7b5eec4abc7421cc171d
|
|
|
|
Test that the same keys included in other descriptors will still be able
to sign a PSBT that requires those keys.
Github-Pull: #26418
Rebased-From: 0de30ed509a9969cb254e00097671625c9e107d2
|
|
To avoid a wallet potentially being able to sign a transaction using
keys from descriptors imported in previous tests, make new wallets for
each test case rather than sharing them.
Github-Pull: #26418
Rebased-From: 6efcdf6b7f6daa83b5937aa630fce358fdaed333
|
|
Github-Pull: #26349
Rebased-From: eb679a7896ce00e322972a011b023661766923b9
|
|
2147483647
This test would cause a crash in bitcoind (see #26274) if the fix given in the
previous commit was not applied.
Github-Pull: #26275
Rebased-From: 9153ff3e274953ea0d92d53ddab4c72deeace1b1
|
|
Github-Pull: #26344
Rebased-From: 315fd4dbabb6b631b755811742a3bdf93e1241bf
|
|
Github-Pull: #26344
Rebased-From: 708b72b7151c855cb5dac2fb6a81e8f35153c46f
|
|
Github-Pull: #25858
Rebased-From: 9e386afb67bf8fa71b72f730da1695eeb11828cd
|
|
Github-Pull: #25858
Rebased-From: 22c051ca70bae73e0430b05fb9d879591df27699
|
|
faeea28753a94c45618c1b0ba83bb8700c53009a test: Avoid race in disconnect_nodes helper (MacroFake)
Pull request description:
Backport of https://github.com/bitcoin/bitcoin/pull/26138
ACKs for top commit:
fanquake:
ACK faeea28753a94c45618c1b0ba83bb8700c53009a
Tree-SHA512: f967c38750220bd6c245db953055f8e6d5402b3a24081ca03795a8403c2ed4eab772b2e9c2d3b581c3bc55d191dd4e22711b5f97d39856d676f10799fc64a9c7
|
|
|
|
...because -i2psam or -cjdnsreachable are not provided.
This mimics existing behavior for -onlynet=onion and non-specified proxy.
|
|
667401a8557f94a3e0b7ec17096077dd8985eac7 [test] only run feature_rbf.py once (glozow)
Pull request description:
There is no need to run this test twice with --descriptors and --legacy-wallet, as it doesn't use the wallet.
ACKs for top commit:
aureleoules:
ACK 667401a8557f94a3e0b7ec17096077dd8985eac7.
theStack:
ACK 667401a8557f94a3e0b7ec17096077dd8985eac7
brunoerg:
ACK 667401a8557f94a3e0b7ec17096077dd8985eac7
Tree-SHA512: 339213159fac29ebc5678461fae41645aed57877d5525e8ca4755890b869a17ae0bea3f590114769c84b71a7df20c59c9530ab8b327912151c82ec58022f7e71
|
|
tx-size check
cc434cbf583ec8d1b0f3aa504417231fdc81f33a wallet: fix sendall creates tx that fails tx-size check (kouloumos)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/26011
The `sendall` RPC doesn't use `CreateTransactionInternal` as the rest of
the wallet RPCs. [This has already been discussed in the original PR](https://github.com/bitcoin/bitcoin/pull/24118#issuecomment-1029462114).
By not going through that path, it never checks the transaction's weight
against the maximum tx weight for transactions we're willing to relay.
https://github.com/bitcoin/bitcoin/blob/447f50e4aed9a8b1d80e1891cda85801aeb80b4e/src/wallet/spend.cpp#L1013-L1018
This PR adds a check for tx-size as well as test coverage for that case.
_Note: It seems that the test takes a bit of time on slower machines,
I'm not sure if dropping it might be for the better._
ACKs for top commit:
glozow:
re ACK cc434cb via range-diff. Changes were addressing https://github.com/bitcoin/bitcoin/pull/26024#discussion_r971325299 and https://github.com/bitcoin/bitcoin/pull/26024#discussion_r970651614.
achow101:
ACK cc434cbf583ec8d1b0f3aa504417231fdc81f33a
w0xlt:
reACK https://github.com/bitcoin/bitcoin/pull/26024/commits/cc434cbf583ec8d1b0f3aa504417231fdc81f33a
Tree-SHA512: 64a1d8f2c737b39f3ccee90689eda1dd9c1b65f11b2c7bc0ec8bfe72f0202ce90ab4033bb0ecfc6080af8c947575059588a00938fe48e7fd553f7fb5ee03b3cc
|
|
fa1ce96184a1815f453e64e14d77cb0025800be9 test: Add missing syncwithvalidationinterfacequeue (MacroFake)
faa4916529699f9a057e2bf2459d957bcec1de84 test/doc: Remove unused syncwithvalidationinterfacequeue (MacroFake)
Pull request description:
Fixes #26071
ACKs for top commit:
achow101:
ACK fa1ce96184a1815f453e64e14d77cb0025800be9
glozow:
ACK fa1ce96184a1815f453e64e14d77cb0025800be9
w0xlt:
ACK https://github.com/bitcoin/bitcoin/commit/fa1ce96184a1815f453e64e14d77cb0025800be9
Tree-SHA512: d1e101b55477360ead2b99ade5d42b922aabe293ec84fb26764e29161c5be6c534aef6f22d2cc5ea63a4bd6b6e77b701f1a7a2283b8e7e815d343a604cd77656
|
|
The `sendall` RPC doesn't use `CreateTransactionInternal`as the rest of
the wallet RPCs and it never checks against the tx-size mempool limit.
Add a check for tx-size as well as test coverage for that case.
|
|
|
|
6f8e3818af7585b961039bf0c768be2e4ee44e0f sendall: check if the maxtxfee has been exceeded (ishaanam)
Pull request description:
Previously the `sendall` RPC didn't check whether the fees of the transaction it creates exceed the set `maxtxfee`. This PR adds this check to `sendall` and a test case for it.
ACKs for top commit:
achow101:
ACK 6f8e3818af7585b961039bf0c768be2e4ee44e0f
Xekyo:
ACK 6f8e3818af7585b961039bf0c768be2e4ee44e0f
glozow:
Concept ACK 6f8e3818af7585b961039bf0c768be2e4ee44e0f. The high feerate is unlikely but sendall should respect the existing wallet options.
Tree-SHA512: 6ef0961937091293d49be16f17e4451cff3159d901c0c7c6e508883999dfe0c20ed4d7126bf74bfea8150d4c1eef961a45f0c28ef64562e6cb817fede2319f1a
|
|
unless 'inputs' are provided
b00fc44ca5cb938f18d31cde7feb4e1c968dcc2f test: add coverage for 'add_inputs' dynamic default value (furszy)
ddbcfdf3d050061f4e8979a0e2bb63bba5a43c47 RPC: bugfix, 'add_inputs' default value is true unless 'inputs' are provided (furszy)
Pull request description:
This bugfix was meant to be in #25685, but decoupled it to try to make it part of 24.0 release.
It's a truly misleading functionality.
This PR doesn't change behavior in any way. Just fixes two invalid RPC help messages and adds test
coverage for the current behavior.
#### Description
In both RPC commands `send()` and `walletcreatefundedpsbt` the help message says
that `add_inputs` default value is false when it's actually dynamically set by the following statement:
```c++
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
```
Which means that, by default, `add_inputs` is true unless there is any pre-set input, in which
case, the default is false.
ACKs for top commit:
achow101:
ACK b00fc44ca5cb938f18d31cde7feb4e1c968dcc2f
S3RK:
ACK b00fc44ca5cb938f18d31cde7feb4e1c968dcc2f
Tree-SHA512: 5c68a40d81c994e0ab6de0817db69c4d3dea3a9a64a60362531bf583b7a4c37d524b740905a3f3a89cdbf221913ff5b504746625adb8622788aea93a35bbcd40
|
|
Covered cases for send() and walletcreatefundedpsbt() RPC commands:
1. Default add_inputs value with no preset inputs (add_inputs=true):
Expect: automatically add coins from the wallet to the tx.
2. Default add_inputs value with preset inputs (add_inputs=false):
Expect: disallow automatic coin selection.
3. Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount).
Expect: include inputs from the wallet.
4. Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount).
Expect: only preset inputs are used.
5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set):
Expect: include inputs from the wallet.
|
|
See https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958562071
Also fix doc typo from https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958571943
|
|
|
|
00eeb31c7660e2c28f189f77a6905dee946ef408 scripted-diff: rename CChainState -> Chainstate (James O'Beirne)
Pull request description:
Alright alright alright, I know: we hate refactors. We especially hate cosmetic refactors.
Nobody knows better than I that changing broad swaths of code out from under our already-abused collaborators, only to send a cascade of rebase bankruptcies, is annoying at best and sadistic at worst. And for a rename! The indignation!
But just for a second, imagine yourself. Programming `bitcoin/bitcoin`, on a sandy beach beneath a lapis lazuli sky. You go to type the name of what is probably the most commonly used data structure in the codebase, and you *only hit shift once*.
What could you do in such a world? You could do anything. [The only limit is yourself.](https://zombo.com/)
---
So maybe you like the idea of this patch but really don't want to deal with rebasing. You're in luck!
Here're the commands that will bail you out of rebase bankruptcy:
```sh
git rebase -i $(git merge-base HEAD master) \
-x 'sed -i "s/CChainState/Chainstate/g" $(git ls-files | grep -E ".*\.(py|cpp|h)$") && git commit --amend --no-edit'
# <commit changed?>
git add -u && git rebase --continue
```
---
~~Anyway I'm not sure how serious I am about this, but I figured it was worth proposing.~~ I have decided I am very serious about this.
Maybe we can have nice things every once in a while?
ACKs for top commit:
MarcoFalke:
cr ACK 00eeb31c7660e2c28f189f77a6905dee946ef408
hebasto:
ACK 00eeb31c7660e2c28f189f77a6905dee946ef408
glozow:
ACK 00eeb31c7660e2c28f189f77a6905dee946ef408, thanks for being the one to propose this
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/24513/commits/00eeb31c7660e2c28f189f77a6905dee946ef408
Tree-SHA512: b828a99780614a9b74f7a9c347ce0687de6f8d75232840f5ffc26e02bbb25a3b1f5f9deabbe44f82ada01459586ee8452a3ee2da05d1b3c48558c8df6f49e1b1
|
|
-listenonion=1
2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 init: allow startup with -onlynet=onion -listenonion=1 (Vasil Dimov)
Pull request description:
It does not make sense to specify `-onlynet=onion` without providing a
Tor proxy (even if other `-onlynet=...` are given). This is checked
during startup. However, it was forgotten that a Tor proxy can also be
retrieved from "Tor control" to which we connect if `-listenonion=1`.
So, the full Tor proxy retrieval logic is:
1. get it from `-onion`
2. get it from `-proxy`
3. if `-listenonion=1`, then connect to "Tor control" and get the proxy
from there (was forgotten before this change)
Fixes https://github.com/bitcoin/bitcoin/issues/24980
ACKs for top commit:
mzumsande:
Tested ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0
MarcoFalke:
ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 🕸
Tree-SHA512: d1d18e07a8a40a47b7f00c31cb291a3d3a9b24eeb28c5e4720d5df4997f488583a3a010d46902b4b600d2ed1136a368e1051c133847ae165e0748b8167603dc3
|
|
We were throwing two different errors for the same problematic:
* "Expected type {expected], got {type}" --> RPCTypeCheckArgument()
* "JSON value of type {type} is not of expected type {expected}" --> UniValue::checkType()
|
|
an unknown block
4f67336f1105b7c34a9e8cdafa603edc1d899fb9 test: verify best blockhash after invalidating an unknown block (brunoerg)
Pull request description:
Fixes #26051
Verify the best blockhash is the same after invalidating an unknown block, not the whole `getchaintip` response.
ACKs for top commit:
instagibbs:
ACK 4f67336f1105b7c34a9e8cdafa603edc1d899fb9
Tree-SHA512: 2d71743c1d3a317ef7b750f88437df71d1aed2728d9edac8b763a343406e168b97865ab25ec4c89caf09d002e076458376618cbd0845496375f7179633c88af9
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-
Co-authored-by: MacroFake <falke.marco@gmail.com>
|
|
dynamic fees in wallet_groups.py
2186608172b0e6c3a9907e06fed27dfd927abd45 test: apply fixed feerate to avoid variable dynamic fees (stickies-v)
Pull request description:
Without specifying a feerate, we let the wallet decide on an appropriate feerate, which can be influenced by various factors
such as what's in the mempool. Since wallet_groups.py fails when feerates are unstable, we should use a fixed feerate across all nodes. The assumed feerate was 20 sats/vbyte, so this PR adopts that.
Closes #25940. I'm not 100% sure, but I think the increased tx relay speed introduced by #25865 caused the transactions to more quickly and often enter the other nodes' mempools, affecting their feerate calculation done in [`wallet:GetMinimumFeeRate()`](https://github.com/bitcoin/bitcoin/blob/ea67232cdb80c4bc3f16fcd823f6f811fd8903e1/src/wallet/fees.cpp#L68-L72) and thus deviating slightly from the expected 20 sats/vbyte.
Ran `wallet_groups.py` over 400 times without failure.
ACKs for top commit:
aureleoules:
ACK 2186608172b0e6c3a9907e06fed27dfd927abd45.
glozow:
Approach ACK 2186608172b0e6c3a9907e06fed27dfd927abd45
Tree-SHA512: 0ea467a67747e6f27369ccd0adacfb21cc36ef0ae728fb28b8ea18e409aab5bd3ede559d6cebb82da0b9703c0c8b2709d686feb3ae009ddf525aa253f44d5816
|
|
07b6e743147f410d8ceb4b6b3282e9fba4cafe50 test: Display skipped tests reason (Aurèle Oulès)
Pull request description:
Attempt to fix #26023.
ACKs for top commit:
brunoerg:
ACK 07b6e743147f410d8ceb4b6b3282e9fba4cafe50
Tree-SHA512: 5d8f7fbd8d65772000a5da8c01276948b157d93d359203c6442cf2681cdcc2426b1fee7ec62cee100019c59a486a96ad98d5e819bffe1fd37624dcd28f42aed2
|
|
4b1d5a10537ab48e3457606ba1cf2ae26a1cb2b2 test: invalidating an unknown block throws an error (brunoerg)
Pull request description:
While playing with `invalidateblock`, I unintentionally tried to invalidate an unknown block and it threw an error. Looking at the tests I just realized there is no test coverage for this case. This PR adds it.
Top commit has no ACKs.
Tree-SHA512: 25286ead809b3ad022e759127ef3134b271fbe76cb7b50ec2b0c7e2409da8d1b01dc5e80afe73e4564cc9c9c03487a1fe772aea3456988552d2f9c8fb34c730b
|
|
|
|
There is no need to run this test twice with --descriptors and
--legacy-wallet, as it doesn't ever use the wallet.
|
|
|
|
for BDB-only wallets
9f3a315c6f6af37d562a72f7d4fb2c53b5940871 test: Fix `wallet_listsinceblock.py` for BDB-only wallets (Hennadii Stepanov)
1941ce6cd15a1123db8769d17a6fd9b1cb3debcc test: Fix `wallet_basic.py` for BDB-only wallets (Hennadii Stepanov)
Pull request description:
Fixes bitcoin/bitcoin#26029.
ACKs for top commit:
brunoerg:
crACK 9f3a315c6f6af37d562a72f7d4fb2c53b5940871
Tree-SHA512: d31c76e558dedea689ff487644e9f2d2f1df1cc2bb9bb041ede4b272884871167fdb19ccc717394c6ba6af8b8c70e9575b344988e0ce55b241a3a4922d0b7f73
|
|
|
|
|
|
disables IPv4 and IPv6
385f5a4c3feb716fcf3f2b4823535df6da6bb67b p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachable (Martin Zumsande)
91f0a7fbb79fe81a75370a4b60dcdd2e55edfa81 p2p: add only reachable addresses to addrman (Martin Zumsande)
Pull request description:
Currently, `-onlynet` does not work well in connection with initial peer discovery, because DNS seeds only resolve to IPv6 and IPv4 adresses:
With `-onlynet=i2p`, we would load clearnet addresses from DNS seeds into addrman, be content our addrman isn't empty so we don't try to query hardcoded seeds (although these exist for i2p!), and never attempt to make an automatic outbound connection.
With `-onlynet=onion` and `-proxy` set, we wouldn't load addresses via DNS, but will make AddrFetch connections (through a tor exit node) to a random clearnet peer the DNS seed resolves to (see https://github.com/bitcoin/bitcoin/issues/6808#issuecomment-147652505), thus breaching the `-onlynet` preference of the user - this has been reported in the two issues listed below.
This PR proposes two changes:
1.) Don't load addresses that are unreachable (so that we wouldn't connect to them) into addrman. This is already the case for addresses received via p2p addr messages, this PR implements the same for addresses received from DNS seeds and fixed seeds. This means that in the case of `-onlynet=onion`, we wouldn't load fixed seed IPv4 addresses into addrman, only the onion ones.
2.) Skip trying the DNS seeds if neither IPv4 nor IPv6 are reachable and move directly to adding the hardcoded seeds from networks we can connect to. This is done by soft-setting `-dnsseed` to 0 in this case, unless `-dnsseed=1` was explicitly specified, in which case we abort with an `InitError`.
Fixes #6808
Fixes #12344
ACKs for top commit:
naumenkogs:
utACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b
vasild:
ACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b
Tree-SHA512: 33a8c29faccb2d9b937b017dba4ef72c10e05e458ccf258f1aed3893bcc37c2e984ec8de998d2ecfa54282abbf44a132e97d98bbcc24a0dcf1871566016a9b91
|
|
This happens, for example, if the user specified -onlynet=onion or
-onlynet=i2p. DNS seeds only resolve to IPv4 / IPv6 addresses,
making their answers useless to us, since we don't want to make
connections to these.
If, within the DNS seed thread, we'd instead do fallback AddrFetch
connections to one of the clearnet addresses the DNS seed resolves to,
we might get usable addresses from other networks
if lucky, but would be violating our -onlynet user preference
in doing so.
Therefore, in this case it is better to rely on fixed seeds for networks we
want to connect to.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
|
|
It does not make sense to specify `-onlynet=onion` without providing a
Tor proxy (even if other `-onlynet=...` are given). This is checked
during startup. However, it was forgotten that a Tor proxy can also be
retrieved from "Tor control" to which we connect if `-listenonion=1`.
So, the full Tor proxy retrieval logic is:
1. get it from `-onion`
2. get it from `-proxy`
3. if `-listenonion=1`, then connect to "Tor control" and get the proxy
from there (was forgotten before this change)
Fixes https://github.com/bitcoin/bitcoin/issues/24980
|