Age | Commit message (Collapse) | Author |
|
fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f doc: Remove irrelevant link to GitHub (MarcoFalke)
fa121b628d51bb0e25eb3fbd716881fa55527dc7 blockstorage: [refactor] Use chainman reference where possible (MarcoFalke)
fa0c7d9ad24d3c9515d3f9c136af4071cbd79055 move-only: Move *Disk functions to blockstorage (MarcoFalke)
fa91b2b2b3447a3645e7958c7dc4e1946a69cb9c move-only: Move AbortNode to shutdown (MarcoFalke)
fa413f07a14744e7d7f7746e861aabd9cf938f61 move-only: Move ThreadImport to blockstorage (MarcoFalke)
faf843c07f99f91603e08ea858f972516f1d669a refactor: Move load block thread into ChainstateManager (MarcoFalke)
Pull request description:
This picks up the closed pull request #21030 and is the first step toward fixing #21220.
The basic idea is to move all disk access into a separate module with benefits:
* Breaking down the massive files init.cpp and validation.cpp into logical units
* Creating a standalone-module to reduce the mental complexity
* Pave the way to fix validation related circular dependencies
* Pave the way to mock disk access for testing, especially where it is performance critical (like fuzzing)
ACKs for top commit:
promag:
Code review ACK fadcd3f78e, checked (almost) moved only changes. This is a nice tidy up change and doesn't change behavior. Easily reviewed commit by commit.
jamesob:
ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f ([`jamesob/ackr/21575.1.MarcoFalke.refactor_create_blocksto`](https://github.com/jamesob/bitcoin/tree/ackr/21575.1.MarcoFalke.refactor_create_blocksto))
ryanofsky:
Code review ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f. New organization makes sense, moves extraneous things outside of validation.cpp. PR is also easy to review with helpfully split up moveonly commits.
Tree-SHA512: 917996592b6d8f9998289d8cb2b1b78b23d1fdb3b07216c9caec1380df33baa09dc2c1e706da669d440b497e79c9c62a01ca20dc202df5ad974a75f3ef7a143b
|
|
This just makes an additional simplification after #21366 replaced
util::Ref with std::any. It was originally suggested
https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but
delayed for a followup. It would have prevented usage bug
https://github.com/bitcoin/bitcoin/pull/21572.
|
|
The doc nicely explains why the directory exists and it is
irrelevant when it was introduced. Even if it was relevant,
it could be trivially found out via `git log ./src/node/ | tail`
without visiting GitHub
|
|
Also, add missing { } for style.
Can be reviewed with `--word-diff-regex=.`
|
|
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
693414d27181cf967f787a2ca72344e52c58c7f0 node/ifaces: ChainImpl: Use an accessor for ChainMan (Carl Dong)
98c4e252f0d09bebb2e4ad3289407459c2cda5d5 node/ifaces: NodeImpl: Use an accessor for ChainMan (Carl Dong)
7e8b5ee814b0b8c34acb20637ed4fc988ccba555 validation: Make BlockManager::LookupBlockIndex const (Carl Dong)
88aead263c61d86e5f836028f517cfbf2a575498 node: Avoid potential UB by asserting assumptions (Carl Dong)
1dd8ed7a8491e51b76eeb236b15b794d9254f674 net_processing: Move comments to declarations (Carl Dong)
07156eb387ea580be5e2ce4a1744992ce7575903 node/coinstats: Replace #include with fwd-declaration (Carl Dong)
7b8e976cd5ac78a22f1be2b2fed8562c693af5d9 miner: Add chainstate member to BlockAssembler (Carl Dong)
e62067e7bcad5a559899afff2e4a8e8b7e9f4301 Revert "miner: Pass in chainstate to BlockAssembler::CreateNewBlock" (Carl Dong)
eede0647b06b6009080c4e536a2705e911d6ee19 Revert "scripted-diff: Invoke CreateNewBlock with chainstate" (Carl Dong)
0c1b2bc549aec77b247f0103652d883227841ac5 Revert "miner: Remove old CreateNewBlock w/o chainstate param" (Carl Dong)
Pull request description:
Chronological history of this changeset:
1. Bundle 4 (#21270) got merged
2. Posthumous reviews were posted
3. These changes were prepended in bundle 5
4. More reviews were added in bundle 5
5. Someone suggested that we split the prepended changes up to another PR
6. This is that PR
In the future, I will just do posthumous review changes in another PR instead. I apologize for the confusion.
Addresses posthumous reviews on bundle 4:
- From jnewbery:
- https://github.com/bitcoin/bitcoin/pull/21270#issuecomment-796738048
- I didn't fix this one, but I added a `TODO` comment so that we don't lost track of it
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592291225
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592296942
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592299738
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592301704
- From MarcoFalke:
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593096212
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593097032
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593097867
- https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593100570
Addresses reviews on bundle 5:
- Checking chainman existence before locking cs_main
- MarcoFalke
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601776
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601876
- Appropriate locking, usage of chainman, and control flow in `src/node/interfaces.cpp`
- MarcoFalke
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601383
- jnewbery
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597029360
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597029921
- ryanofsky
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597163828
- Style/comment formatting changes
- jnewbery
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597026552
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597027186
- Making LookupBlockIndex const
- jnewbery
- https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597035062
ACKs for top commit:
MarcoFalke:
review ACK 693414d27181cf967f787a2ca72344e52c58c7f0 🛐
ryanofsky:
Code review ACK 693414d27181cf967f787a2ca72344e52c58c7f0. I reviewed this previously as part of #21391. I am a fan of the increasingly complicated bundle numbering, and kind of hope there in the next round there is some way we can get bundles 5.333333 and 5.666667!
jamesob:
ACK 693414d27181cf967f787a2ca72344e52c58c7f0 ([`jamesob/ackr/21525.1.dongcarl.bundle_4_5_n_followup_f`](https://github.com/jamesob/bitcoin/tree/ackr/21525.1.dongcarl.bundle_4_5_n_followup_f))
Tree-SHA512: 9bdc199f70400d01764e1bd03c25bdb6cff26dcef60e4ca3b649baf8d017a2dfc1f058099067962b4b6ccd32d078002b1389d733039f4c337558cb70324c0ee3
|
|
916ab0195d567fd0a9097045e73a6654c453adea remove unused class util::Ref and its unit test (Sebastian Falbesoner)
8dbb87a3932f81e23ba7afd865b9aeeb535f0c20 refactor: replace util::Ref by std::any (C++17) (Sebastian Falbesoner)
95cccf8a4b392959c1fd7ec0647e04eb13880865 util: introduce helper AnyPtr to access std::any instances (Sebastian Falbesoner)
Pull request description:
As described in `util/ref.h`: "_This implements a small subset of the functionality in C++17's std::any class, and **can be dropped when the project updates to C++17**_". For accessing the contained object of a `std::any` instance, a helper template function `AnyPtr` is introduced (thanks to ryanofsky).
ACKs for top commit:
hebasto:
re-ACK 916ab0195d567fd0a9097045e73a6654c453adea, with command
ryanofsky:
Code review ACK 916ab0195d567fd0a9097045e73a6654c453adea. Changes since last review: rebase and replacing types with `auto`. I might have used `const auto*` and `auto*` instead of plain `auto` because I think the qualifiers are useful, but this is all good.
Tree-SHA512: fe2c3e4f5726f8ad40c61128339bb24ad11d2c261f71f7b934b1efe3e3279df14046452b0d9b566917ef61d5c7e0fd96ccbf35ff810357e305710f5002c27d47
|
|
|
|
|
|
|
|
3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 [net] remove fUpdateConnectionTime from FinalizeNode (John Newbery)
7c4cc67c0c3c50df004ee53cac5b2884b7fbab29 [net] remove CConnman::AddNewAddresses (John Newbery)
bcd7f30b7944892db7ae37069175804567bb0cdf [net] remove CConnman::MarkAddressGood (John Newbery)
8073673dbcb2744fcc9c011edf2d61388ca929cd [net] remove CConnman::SetServices (John Newbery)
392a95d393a9af01b53e5e68197e81968efb84fc [net_processing] Keep addrman reference in PeerManager (John Newbery)
1c25adf6d278eb1a1f018986a126d0eb8137e0ee [net] Construct addrman outside connman (John Newbery)
Pull request description:
Addrman is currently a member variable of connman. Make it a top-level component with lifetime owned by node.context, and add a reference to addrman in peerman. This allows us to eliminate some functions in connman that are simply forwarding requests to addrman, and simplifies the connman-peerman interface.
By constructing the addrman in init, we can also add parameters to the ctor, which allows us to test it better. See #20233, where we enable consistency checking for addrman in our functional tests.
ACKs for top commit:
MarcoFalke:
re-ACK 3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 only change is squash 🏀
vasild:
ACK 3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682
Tree-SHA512: 17662c65cbedcd9bd1c194914bc4bb4216f4e3581a06222de78f026d6796f1da6fe3e0bf28c2d26a102a12ad4fbf13f815944a297f000e3acf46faea42855e07
|
|
|
|
4f2653a89018fa4d24bd2a551832a7410b682600 test: Use deterministic chain in utxo set hash test (Fabian Jahr)
4973c5175c5fd1f4791ea26e8ddefd6fb11ac1c3 test: Remove wallet dependency of utxo set hash test (Fabian Jahr)
1a27af1d7b5ec18b4248ead1eaf0f381047b4b24 rpc: Improve gettxoutsetinfo help (Fabian Jahr)
Pull request description:
Follow-ups to #19145:
- Small improvement on the help text of RPC gettxoutsetinfo
- Using deterministic blockchain in the test `functional/feature_utxo_set_hash.py`
- Removing wallet dependency in the test `functional/feature_utxo_set_hash.py`
Split out of #19521.
ACKs for top commit:
MarcoFalke:
review ACK 4f2653a89018fa4d24bd2a551832a7410b682600 👲
Tree-SHA512: 92927b3aa22b6324eb4fc9d346755313dec44d973aa69a0ebf80a8569b5f3a7cf3539721ebdba183737534b9e29b3e33f412515890f0d0b819878032a3bba8f9
|
|
|
|
|
|
node.context owns the CAddrMan. CConnman holds a reference to
the CAddrMan.
|
|
680eb56d828ce358b4e000c140f5b247ff5e6179 [net processing] Don't pass CConnman to RelayTransactions (John Newbery)
a38a4e8f039dfabfd9435f3a63f1a9b56de086d6 [net processing] Move RelayTransaction into PeerManager (John Newbery)
Pull request description:
This is the first part of #21160. It moves the RelayTransaction() function to be a member function of the PeerManager class. This is required in order to move the transaction inventory data into the Peer object, since Peer objects are only accessible from within PeerManager.
ACKs for top commit:
ajtowns:
ACK 680eb56d828ce358b4e000c140f5b247ff5e6179
Tree-SHA512: 8c93491a4392b6369bb7f090de326a63cd62a088de59026e202f226f64ded50a0cf1a95ed703328860f02a9d2f64d3a87ca1bca9a6075b978bd111d384766235
|
|
|
|
-BEGIN VERIFY SCRIPT-
git rm src/optional.h
sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src)
sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src)
sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src)
sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e '/optional.h \\/d' src/Makefile.am
sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp
sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src)
-END VERIFY SCRIPT-
|
|
1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff doc: update developer notes for removal of MakeUnique (fanquake)
3ba2840e7ee81341b0748c0121aedc2e9305041a scripted-diff: remove MakeUnique<T>() (fanquake)
Pull request description:
Since requiring C++17, this is just pointless abstraction. I think we should just "tear the band-aid off" and remove it. Similar to the changes happening in #21366.
Also, having a comment saying this is deprecated doesn't prevent it's usage in new code. i.e : https://github.com/bitcoin/bitcoin/pull/20946#discussion_r561949731.
The repository is fairly quiet at the moment, so any potential complaints about having to rebase should be minimal. Might as well get this over and done with.
ACKs for top commit:
jnewbery:
utACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff
practicalswift:
cr ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff: patch looks correct
ajtowns:
ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff -- code review only
glozow:
ACK https://github.com/bitcoin/bitcoin/commit/1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff looks correct
Tree-SHA512: 4a14b9611b60b9b3026b54d6f5a2dce4c5d9b63a7b93d7de1307512df736503ed84bac66e7b93372c76e3117f49bf9f29cd473d3a47cb41fb2775bc10234736f
|
|
a67983cd6d8e61565da4e03f3ba401d0148fe195 net_processing: Add review-only assertion to PeerManager (Carl Dong)
272d993e759e7fcfe883b84e9a2a3be3c75177ec scripted-diff: net_processing: Use existing chainman (Carl Dong)
021a04a46915468e7508a6ef44e7fbab1426343d net_processing: Move some static functions to PeerManager (Carl Dong)
91c5b68acd12cf7c2b4888d54d8fdd21837b2817 node/ifaces: ChainImpl: Use existing NodeContext member (Carl Dong)
8a1d580b2156268e3ab30f902b3fc9aa87bd2819 node/ifaces: NodeImpl: Use existing NodeContext member (Carl Dong)
4cde4a701b8856ac4ec9721b0226dbbfc52a71c3 node: Use existing NodeContext (Carl Dong)
106bcd4f390137904b5579cfef023fb8a5c8b4b5 node/coinstats: Pass in BlockManager to GetUTXOStats (Carl Dong)
2c3ba006930a5bbbf5a33bd530f3c1b2c4103c74 miner: Pass in blockman to ::RegenerateCommitments (Carl Dong)
2afcf24408b4453e4418ebfb326b141f6ea8647c miner: Remove old CreateNewBlock w/o chainstate param (Carl Dong)
46b7f29340acb399fbd2378508a204d8d8ee8fca scripted-diff: Invoke CreateNewBlock with chainstate (Carl Dong)
d0de61b764fc7e9c670b69d8210705da296dd245 miner: Pass in chainstate to BlockAssembler::CreateNewBlock (Carl Dong)
a04aac493fd564894166d58ed4cdfd9ad4f561cb validation: Remove extraneous LoadGenesisBlock function prototype (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Based on:
- [x] #21055 | [Bundle 3/n] Prune g_chainman usage in mempool-related validation functions
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
laanwj:
Code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195
ryanofsky:
Code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195. Only change since last review new first commit fixing header declaration, and rebase
glozow:
code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195
Tree-SHA512: dce182a18b88be80cbf50978d4ba8fa6ab0f01e861d09bae0ae9364051bb78f9334859d164b185b07f1d70a583e739557fab6d820cac8c37b3855b85c2a6771b
|
|
-BEGIN VERIFY SCRIPT-
git rm src/util/memory.h
sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src)
sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src)
sed -i -e '/util\/memory.h \\/d' src/Makefile.am
-END VERIFY SCRIPT-
|
|
fa4e088cbac035b8029a10b492849540150d0622 wallet: Mark replaced tx to not be in the mempool anymore (MarcoFalke)
Pull request description:
The wallet does not mark the replaced tx as out-of-mempool. This causes failures in user scripts, because later RPCs may depend on this state change from `bumpfee`.
For example, the following might fail on current master:
```
txid = sendtoaddress(...)
bumpfee(txid)
abandontransaction(txid) # fails because txid is still marked as "in mempool"
```
Fixes #18831
ACKs for top commit:
meshcollider:
utACK fa4e088cbac035b8029a10b492849540150d0622
ryanofsky:
Code review ACK fa4e088cbac035b8029a10b492849540150d0622, and previous ACK faeedff5c87091fd83d2fb2b29eb49c948363f29 is also still valid in case there's a preference for the original fix
Tree-SHA512: 9858f40f5fb5a43a7b584b5c4268b6befa82e6a84583be5206fe721bcb6c255e8d35479d347d0b9aed72703df49887c02b14ab680e8efdd28b90dd6b93d9439a
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Use the local m_connman instead
|
|
We don't mark RelayTransaction as const. Even though it doesn't mutate
PeerManagerImpl state, it _is_ mutating the internal state of a CNode
object, by updating setInventoryTxToSend. In a subsequent commit, that
field will be moved to the Peer object, which is owned by
PeerMangerImpl.
This requires PeerManagerImpl::ReattemptInitialBroadcast() to no longer
be const.
|
|
-BEGIN VERIFY SCRIPT-
find_regex='\bAcceptToMemoryPool\(' \
&& git grep -l -E "$find_regex" -- src \
| grep -v '^src/validation\.\(cpp\|h\)$' \
| xargs sed -i -E 's@'"$find_regex"'@\0::ChainstateActive(), @g'
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
find_regex='\bCheckFinalTx\(' \
&& git grep -l -E "$find_regex" -- src \
| grep -v '^src/validation\.\(cpp\|h\)$' \
| xargs sed -i -E 's@'"$find_regex"'@\0::ChainActive().Tip(), @g'
-END VERIFY SCRIPT-
|
|
Values for mainnet and testnet will be specified in a follow-up PR that can be
scrutinized accordingly. This structure is required for use in snapshot activation
logic.
|
|
e987ae5a554c9952812746c29f2766bacea4b727 test: Add test for deterministic UTXO set hash results (Fabian Jahr)
6ccc8fc067bf516cda7bc5d7d721945be5ac2003 test: Add test for gettxoutsetinfo RPC with MuHash (Fabian Jahr)
0d3b2f643d7da3202c0a0e757539208c4aa7c450 rpc: Add hash_type MUHASH to gettxoutsetinfo (Fabian Jahr)
2474645f3b15687e7f196b89eb935d6e6a98a9da refactor: Separate hash and stats calculation in coinstats (Fabian Jahr)
a1fcceac69097a8e6540a6fd8121a5d53022528f refactor: Improve encapsulation between MuHash3072 and Num3072 (Fabian Jahr)
Pull request description:
This is another Pr in the series PRs for Coinstatsindex (see overview in #18000). This PR adds the `hash_type` option `muhash` to `gettxoutsetinfo` through which the user can calculate the serialized muhash of the utxo set. This PR does not use the index yet.
ACKs for top commit:
Sjors:
tACK e987ae5
achow101:
ACK e987ae5a554c9952812746c29f2766bacea4b727
jonatack:
Tested re-ACK e987ae5a554c9952812746c29f2766bacea4b727 per `git diff 3506d90 e987ae5`, reviewed diff, debug built, ran gettxoutsetinfo -signet and help on this branch vs master, at height 23127 both returned `hash_serialized_2` of `2b72d65f3b6efb2311f58374ea2b939abf49684d44f4bafda45faa3b5452a454` and this branch returned `muhash` of `c9f1ff12d345ccf9939c6bbf087e6f7399b6115adee1569287e9c5c43dbb475c`
ryanofsky:
Code review ACK e987ae5a554c9952812746c29f2766bacea4b727. Looks very good. I left one suggestion to simplify code, but feel free to ignore it here and maybe consider it for later since PR has already had a lot of review.
Tree-SHA512: 9a739ce375e73749fa69a467262b60d3e5314ef384e2d7150b3bbc8e4125cd9fd1db95306623bb9a632fcbaf5d9d2bf2f5cc43bf717d4ff5e2c9c4b52dd9296c
|
|
This creates a cleaner interface with ATMP, allows us to make results const,
and makes accessing values that don't make sense (e.g. fee when tx is
invalid) an error.
|
|
Also small style fix in rpc/util.cpp
|
|
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
FindForkInGlobalIndex only acts on BlockManager.
Note to reviewers: Since FindForkInGlobalIndex is always called with
::ChainActive() as its first parameter, it is possible to move
FindForkInGlobalIndex to CChainState and remove this const CChain&
parameter to instead use m_chain. However, it seems like the original
intention was for FindForkInGlobalIndex to work with _any_ chain, not
just the current active chain. Let me know if this should be changed.
|
|
[META] In a previous commit, we moved ::LookupBlockIndex to become a
member function of BlockManager. This commit is split out from
that one since it can be expressed nicely as a scripted-diff.
-BEGIN VERIFY SCRIPT-
find_regex='LookupBlockIndex' \
&& git grep -l -E "$find_regex" -- src \
| grep -v '^src/validation\.\(cpp\|h\)$' \
| xargs sed -i -E "s@${find_regex}@g_chainman.m_blockman.LookupBlockIndex@g"
-END VERIFY SCRIPT-
|
|
|
|
|
|
This commit does not change behavior.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54 [net processing] Add RemovePeer() (John Newbery)
a20ab22786466fe5164b53e62de9d23a4062fbca [net processing] Make GetPeerRef const (John Newbery)
ed7e469ceec6f7101a3fb7b15c21a6fb69697866 [net_processing] Move peer_map to PeerManager (John Newbery)
a529fd3e3f2391e592ac937e291fec51e067ea2e [net processing] Move GetNodeStateStats into PeerManager (John Newbery)
Pull request description:
This moves `g_peer_map` from a global in net_processing.cpp's unnamed namespace to being a member `m_peer_map` of `PeerManager`.
ACKs for top commit:
theuni:
Re-ACK 3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54.
dongcarl:
Re-ACK 3025ca9
hebasto:
re-ACK 3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54, since my [previous](https://github.com/bitcoin/bitcoin/pull/19910#pullrequestreview-545574237) review only reverted the change that introduced NRVO in `PeerManager::GetPeerRef`, and comments are fixed in the proper commits.
Tree-SHA512: 6369eb3c688ac5b84f89f7674115f78ff02edbed76063ac2ebb1759894c9e973883e10821a35dab92bd3d738280acc095bd5368f552a060b83cd309330387d47
|
|
Suggested https://github.com/bitcoin/bitcoin/pull/19425#discussion_r456236407
|
|
This just drops three interfaces::Chain methods replacing them with other calls.
Motivation for removing these chain methods:
- Need to get rid of findFirstBlockWithTimeAndHeight for #10102, which doesn't
support overloaded methods
- Followup from
https://github.com/bitcoin/bitcoin/pull/16426#discussion_r412487403
- phantomcircuit comments about findNextBlock test
http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-06.html#l-214
Behavior is not changing in any way here. A TODO comment in
ScanForWalletTransactions was removed, but just because it was invalid (see
https://github.com/bitcoin/bitcoin/pull/19195#discussion_r448020762), not
because it was implemented.
|
|
estimates global)
4e28753f60613ecd35cdef87bef5f99c302c3fbd feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c16997bdc7e22a20eca16e234290b7ff init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202bfb9d9b50800b8ffe3fead3f77f5fa Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957ab7e3b6aece82b9561774648094f54 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)
Pull request description:
If the `blocksonly` mode is turned on after running with transaction
relay enabled for a while, the fee estimation will serve outdated data
to both the internal wallet and to external applications that might be
feerate-sensitive and make use of `estimatesmartfee` (for example a
Lightning Network node).
This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.
This fixes #16840, and closes #16890 which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
> If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).
ACKs for top commit:
MarcoFalke:
re-ACK 4e28753f60 👋
jnewbery:
utACK 4e28753f60613ecd35cdef87bef5f99c302c3fbd
Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8
|