| Age | Commit message (Collapse) | Author |
|---|
|
|
|
In previous commits in this patchset, we've made sure that every
Unload/UnloadBlockIndex member function resets its own members, and does
not reach out to globals.
This means that their corresponding classes' default destructors can now
replace them, and do an even more thorough job without the need to be
updated for every new member variable.
Therefore, we can remove them, and also remove UnloadBlockIndex since
that's not used anymore.
Unfortunately, chainstatemanager_loadblockindex relies on
CChainState::UnloadBlockIndex, so that needs to stay for now.
|
|
The only caller that uses this is ~ChainTestingSetup() where we
immediately destroy the mempool afterwards.
|
|
Also add TODO item to deglobalize the {versionbits,warning}cache, which
should really only need to be cleared if we change the chainparams.
|
|
|
|
Prior to this change blocks could be pruned up to the last block before the blockfilterindex current best block.
|
|
This change also introduces an aditional buffer of 10 blocks (PRUNE_LOCK_BUFFER) that will not be pruned before the best block.
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
|
|
Add more fs::path operator/ and operator+ overloads to prevent unsafe
string->path conversions on Windows that would cause strings to be
decoded according to the current Windows locale & code page instead of
the correct string encoding.
Update application code to deal with loss of implicit string->path
conversions by calling fs::u8path or fs::PathFromString explicitly, or
by just changing variable types from std::string to fs::path to avoid
conversions altoghther, or make them happen earlier.
In all cases, there's no change in behavior either (1) because strings
only contained ASCII characters and would be decoded the same regardless
of what encoding was used, or (2) because of the 1:1 mapping between
paths and strings using the PathToString and PathFromString functions.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
...to m_best_header and m_have_pruned
-BEGIN VERIFY SCRIPT-
find_regex="\bpindexBestHeader\b" \
&& git grep -l -E "$find_regex" -- src \
| xargs sed -i -E "s@$find_regex@m_best_header@g"
find_regex="\bfHavePruned\b" \
&& git grep -l -E "$find_regex" -- src \
| xargs sed -i -E "s@$find_regex@m_have_pruned@g"
-END VERIFY SCRIPT-
|
|
-----
Code Reviewer Notes
Call graph of relevant functions:
UnloadBlockIndex() <-- Moved from
calls ChainstateManager::Unload()
which calls BlockManager::Unload() <-- Moved to
So calling UnloadBlockIndex() would still run this moved code. The code
will also now run when ~BlockManager gets called, which makes sense.
|
|
[META] In the next commit, we move the clearing of fHavePruned to
BlockManager::Unload()
|
|
-----
Code Reviewer Notes
Call graph of relevant functions:
UnloadBlockIndex() <-- Moved from
calls ChainstateManager::Unload() <-- Moved to
Safe because ChainstateManager::Unload() is called only by
UnloadBlockIndex() and no other callers.
|
|
[META] In the next commit, we move the clearing of pindexBestHeader to
ChainstateManager::Unload()
|
|
Co-authored-by: James O'Beirne <james.obeirne@pm.me>
|
|
When using checklevel=4, block verification fails because of duplicate coinbase transactions
involving blocks 91812 and 91722. There was already a check in place for ConnectBlock to
ignore the problematic blocks, but DisconnectBlock did not contain a similar check.
This change ignores the blocks where these inconsistencies surface so
that block verification will succeed at checklevel=4.
|
|
Now BlockManager::LoadBlockIndex() will ACTUALLY only load BlockMan
members.
[META] In a later commit, pindexBestHeader will be moved to ChainMan as
a member
-----
Code Reviewer Notes
Call graph of relevant functions:
ChainstateManager::LoadBlockIndex() <-- Moved to
calls BlockManager::LoadBlockIndexDB()
which calls BlockManager::LoadBlockIndex() <-- Moved from
There is only one call to each of inner functions, meaning that no
behavior is changing.
|
|
packages
9bebf35e269b2a918df27708565ecd0c5bd3f116 [validation] don't package validate if not policy or missing inputs (glozow)
51edcffa0e156dba06191a8d5c636ba01fa5b65f [unit test] package feerate and package cpfp (glozow)
1b93748c937e870e7574a8e120a85bee6f9013ff [validation] try individual validation before package validation (glozow)
17a8ffd8020375d60428695858558f2be264aa36 [packages/policy] use package feerate in package validation (glozow)
09f32cffa6c3e8b2d77281a5983ffe8f482a5945 [docs] package feerate (glozow)
Pull request description:
Part of #22290, aka [Package Mempool Accept](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a).
This enables CPFP fee bumping in child-with-unconfirmed-parents packages by introducing [package feerate](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#fee-related-checks-use-package-feerate) (total modified fees divided by total virtual size) and using it in place of individual feerate. We also always [validate individual transactions first](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#always-try-individual-submission-first) to avoid incentive-incompatible policies like "parents pay for children" or "siblings pay for siblings" behavior.
ACKs for top commit:
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/24152/commits/9bebf35e269b2a918df27708565ecd0c5bd3f116
mzumsande:
Code review ACK 9bebf35e269b2a918df27708565ecd0c5bd3f116
t-bast:
ACK https://github.com/bitcoin/bitcoin/pull/24152/commits/9bebf35e269b2a918df27708565ecd0c5bd3f116
Tree-SHA512: 5117cfcc3ce55c00384d9e8003a0589ceac1e6f738b1c299007d9cd9cdd2d7c530d31cfd23658b041a6604d39073bcc6e81f0639a300082a92097682a6ea8c8f
|
|
|
|
Package validation policy only differs from individual policy in its
evaluation of feerate. Minimize DoS surface; don't validate all over
again if we know the result will be the same.
|
|
This avoids "parents pay for children" and "siblings pay for siblings"
behavior, since package feerate is calculated with totals and is
topology-unaware.
It also ensures that package validation never causes us to reject a
transaction that we would have otherwise accepted in single-tx
validation.
|
|
This allows CPFP within a package prior to submission to mempool.
|
|
304ef73c83900a5439cdf8e6f0519471f59444d3 validation: improve connect bench logging (Sjors Provoost)
Pull request description:
* mention when we're using a cached block rather than actually loading it from disk
* add ms/blk to load block from disk
* log writing of Undo data, so it's tracked separate from writing indexes
Example outputs from `src/bitcoind -debug=bench` during IBD.
When the pass the block in memory:
```
2022-01-31T14:21:35Z - Connect block: 251.79ms [59.59s (419.65ms/blk)]
2022-01-31T14:21:35Z - Using cached block
2022-01-31T14:21:35Z - Load block from disk: 0.07ms [9.67s (68.12ms/blk)]
2022-01-31T14:21:35Z - Sanity checks: 0.00ms [0.27s (1.87ms/blk)]
2022-01-31T14:21:35Z - Fork checks: 0.02ms [0.26s (1.84ms/blk)]
2022-01-31T14:21:35Z - Connect 597 transactions: 154.84ms (0.259ms/tx, 0.022ms/txin) [34.89s (243.96ms/blk)]
2022-01-31T14:21:35Z - Verify 7043 txins: 169.60ms (0.024ms/txin) [35.67s (249.46ms/blk)]
2022-01-31T14:21:35Z - Write undo data: 19.72ms [10.68s (74.68ms/blk)]
2022-01-31T14:21:35Z - Index writing: 0.05ms [0.73s (5.12ms/blk)]
2022-01-31T14:21:35Z - Connect total: 189.66ms [48.18s (336.93ms/blk)]
2022-01-31T14:21:35Z - Flush: 5.23ms [1.19s (8.30ms/blk)]
2022-01-31T14:21:35Z - Writing chainstate: 0.04ms [0.58s (4.03ms/blk)]
2022-01-31T14:21:35Z UpdateTip: new best=0000000000000000000b94a079a58d64f640f66b0cc338b5831b94c8739439a6 height=660135 version=0x20a00000 log2_work=92.494955 tx=593512887 date='2020-12-06T01:43:07Z' progress=0.850804 cache=138.8MiB(1031560txo)
2022-01-31T14:21:35Z - Connect postprocess: 0.26ms [0.17s (1.17ms/blk)]
```
When we have to load the block from disk (when blocks are received out of order, they are saved after initial validation steps and then loaded again for the final validation steps and connecting to tip):
```
2022-01-31T14:21:35Z - Connect block: 195.27ms [59.79s (418.08ms/blk)]
2022-01-31T14:21:38Z - Load block from disk: 23.35ms [9.70s (67.80ms/blk)]
2022-01-31T14:21:38Z - Sanity checks: 1.96ms [0.27s (1.87ms/blk)]
2022-01-31T14:21:38Z - Fork checks: 0.05ms [0.26s (1.83ms/blk)]
2022-01-31T14:21:38Z - Connect 404 transactions: 116.03ms (0.287ms/tx, 0.017ms/txin) [35.00s (243.07ms/blk)]
2022-01-31T14:21:38Z - Verify 7031 txins: 119.58ms (0.017ms/txin) [35.79s (248.56ms/blk)]
2022-01-31T14:21:38Z - Write undo data: 23.54ms [10.70s (74.33ms/blk)]
2022-01-31T14:21:38Z - Index writing: 1.42ms [0.73s (5.09ms/blk)]
2022-01-31T14:21:38Z - Connect total: 146.84ms [48.33s (335.61ms/blk)]
2022-01-31T14:21:38Z - Flush: 4.84ms [1.19s (8.28ms/blk)]
2022-01-31T14:21:38Z - Writing chainstate: 0.04ms [0.58s (4.00ms/blk)]
2022-01-31T14:21:38Z UpdateTip: new best=00000000000000000004c2cad14fec645807ce236f8e1cc43fe106ee4f27692e height=660136 version=0x2000e000 log2_work=92.494972 tx=593513291 date='2020-12-06T01:46:08Z' progress=0.850804 cache=139.4MiB(1036010txo)
2022-01-31T14:21:38Z - Connect postprocess: 0.16ms [0.17s (1.16ms/blk)]
```
ACKs for top commit:
laanwj:
Code review ACK 304ef73c83900a5439cdf8e6f0519471f59444d3
theStack:
Concept and code-review ACK 304ef73c83900a5439cdf8e6f0519471f59444d3
jonatack:
re-ACK 304ef73c83900a5439cdf8e6f0519471f59444d3 per `git range-diff 4faf7a1d8 cfb027d 304ef7` rebase only since my last review
Tree-SHA512: 2009e1db3c30eacd15c11629903334eda4f016da614afe2a1275d00c2a80d42e37edb10e79ceb58b3bd8855e7c6915e67d455aec15f73a25f48f48f64aa51a29
|
|
is set
cccc1e70b8a14430cc94143da97936a60d6c83d3 Enforce Taproot script flags whenever WITNESS is set (MarcoFalke)
fa422994116a7a053789304d56159760081479eb Remove nullptr check in GetBlockScriptFlags (MarcoFalke)
faadc606c7644f2934de390e261d9d65a81a7592 refactor: Pass const reference instead of pointer to GetBlockScriptFlags (MarcoFalke)
Pull request description:
Now that Taproot is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status.
### Benefits:
(With "script flags" I mean "taproot script verification flags".)
* Script flags are known ahead for all blocks (even blocks not yet created) and do not change. This may benefit static analysis, code review, and development of new script features that build on Taproot.
* Any future bugs introduced in the deployment code won't have any effect on the script flags, as they are independent of deployment.
* Enforcing the taproot rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases.
* It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. While `nMinimumChainWork` already protects against this, the cost for a few months worth of POW might be lowered until a major version release of Bitcoin Core reaches EOL. The needed work for the attack is the difference between `nMinimumChainWork` and the work at block 709632.
For reference, previously the same was done for P2SH and WITNESS in commit 0a8b7b4b33c9d78574627fc606267e2d8955cd1c.
### Implementation:
I found one block which fails verification with the flags applied, so I added a `TaprootException`, similar to the `BIP16Exception`.
For reference, the debug log:
```
ERROR: ConnectBlock(): CheckInputScripts on b10c007c60e14f9d087e0291d4d0c7869697c6681d979c6639dbd960792b4d41 failed with non-mandatory-script-verify-flag (Witness program was passed an empty witness)
BlockChecked: block hash=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad state=non-mandatory-script-verify-flag (Witness program was passed an empty witness)
InvalidChainFound: invalid block=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad height=692261 log2_work=92.988459 date=2021-07-23T08:24:20Z
InvalidChainFound: current best=0000000000000000000067b17a4c0ffd77c29941b15ad356ca8f980af137a25d height=692260 log2_work=92.988450 date=2021-07-23T07:47:31Z
ERROR: ConnectTip: ConnectBlock 0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad failed, non-mandatory-script-verify-flag (Witness program was passed an empty witness)
```
Hint for testing, make sure to set `-noassumevalid`.
### Considerations
Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before Taproot activation, that is the last block without the Taproot script flags set, is only buried by a few days of POW. However, when and if this patch is included in the next major release, it will be buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large.
ACKs for top commit:
Sjors:
tACK cccc1e70b8a14430cc94143da97936a60d6c83d3
achow101:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3
laanwj:
Code review ACK cccc1e70b8a14430cc94143da97936a60d6c83d3
ajtowns:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ; code review; wrote a "getblockscriptflags" rpc to quickly check that blocks just had bit 17 (taproot) added; review of earlier revisions had established non-exception blocks do validate with taproot rules enabled.
jamesob:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ([`jamesob/ackr/23536.1.MarcoFalke.enforce_taproot_script_f`](https://github.com/jamesob/bitcoin/tree/ackr/23536.1.MarcoFalke.enforce_taproot_script_f))
Tree-SHA512: 00044de68939caef6420ffd588c1291c041a8b397c80a3df1e3e3487fbeae1821d23975c51c95e44e774558db76f943b00b4e27cbd0213f64a9253116dc6edde
|
|
f865cf8ded2b2fbc82a6fbc41226d991909a6880 Add and use BlockManager::GetAllBlockIndices (Carl Dong)
28ba0313eac37e4a900b7e97af7169ce999c4024 Add and use CBlockIndexHeightOnlyComparator (Carl Dong)
12eb05df63f930969115af6dc66e2e5d02f2a517 move-only: Move CBlockIndexWorkComparator to blockstorage (Carl Dong)
c600ee38168a460d3026eae0e289c976194aad14 Only load BlockMan in BlockMan member functions (Carl Dong)
42e56d9b188f97c077ed2269e24acc0be35ece17 style-only: No need for std::pair for vSortedByHeight (Carl Dong)
3bbb6fea051f4e19bd2448e401a6c4e9b4cc7a41 style-only: Various blockstorage.cpp cleanups (Carl Dong)
5be9ee3c54dcb396ff52fc8c8b7e4e6e39ec4a3b refactor: more const annotations for uses of CBlockIndex* (Anthony Towns)
Pull request description:
The only important commit is "Only load BlockMan in BlockMan member functions", everything else is all just small style changes.
Here's the commit message, reproduced:
```
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
```
ACKs for top commit:
ajtowns:
ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 ; code review only
MarcoFalke:
review ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 🗂
Tree-SHA512: 7b204d782834e06fd7329d022e2ae860181b4e8105c33bfb928539a4ec24161dc7438a9c4d4ee279dcad77de310c160b997bb8aa18923243d0fd55ccf4ad7c3a
|
|
|
|
...also use std::sort for clarity
|
|
...it's declared in blockstorage.h
|
|
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
|
|
fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 scripted-diff: Clarify CheckFinalTxAtTip name (MarcoFalke)
fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35 policy: Remove unused locktime flags (MarcoFalke)
Pull request description:
The locktime flags have many issues:
* They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5.
* They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (https://github.com/bitcoin/bitcoin/pull/6566#issuecomment-150310861)
* No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested.
* The dead code calls `GetAdjustedTime` (network adjusted time), which has its own issues. See https://github.com/bitcoin/bitcoin/issues/4521
Fix all issues by removing them
ACKs for top commit:
ajtowns:
ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1
theStack:
Code-review ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1
glozow:
ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1, agree the default arg `flags` is a massive footgun and just setting max flags is weird. Adding `AtTip` to the names makes sense to me, since they're both testing for *next* block and only ever used for {,re}addition to mempool.
Tree-SHA512: 79f4a52f34909eb598d88bbae7afe8abe5f85f45c128483d16aa83dacd0e5579e561b725d01b1e9a931d1821012a51ad2bc6fb2867f8d09ee541f9d234d696f8
|
|
ATMPArgs
faa1aec26b3f354c832e6b995323c9429b178931 Remove confusing P1008R1 violation in ATMPArgs (MarcoFalke)
Pull request description:
The `= delete` doesn't achieve the stated goal and it is also redundant, since it is not possible to default construct the `ATMPArgs` type.
This can be tested with:
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 2813b62462..1c939c0b8a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -519,6 +519,7 @@ public:
/** Parameters for child-with-unconfirmed-parents package validation. */
static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time,
std::vector<COutPoint>& coins_to_uncache) {
+ ATMPArgs{};
return ATMPArgs{/* m_chainparams */ chainparams,
/* m_accept_time */ accept_time,
/* m_bypass_limits */ false,
```
Which fails on current master *and* this pull with the following error:
```
validation.cpp:525:22: error: reference member of type 'const CChainParams &' uninitialized
ATMPArgs{};
~^
validation.cpp:470:29: note: uninitialized reference member is here
const CChainParams& m_chainparams;
^
1 error generated.
```
Further reading (optional):
* http://open-std.org/JTC1/SC22/WG21/docs/papers/2018/p1008r1.pdf
ACKs for top commit:
achow101:
ACK faa1aec26b3f354c832e6b995323c9429b178931
glozow:
code review ACK faa1aec26b3f354c832e6b995323c9429b178931
Tree-SHA512: 16db2c9959a1996eafbfa533dc4d1483761b9d28295aed5a82b86abd7268da37c51c59ddc67c205165ecb415dbe637b12a0e1b3234d50ab0b3b79de66d7bd73e
|
|
|
|
`CBlockIndex`s
6c23c415613d8b847e6f6a2f872be893da9f4384 refactor: Rewrite AddToBlockIndex with try_emplace (Carl Dong)
c05cf7aa1e1c15089753897a10c14762027d4b99 style: Modernize range-based loops over m_block_index (Carl Dong)
c2a1655799c5d5dab9b14bd2a6b2d2296efd6964 style-only: Use using instead of typedef for BlockMap (Carl Dong)
dd79dad17545424d145e846026518d70da594380 refactor: Rewrite InsertBlockIndex with try_emplace (Carl Dong)
531dce034718523967808a89c18ba69a1e3e5a1f tests: Remove now-unnecessary manual Unload's (Carl Dong)
bec86ae32683ac56b4e6ba9c9b7d21cfbdf4ac03 blockstorage: Make m_block_index own CBlockIndex's (Carl Dong)
Pull request description:
Part of: #24303
Split off from: #22564
```
Instead of having CBlockIndex's live on the heap, which requires manual
memory management, have them be owned by m_block_index. This means that
they will live and die with BlockManager.
```
The second commit demonstrates how this makes calls to `Unload()` to satisfy the address sanitizer unnecessary.
ACKs for top commit:
ajtowns:
ACK 6c23c415613d8b847e6f6a2f872be893da9f4384
MarcoFalke:
re-ACK 6c23c415613d8b847e6f6a2f872be893da9f4384 🎨
Tree-SHA512: 81b2b5119be27cc0f8a9457b11da60cc60930315d2a5be36be89fe253d32073ffe622348ff153114b9b3212197bddbc791810913a43811b33cc58e7162bd105b
|
|
ChainstateManager::Reset thread safety cleanups
ae9ceed3e23288b163b7d7b1840b06b8d332f4ce validation, refactoring: remove ChainstateManager::Reset() (Jon Atack)
daad0093e3d1466789d0ce687902636c80cd74a1 validation: replace lock with annotation in UnloadBlockIndex() (Jon Atack)
Pull request description:
Thread safety refactoring seen in #24177:
- replace re-acquiring lock cs_main with a thread safety annotation in UnloadBlockIndex()
- remove ChainstateManager::Reset(), as it is currently unused (can be reintroduced in the test utilities if needed for unit testing)
ACKs for top commit:
laanwj:
Code review ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce
vasild:
ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce
klementtan:
crACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce
Tree-SHA512: cebb782572997cc2dda01590d6bb6c5e479e8202324d8b6ff459b814ce09e818b996c881736bfebd1b8bf4b6d7a0f79faf3ffea176a4699dd7d7429de2db2d13
|
|
VerifyLoadedChainstate
fa7991601c93761bc12ef33b672a927d48a95569 Fixup style of VerifyDB (MarcoFalke)
fa462ea787d124c56d6ba7ef79a9b5b23f0411c5 Avoid implicit-integer-sign-change in VerifyLoadedChainstate (MarcoFalke)
Pull request description:
This happens when checking all blocks (`-1`).
To test:
```
./configure CC=clang CXX=clang++ --with-sanitizers=undefined,integer
make
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./test/functional/rpc_blockchain.py
ACKs for top commit:
theStack:
Code-review ACK fa7991601c93761bc12ef33b672a927d48a95569
brunoerg:
crACK fa7991601c93761bc12ef33b672a927d48a95569
Tree-SHA512: bcbe6becf2fbedd21bbde83a544122e79465937346802039532143b2e4165784905a8852c0ccb088b964874df5e5550931fdde3629cbcee3ae237f2f63c43a8e
|
|
|
|
|
|
Instead of having CBlockIndex's live on the heap, which requires manual
memory management, have them be owned by m_block_index. This means that
they will live and die with BlockManager.
A change to BlockManager::LookupBlockIndex:
- Previously, it was a const member function returning a non-const CBlockIndex*
- Now, there's are const and non-const versions of
BlockManager::LookupBlockIndex returning a CBlockIndex with the same
const-ness as the member function:
(e.g. const CBlockIndex* LookupBlockIndex(...) const)
See next commit for some weirdness that this eliminates.
The range based for-loops are modernize (using auto + destructuring) in
a future commit.
|
|
from transifex translator feedback
48742693acc9de837735674057c9aae2fe90bd1d Replace "can not" with "cannot" in docs, user messages, and tests (Jon Atack)
e670edd43441ecb6e5978d65348501c57d856030 User-facing content fixups from transifex translator feedback (Jon Atack)
Pull request description:
Closes #24366.
ACKs for top commit:
laanwj:
Code review re-ACK 48742693acc9de837735674057c9aae2fe90bd1d
hebasto:
re-ACK 48742693acc9de837735674057c9aae2fe90bd1d, only suggested change since my previous [review](https://github.com/bitcoin/bitcoin/pull/24367#pullrequestreview-885938219).
Tree-SHA512: 4dcdcb417251a413e65fab6070515e13a1267c8e0dbcf521386b842511391f24c84a0c2168fe13458c977682034466509bf2a3453719d4d94d3c568fd9f4adb4
|
|
77202f0554dcbbbb167d0ed3927cca0bf4609ce8 [doc] package deduplication (glozow)
d35a3cb3968d7584c7d5c42b121a80f34ea656bf [doc] clarify inaccurate comment about replacements paying higher feerate (glozow)
5ae187f8761f5f85a1ef41d24f75afb7eecf366f [validation] look up transaction by txid (glozow)
Pull request description:
- Use txid, not wtxid, for `mempool.GetIter()`: https://github.com/bitcoin/bitcoin/pull/22674#discussion_r772934994
- Fix a historically inaccurate comment about RBF during the refactors: https://github.com/bitcoin/bitcoin/pull/22855#discussion_r777130441
- Add a section about package deduplication to policy/packages.md: https://github.com/bitcoin/bitcoin/pull/24152#discussion_r802955759 and https://github.com/bitcoin/bitcoin/pull/24152#discussion_r802723149
(I'm intending for this to be in v23 since it's fixups for things that are already merged, which is why I split it from #24152)
ACKs for top commit:
t-bast:
LGTM, ACK https://github.com/bitcoin/bitcoin/pull/24310/commits/77202f0554dcbbbb167d0ed3927cca0bf4609ce8
darosior:
ACK 77202f0554dcbbbb167d0ed3927cca0bf4609ce8
LarryRuane:
ACK 77202f0554dcbbbb167d0ed3927cca0bf4609ce8
Tree-SHA512: a428e791dfa59c359d3ccc67e8d3a4c1239815d2f6b29898e129700079271c00b3a45f091f70b65a6e54aa00a3d5b678b6da29d2a76b6cd6f946eaa7082ea696
|
|
|
|
|
|
799968e8b38833dc7fd7b6d488a66a14580ef674 tracing: misc follow-ups to 22902 (0xb10c)
36a65847033540cf2203252c7baf42bc5ec97579 tracing: correctly scope utxocache:flush tracepoint (Arnab Sen)
Pull request description:
This PR is a follow-up to the [#22902](https://github.com/bitcoin/bitcoin/pull/22902).
Previously, the tracepoint `utxocache:flush` was called, even when it was not flushing. So, the tracepoint is now scoped to write only when coins cache to disk.
ACKs for top commit:
0xB10C:
ACK 799968e8b38833dc7fd7b6d488a66a14580ef674
Tree-SHA512: ebb096cbf991c551c81e4339821f10d9768c14cf3d8cb14d0ad851acff5980962228a1c746914c6aba3bdb27e8be53b33349c41efe8bab5542f639916e437b5f
|
|
eb8b22d5176d7abc6f93b4473df446105ca595e6 block_connected: re-use previous GetTimeMicros (William Casarin)
80e1c55687aae61767f1ade0826746cda00d6a24 block_connected: don't serialize block hash twice (William Casarin)
Pull request description:
In the validation:block_connected tracepoint, we call block->GetHash(), which
ends up calling CBlockHeader::GetHash(), executing around 8000 serialization
instructions. We don't need to do this extra work, because block->GetHash() is
already called further up in the function. Let's save that value as a local
variable and re-use it in our tracepoint so there is no unnecessary tracepoint
overhead.
Shave off an extra 100 or so instructions from the validation:block_connected
tracepoint by reusing a nearby GetTimeMicros(). This brings the tracepoint down
to 54 instructions. Still high, but much better than the previous ~154 and
8000 instructions which it was originally.
Signed-off-by: William Casarin <jb55@jb55.com>
ACKs for top commit:
0xB10C:
ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
laanwj:
Code review ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
theStack:
re-ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
Tree-SHA512: 92ae585e487554e0f73042a8abaa239f630502c1d198e010bd7c1de252d882bccb627bbf0e4faec09c1253e782b145bcf153f9fee78cdb8456188044a96f8267
|
|
lock assertions
f485a0745455b46390f1c14260643ad69c8fe2ad Add missing thread safety lock assertions in validation.h (Jon Atack)
37af8a20cf39ed8ee4b3ba4e1d8d55178eaacb78 Add missing thread safety lock assertions in validation.cpp (Jon Atack)
Pull request description:
A number of functions in validation.{h,cpp} have a thread safety lock annotation in the declaration but are missing the corresponding run-time lock assertion in the definition.
ACKs for top commit:
hebasto:
re-ACK f485a0745455b46390f1c14260643ad69c8fe2ad, only suggested change since my [previous](https://github.com/bitcoin/bitcoin/pull/24177#pullrequestreview-877810465) review.
vasild:
ACK f485a0745455b46390f1c14260643ad69c8fe2ad
Tree-SHA512: c86c0c0e8fe6ec7ae9ed9890f1dd7d042aa482ecf99feb6679a670aa004f6e9a99f7bc047205a34968fab7f1f841898c59b48c3ed6245c166e3b5abbf0867445
|
|
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
|
|
GetIter takes a txid, not wtxid.
|
|
Co-authored-by: Shashwat <svangani239@gmail.com>
|
|
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
|
|
|
|
|
