Age | Commit message (Collapse) | Author |
|
Often when we're updating mempool entries we update entries that we
ultimately end up removing the updated entries shortly thereafter. This
patch makes it so that we filter for such entries a bit earlier in
processing, which yields a mild improvement for these cases, and is
negligible overhead otherwise.
|
|
|
|
When build CHDChains out of CKeyMetadata, the chain counters are
actually 1 based, not 0 based, so 1 must be added to each index.
|
|
When topping up an inactive HD chain, either key_origin will be
available and we can use the path given there, or we need to figure out
the path from the string hdKeypath.
|
|
Resolves segfault on legacy wallet
Log warning when meta.key_origin.path is below expected size
|
|
|
|
Make the behavior align with the help text by actually using
SIGHASH_DEFAULT as the default sighash for signing PSBTs.
|
|
It is better to ues an optional to determine whether the sighash type
is set rather than using 0 as a magic number.
|
|
|
|
aaaa34e34db6ff0f9ca3407bda42e7f0db887737 doc: Add missing optional to getblockfrompeer (MarcoFalke)
Pull request description:
Can be reviewed with `--word-diff-regex=. --ignore-all-space`
ACKs for top commit:
Sjors:
utACK aaaa34e34db6ff0f9ca3407bda42e7f0db887737
Tree-SHA512: 7f46c82a46b8cc19f7eb549b9aa13be8cd6849a8ef8a2ddda6d1eee6978d099fccadd29a2bf817f44d601b905f5d5f6b5d8f4f54be5ee8b914b520359c058e68
|
|
-BEGIN VERIFY SCRIPT-
sed -i -e 's|, /\* optional \*/ true,|, /*optional=*/true,|g' $( git grep -l ', /\* optional \*/ true,' )
-END VERIFY SCRIPT-
|
|
|
|
ffd11ea87640c8a3d83b6f83ac18e65234fc6002 Fix typo and grammar (Heebs)
Pull request description:
Fix typo and grammar in the coin selection algorithm's description.
ACKs for top commit:
meshcollider:
ACK ffd11ea87640c8a3d83b6f83ac18e65234fc6002
Tree-SHA512: bba07c2efd5140fb3e021618739d70aaa761bbc274afb8158809492b0606773c217e42e58e58b18a2454b9c45ebc883ebece17cdc467ac60e3d3140d7a979db7
|
|
dce8c4c38111556ca480aa0e63c46b71f66b508f rpc: getblockfrompeer (Sjors Provoost)
b884ababc29ce963826d8a4327ed6a5e629ff175 rpc: move Ensure* helpers to server_util.h (Sjors Provoost)
Pull request description:
This adds an RPC method to fetch a block directly from a peer. This can used to fetch stale blocks with lower proof of work that are normally ignored by the node (`headers-only` in `getchaintips`).
Usage:
```
bitcoin-cli getblockfrompeer HASH peer_n
```
Closes #20155
Limitations:
* you have to specify which peer to fetch the block from
* the node must already have the header
ACKs for top commit:
jnewbery:
ACK dce8c4c38111556ca480aa0e63c46b71f66b508f
fjahr:
re-ACK dce8c4c38111556ca480aa0e63c46b71f66b508f
Tree-SHA512: 843ba2b7a308f640770d624d0aa3265fdc5c6ea48e8db32269b96a082b7420f7953d1d8d1ef2e6529392c7172dded9d15639fbc9c24e7bfa5cfb79e13a5498c8
|
|
f1f10c0514fe81318c8b064f9dad0e2f9a2cd037 Remove CTxMemPool params from ATMP (lsilva01)
Pull request description:
Remove `CTxMemPool` parameter from `AcceptToMemoryPool` function, as suggested in https://github.com/bitcoin/bitcoin/pull/23437#issuecomment-962536149 .
This requires that `CChainState` has access to `MockedTxPool` in `tx_pool.cpp` as mentioned https://github.com/bitcoin/bitcoin/pull/23173#discussion_r731895386. So the `MockedTxPool` is attributed to `CChainState::m_mempool` before calling `AcceptToMemoryPool`.
Requires #23437.
ACKs for top commit:
jnewbery:
utACK f1f10c0514fe81318c8b064f9dad0e2f9a2cd037
MarcoFalke:
review ACK f1f10c0514fe81318c8b064f9dad0e2f9a2cd037 🔙
Tree-SHA512: 2a4885f4645014fc1fa98bb1090f13721c1a0796bc0021b9cb43bc8cc13920b6eaf057d1f5ed796e0a110e7813e41fe0196334ce7c80d1231fc057a9a3bdf349
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Co-authored-by: John Newbery <1063656+jnewbery@users.noreply.github.com>
Co-authored-by: Jon Atack <jon@atack.com>
|
|
receivedby RPCs
1dcba996d30d83aebe8c73f42f5d4056d6472166 Coinbase receivedby rpcs release notes (Andrew Toth)
b5696750a925c07261287b043ffdfb393cbb1327 Test including coinbase transactions in receivedby wallet rpcs (Andrew Toth)
bce20c34d6b999e700a560f95351c212ed8c36f4 Include coinbase transactions in receivedby wallet rpcs (Andrew Toth)
Pull request description:
The current `*receivedby*` RPCs filter out coinbase transactions. This doesn't seem correct since an output to your address in a coinbase transaction *is* receiving those coins.
This PR corrects this behaviour. Also, a new option `include_immature_coinbase` is added (default=`false`) that includes immature coinbase transactions when set to true.
However, since this is potentially a breaking change this PR introduces a hidden configuration option `-deprecatedrpc=exclude_coinbase`. This can be set to revert to previous behaviour. If no reports of broken workflow are received, then this option can be removed in a future release.
Fixes https://github.com/bitcoin/bitcoin/issues/14654.
ACKs for top commit:
jnewbery:
reACK 1dcba996d30d83aebe8c73f42f5d4056d6472166
Tree-SHA512: bfc43b81279fea5b6770a4620b196f6bc7c818d221b228623e9f535ec75a2406bc440e3df911608a3680f11ab64c5a4103917162114f5ff7c4ca8ab07bb9d3df
|
|
I strongly recommend reviewing with the following git-diff flags:
--ignore-space-change
|
|
|
|
|
|
This commit coalesces the chainstate loading sequence between our unit
test and non-unit test init codepaths.
|
|
[META] In a future commit, these options will be used in TestingSetup to
ensure that the DBs are in-memory.
|
|
|
|
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
[META] In a future commit, this function will be re-used in TestingSetup
so that the behaviour matches across test and non-test init
codepaths.
|
|
Previously we were passing in CChainParams, when VerifyDB only needed
the Consensus::Params subset.
|
|
...instead allow optionally passing in a std::function<bool()>
|
|
...instead pass in a std::function<int64_t()>
Note that the static_cast is needed (apparently) for the compiler to
know which overloaded GetTime to choose.
|
|
See added code comment for more details.
|
|
safety lock assertions
275e9390e1c84ac021b3c781ee239ad9ba7b78d4 mining, refactor: add m_mempool.cs thread safety lock assertions (Jon Atack)
Pull request description:
in src/node/miner to
- BlockAssembler::addPackageTxs()
- BlockAssembler::SkipMapTxEntry()
- BlockAssembler::UpdatePackagesForAdded()
These functions have thread safety lock annotations in their declarations but are missing the corresponding run-time lock assertions in their definitions.
Per doc/developer-notes.md: "Combine annotations in function declarations with run-time asserts in function definitions."
ACKs for top commit:
shaavan:
ACK 275e9390e1c84ac021b3c781ee239ad9ba7b78d4. Thanks for catching and fixing this!
Tree-SHA512: 1c6f1ad1bbd94ff391fc8ce1e3b95d88bd3db5db804a1a5ef4636e54b29f5801f79aa9ed753d34c9a79a58cf01c7ed890e7681ff1c7b0f16335dc062bbac31cc
|
|
|
|
|
|
getmempool RPCs
2f9515f37addabde84c79926d7a24b2897a21dd1 rpc: move fees object to match help (josibake)
07ade7db8f919826c5e69bdaf7d54a6ae653175e doc: add release note for fee field deprecation (josibake)
2ee406ce3e9c252734cb391d85044ac389c34279 test: add functional test for deprecatedrpc=fees (josibake)
35d928c63237e31c99215e2d9d84782befd618d5 rpc: deprecate fee fields from mempool entries (josibake)
Pull request description:
per #22682 , top level fee fields for mempool entries have been deprecated since 0.17 but are still returned. this PR properly deprecates them so that they are no longer returned unless `-deprecatedrpc=fees` is passed.
the first commit takes care of deprecation and also updates `test/functional/mempool_packages.py` to only use the `fees` object. the second commit adds a new functional test for `-deprecatedrpc=fees`
closes #22682
## questions for the reviewer
* `-deprecatedrpc=fees` made the most sense to me, but happy to change if there is a name that makes more sense
* #22682 seems to indicate that after some period of time, the fields will be removed all together. if we have a rough idea of when this will be, i can add a `TODO: fully remove in vXX` comment to `entryToJSON`
## testing
to get started on testing, compile, run the tests, and start your node with the deprecated rpcs flag:
```bash
./src/bitcoind -daemon -deprecatedrpc=fees
```
you should see entries with the deprecated fields like so:
```json
{
"<txid>": {
"fees": {
"base": 0.00000671,
"modified": 0.00000671,
"ancestor": 0.00000671,
"descendant": 0.00000671
},
"fee": 0.00000671,
"modifiedfee": 0.00000671,
"descendantfees": 671,
"ancestorfees": 671,
"vsize": 144,
"weight": 573,
...
},
```
you can also check `getmempoolentry` using any of the txid's from the output above.
next start the node without the deprecated flag, repeat the commands from above and verify that the deprecated fields are no longer present at the top level, but present in the "fees" object
ACKs for top commit:
jnewbery:
reACK 2f9515f37addabde84c79926d7a24b2897a21dd1
glozow:
utACK 2f9515f37addabde84c79926d7a24b2897a21dd1
Tree-SHA512: b175f4d39d26d96dc5bae26717d3ccfa5842d98ab402065880bfdcf4921b14ca692a8919fe4e9969acbb5c4d6e6d07dd6462a7e0a0a7342556279b381e1a004e
|
|
in src/node/miner to:
- BlockAssembler::addPackageTxs()
- BlockAssembler::SkipMapTxEntry()
- BlockAssembler::UpdatePackagesForAdded()
These functions have thread safety lock annotations in
their declarations but are missing the corresponding
run-time lock assertions in their definitions.
Per doc/developer-notes.md: "Combine annotations in function
declarations with run-time asserts in function definitions."
|
|
ComplexMempool benchmark"
faa185bb3abe5fdaeeae14706bad9437acac6a69 Revert "Fixes Bug in Transaction generation in ComplexMempool benchmark" (MarcoFalke)
Pull request description:
Developers are reporting crashes (potentially OOM) on IRC, but I can't reproduce. Still, revert this for now, since one developer reported the bare metal this was running on crashed.
Top commit has no ACKs.
Tree-SHA512: 080db4fcfc682b68f4cc40dfabd9d3e0e3f6e6297ce4b782d5de2c83bc18f85f60efb1cda64c51e23c4fd2a05222a904e7a11853d9f9c052dcd26a53aa00b235
|
|
This reverts commit 29e983386b0aecf99cdb7d0e08ba6b450bed313e.
|
|
|
|
- consistent param naming between function declaration and definition
- brackets, param naming and localvar naming per current standards
in doc/developer-notes.md
- update/improve doxygen documentation in the declaration
- improve comments and other localvar names
- constness
- named args
|
|
1ed5681407adc1acc60c9bfebde5819f077f0bf3 rpc: add missing scantxoutset examples (Sebastian Falbesoner)
Pull request description:
The scantxoutset RPC and its help text was at last improved in #16285, but it's still missing examples (see https://github.com/bitcoin/bitcoin/pull/16285#issuecomment-529313781).
~Note that the example descriptor used doesn't follow the developer guideline of using invalid bech32 addresses, as the RPC is not wallet-related and it's use-case is merely to look up state information (i.e. there is no danger of sending funds to a wrong address).~ For the sake of simplicity, the raw descriptor for an early coinbase payout address (block 9) is taken, i.e. it yields results even at an early stage of IBD. Happy to change that though if there are other suggestions.
ACKs for top commit:
shaavan:
reACK 1ed5681407adc1acc60c9bfebde5819f077f0bf3
Tree-SHA512: 057ad9ac0d019035bee2332440128de0ef08580bbeae80182ff74771beead3555c4bf7008071a97bbb6a8d85fb85d0f0754fb7941db2c5b755eae1ac9aa65318
|
|
ComplexMempool benchmark
29e983386b0aecf99cdb7d0e08ba6b450bed313e Fixes Bug in Transaction generation in ComplexMempool benchmark (Shorya)
Pull request description:
This fixes issues with `ComplexMempool` benchmark introduced in [#17292](https://github.com/bitcoin/bitcoin/pull/17292) , this stress test benchmarks performance of ancestor and descendant tracking of mempool graph algorithms on a complex Mempool.
This Benchmark first creates 100 base transactions and stores them in `available_coins` vector. `available_coins` is used for selecting ancestor transactions while creating 800 new transactions. For this a random transaction is picked from `available_coins` and some of its outputs are mapped to the inputs of the new transaction being created.
Now in case we exhaust all the outputs of an entry in `available_coins` then we need to remove it from `available_coins` before the next iteration of choosing a potential ancestor , it is now implemented with this patch.
As the index of the entry is randomly chosen from `available_coins` , In order to remove it from the vector , if index of the selected entry is not at the end of `available_coins` vector , it is swapped with the entry at the back of the vector , then the entry at the end of `available_coins` is popped out.
Earlier the code responsible for constructing outputs of the newly created transaction was inside the loop used for assigning ancestors to the transaction , which does some unnecessary work as it creates outputs of the transaction again and again , now it is moved out of the loop so outputs of the transaction are created just once before adding it to the final list of the transactions created. This one is a minor change to save some computation.
These changes have changed the `ComplexMempool` benchmark results on `bitcoin:master` as follows :
**Before**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 232,881,625.00 | 4.29 | 0.7% | 2.55 | `ComplexMemPool`
**After**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 497,275,135.00 | 2.01 | 0.5% | 5.49 | `ComplexMemPool`
Top commit has no ACKs.
Tree-SHA512: d6946d7e65c55f54c84cc49d7abee52e59ffc8b7668b3c80b4ce15a57690ab00a600c6241cc71a2a075def9c30792a311256fed325ef162f37aeacd2cce93624
|
|
for Adjusted Time
0c85dc30e6b628f7538a67776c7eefcb84ef4f82 p2p: Don't use timestamps from inbound peers (Martin Zumsande)
Pull request description:
`GetAdjustedTime()` (used e.g. in validation and addrman) returns a time with an offset that is influenced by timestamps that our peers have sent us in their version message.
Currently, timestamps from all peers are used for this.
However, I think that it would make sense to ignore the timedata samples from inbound peers, making it much harder for others to influence the Adjusted Time in a targeted way.
With the extra feeler connections (every 2 minutes on average) and extra block-relay-only connections (every 5 minutes on average) there are also now plenty of opportunities to gather a meaningful number of timedata samples from outbound peers.
There are some measures in place to prevent abuse: the `-maxtimeadjustment` parameter with a default of 70 minutes, warnings in cases of large deviations, only using the first 200 samples ([explanation](https://github.com/bitcoin/bitcoin/blob/383d350bd5107bfe00e3b90a00cab9a3c1397c72/src/timedata.cpp#L57-L72)), but I think that only using samples from outbound connections in the first place would be an additional safety measure that would make sense.
See also issue #4521 for further context and links: There have been several discussions in the past about replacing or abolishing the existing timedata system.
ACKs for top commit:
jnewbery:
Concept and code review ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
naumenkogs:
ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
vasild:
ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
Tree-SHA512: 2d6375305bcae034d68b58b7a07777b40ac430dfed554c88e681a048c527536691e1b7d08c0ef995247d356f8e81aa0a4b983bf2674faf6a416264e5f1af0a96
|
|
estimatesmartfee if smart fee data is unavailable
cd8d156354ed32a215de5eab5c394a1d74d91ed4 Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable (Luke Dashjr)
Pull request description:
Fixes a regression introduced by #22722
(Not entirely sure on the solution)
ACKs for top commit:
prayank23:
crACK https://github.com/bitcoin/bitcoin/pull/23547/commits/cd8d156354ed32a215de5eab5c394a1d74d91ed4
darosior:
utACK cd8d156354ed32a215de5eab5c394a1d74d91ed4
kristapsk:
utACK cd8d156354ed32a215de5eab5c394a1d74d91ed4
Tree-SHA512: eb4aa3cc345c69c44ffd5733b51b90eefe1d7854b7a2855e8cbb98268db24d43b7d0ae9fbb0eccf9b6dc01da644d19433cc77fec52ff67bf890be1fc53a67fc4
|