aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-01-24Better error messages for invalid addressesBezdrighin
This commit addresses #20809. We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
2021-01-22Merge #20927: [refactor] [net] Clean up InactivityCheck()MarcoFalke
bf100f8170770544fb39ae6802175c564cde532f [net] Cleanup InactivityChecks() and add commenting about time (John Newbery) 06fa85cd50b718fecd69f0481740d2b8714a1397 [net] InactivityCheck() takes a CNode reference (John Newbery) Pull request description: This is a pure refactor and should not change any behavior. It clarifies and documents the InactivityCheck() function This makes #20721 easier to review. In particular, this function uses a mixture of (unmockable) system time and mockable time. It's important to understand where those are being used when reviewing #20721. #20721 doesn't require this change, so if others don't agree that it's useful and makes review easier, then I'm happy to close this and just do #20721 directly. ACKs for top commit: fanquake: ACK bf100f8170770544fb39ae6802175c564cde532f MarcoFalke: review ACK bf100f8170770544fb39ae6802175c564cde532f 💫 Tree-SHA512: 7b001de2a5fbe8a6dc37baeae930db5775290afb2e8a6aecdf13161f1e5b06ef813bc6291d8ee5cefcf1e430c955ea702833a8db84192eebe6e6acf0b9304cb2
2021-01-22Merge #20985: doc: add xorriso to macOS depends packagesfanquake
5b41d84b3469484ff6a1f4eb7c890b6444a16091 doc: add xorriso to macOS depends packages (fanquake) Pull request description: This was missed in #20470. ACKs for top commit: hebasto: ACK 5b41d84b3469484ff6a1f4eb7c890b6444a16091, tested on Linux Mint 20.1 (x86_64). Tree-SHA512: bcfd8468a099c69175f8a9d295c1466764ab25d6a61121b28675a09c3e96f45b6309e1523d341f4cb21d0ddee4945f00ba060ba02da835f2f0db7e694fd6c44b
2021-01-22doc: add xorriso to macOS depends packagesfanquake
This was missed in #20470.
2021-01-22Merge #20523: zmq: deduplicate 'sequence' publisher message creation/sendingfanquake
962444295d92744e6f671dac2462037f9f3d79d7 zmq: deduplicate 'sequence' publisher message creation/sending (Sebastian Falbesoner) Pull request description: This small PR deduplicates common low-level creation and sending code for the 'sequence' zmq publisher message (methods `NotifyBlock{Connect,Disconnect}()`, `NotifyTransaction{Acceptance,Removal}()` in the class `CZMQPublishSequenceNotifier`) by introducing a helper function. ACKs for top commit: jonatack: Code review re-ACK 962444295d92744e6f671dac2462037f9f3d79d7 per `git diff f231ffd 9624442` instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/20523/commits/962444295d92744e6f671dac2462037f9f3d79d7 Tree-SHA512: de0750d923f36d1a5751331e88eec8a1605cb88c93318830913210485e2bff712310484f18a0fb626df6ef32ce0b0cf57f4421ce62656e05fce7011a0c3c2d0e
2021-01-22Merge #17920: guix: Build support for macOSfanquake
f1694757ddbcb3635213b085e864851e285c8c12 guix: Fix typo (Carl Dong) 771c4b98a8693eee642f2b118b3193fe6e022291 guix: README: Add darwin HOSTS entry (Carl Dong) 8dbf18cb1d3260d34ba822ceb12e67b1f124ea13 guix: Check for macOS SDK before building anything (Carl Dong) 34b23f597ec52efb795d72e9e5620712d0010edd guix: Set ZERO_AR_DATE for darwin build determinism (Carl Dong) f3835dc6a3732dcd4afbb5987f84dc27f2bf55af build: Make xorrisofs reproducible with -volume_date (Carl Dong) c9eb4cf3a0f81bfd72f06fd43b5610f0a4f5e804 guix: Add support for darwin builds (Carl Dong) 37fe73a092b08fe9d7ce636a1021429de6cda757 build: Add var printing target to src/Makefile.am (Carl Dong) Pull request description: This PR brings our Guix builds on par with Gitian in terms of supported architectures. Reviewers: if you run a build, please submit: ``` find output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum ``` So that we can compare hashes and ensure reproducibility! ACKs for top commit: fanquake: ACK f1694757ddbcb3635213b085e864851e285c8c12 - I think we can make some small usability improvements, but this is ok to merge now. Tree-SHA512: 4af2b71654a9736467dcc681d10601c6eee37800d7847011a50585455b67b55d61742ca5604585f310a2fd75335b674e5e27dfb5169cb2f26e112aa4c411d8be
2021-01-22Merge #20983: Fix MSVC build after gui#176fanquake
c5354e4641d8a92807e4183894d4bb32241e4b5b Fix MSVC build after gui#176 (Hennadii Stepanov) Pull request description: Close #20982. ACKs for top commit: fanquake: ACK c5354e4641d8a92807e4183894d4bb32241e4b5b Tree-SHA512: 548ae0616fad0373950fa842d7009dbf285882e8317767c699903eeca90a90cefb7a03674ddeced9a4ac8c25f3c35dfed3a6d6da1aeb6609feb19a83b1b170ee
2021-01-21Fix MSVC build after gui#176Hennadii Stepanov
2021-01-21Merge bitcoin-core/gui#176: Fix TxViewDelegate layoutMarcoFalke
af58f5b12cea91467692dd4ae71d8cc916a608ed qt: Stop the effect of hidden widgets on the size of QStackedWidget (Hennadii Stepanov) f0d04795e23606399414d074d78efe5aa0da7259 qt: Fix TxViewDelegate layout (Hennadii Stepanov) d43992140679fb9a5ebc7850923679033f9837f3 qt: Add TransactionOverviewWidget class (Hennadii Stepanov) Pull request description: This change: - prevents overlapping date and amount strings - guaranties that "eye" sign at the end of the watch-only address/label is always visible Fix https://github.com/bitcoin/bitcoin/issues/20826 Here are some screenshots with this PR with the _minimum available width_ of the transaction list widget: ![Screenshot from 2021-01-03 20-23-56](https://user-images.githubusercontent.com/32963518/103486411-6408ca00-4e06-11eb-9c21-627a65e532c1.png) ![Screenshot from 2021-01-03 20-24-47](https://user-images.githubusercontent.com/32963518/103486413-6834e780-4e06-11eb-8221-478d98bbdf69.png) ![Screenshot from 2021-01-03 20-25-27](https://user-images.githubusercontent.com/32963518/103486418-6d923200-4e06-11eb-8625-a4ed3089b6ab.png) ![Screenshot from 2021-01-03 20-33-20](https://user-images.githubusercontent.com/32963518/103486420-708d2280-4e06-11eb-90c2-f2463fb3c4b3.png) ACKs for top commit: dooglus: ACK af58f5b. jarolrod: re-ACK af58f5b12cea91467692dd4ae71d8cc916a608ed Tree-SHA512: 6dae682490ec50fa0335d220bc2d153fa3e6ed578f07c6353a3b180f8f6cf1c2f9e52ebd7b3076f51d7004d86bf5cca14e6b5db9cdf786e85a57a81eacbb4988
2021-01-21Merge bitcoin-core/gui#171: Use layout manager for Create Wallet dialogMarcoFalke
d4feb6812a2707ef85d75dda4372086ec62eb922 qt: Use layout manager for Create Wallet dialog (Hennadii Stepanov) Pull request description: On master (e75f91eae3936269b40b4bfdfe540d5526270936) not using layout manager causes problems with resizing: ![Screenshot from 2021-01-01 13-03-13](https://user-images.githubusercontent.com/32963518/103437728-ce1d4580-4c33-11eb-8915-1e9482775653.png) ![Screenshot from 2021-01-01 13-03-26](https://user-images.githubusercontent.com/32963518/103437730-d6758080-4c33-11eb-9e0f-87d0dd487fcb.png) Also text labels are not resized properly on some window managers (https://github.com/bitcoin/bitcoin/issues/20777), or if their lengths are changed (after translation). This PR introduces a standard layout manager for the "Create Wallet" dialog that fixes all layout issues (actually, the `createwalletdialog.ui` has been re-written from scratch): ![Screenshot from 2021-01-01 13-10-03](https://user-images.githubusercontent.com/32963518/103437822-d0cc6a80-4c34-11eb-84fd-fcb10a16d9ef.png) ![Screenshot from 2021-01-06 23-50-36](https://user-images.githubusercontent.com/32963518/103823090-0b416780-507a-11eb-89dd-3f48a358e168.png) Additional visual changes: - advanced options are grouped in `QGroupBox` (https://github.com/bitcoin-core/gui/pull/96#issuecomment-726337165) - enabled the [size grip](https://doc.qt.io/qt-5/qsizegrip.html#details) Fix https://github.com/bitcoin/bitcoin/issues/20777 ACKs for top commit: jarolrod: ACK d4feb6812a2707ef85d75dda4372086ec62eb922 Sjors: re-tACK d4feb6812a2707ef85d75dda4372086ec62eb922 promag: Tested ACK d4feb6812a2707ef85d75dda4372086ec62eb922 on macos. Tree-SHA512: 4c055962e49f88624900b880b33a866976d224628784593428b712d2e94563d77ddefddea3397134d20e72f738a8cf9aa885c1272fd9ffc90213c104435fb9f4
2021-01-21Merge #20932: refactor: Replace fs::absolute calls with AbsPathJoin callsMarcoFalke
da9caa1cedd69702aea44cb44b2fd0a2d6d56916 Replace fs::absolute calls with AbsPathJoin calls (Kiminuo) 66576c4fd532ac18b8b355ea93d25581a2c15654 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo) Pull request description: This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17. This PR doesn't change behavior aside from adding an assert and fixing a test bug. ACKs for top commit: jonatack: Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 only doxygen improvements since my last review per `git diff d867d7a da9caa1` MarcoFalke: review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 📯 ryanofsky: Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916. Just comment and test tweaks since previous review. Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
2021-01-21guix: Fix typoCarl Dong
2021-01-21guix: README: Add darwin HOSTS entryCarl Dong
2021-01-21guix: Check for macOS SDK before building anythingCarl Dong
2021-01-21guix: Set ZERO_AR_DATE for darwin build determinismCarl Dong
See comments inserted in this commit.
2021-01-21build: Make xorrisofs reproducible with -volume_dateCarl Dong
We need this to be after a '--' as '-volume_date' is a xorriso flag, not a xorrisofs flag. See the respective man pages. For more details: https://issues.guix.info/issue/35283#2
2021-01-21guix: Add support for darwin buildsCarl Dong
2021-01-21build: Add var printing target to src/Makefile.amCarl Dong
See 181989f6c9427fc266dbdcc84cb60ac03e67cdb2 for more info. I missed this one last time.
2021-01-21Merge #17556: test: Change feature_config_args.py not to rely on strange ↵MarcoFalke
regtest=0 behavior ff44cae279bef7997f76db18deb1e41b39f05cb6 test: Change feature_config_args.py not to rely on strange regtest=0 behavior (Russell Yanofsky) Pull request description: Update test to simply generate a normal mainnet configuration file instead of using a crazy setup where a regtest=1 config file using an includeconf in the [regtest] section includes another config file that specifies regtest=0, retroactively switching the network to mainnet. This setup was fragile and only worked because the triggered InitError happened early enough that none of the ignored [regtest] options mattered (only affecting log output). This change was originally made as part of #17493 Top commit has no ACKs. Tree-SHA512: 3f77305454f04438493dfc2abd78a00434b30869454d1c3f54587b9c1f63239c49c90fb3b4d3a777ad130f2184e0f2dac87cee4cd23c50f1b3496a375943da01
2021-01-21Merge #20972: locks: Annotate CTxMemPool::check to require cs_mainMarcoFalke
b3964670537d0943b8fb2d8f2ea419cbefd4835a locks: Annotate CTxMemPool::check to require cs_main (Carl Dong) Pull request description: ``` Currently, CTxMemPool::check locks CTxMemPool's own cs member, then calls GetSpendHeight which locks cs_main. This can potentially cause an undesirable lock invesion since CTxMemPool's cs is supposed to be locked after cs_main. This does not cause us any problems right now because all callers of CTxMemPool already lock cs_main before calling CTxMemPool::check, which means that the LOCK(cs_main) in GetSpendHeight becomes benign. However, it is currently possible for new code to be added which calls CTxMemPool::check without locking cs_main (which would be dangerous). Therefore we should make it explicit that cs_main needs to be held before calling CTxMemPool::check. NOTE: After all review-only assertions are removed in "#20158 | tree-wide: De-globalize ChainstateManager", and assuming that we keep the changes in "validation: Pass in spendheight to CTxMemPool::check", we can re-evaluate to see if this annotation is still necessary. ``` ----- Previous discussions: 1. https://github.com/bitcoin/bitcoin/pull/20158#discussion_r520639845 2. https://github.com/bitcoin/bitcoin/pull/20158#pullrequestreview-557117202 3. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559425521 ACKs for top commit: jnewbery: Code review ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a MarcoFalke: ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a jonatack: ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a review and debug built, verified that `cs_main` is held by callers of `CTxMemPool::check()` in `PeerManagerImpl::ProcessOrphanTx()`, `PeerManagerImpl::ProcessMessage()`, and `CChainState::ActivateBestChainStep()` Tree-SHA512: 4635cddb4aa1af9532bb657b2f9c4deec4568d16ba28c574eae91bb77368cd40e23c3c720a9de11cec78e7ad678a44a5e25af67f13214b86b56e777e0c35a026
2021-01-21Merge #20946: fuzz: Consolidate fuzzing TestingSetup initializationMarcoFalke
abb6fa728598c4cc8874eae1c3c5e587e36424cd fuzz: Initialize a full TestingSetup where appropriate (Carl Dong) 713314abfa224efceb7ba6fa7fef37c449902936 fuzz: Consolidate fuzzing TestingSetup initialization (Carl Dong) Pull request description: ``` Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways: 1. Calling InitializeFuzzingContext, which implicitly constructs a static const BasicTestingSetup 2. Directly constructing a static const BasicTestingSetup in the initialize_* function 3. Directly constructing a static TestingSetup and reproducing the initialization arguments (I'm assuming because InitializeFuzzingContext only initializes a BasicTestingSetup) The new, relatively-simple MakeFuzzingContext function allows us to consolidate these methods of initialization by being flexible enough to be used in all situations. It: 1. Is templated so that we can choose to initialize any of the *TestingSetup classes 2. Has sane defaults which are often used in fuzzers but are also easily overridable 3. Returns a unique_ptr, explicitly transferring ownership to the caller to deal with according to its situation ``` ~~Question for fuzzing people: was it intentional that `src/test/fuzz/net.cpp` would directly instantiate the `BasicTestingSetup` and thus omit the `"-nodebuglogfile"` flag?~~ [Answered](https://github.com/bitcoin/bitcoin/pull/20946#issuecomment-761537108) ACKs for top commit: MarcoFalke: ACK abb6fa728598c4cc8874eae1c3c5e587e36424cd Tree-SHA512: 96a5ca6f4cd5ea0e9483b60165b31ae3e9003918c700a7f6ade48010f419f2a6312e10b816b3187f1d263798827571866e4c4ac0bbfb2e0c79dfad254cda68e7
2021-01-21fuzz: Initialize a full TestingSetup where appropriateCarl Dong
A full TestingSetup is required for both coins_view and load_external_block_file as they interact with the active chainstate.
2021-01-21fuzz: Consolidate fuzzing TestingSetup initializationCarl Dong
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways: 1. Calling InitializeFuzzingContext, which implicitly constructs a static const BasicTestingSetup 2. Directly constructing a static const BasicTestingSetup in the initialize_* function 3. Directly constructing a static TestingSetup and reproducing the initialization arguments (I'm assuming because InitializeFuzzingContext only initializes a BasicTestingSetup) The new, relatively-simple MakeFuzzingContext function allows us to consolidate these methods of initialization by being flexible enough to be used in all situations. It: 1. Is templated so that we can choose to initialize any of the *TestingSetup classes 2. Has sane defaults which are often used in fuzzers but are also easily overridable 3. Returns a unique_ptr, explicitly transferring ownership to the caller to deal with according to its situation
2021-01-21zmq: deduplicate 'sequence' publisher message creation/sendingSebastian Falbesoner
2021-01-21Merge #20953: test: dedup zmq test setup code (node restart, topics ↵fanquake
subscription) 4efb6c2d3b41c6badb4d8d5d9370387a91bf7066 zmq test: deduplicate test setup code (node restart, topics subscription) (Sebastian Falbesoner) Pull request description: This PR deduplicates common setup code for the ZMQ functional test. The following steps, previously duplicated in each sub-test, are put into a new method `setup_zmq_test(...)`: - create subscriber sockets (`zmq.SUB`) for each topic with the specified timeout (default 60s) - restart node0 with specified zmq notifications enabled (`-zmqpub...=tcp://127.0.0.1:...`...) - if desired, connect node0 with node1 (note done by default) - connect all susbcriber sockets to publisher (running on node0) - wait a bit (currently 200ms), to _"Relax so that the subscribers are ready before publishing zmq messages"_ Note that the last point should be repaced by a more robust method, as this test is still flaky, see #20934 (also #20590 and #20538). ACKs for top commit: instagibbs: ACK 4efb6c2d3b41c6badb4d8d5d9370387a91bf7066 laanwj: Code review ACK https://github.com/bitcoin/bitcoin/pull/20953/commits/4efb6c2d3b41c6badb4d8d5d9370387a91bf7066 Tree-SHA512: d49626756a9c669f1133f1b73ce273994b58c760ce0d6a4bdaa384f043a74149dc2b9fa66fe990413d9105f9c3b6ea973e099669e8e02f2902a5b84fa995028c
2021-01-20locks: Annotate CTxMemPool::check to require cs_mainCarl Dong
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then calls GetSpendHeight which locks cs_main. This can potentially cause an undesirable lock invesion since CTxMemPool's cs is supposed to be locked after cs_main. This does not cause us any problems right now because all callers of CTxMemPool already lock cs_main before calling CTxMemPool::check, which means that the LOCK(cs_main) in GetSpendHeight becomes benign. However, it is currently possible for new code to be added which calls CTxMemPool::check without locking cs_main (which would be dangerous). Therefore we should make it explicit that cs_main needs to be held before calling CTxMemPool::check. NOTE: After all review-only assertions are removed in "#20158 | tree-wide: De-globalize ChainstateManager", and assuming that we keep the changes in "validation: Pass in spendheight to CTxMemPool::check", we can re-evaluate to see if this annotation is still necessary.
2021-01-20Merge #20952: wallet: Add BerkeleyDB version sanity check at init timeSamuel Dobson
ad57fb756b1c2df625790bd9c296ec28daa93740 wallet: Add BerkeleyDB version sanity check at init time (Wladimir J. van der Laan) Pull request description: Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation. This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected. ACKs for top commit: decryp2kanon: utACK ad57fb7 achow101: ACK ad57fb756b1c2df625790bd9c296ec28daa93740 theStack: utACK ad57fb756b1c2df625790bd9c296ec28daa93740 meshcollider: Code review ACK ad57fb756b1c2df625790bd9c296ec28daa93740 Tree-SHA512: 99cd7d836bffbdeb3d4e14053f7139cc85a6d42e631a3f9a3058a848042446b364faee127500f5acb374616e6a61ab2bedebfac1ba9bc993b4d6227114c2a6c2
2021-01-20Merge #20937: guix: Make nsis reproducible by respecting SOURCE-DATE-EPOCHfanquake
1fca9811e1331ac5dae8188f6178cc37da4929a7 lint: Skip whitespace lint for guix patches (Carl Dong) a91c46c57d88fc399432afab7bb0fb14c3e490a7 guix: Make nsis reproducible by respecting SOURCE-DATE-EPOCH (Carl Dong) Pull request description: ``` When building nsis, if VERSION is not specified, it defaults to cvs_version which is non-deterministic as it includes the current date. This patches nsis to default to SOURCE_DATE_EPOCH if it exists so that nsis is reproducible. Upstream change: https://github.com/kichik/nsis/pull/13 ``` Sidenote: also a good demonstration of how Guix allows us to flexibly patch our tools! Note to reviewers: if you want to compare hashes, please build after Jan 16th 2021 without my substitute server enabled! ACKs for top commit: fanquake: ACK 1fca9811e1331ac5dae8188f6178cc37da4929a7 Tree-SHA512: b800e0ce5f73827ad353739effb9167ec3a6bdb362c725ae20dd3f025ce78660f85c70ce1d75cd0896facf1e8fe38a9e058459ed13dec71ab3a2fe41e20eaa5d
2021-01-19Merge #20891: rpc: Remove deprecated bumpfee behaviorWladimir J. van der Laan
ea0a7ec949f0f7e212f0d8819f7a54cad2258bdd Remove deprecated bumpfee behavior (Andrew Chow) Pull request description: Removes the deprecation message, behavior, and test. This was marked for removal in 22.0. ACKs for top commit: promag: ACK ea0a7ec949f0f7e212f0d8819f7a54cad2258bdd, maybe add need release notes tag. Tree-SHA512: d1626906849f6ee37213c32e5f8c1433ad8fb7beabcd88f5801b1964b322171a2341bdfbd9a3a5ab39b2fd9d9c6a05f73298583423a73cab1275653105c03e8e
2021-01-19Merge #20906: contrib: embed C++11 patch in install_db4.shWladimir J. van der Laan
92370033a2606ee88f4d080193e0bf6da481dc2e contrib: embed C++11 patch in install_db4.sh (jackielove4u) Pull request description: This is a continuation of https://github.com/bitcoin/bitcoin/pull/20665. Closes #20722. ACKs for top commit: laanwj: ACK 92370033a2606ee88f4d080193e0bf6da481dc2e fanquake: ACK 92370033a2606ee88f4d080193e0bf6da481dc2e. Tree-SHA512: ebfd16f5301158de1acc1b8eeca43b3d94f0a6d438832133a30648e5e8a88268b4af983be0bb57f3018e3af8459f32f0de676c1b4e8942e199a4497c776631c5
2021-01-19[net] Cleanup InactivityChecks() and add commenting about timeJohn Newbery
Also clean up and better comment the function. InactivityChecks() uses a mixture of (non-mockable) system time and mockable time. Make sure that's well documented. Despite being marked as const in CConnman before this commit, the function did mutate the state of the passed in CNode, which is contained in vNodes, which is a member of CConnman. To make the function truly const in CConnman and all its data, instead make InactivityChecks() a pure function, return whether the peer should be disconnected, and let the calling function (SocketHandler()) update the CNode object. Also make the CNode& argument const.
2021-01-18Merge #19866: eBPF Linux tracepointsWladimir J. van der Laan
22eb7930a6ae021438aa0b8e750170534944f296 tracing: add tracing framework (William Casarin) 933ab8a720cb9b3341adec4109cffb6dc5b322a5 build: detect sys/sdt.h for eBPF tracing (William Casarin) Pull request description: Instead of writing ad-hoc logging everywhere (eg: #19509), we can take advantage of linux user static defined traces, aka. USDTs ( not the stablecoin :sweat_smile: ) The linux kernel can hook into these tracepoints at runtime, but otherwise they have little to no performance impact. Traces can pass data which can be printed externally via tools such as bpftrace. For example, here's one that prints incoming and outgoing network messages: # Examples ## Network Messages ``` #!/usr/bin/env bpftrace BEGIN { printf("bitcoin net msgs\n"); @start = nsecs; } usdt:./src/bitcoind:net:push_message { $ip = str(arg0); $peer_id = (int64)arg1; $command = str(arg2); $data_len = arg3; $data = buf(arg3,arg4); $t = (nsecs - @start) / 100000; printf("%zu outbound %s %s %zu %d %r\n", $t, $command, $ip, $peer_id, $data_len, $data); @outbound[$command]++; } usdt:./src/bitcoind:net:process_message { $ip = str(arg0); $peer_id = (int64)arg1; $command = str(arg2); $data_len = arg3; $data = buf(arg3,arg4); $t = (nsecs - @start) / 100000; printf("%zu inbound %s %s %zu %d %r\n", $t, $command, $ip, $peer_id, $data_len, $data); @inbound[$ip, $command]++; } ``` $ sudo bpftrace netmsg.bt output: https://jb55.com/s/b11312484b601fb3.txt if you look at the bottom of the output you can see a histogram of all the messages grouped by message type and IP. nice! ## IBD Benchmarking ``` #!/usr/bin/env bpftrace BEGIN { printf("IBD to 500,000 bench\n"); } usdt:./src/bitcoind:CChainState:ConnectBlock { $height = (uint32)arg0; if ($height == 1) { printf("block 1 found, starting benchmark\n"); @start = nsecs; } if ($height >= 500000) { @end = nsecs; @duration = @end - @start; exit(); } } END { printf("duration %d ms\n", @duration / 1000000) } ``` This one hooks into ConnectBlock and prints the IBD time to height 500,000 starting from the first call to ConnectBlock Userspace static tracepoints give lots of flexibility without invasive logging code. It's also more flexible than ad-hoc logging code, allowing you to instrument many different aspects of the system without having to enable per-subsystem logging. Other ideas: tracepoints for lock contention, threads, what else? Let me know what ya'll think and if this is worth adding to bitcoin. ## TODO - [ ] docs? - [x] Integrate systemtap-std-dev/libsystemtap into build (provides the <sys/sdt.h> header) - [x] ~dtrace macos support? (is this still a thing?)~ going to focus on linux for now ACKs for top commit: laanwj: Tested ACK 22eb7930a6ae021438aa0b8e750170534944f296 0xB10C: Tested ACK 22eb7930a6ae021438aa0b8e750170534944f296 Tree-SHA512: 69242242112b679c8a12a22b3bc50252c305894fb3055ae6e13d5f56221d858e58af1d698af55e23b69bdb7abedb5565ac6b45fa5144087b77a17acd04646a75
2021-01-18Merge #20880: gitian: Use custom MacOS code signing toolWladimir J. van der Laan
2c403279e2f0f7c8c27c56d4e7b0573c59571f0a gitian: Remove codesign_allocate and pagestuff from MacOS build (Andrew Chow) f55eed251488d70d5e2e3a2965a4f8ec0c476853 gitian: use signapple to create the MacOS code signature (Andrew Chow) 95b06d21852b28712db6c710e420a58bdc1a0944 gitian: use signapple to apply the MacOS code signature (Andrew Chow) 42bb1ea363286b088257cabccb686ef1887c1d3b gitian: install signapple in gitian-osx-signer.yml (Andrew Chow) Pull request description: The MacOS code signing issues that were encountered during the 0.21.0 release cycle have shown that it is necessary for us to use a code signing tool for which the source code is available and modifiable by us. Given that there appears to not be such a tool available, I have written such a tool, [signapple](https://github.com/achow101/signapple), that we can use. This tool is able to create a valid MacOS code signature, detach it in a way that we were doing previously, and attach it to the unsigned binary. This tool can also verify that the signature is correct. This PR implements the usage of that tool in the gitian build for the code signed MacOS binary. The code signer will use this tool to create the detached signature. Gitian builders will use this tool to apply the detached signature. The `gitian-osx-signer.yml` descriptor has been modified to install this tool so that the `detached-sig-apply.sh` script can use it. Additionally, the `codesign_allocate` and `pagestuff` tools are no longer necessary so they are no longer added to the tarball used in code signing. Lastly, both the `detached-sig-create.sh` and `detached-sig-apply.sh` scripts are made to be significantly less complex and to not do unexpected things such as unpacking an already unpacked tarball. The detached code signature that signapple creates is almost identical to that which we were previously creating. The only difference is that the cpu architecture name is included in the extension (e.g. we have `bitcoin-qt.x86_64sign` instead of `bitcoin-qt.sign`). This was done in order to support signing universal binaries which we may want to do in the future. However signapple can still apply existing code signatures as it will accept the `.sign` extension. If it is desired, it can be modified to produce signatures with just the `.sign` extension. However I do not think it is necessary to maintain compatibility with the old process. ACKs for top commit: laanwj: Code review ACK 2c403279e2f0f7c8c27c56d4e7b0573c59571f0a Tree-SHA512: 2a0e01e9133f8859b9de26e7e8fe1d2610d2cbdee2845e6008b12c083c7e3622cbb2d9b83c50a269e2c3074ab95914a8225d3cd4108017f58b77a62bf10951e0
2021-01-18Merge #20884: script: Improve robustness of bitcoind.service on startupWladimir J. van der Laan
9d026546778629472574b26fa73338efc63d02da doc: Fix systemd spelling and link to doc/init.md (Hennadii Stepanov) 601778c3107adbd8d96eb0bb5c16a9d0a4b81594 script: Add Documentation key to bitcoind.service (Hennadii Stepanov) d9392b724cae53b7a16fa5f84ebe152eea496502 script: Improve robustness of bitcoind.service on startup (Hennadii Stepanov) Pull request description: If network interfaces are not properly up the following happens: ``` ... 2021-01-08T10:17:11Z scheduler thread start 2021-01-08T10:17:11Z libevent: getaddrinfo: address family for nodename not supported 2021-01-08T10:17:11Z Binding RPC on address 127.0.0.1 port 8332 failed. 2021-01-08T10:17:11Z HTTP: creating work queue of depth 16 2021-01-08T10:17:11Z Using random cookie authentication. 2021-01-08T10:17:11Z Generated RPC authentication cookie /var/lib/bitcoind/.cookie 2021-01-08T10:17:11Z HTTP: starting 2 worker threads 2021-01-08T10:17:11Z init message: Loading banlist... 2021-01-08T10:17:11Z SetNetworkActive: true 2021-01-08T10:17:11Z Error: Cannot resolve -externalip address: <EDITED> 2021-01-08T10:17:11Z Shutdown: In progress... 2021-01-08T10:17:11Z scheduler thread exit 2021-01-08T10:17:11Z Shutdown: done ``` This PR improves robustness on startup in such cases in documented way: https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/ Also minor doc improvements are added. ACKs for top commit: Sjors: ACK 9d02654 practicalswift: ACK 9d026546778629472574b26fa73338efc63d02da: patch looks correct darosior: ACK 9d026546778629472574b26fa73338efc63d02da -- been using the first patch too Tree-SHA512: 38294f5682c09e6ea9008de7d7459098c920cf1b98ad8ef8a5d2ca01f2f781c0fec5591dc40ef36eeb19d94991b0c7fb7cb38c4e716bc7219875c9bcd0a55e1b
2021-01-18Merge #20955: test: Fix get_previous_releases.py for aarch64Wladimir J. van der Laan
fa1d5e51374bce9607b429836bbd3fe059e3c78c test: Fix get_previous_releases.py for aarch64 (MarcoFalke) Pull request description: Otherwise it will fail with "Not sure which binary to download..." ACKs for top commit: laanwj: Code review ACK fa1d5e51374bce9607b429836bbd3fe059e3c78c Tree-SHA512: 0db71e898a431665757ce835016a4e05c629a95abc4a2951eac9bd9b5876ec3dc3d6f156d58565e2bcdf918cde4f2649183d4a58038ac13c705a7e914c0094d1
2021-01-18Merge #20938: build: fix linking against -latomic when building for riscvWladimir J. van der Laan
54ce4fac80689621dcbcc76169b2b00b179ee743 build: improve macro for testing -latomic requirement (fanquake) 2c010b9c56f069efad2b2c10bffaef4ed059736e add std::atomic include to bitcoin-util.cpp (fanquake) Pull request description: Since the merge of #19937, riscv builds have been failing, due to a link issue with [`std::atomic_exchange`](https://en.cppreference.com/w/cpp/atomic/atomic_exchange) in `bitcoin-util`: ```bash CXXLD bitcoin-util bitcoin_util-bitcoin-util.o: In function `grind_task': /home/ubuntu/build/bitcoin/distsrc-riscv64-linux-gnu/src/bitcoin-util.cpp:98: undefined reference to `__atomic_exchange_1' collect2: error: ld returned 1 exit status ``` We have a [macro](https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/l_atomic.m4) that tries to determine when `-latomic` is required, however it doesn't quite work well enough, as it's currently determining it isn't needed: ```bash ./autogen.sh ./configure --prefix=/home/ubuntu/bitcoin/depends/riscv64-linux-gnu ... checking whether std::atomic can be used without link library... yes ``` This PR adds a call to `std::atomic_exchange` to the macro, which will get us properly linked against `-latomic` on riscv: ```bash checking whether std::atomic can be used without link library... no checking whether std::atomic needs -latomic... yes ``` Also adds an `<atomic>` include to `bitcoin-util.cpp`. ACKs for top commit: laanwj: Tested ACK 54ce4fac80689621dcbcc76169b2b00b179ee743 Tree-SHA512: 963c875097ee96b131163ae8109bcf8fecf4451d20faa2f3d223f9938ea3d8d1ed5604e12ad82c2b4b1c605fd293a9b6b08fefc00dd3e68d09c49e95029c6f50
2021-01-18test: Fix get_previous_releases.py for aarch64MarcoFalke
2021-01-17lint: Skip whitespace lint for guix patchesCarl Dong
2021-01-17guix: Make nsis reproducible by respecting SOURCE-DATE-EPOCHCarl Dong
When building nsis, if VERSION is not specified, it defaults to cvs_version which is non-deterministic as it includes the current date. This patches nsis to default to SOURCE_DATE_EPOCH if it exists so that nsis is reproducible. Upstream change: https://github.com/kichik/nsis/pull/13
2021-01-17Merge #20939: build: fix RELOC_SECTION security check for bitcoin-utilWladimir J. van der Laan
c061800bb17ce8005ac7b189135dd7e95155e7de build: fix RELOC_SECTION security check for bitcoin-util (fanquake) Pull request description: The binutils we use for gitian builds strips the reloc section from Windows binaries, which breaks ASLR. As a temporary workaround, export main(). This is the same workaround as #18702 (bitcoin-cli), and will fix the currently failing security check: ```bash + make -j1 -C src check-security make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src' Checking binary security... bitcoin-util.exe: failed RELOC_SECTION make: *** [check-security] Error 1 ``` Relevant upstream issue: https://sourceware.org/bugzilla/show_bug.cgi?id=19011 ACKs for top commit: dongcarl: ACK c061800bb17ce8005ac7b189135dd7e95155e7de laanwj: ACK c061800bb17ce8005ac7b189135dd7e95155e7de Tree-SHA512: a1a4da0b2cddfc377190b9044a04f42a859ca79f11ce2c2ab4c3d066a2786c34d5446d75f8eec634f308d2d3349ebbd7c9f76dcaebeeb28e471c829851592367
2021-01-17wallet: Add BerkeleyDB version sanity check at init timeWladimir J. van der Laan
Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation. This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected.
2021-01-17zmq test: deduplicate test setup code (node restart, topics subscription)Sebastian Falbesoner
2021-01-16Merge #20945: Fix 0.21.0 release note to specify correct option BIP 157 supportWladimir J. van der Laan
9a42b5e6553424908830db0cf1aafe9607d30b62 Fix 0.21.0 release note to specify correct option BIP 157 support (benthecarman) Pull request description: https://github.com/bitcoin/bitcoin/blob/master/src/init.cpp#L452 ACKs for top commit: mjdietzx: ACK 9a42b5e6553424908830db0cf1aafe9607d30b62 laanwj: ACK 9a42b5e6553424908830db0cf1aafe9607d30b62 Tree-SHA512: 901195434046fd8a18a2329104b69e0ecc1bdb2bba1fe0688fb4a7c5992e9bdd66b8e50f46873f4ddf5ffe3c9aed860ccb01584812b06d865bd06fcd2b14c42b
2021-01-15Replace fs::absolute calls with AbsPathJoin callsKiminuo
This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.
2021-01-15Fix 0.21.0 release note to specify correct option BIP 157 supportbenthecarman
2021-01-15test: Clear forced -walletdir setting after wallet init_testsKiminuo
Leaving this value set interfered with the CreateWallet test if it happened to execute later in the test ordering. Specifically it would cause CreateWallet test to write data to the current directory instead of temporary test directory.
2021-01-15Merge #20916: rpc: Return wtxid from testmempoolacceptMarcoFalke
fa0aa87071eaec8a5df17774cdb352195e5e09de rpc: Return wtxid from testmempoolaccept (MarcoFalke) Pull request description: It would be nice if `testmempoolaccept` returned the unique wtxid directly to avoid a costly `decoderawtransaction` roundtrip ACKs for top commit: mjdietzx: utACK fa0aa87071eaec8a5df17774cdb352195e5e09de stackman27: utACK [`fa0aa87`](https://github.com/bitcoin/bitcoin/pull/20916/commits/fa0aa87071eaec8a5df17774cdb352195e5e09de) glozow: cr ACK https://github.com/bitcoin/bitcoin/pull/20916/commits/fa0aa87071eaec8a5df17774cdb352195e5e09de Tree-SHA512: 05dbaf46d93e47e9eedb725c2f57175e6d4e1722da0531fe4f80e74fc2518911da87634f25f61fa2bc8d87a3017e82fd0684b09a0a9706d71deed970035c2e7a
2021-01-15Merge #20908: fuzz: Use mocktime in process_message* fuzz targetsMarcoFalke
fa0a864b383a794910cdb215bb836dae12357bc6 fuzz: Use mocktime in process_message* fuzz targets (MarcoFalke) Pull request description: Use mocktime to allow time to advance deterministically during execution of a fuzz input. This also allows to drop the call to `JumpOutOfIbd`. ACKs for top commit: practicalswift: cr ACK fa0a864b383a794910cdb215bb836dae12357bc6 Tree-SHA512: e92fc70ec6bd49760173cb202549f364304e22b3f7127b9a4da8447cf9341008e477ad42c2599c2fde167bbcbc0e2d139709b4ef6371788bc2c1c3b7f589e11d
2021-01-15build: fix RELOC_SECTION security check for bitcoin-utilfanquake
The binutils we use for gitian builds strips the reloc section from Windows binaries, which breaks ASLR. As a temporary workaround, export main(). This is the same workaround as #18702 (bitcoin-cli), and will fix the currently failing security check: ```bash + make -j1 -C src check-security make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src' Checking binary security... bitcoin-util.exe: failed RELOC_SECTION make: *** [check-security] Error 1 ``` Relevant upstream issue: https://sourceware.org/bugzilla/show_bug.cgi?id=19011
2021-01-15build: improve macro for testing -latomic requirementfanquake
riscv builds are currently failing because -latomic isn't being linked against, when it is needed: ```bash /home/ubuntu/build/bitcoin/distsrc-riscv64-linux-gnu/src/bitcoin-util.cpp:98: undefined reference to `__atomic_exchange_1' ``` This exteneds our macro to ensure that -latomic is linked against when required.