When mining the first block of a new signet chain, pick a timestamp for
the first block so that after mining 100 blocks the timestamp will be
back to the current time -- this prevents an unnecessary delay before
any miner rewards have matured enough to be spent. This takes into
account that the delta between blocks may be shorter than 10 minutes due
to attempting to increase the difficulty to match --nbits, but does not
take into account the time spent actually generating the 100 blocks.
506e6585a54818d0613a067f91c0bac2f308a48c gui: display plain "Inbound" in peer details (Jon Atack)
Pull request description:
Alternative version to #201.
ACKs for top commit:
MarcoFalke:
ACK 506e6585a54818d0613a067f91c0bac2f308a48c
jonasschnelli:
utACK 506e6585a54818d0613a067f91c0bac2f308a48c
Tree-SHA512: 88d141b14684c1dcdff47f7ba241e5a7c42c14da3d9aaa89f1649235a64fd26bc5a6055707dc07992cd9d8c05d143754f6dd51ccee69fd4309336dd07c52e61c
faf7d7418cf01cb04cd457bcc630654da958a777 fuzz: Avoid extraneous copy of input data, using Span<> (MarcoFalke)
Pull request description:
Seeing speedup here in the fuzz framework part (non-fuzz-target part). Speedup is only visible for input data larger than 100kB.
ACKs for top commit:
practicalswift:
cr ACK faf7d7418cf01cb04cd457bcc630654da958a777: patch looks correct :)
laanwj:
Code review ACK faf7d7418cf01cb04cd457bcc630654da958a777
Tree-SHA512: 41af7118846e0dfee237a6d5269a6c7cfbc775d7bd1cc2a85814cb60f6c2b37fe7fd35f1a788d4f08e6e0202c48b71054b67d2931160c445c79fc59e5347dadf
747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 netinfo: display only outbound block relay counts (Jon Atack)
76d198a5c15a9376c7d3a91754320334337a9e50 netinfo: add i2p network (Jon Atack)
9d6aeca2c5ec1df579c27c39e82fa3ddf1d25986 netinfo: add bip152 high-bandwidth to/from fields (Jon Atack)
5de7a6cf63ef39b0474ea9c90a968f867635d98e netinfo: display manual peers count (Jon Atack)
d3cca3be63afeb19a41e9892444fc6e02ea1c7c8 netinfo: update to use peer connection types (Jon Atack)
62bf5b785087981d9c0f8ddc8a3ceda911845a53 netinfo: add ConnectionTypeForNetinfo member helper function (Jon Atack)
Pull request description:
Merry Bitcoin Christmas! Ho ho ho 🎄✨
This PR updates `-netinfo` to:
- use the getpeerinfo `connection_type` field (and no longer use getpeerinfo `relaytxes` for block-relay detection)
- display manual peers count, if any, in the outbound row
- display the block relay counts in the outbound row only
- display high-bandwidth BIP152 compact block relay peers (`hb` column, to `.` and from `*`)
- add support for displaying I2P network peers, if any are present
Testing and review welcome! How to test:
- to run the full live dashboard (on Linux): `$ watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4`
- to run the full dashboard: ``$ ./src/bitcoin-cli -netinfo 4``
- to see the help: `$ ./src/bitcoin-cli -netinfo help`
- to see the help summary: `$ ./src/bitcoin-cli -help | grep -A4 netinfo`
ACKs for top commit:
laanwj:
re-ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6
michaelfolkson:
ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6
jonasschnelli:
Tested ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 - works nicely. Great that this PR only changes bitcoin-cli.
Tree-SHA512: 48fe23dddf3005a039190fcbc84167cd25b0a63489617fe14ea5db9a641a829b46b6e8dc7924aab6577d82a13909d157e82f715bd2ed3a8a15071957c35c19f3
49797c3ccfbb9f7ac9c1fbb574d35b315c103805 tests: Disable bdb dump test when no bdb (Andrew Chow)
1194cf9269e6c4bf67b09c4766f42bf173d12c0a Fix wallet_send.py wallet setup to work with descriptors (Andrew Chow)
fbaea7bfe44822710a36601c6b0febbd5e33dfbd Require legacy wallet for wallet_upgradewallet.py (Andrew Chow)
b1b679e0ab7a9981e3e78424fe8836edd59abf6f Explicitly mark legacy wallet tests as such (Andrew Chow)
09514e1bef46444a67cde9ff13e76bd4b9f8c7ac Setup wallets for interface_zmq.py (Andrew Chow)
4d03ef9a73ceb5ef4e9d184a135bca6bdeb8c311 Use MiniWallet in rpc_net.py (Andrew Chow)
4de23824b0c711ece68f9fc007ffac12126710aa Setup wallets for interface_bitcoin_cli.py (Andrew Chow)
7c71c627d28f0cddaf2349a55336278a681c27c2 Setup wallets with descriptors for feature_notifications (Andrew Chow)
1f1bef8dbab7225884d769a45477ee11d0ebf654 Have feature_filelock.py test both bdb and sqlite, depending on compiled (Andrew Chow)
c77975abc0123b29b0eb3481b8916e7c025b7c4c Disable upgrades tests that require BDB if BDB is not compiled (Andrew Chow)
1f20cac9d41e507901a2811d6db7147d7ab0321b Disable wallet_descriptor.py bdb format check if BDB is not compiled (Andrew Chow)
3641597d7ef6f5097a9e93cab3ef7e0f9c820296 tests: Don't make any wallets unless wallet is required (Andrew Chow)
b9b88f57a9b9a28e0f0614c12ae3012cf5050b10 Skip legacy wallet reliant tests if BDB is not compiled (Andrew Chow)
6f36242389bd3e7eacf594ce90491e8ccca70f3a tests: Set descriptors default based on compilation (Andrew Chow)
Pull request description:
This PR fixes tests for when BDB is not compiled. Tests which rely on or test legacy wallet behavior are disabled and skipped when BDB is not compiled. For the components of some tests that are for legacy wallet things, those parts of the tests are skipped.
For the majority of tests, changes are made so that they can be run with either legacy wallets or descriptor wallets without materially effecting the test. Most tests only need the wallet for balance and transactions, so the type of wallet is not an important part of those tests. Additionally, some tests are wallet agnostic and modified to instead use the test framework's MiniWallet.
ACKs for top commit:
laanwj:
ACK 49797c3ccfbb9f7ac9c1fbb574d35b315c103805
ryanofsky:
Code review ACK 49797c3ccfbb9f7ac9c1fbb574d35b315c103805. Only change since last review is dropping last commit. Previous review w/ suggestions for future followup is https://github.com/bitcoin/bitcoin/pull/20267#pullrequestreview-581508843
Tree-SHA512: 69659f8a81fb437ecbca962f4082c12835282dbf1fba7d9952f727a49e01981d749af9b09feda1c8ca737516c7d7a08ef17e782795df3fa69892d5021b41c1ed
e1e67148321cff0de9eb5e63d2604f05c12e69d1 doc: refer to BIPs 339/155 in feature negotiation (Jon Atack)
Pull request description:
of `wtxidrelay` and `addrv2`/`sendaddrv2`, and add `fSuccessfullyConnected` doxygen documentation to clarify that it is set to true on VERACK.
ACKs for top commit:
laanwj:
re-ACK e1e67148321cff0de9eb5e63d2604f05c12e69d1
Tree-SHA512: 3e6af5b246e4ee1ec68ee34db525746717871bc986ad4840f5a8edce55740768389f6fd0ec69046eda2fb4c69939440a96571f79d36e6cbff4fd3b7f2ebc74c0
eecb7ab105a4a59d09cd55b124c5ad563846fe11 [doc] clarify -peertimeout and -timeout descriptions (gzhao408)
Pull request description:
The debug-only option `-peertimeout` is used to delay `InactivityCheck()`, whereas the `-timeout` option specifies socket timeouts (`nConnectTimeout`). The current descriptions are a bit misleading and hard to tell apart. I think it would save dev/review time to update them 🤷
ACKs for top commit:
MarcoFalke:
ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11 nice doc fixup
jnewbery:
ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11
Tree-SHA512: 71d2e6c31664b9f7f0b053ecf3be21c6c55472553fa7478d8526ba3be8d54979bceafca63d87b8b2488c11f409c332ac795da613ff8101546b18d9cd8bcceb50
fac4be30482c21ac330e09ef8756c49e37faa6fa fuzz: Configure check for main function (take 2) (MarcoFalke)
Pull request description:
Actually fix https://github.com/google/honggfuzz/issues/336#issuecomment-702972138
Follow-up to #20065
Steps to test: `honggfuzz` section in doc/fuzzing.md
ACKs for top commit:
practicalswift:
cr ACK fac4be30482c21ac330e09ef8756c49e37faa6fa: patch looks correct!
Tree-SHA512: 893768c80439fe5d90b883ade89dc02f5bb80e27637916cf5575b6a9ed0b1c04942ff851342f5bbabb8666e6696715427feeb98f5301ad23c7b87b09e5872f97
faf3b4b53359850e0c47e67ead1c2b6dccc55531 refactor: Treat ArgsManager::Flags as uint32_t explicitly (MarcoFalke)
Pull request description:
The underlying type might be implementation defined, which is probably why the sanitizer kills the fuzz tests.
Fix that by pinning the underlying type.
This refactor does not change behaviour and only affects the sanitizer in tests.
ACKs for top commit:
practicalswift:
cr ACK faf3b4b53359850e0c47e67ead1c2b6dccc55531
Tree-SHA512: d446824836e1037b4200ba3630c8628090678cfad45559866275d8e06349f7c8cdb7e816619f5afb35f9f65299cc00e046d2f81b73cd8eb843e2e15676b647d5
590bda79e876d9b959083105b8c7c41dd87706eb scripted-diff: Remove setup_clean_chain if default is not changed (Fabian Jahr)
98892f39e3d079c73bff7f2a5d5420fa95270497 doc: Improve setup_clean_chain documentation (Fabian Jahr)
Pull request description:
The first commit improves documentation on setup_clean_chain which is misunderstood quite frequently. Most importantly it fixes the TestShell docs which are simply incorrect.
The second commit removes the instances of `setup_clean_clain` in functional tests where it is not changing the default.
This used to be part of #19168 which also sought to rename`setup_clean_chain`.
ACKs for top commit:
jonatack:
ACK 590bda79e876d9b959083105b8c7c41dd87706eb
Tree-SHA512: a7881186e65d31160b8f84107fb185973b37c6e50f190a85c6e2906a13a7472bb4efa9440bd37fe0a9ac5cd2d1e8559870a7e4380632d9a249eca8980b945f3e
20677ffa22e93e7408daadbd15d433f1e42faa86 validation: Guard all chainstates with cs_main (Carl Dong)
Pull request description:
```
This avoids a potential race-condition where a thread is reading the
ChainstateManager::m_active_chainstate pointer while another one is
writing to it. There is no portable guarantee that reading/writing the
pointer is thread-safe.
This is also done in way that mimics ::ChainstateActive(), so the
transition from that function to this method is easy.
More discussion:
1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027
2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961
3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522
4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695
```
Basically this PR removes the loaded-but-unfired footgun, which:
- Is multiplied (but still unshot) in the chainman deglobalization PRs (#20158)
- Is shot in the test framework in the au.activate PR (#19806)
ACKs for top commit:
jnewbery:
code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. I've verified by eye that neither of these members are accessed without cs_main.
ryanofsky:
Code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. It is safer to have these new `GUARDED_BY` annotations and locks than not to have them, but in the longer run I think every `LOCK(cs_main)` added here and added earlier in f92dc6557a153b390a1ae1d0808ff7ed5d02c66e from #20749 should be removed and replaced with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` on the accessor methods instead. `cs_main` is a high level lock that should be explicitly acquired at a high level to prevent the chain state from changing. It shouldn't be acquired recursively in low-level methods just to read pointer values atomically.
Tree-SHA512: 68a3a46d79a407b774eab77e1d682a97e95f1672db0a5fcb877572e188bec09f3a7b47c5d0cc1f2769ea276896dcbe97cb35c861acf7d8e3e513e955dc773f89
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke)
fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)
Pull request description:
This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937Fixes: #20902
ACKs for top commit:
ajtowns:
ACK fa61b9d1a68820758f9540653920deaeae6abe79
fjahr:
Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79
Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
f22a3ec1403293f45a06558d105f0da624c5ebed build: make macOS HOST in download-osx generic (fanquake)
Pull request description:
This was missed in #20419, and the update before that, so just make this non-versioned so that we don't have to worry about it. This is fine, because it's just for downloading sources.
ACKs for top commit:
RandyMcMillan:
ACK f22a3ec1403293f45a06558d105f0da624c5ebed
dongcarl:
utACK f22a3ec1403293f45a06558d105f0da624c5ebed
Tree-SHA512: 3a6993a69594a793a5185e4ba48858443a1002a37b96ff881d39ca7719c79432b35d709bd9a9379f8046bdbeb716c5e1598f273a7e7e3f3bf528b6a807abe5ec
the i2p peer counts column is displayed iff the node is connected
to at least one i2p peer, so this doesn't add clutter for users
who are not running an i2p service
572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 doc: More precise -debug and -debugexclude doc (wodry)
Pull request description:
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.
This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
ACKs for top commit:
laanwj:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
MarcoFalke:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
theStack:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
This was missed in #20419, and the update before that, so just
make this un-versioned so that we don't have to worry about it.
This is fine, because it's just for downloading sources.
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev)
381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev)
e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev)
4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev)
f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev)
dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev)
Pull request description:
This PR introduces per-peer message capture into Bitcoin Core. 📓
## Purpose
The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?".
## Functionality
When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue.
The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so:
```
message_capture/203.0.113.7:56072/msgs_recv.dat
message_capture/203.0.113.7:56072/msgs_sent.dat
```
Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option.
## Future Maintenance
I sympathize greatly with anyone who says "the best code is no code".
The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool.
Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small.
## FAQ
"Why not just use Wireshark"
Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark.
Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done.
Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable.
ACKs for top commit:
MarcoFalke:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚
jnewbery:
utACK bff7c66e67aa2f18ef70139338643656a54444fe
theStack:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe
Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
Since these chainstates are:
1. Also vulnerable to the race condition described in the previous
commit
2. Documented as having similar semantics as m_active_chainstate
we should also protect them with ::cs_main.
b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb build: Add -Werror=mismatched-tags (Hennadii Stepanov)
1485124291368c4a2ca8ea09c18e813f1dbabf5c Fix -Wmismatched-tags warnings (Hennadii Stepanov)
Pull request description:
Warnings were introduced in #20749:
```
./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
class CCheckpointData;
^
./chainparams.h:24:8: note: previous use is here
struct CCheckpointData {
^
./validation.h:43:1: note: did you mean struct here?
class CCheckpointData;
^~~~~
struct
1 warning generated.
```
This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435
ACKs for top commit:
glozow:
utACK b6aadcd5b4🚗
practicalswift:
cr ACK b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb: patch looks correct
Tree-SHA512: 3ac887ebdbf9a1ae33c1fd5381b3b8d83388ad557ddeb55013acd42bb9752a5bd009e3a0eed52644a023a7a0dda1c159277981af82f58fb0abfe60b84e01bf29
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.
This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
fa292724598c273867bc6dbf311f1440fe2541ba Remove redundant MakeUCharSpan wrappers (MarcoFalke)
faf4aa2f47c0de4f3a0c5f5fe5b3ec32f611eefd Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke)
fada14b948cac147198e3b685b5dd8cb72dc2911 Treat CDataStream bytes as uint8_t (MarcoFalke)
fa8bdb048e65cae2d26bea3f991717a856e2fb39 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke)
faa96f841fe45bc49ebb6e07ac82a129fa9c40bf Remove unused CDataStream methods (MarcoFalke)
Pull request description:
Using `uint8_t` for raw bytes has a style benefit:
* The signedness is clear from reading the code, as it does not depend on the architecture
Other clean-ups in this pull include:
* Remove unused methods
* Constructor is simplified with `Span`
* Remove `Init()` member in favor of C++11 member initialization
ACKs for top commit:
laanwj:
code review ACK fa292724598c273867bc6dbf311f1440fe2541ba
theStack:
ACK fa292724598c273867bc6dbf311f1440fe2541ba 🍾
Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
cc30dfbd4bab5fe4acc82dca9f6979e5a3a41e32 doc: Document use of make-tag script to make tags (Wladimir J. van der Laan)
Pull request description:
To make release tags the `make-tag.py` script from the maintainer tools should be used. This ensures that all the various occurrences of the version in different files match the tagged version before proceeding. And move it into a separate section with the other per-release actions.
Also replace other "ping wumpus" references.
ACKs for top commit:
jonatack:
ACK cc30dfbd4bab5fe4acc82dca9f6979e5a3a41e32
Tree-SHA512: c09748a0bea85573b3f04fdb86430a53b683ff4d956edc1f49d471e2e526715cbde7cf6ad83a43a1a3fca4ff3c5af011e3d1b8cb61f5d8e065cfa71ba0138c88
dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f refactor: remove boost::thread_group usage (fanquake)
Pull request description:
Post #18710, there isn't much left using `boost::thread_group`, so should just be able to replace it with the standard library. This also removes the last use of `boost::thread_interrupted`.
After this change, last piece of Boost Thread we'd be using is `boost::shared_mutex`. See the commentary [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) as to why it may be non-trivial to swap that for `std::shared_mutex` in the near future.
Closes#17307
ACKs for top commit:
laanwj:
Code review re-ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f
MarcoFalke:
review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f 🔁
jonatack:
Non-expert code review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f, also checked range-diff since last review and that local debug build is clean with gcc 10.2.1-6 on Debian
Tree-SHA512: 5510e2d760cce824234207dc86b1551ca8f21cbf3a2ce753c0254a0d03ffd83c94e449aec202fb7bd76e6fc64df783a6b70a736b0add9ece3734bb9c8ce8fc2f
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong)
b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong)
0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong)
ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong)
e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong)
5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong)
2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong)
9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong)
0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong)
d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong)
f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong)
e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong)
b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong)
3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong)
eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong)
15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong)
f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
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:
jnewbery:
utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
laanwj:
re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
ryanofsky:
Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review:
Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
74d23bf7fbb6169ec658c36af57cd1f937823d9c rpc: document RPC_TRANSACTION_ALREADY_IN_CHAIN exception (Jarol Rodriguez)
Pull request description:
It is not documented in the `RPCHelpMan` of `sendrawtransaction` that if you attempt to send a transaction which already exists in a block, an `RPC_TRANSACTION_ALREADY_IN_CHAIN` exception will be raised. It is best to make developers aware of this so that it can be properly caught and avoid any headaches.
Closes#5638
ACKs for top commit:
jonatack:
ACK 74d23bf7fbb6169ec658c36af57cd1f937823d9c
Tree-SHA512: d1d5fc242574377c8a76b4ef7b12239996424d8bee186533b5a8fe337bbeb3186e51dbdd28c5eafb982601e44e17b68a7f52db5dd7bc647429f6f95e2de289f6
c943282b5e6312537f885c811d43120ce2f5b766 validation: remove redundant check on pindex (jarolrod)
Pull request description:
This removes a redundant check on `pindex` being a `nullptr`. By the time we get to this step `pindex` is always a `nullptr` as the branch where it has been set would have already returned.
Closes#19223
ACKs for top commit:
Zero-1729:
re-ACK c943282
ajtowns:
ACK c943282b5e6312537f885c811d43120ce2f5b766 - code review only
MarcoFalke:
review ACK c943282b5e6312537f885c811d43120ce2f5b766 📨
theStack:
re-ACK c943282b5e6312537f885c811d43120ce2f5b766
Tree-SHA512: d2dc58206be61d2897b0703ee93af67abed492a80e84ea03dcfbf7116833173da3bdafa18ff80422d5296729d5254d57cc1db03fdaf817c8f57c62c3abef673c
