| Age | Commit message (Collapse) | Author |
|---|
|
wallet flag should be un/set
cdba23db353a1beff831ff4fc83d01ed64e8c2a9 wallet: Document blank flag use in descriptor wallets (Ryan Ofsky)
43310200dce8d450ae5808824af788cefaa5d6db wallet: Ensure that the blank wallet flag is unset after imports (Andrew Chow)
e9379f1ffa7a4eebce397f1150317e840655e021 rpc, wallet: Include information about blank flag (Andrew Chow)
Pull request description:
The `blank` wallet flag is used to indicate that the wallet intentionally does not have any keys, scripts, or descriptors, and it prevents the automatic generation of those things for such a wallet. Once the wallet contains any of those data, it is unnecessary, and possibly incorrect, to have `blank` set. This PR fixes a few places where this was not properly happening. It also adds a test for this unset behavior.
ACKs for top commit:
S3RK:
reACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9
ryanofsky:
Code review ACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9. Only change since last review is dropping the commit which makes createwallet RPC set BLANK flag automatically when DISABLE_PRIVATE_KEYS flag is set
Tree-SHA512: 85bc2a9754df0531575d5c8f4ad7e8f38dcd50083dc29b3283dacf56feae842e81f34654c5e1781f2dadb0560ff80e454bbc8ca3b2d1fab1b236499ae9abd7da
|
|
|
|
size/weight values
3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f Remove txmempool implicit-integer-sign-change sanitizer suppressions (Hennadii Stepanov)
d2f6d2a95a9f6c1632c1ed3b5b5b67a49eb71d6b Use `int32_t` type for most transaction size/weight values (Hennadii Stepanov)
Pull request description:
From bitcoin/bitcoin#23957 which has been incorporated into this PR:
> A file-wide suppression is problematic because it will wave through future violations, potentially bugs.
>
> Fix that by using per-statement casts.
>
> This refactor doesn't change behavior because the now explicit casts were previously done implicitly.
>
> Similar to commit 8b5a4de904b414fb3a818732cd0a2c90b91bc275
ACKs for top commit:
achow101:
ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f
0xB10C:
ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f. I've focused my testing and code review on the tracepoint related changes. The docs, the test, and the mempool_monitor.py demo script are updated. I ran the `interface_usdt_mempool.py` test and the `mempool_monitor.py` script. The `mempool_monitor.py` output looks correct.
Xekyo:
codereview ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f
ryanofsky:
Code review ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f. Since last review, just rebased with more type changes in test and tracing code
Tree-SHA512: 397407f72165b6fb85ff1794eb1447836c4f903efed1a05d7a9704c88aa9b86f330063964370bbd59f6b5e322e04e7ea8e467805d58dce381e68f7596433330f
|
|
ee2417ed614d6a298f932ac068702ab2abee3cdf test: fix intermittent failure in p2p_leak_tx.py (Martin Zumsande)
Pull request description:
Fixes #27860
The problem was that the replacement tx `tx_b` would sometimes be sent out to the inbound peer after the `notfound`, so that threre would be an unexpected `tx` message and the test fails.
```
node0 2023-06-12T12:48:24.903204Z [msghand] [net.cpp:2856] [PushMessage] [net] sending notfound (73 bytes) peer=1
node0 2023-06-12T12:48:24.903916Z [msghand] [net.cpp:2856] [PushMessage] [net] sending tx (133 bytes) peer=1
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_leak_tx.py", line 74, in test_notfound_on_replaced_tx
assert "tx" not in inbound_peer.last_message
```
Fix this by letting the peer wait for the initial broadcast of the replacement tx before continuing with the test.
ACKs for top commit:
MarcoFalke:
lgtm ACK ee2417ed614d6a298f932ac068702ab2abee3cdf
Tree-SHA512: ecc8fb44cac6097a949e4ee622f6f654f49851d7966359532ab3af4c5ed9d587bf08110820b473a616cde3ae6fc8d0da9bb3cee39347655a8c433e819d4d1065
|
|
`/deploymentinfo`
7d452d826a7056411077b870efc3872bb2fa45e4 test: add coverage for `/deploymentinfo` passing a blockhash (brunoerg)
ce887eaf4917c337b21aa2e7811804ce003d36be rest: bugfix, fix crash error when calling `/deploymentinfo` (brunoerg)
Pull request description:
Calling `/deploymentinfo` passing a valid blockhash makes bitcoind to crash. It happens because we're pushing a JSON value of type array when it expects type object. See:
```cpp
jsonRequest.params = UniValue(UniValue::VARR);
```
```cpp
jsonRequest.params.pushKV("blockhash", hash_str);
```
This PR fixes it by changing `pushKV` to `push_back` and adds more test coverage.
ACKs for top commit:
achow101:
ACK 7d452d826a7056411077b870efc3872bb2fa45e4
stickies-v:
ACK 7d452d826a7056411077b870efc3872bb2fa45e4
Tree-SHA512: f01551e556aba2380c3eaed0bc59057304302c202d317d7c1eec5f7ef839851f672aed80819a8719cb1cbbad2aad735d6d44314ac7d6d98bff8217f5a16c312b
|
|
This change gets rid of a few casts and makes the following commit diff
smaller.
|
|
|
|
61c569ab6069d04079a0831468eb713983919636 refactor: decouple early return commands from AppInit (furszy)
4927167f855f8ed3bbf6d2766f61229f742e632a gui: return EXIT_FAILURE on post-init fatal errors (furszy)
3b2c61e8198bcefb1c2343caff1d705951026cc4 Return EXIT_FAILURE on post-init fatal errors (furszy)
3c06926cf21dcca3074ef51506f556b2286c299b refactor: index: use `AbortNode` in fatal error helper (Sebastian Falbesoner)
9ddf7e03a35592617a016418fd320cc93c8d1abd move ThreadImport ABC error to use AbortNode (furszy)
Pull request description:
It seems odd to return `EXIT_SUCCESS` when the node aborted execution due a fatal internal error
or any post-init problem that triggers an unrequested shutdown.
e.g. blocks or coins db I/O errors, disconnect block failure, failure during thread import (external
blocks loading process error), among others.
ACKs for top commit:
TheCharlatan:
ACK 61c569ab6069d04079a0831468eb713983919636
ryanofsky:
Code review ACK 61c569ab6069d04079a0831468eb713983919636
pinheadmz:
ACK 61c569ab6069d04079a0831468eb713983919636
theStack:
Code-review ACK 61c569ab6069d04079a0831468eb713983919636
Tree-SHA512: 18a59c3acc1c6d12cbc74a20a401e89659740c6477fccb59070c9f97922dfe588468e9e5eef56c5f395762187c34179a5e3954aa5b844787fa13da2e666c63d3
|
|
|
|
It seems odd to return `EXIT_SUCCESS` when the node aborted
execution due a fatal internal error or any post-init problem
that triggers an unrequested shutdown.
e.g. blocks or coins db I/O errors, disconnect block failure,
failure during thread import (external blocks loading process
error), among others.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
|
|
This allows us to test that the blank flag is being set appropriately.
|
|
|
|
delete a mapDeltas entry when delta==0
67b7fecacd0489809690982c89ba2d0acdca938c [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow)
c1061acb9d502cdf8c6996c818d9a8a281cbe40c [functional test] prioritisation is not removed during replacement and expiry (glozow)
0e5874f0b06114d9b077e0ff582915e4f83059e6 [functional test] getprioritisedtransactions RPC (glozow)
99f8046829f699ff2eace266aa8cea1d9f7cb65a [rpc] add getprioritisedtransactions (glozow)
9e9ca36c80013749faaf2aa777d52bd07d9d24ec [mempool] add GetPrioritisedTransactions (glozow)
Pull request description:
Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.
Motivation / Background
- `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
- Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen.
- Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart.
- Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them.
- Related: #4818 and #6464.
- There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat.
- Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while.
Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are.
ACKs for top commit:
achow101:
ACK 67b7fecacd0489809690982c89ba2d0acdca938c
theStack:
Code-review ACK 67b7fecacd0489809690982c89ba2d0acdca938c
instagibbs:
code review ACK 67b7fecacd0489809690982c89ba2d0acdca938c
ajtowns:
ACK 67b7fecacd0489809690982c89ba2d0acdca938c code review only, some nits
Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
|
|
a7b46a1feae60e38fe4bdcacf5034f44cae49222 test: added coverage to mining_basic.py (kevkevin)
Pull request description:
Included a test that checks if we call submitblock with block.vtx.empty() then it throws an rpc deserialization error, currently we only test if !block.vtx->IsCoinBase() throws an rpc deserialization error
I've tested to make sure this actually doing what I intended by breaking up this if block into two if blocks with different error messages and running the functional test
https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/rpc/mining.cpp#L963
This change should increase the test coverage for the `submitblock()` rpc in `./src/rpc/mining.cpp`
ACKs for top commit:
theStack:
ACK a7b46a1feae60e38fe4bdcacf5034f44cae49222
Tree-SHA512: 4078cb1fa879cc9e34438319f73085b521b90a5a95348b23e494cf8e5ac792ec426bc0e1a63e949645e16afebe54c5f35a194f02e20b7273871163d89a5c44e6
|
|
2cd28e9fef5dd743bcd70025196ee311fcfdcae4 rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky)
95d7de0964620a3f7386a4adc5707559868abf84 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky)
96233146dd31c1d99fd1619be4449944623ef750 RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky)
702b56d2a8ce48bc3b66a2867d09fa11dcf12fc5 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky)
Pull request description:
Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects.
This makes it possible to make calls like:
```sh
src/bitcoin-cli -named bumpfee txid fee_rate=10
```
instead of
```sh
src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}'
```
RPC help is also updated to show options as top level named arguments instead of as nested objects.
<details><summary>diff</summary>
<p>
```diff
@@ -15,16 +15,17 @@
Arguments:
1. txid (string, required) The txid to be bumped
-2. options (json object, optional)
+2. options (json object, optional) Options object that can be used to pass named arguments, listed below.
+
+Named Arguments:
- {
- "conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
+conf_target (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
- "fee_rate": amount, (numeric or string, optional, default=not set, fall back to wallet fee estimation)
+fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation)
Specify a fee rate in sat/vB instead of relying on the built-in fee estimator.
Must be at least 1.000 sat/vB higher than the current transaction fee rate.
WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB.
- "replaceable": bool, (boolean, optional, default=true) Whether the new transaction should still be
+replaceable (boolean, optional, default=true) Whether the new transaction should still be
marked bip-125 replaceable. If true, the sequence numbers in the transaction will
be left unchanged from the original. If false, any input sequence numbers in the
original transaction that were less than 0xfffffffe will be increased to 0xfffffffe
@@ -32,11 +33,10 @@
still be replaceable in practice, for example if it has unconfirmed ancestors which
are replaceable).
- "estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
+estimate_mode (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
"unset"
"economical"
"conservative"
- }
Result:
{ (json object)
```
</p>
</details>
**Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation.
ACKs for top commit:
achow101:
ACK 2cd28e9fef5dd743bcd70025196ee311fcfdcae4
Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436
|
|
This fixes an intermittent error, caused by blocks arriving
out of order due to how compact block relay may revert to headers
processing when the tip hasn't caught up, and resulting in slightly
different pruning behavior.
Making sure that all blocks from the previous tests are synced before
generating more blocks makes this impossible.
See Issue #27749 for more details.
|
|
fa6b11a55663e70369bfbbba5fccc55b33f2b310 test: Throw error when -signetchallenge is non-hex (MarcoFalke)
Pull request description:
Instead of silently parsing non-hex to an empty challenge, throw an error.
Also, add missing includes while touching the file.
ACKs for top commit:
kevkevinpal:
ACK [fa6b11a](https://github.com/bitcoin/bitcoin/pull/27765/commits/fa6b11a55663e70369bfbbba5fccc55b33f2b310)
kallewoof:
ACK fa6b11a
TheCharlatan:
Nice, ACK fa6b11a55663e70369bfbbba5fccc55b33f2b310
Tree-SHA512: 018ebbbf819ba7cdf0c6dd294fdfaa5ddb81b87058a8b9c57b96066d5b07e1656fd78f18e3cef375aebefa191fa515c2c70bc764880fa05f98f526334431a616
|
|
transactions that are no longer conflicted
89df7987c2f1eea42454c2b0efc31a924fbfd3a8 Add wallets_conflicts (Antoine Riard)
dced203162d45e542f187f8d0d07dab85c52cf28 wallet, tests: mark unconflicted txs as inactive (ishaanam)
096487c4dcfadebe5ca959927f5426cae1c304d5 wallet: introduce generic recursive tx state updating function (ishaanam)
Pull request description:
This implements a fix for #7315. Previously when a block was disconnected any transactions that were conflicting with transactions mined in that block were not updated to be marked as inactive. The fix implemented here is described on the [Bitcoin DevWiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking#idea-refresh-conflicted). A test which tested the previous behavior has also been updated.
Second attempt at #17543
ACKs for top commit:
achow101:
ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8
rajarshimaitra:
tACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8.
glozow:
ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8
furszy:
Tested ACK 89df7987
Tree-SHA512: 3133b151477e8818302fac29e96de30cd64c09a8fe3a7612074a34ba1a332e69148162e6cb3f1074d9d9c9bab5ef9996967d325d8c4c99ba42b5fe3b66a60546
|
|
from mempool_packages to wallet_basic
ffffe622e9cbf926326135bb23958380dcf09df1 test: Move test_chain_listunspent wallet check from mempool_packages to wallet_basic (MarcoFalke)
Pull request description:
This fixes a bug.
On master:
```
$ ./test/functional/mempool_packages.py --legacy-wallet
File "./test/functional/mempool_packages.py", line 52, in run_test
self.nodes[0].importaddress(self.wallet.get_address())
test_framework.authproxy.JSONRPCException: Bech32m addresses cannot be imported into legacy wallets (-5)
```
On this pull, all tests pass.
ACKs for top commit:
glozow:
ACK ffffe622e9cbf926326135bb23958380dcf09df1, thanks for changing! Nice to remove wallet from another non-wallet test.
Tree-SHA512: 842c3b7c2e90285a155b8ed9924ef0c99f7773892be4f1847e5d7ece79c914ea5acee0d71de2ce46c354ee95fb74a03c20c0afb5e49c0b8e1c0ce406df963650
|
|
eefe56967b4eb4b5144325cde4f40fc1cbde3e65 bugfix: Fix incorrect debug.log config file path (Ryan Ofsky)
3746f00be1b732a04976fc70cbb0661f97bbbd99 init: Error if ignored bitcoin.conf file is found (Ryan Ofsky)
398c3719b02197ad92fded20f6ff83b364747297 lint: Fix lint-format-strings false positives when format specifiers have argument positions (Ryan Ofsky)
Pull request description:
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:
- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.conf` file is ignored with no warning.
- Another way this could happen is if a `-conf=` command line argument points to a configuration file with a `datadir=/path` line and that path contains a `bitcoin.conf` file, which is currently ignored.
This change only adds an error message and doesn't change anything about way settings are applied. It also doesn't trigger errors if there are redundant `-datadir` or `-conf` settings pointing at the same configuration file, only if they are pointing at different files and one file is being ignored.
ACKs for top commit:
pinheadmz:
re-ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
willcl-ark:
re-ACK eefe56967b
TheCharlatan:
ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
Tree-SHA512: 939a98a4b271b5263d64a2df3054c56fcde94784edf6f010d78693a371c38aa03138ae9cebb026b6164bbd898d8fd0845a61a454fd996e328fd7bcf51c580c2b
|
|
wallet_basic
|
|
|
|
changed from std::string -> std::string_view
applied snake case to byteStr -> byte_str
|
|
eeee55f9288740747b6e8d806ce8177fd92635cf rpc: Fix invalid bech32 handling (MarcoFalke)
Pull request description:
Currently the handling of invalid bech32(m) addresses over RPC has many issues:
* No error for invalid addresses is reported, leading to internal bugs via `CHECK_NONFATAL`, see https://github.com/bitcoin/bitcoin/issues/27723
* The error messages use "data size" (the meaning of which is unclear to the user, because the witness program data and bech32 section data are related but different) when they mean "program size"
Fix all issues. Also, use the BIP 173 and BIP 350 test vectors.
ACKs for top commit:
achow101:
ACK eeee55f9288740747b6e8d806ce8177fd92635cf
brunoerg:
crACK eeee55f9288740747b6e8d806ce8177fd92635cf
Tree-SHA512: c8639ee49e2a54b740b72d66bc4a40352dd553a6e3220dea9f94e48e33124f21f597a2817cb405d0a4c88d21df1013c0a4877a01370a2d326aa2cff1f9c381a8
|
|
d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83 Add tests for parallel compact block downloads (Greg Sanders)
03423f8bd12b95a06a4a9d8377e781625dd38aae Support up to 3 parallel compact block txn fetchings (Greg Sanders)
13f9b20b4cb2f3f26e81184a77e9cf1f626d4f57 Only request full blocks from the peer we thought had the block in-flight (Greg Sanders)
cce96182ba2457335868c65dc16b081c3dee32ee Convert mapBlocksInFlight to a multimap (Greg Sanders)
a90595478dcf4e443cd15bbb822d485dc42bdb18 Remove nBlocksInFlight (Greg Sanders)
86cff8bf18f2c6344a25ad8b81cf366201a73c36 alias BlockDownloadMap for mapBlocksInFlight (Greg Sanders)
Pull request description:
This is an attempt at mitigating https://github.com/bitcoin/bitcoin/issues/25258 , which is a revival of https://github.com/bitcoin/bitcoin/pull/10984, which is a revival of https://github.com/bitcoin/bitcoin/pull/9447.
This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey
peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block.
This would hopefully allow the chance for an honest high bandwidth peer to hand us the transactions
even if the first in flight peer stalls out.
In contrast to previous effort:
1) it will not help if subsequent peers send block headers only, so only high-bandwidth peers this time. See: https://github.com/bitcoin/bitcoin/pull/10984/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1411
2) `MAX_GETBLOCKTXN_TXN_AFTER_FIRST_IN_FLIGHT` is removed, in favor of aiding recovery during turbulent mempools
3) We require one of the 3 block fetching slots to be an outbound peer. This can be the original offering peer, or subsequent compact blocks given by high bandwidth peers.
ACKs for top commit:
sdaftuar:
ACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83
mzumsande:
Code Review ACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83
Tree-SHA512: 54980eac179e30f12a0bd49df147b2c3d63cd8f9401abb23c7baf02f76eeb59f2cfaaa155227990d0d39384de9fa38663f88774e891600a3837ae927f04f0db3
|
|
`feature_bip68_sequence`
272eb5561667482f8226bcf98eea00689dccefb8 test: fix `include_immature_coinbase` logic in `get_utxos` (brunoerg)
a951c34f179dad0c7059b04a7f1e6b0804462168 test: fix `interface_usdt_mempool` by mining a block after each test (brunoerg)
1557bf1196bc2bdf00fd32f3b5d525796b4d194c test: fix mature utxos addition to wallet in `mempool_package_limits` (brunoerg)
60ced9007d518d542ce489b91076f9bbaf3312e3 test: fix intermittent issue in `feature_bip68_sequence` (brunoerg)
Pull request description:
Fixes #27129
To avoid `bad-txns-premature-spend-of-coinbase` error,
when getting a utxo (using `get_utxo`) to create a new
transaction `get_utxo` shouldn't return (if possible)
by default immature coinbase.
ACKs for top commit:
achow101:
ACK 272eb5561667482f8226bcf98eea00689dccefb8
pinheadmz:
re-ACK 272eb5561667482f8226bcf98eea00689dccefb8
Tree-SHA512: eae821c7833bf084d8b907c94876ed010a7925d2177c3013a0c61b69d9571df006da83397a19487d93b0d1fa415951152f0b8ad0de2a55d86c39f6917934f050
|
|
|
|
|
|
1bce12acd3e271a7c88d9400b4e3a5645bc8a911 test: add test for `descriptorprocesspsbt` RPC (ishaanam)
fb2a3a70e860aa87fb7a21f6554ed9f3ce901e2d rpc: add descriptorprocesspsbt rpc (ishaanam)
Pull request description:
This PR implements an RPC called `descriptorprocesspsbt`. This RPC is based off of `walletprocesspsbt`, but instead of interacting with the wallet to update, sign and finalize a psbt, it instead accepts an array of output descriptors and uses that information along with information from the mempool, txindex, and the utxo set to do so. `utxoupdatepsbt` also updates a psbt in this manner, but doesn't sign or finalize it. Because of this overlap, a helper function that is added in this PR is called by both `utxoupdatepsbt` and `descriptorprocesspsbt`. Whether or not the helper function signs a psbt is dictated by if the HidingSigningProvider passed to it contains any private information. There is also a test added in this PR for this new RPC that uses p2wsh, p2wpkh, and legacy outputs.
Edit: see https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1228830963
ACKs for top commit:
achow101:
re-ACK 1bce12acd3e271a7c88d9400b4e3a5645bc8a911
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/25796/commits/1bce12acd3e271a7c88d9400b4e3a5645bc8a911
Tree-SHA512: e1d0334739943e71f2ee68b4db7637ebe725da62e7aa4be071f71c7196d2a5970a31ece96d91e372d34454cde8509e95ab0eebd2c8edb94f7d5a781a84f8fc5d
|
|
This change allows `test_runner.py` to work from an out-of-source build
directory using a symlink, a hard link or a copy on any platform.
|
|
Use current block height to compute the confirmation count
instead of using the value from utxo object
|
|
Co-authored-by: josibake <josibake@protonmail.com>
|
|
|
|
To avoid `bad-txns-premature-spend-of-coinbase` error,
when getting a utxo (using `get_utxo`) to create a new
transaction `get_utxo` shouldn't return by default
immature coinbase.
|
|
can be downloaded
fa4c16b186a34f4172deda617166813c8cb92c59 test: Add test to check tx in the last block can be downloaded (MarcoFalke)
fadc8490ab4fdd91aaf446e08108657b304a3ded test: Split up test_notfound_on_unannounced_tx test case (MarcoFalke)
Pull request description:
If a peer received an `inv` about a transaction, which was included in a block before receiving the corresponding `getdata`, it can be beneficial to send this transaction to the peer to aid compact block relay.
Add a test for this to avoid breaking it in the future.
ACKs for top commit:
sdaftuar:
ACK fa4c16b186a34f4172deda617166813c8cb92c59
instagibbs:
ACK fa4c16b186a34f4172deda617166813c8cb92c59
Tree-SHA512: 1ec16dcc216dd29c849928e753158d45c409612e9ac528db16e5af465bb5b69cd42241ac6f67e5a4583b8e558eeba49fa0a0889a4247b3fb0053b2758eec9490
|
|
faf4315c88d8c81c2ff84870bc81aef3cf719816 test: Return dict in MiniWallet::send_to (MarcoFalke)
Pull request description:
Returning a tuple has many issues:
* If only one value is needed, it can not be indexed by name
* If another value is added to the return value, all call sites need to be updated
Bite the bullet now and update all call sites to fix the above issues.
ACKs for top commit:
brunoerg:
crACK faf4315c88d8c81c2ff84870bc81aef3cf719816
theStack:
Code-review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816
stickies-v:
Code review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816
Tree-SHA512: 8ce1aca237df21f04b3990d0e5fcb49cc408fe6404399d3769a64eae1b5218941157d9785fce1bd9e45140cf70e06c3aa42646ee8f7b57855beb784fc3ef0261
|
|
|
|
|
|
This is achieved by letting the index sync thread wait until
reindex-chainstate is finished.
This also disables the pruning check when reindexing the chainstate (which is
incompatible with prune mode) because there would be no chain at this point
in init.
|
|
The index sync code has logic to go back the chain to the forking point, while
also updating index-specific state, which is necessary to prevent
possible corruption of the coinstatsindex.
Also add a test for this (a reorg happens while the index is deactivated)
that would not pass before this change.
|
|
|
|
Test the case of a tx being conflicted by multiple
txs with different depths. The conflicted tx is also spent by
a child tx for which confirmation status is tied to the parent's.
After a reorg of conflicting txs, the conflicted status should be
undone properly.
Co-authored-by: furszy <mfurszy@protonmail.com>
|
|
In `blockDisconnected`, for each transaction in the block, look
for any wallet transactions spending the same inputs. If any of
these transactions were marked conflicted, they are now marked as
inactive.
Co-authored-by: ariard <antoine.riard@gmail.com>
|
|
|
|
It's unnecessary to keep the data around, as it doesn't do anything. If
prioritisetransaction is called again, we'll make a new entry in
mapDeltas.
These entries are only deleted when the transaction is mined or conflicted
from a block (i.e. not in replacement or eviction), are persisted in
mempool.dat, and never expire. If node operators use the RPC to
regularly prioritise/de-prioritise transactions, these (meaningless)
map entries may hang around forever and take up valuable mempool memory.
|
|
|
|
|
|
Included a test that checks if we call submitblock with
block.vtx.empty() then it throws an rpc deserialization error, currently
we only test if !block.vtx->IsCoinBase() throws an rpc deserialization
error
|
|
fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost)
Pull request description:
This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.
For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.
Both markers can still be parsed.
The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.
See discussion in #15740
ACKs for top commit:
achow101:
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
darosior:
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
|
|
|
