Helix pull upstream #10
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62defc8 Clear EvoDB cache when hitting specific size (Liquid) Pull request description: Perpetual hunt to reduce memory! This is a simple PR to clear the EvoDB cache and flush it to disk when it grows. In Dash, users experience persistently growing memory because the EvoDB cache until v.18 dashpay#5007 This is flushing the evodb cache and chain state when hit its "critical" memory and will keep from persistently growing. ACKs for top commit: panleone: utACK 62defc8 Fuzzbawls: ACK 62defc8 Tree-SHA512: 13ab1bd1b66c7492b792c59b1821aa8bd2ee5d65615045d9c66d35d44f70604367fd198a3420faae19d8cb53d9a22a69ae7b5464029904a150d8449e3d96d7fc
5c0be19abc Merge PIVX-Project/bls-signatures#21: Add fix for RELIC parsing invalid inputs. 144680469e Merge PIVX-Project/bls-signatures#20: CMake: Squelch implicit conversion warnings 98d6c06453 Merge PIVX-Project/bls-signatures#19: [GA] Add macOS/Ubuntu latest runners adb71101a3 Add fix for RELIC parsing invalid inputs. 4581ffab2c CMake: Squelch implicit conversion warnings 6b178f9f94 [GA] Add macOS/Ubuntu latest runners 188967ec6c Merge PIVX-Project/bls-signatures#18: Autotools: Bump relic version 9c27c3a468 Merge PIVX-Project/bls-signatures#17: Update Relic git-tag fa5c05a5dd Autotools: Consistently use explicit quoting and improve readability 7a069139c1 build: Update autotools Makefile.am for new relic 779fdb24b7 Merge commit '98c9be9bc82c69b146a010a26c03e3ead7387624' into 2023_contrib-relic-bump 98c9be9bc8 Squashed 'contrib/relic/' changes from 1885ae3b..e6209fd8 6e619c55f1 Merge PIVX-Project/bls-signatures#16: [GA] Bump action-apt to `master` version 765a72b5cd Merge PIVX-Project/bls-signatures#15: [Build] Add syslib memory include to threshold.cpp 82995ba8be Merge PIVX-Project/bls-signatures#14: [Contrib] Update catch to v2.13.10 b7b5e2bde9 Update Relic git-tag 551f2c4e1d [GA] Bump action-apt to `master` version 0c8c9cd84e [Build] Add syslib memory include to threshold.cpp c3f7315c28 scripted-diff: Update catch to v2.13.10 git-subtree-dir: src/chiabls git-subtree-split: 5c0be19abcd3061bb696f236f2c8b6698ca925d5
1c0a068 Fix POW block sign (panleone) Pull request description: I know that we are no longer POW but nevertheless mining in POW phase would no longer work since we are signing the block ACKs for top commit: Liquid369: tACK 1c0a068 Fuzzbawls: utACK 1c0a068 Tree-SHA512: d4b41bb0e567104ceb232434a059430162609fa8f012c10e25d4b24fa7c706b01898d50c7a77a522108ad7356166ed7b619ebcadf0e0d80c8c8eb7fcf08007ed
815bbc2 [Tests] Cover startmasternode all option (Fuzzbawls) 189f263 [Tests] Test newly invalid startmasternode set options (Fuzzbawls) b86632f [RPC] Cleanup startmasternode command (Fuzzbawls) Pull request description: General cleanup of the `startmasternode` RPC command. This has been a long time coming, and rectifies some long-standing issues with the design and operability of the command. - Remove 'local' set support. Long-time NonOp. - Remove 'many' set support. Always NonOp. - General cleanup of the command's logic. Adds code coverage for new failure cases as well as code coverage for multi-MN start (ie, not "alias"-only) start commands. ACKs for top commit: Liquid369: tACK 815bbc2 panleone: tACK 815bbc2 Tree-SHA512: b1d3a0926691e7f73d559872334935384c029766b46b9506a51389a496df928052641629dffbb8b75d77af8a6fd418ef071cc64ea369abfa38119ac08a411b7a
fb39e71 [Core] Unify binary exit codes (Fuzzbawls) Pull request description: * `--help`, `--version` etc should exit with `0` i.e. no error ("not enough args" case should still trigger an error) * error reading config file should exit with `1` Ref: bitcoin#9067, bitcoin#9120, bitcoin#13349 ACKs for top commit: panleone: utACK fb39e71 Liquid369: tACK fb39e71 Tree-SHA512: d2ea55dbc1106f7cdb1062ef509de4191c0a2945f957204d9a0c8f94babcd5c0618b1e10fd7ac2792078327a2508054e9e64b0075156de59fd224394549eb9d9
89b31d6 [zmq] Call va_end() on va_start()ed args. (Karl-Johan Alm) Pull request description: > A function that invokes `va_start`, shall also invoke `va_end` before it returns. (http://www.cplusplus.com/reference/cstdarg/va_start/) > If there is no corresponding call to `va_start` or `va_copy`, or if `va_end` is not called before a function that calls `va_start` or `va_copy` returns, the behavior is undefined. http://en.cppreference.com/w/cpp/utility/variadic/va_end Ref: bitcoin#10056 ACKs for top commit: panleone: utACK 89b31d6 Liquid369: ACK 89b31d6 Tree-SHA512: 20ac403ba901b98ac969c3c021311707740b6a3b5f69a10117518331af8460f764e0dedc80205a29210d283914380e6edf1b9e50394c3e9262ae4b6f5f520b64
We use a more human-friendly visual layout for RPC tables in our source code that doesn't play nicely with `clang-format`. This does a sweep of whitespace cleanup in the various tables and adds guard macros so that `clang-format` will not try to automatically reformat the code layout.
5a492f2 [Tests] Add further RPC test coverage for cold staking (Fuzzbawls) Pull request description: Adds RPC test coverage for the following RPC Commands: - liststakingaddresses - listdelegators - delegatorremove ACKs for top commit: panleone: utACK 5a492f2 Liquid369: ACK 5a492f2 Tree-SHA512: 00996606372cc1a38912abd1b8941b38566d39a45efab3deee384c1009532fb8c873351304d15731e86f0e108144f9a76408ae7ef5ce8c957960c7db8dc1bf08
39163e7 Show memo for self s->s txs (panleone) Pull request description: Solves #2838 With the current code we already show the memo for a self t->s transaction but not for a self s->s transaction. With this PR I fix that issue ACKs for top commit: Liquid369: tACK 39163e7 Fuzzbawls: ACK 39163e7 Tree-SHA512: c5ce679218511117138341ef8670e97dc20b794f88c41eda2686cf363af478061ac18ef94a38e719cbf1d67eab35c091b1dba743d45f0b15b1792d004c98023d
…g-format a94c842 [Misc] Cleanup whitespace and guard RPC tables from clang-format (Fuzzbawls) Pull request description: We use a more human-friendly visual layout for RPC tables in our source code that doesn't play nicely with `clang-format`. This does a sweep of whitespace cleanup in the various tables and adds guard macros so that `clang-format` will not try to automatically reformat the code layout. ACKs for top commit: panleone: re-utACK a94c842 Liquid369: ACK a94c842 Tree-SHA512: fd8e117ca6de9a6ae7116d6e4e3cae267c8559c0d2d90cb0e3407755a8348bfb8133a3bab3b20b8029e8c19a3329c9f00b96527faefee143dab8aeba507108f9
e9b78ed Remove furszy dead seeder with working one (Liquid) Pull request description: This is a newly setup DNS Seeder to help the PIVX network in peer discovery. Furszy’s old seeder is now defunct and not working, in order for everything not to fall onto @Fuzzbawls shoulders(and seeders), I have created another instance. ACKs for top commit: Fuzzbawls: ACK e9b78ed panleone: tACK e9b78ed Tree-SHA512: 7545419453fd95fe93c02d0e11a6eee110a83da318ccd70ac87472d6d9f4053a025bbcf97e1beda40d538b1bb84e9da5ec25c42c619f57985356da7a1646d8f1
2e63747 [Doc] Remove bitness from pivx-qt help message and manpage (Wladimir J. van der Laan) Pull request description: Remove the `(64-bit)` from the pivx-qt help message. Since removing the Windows 32-bit builds, it is no longer information that is often useful for troubleshooting. This never worked for other architectures than x86, and the only 32-bit x86 build left is the Linux one. Linux users tend to know what architecture they are using. It also accidentally ends up in the pivx-qt manpage. Ref: bitcoin#17503 ACKs for top commit: panleone: tACK 2e63747 Liquid369: ACK 2e63747 Tree-SHA512: 7931b554bbad402f2d4c3dd94b6452829a44dcda8e3cd39d38536cc12adbac1e6a27648bbb737f6a5c5000db762c0e86087092e68df5eef41221eb1854fd4f3e
69ce560 Squashed 'src/chiabls/' changes from 4ec2ecc777..5c0be19abc (Fuzzbawls) Pull request description: Bump the chiabls subtree to commit PIVX-Project/bls-signatures@5c0be19. The most notable change here is added support for newer versions of gcc. ACKs for top commit: panleone: tACK 3492719: Liquid369: tACK 3492719 Tree-SHA512: 40da8a1925c0e7ae9ced8da8e1a4998079d595384c877681fde09e6750f956a09e4b3326830402bf40e40c574572c4463c5644b49b2f9961cd379c5e4858691e
Do away with timestamped chache key naming. This simplifies our cache storage situation considerably, especially given that our caches are automatically validated/refreshed as-needed by our internal build systems.
Simplify native builds by skipping the `distdir` step
These two jobs will serve as early detectors of issues in the most recent versions of each OS.
…rThread twice Squash where I add interrupt
793667a Improve shutdown process (João Barbosa) Pull request description: Improve the shutdown time by not having to wait up to 2 seconds. Here is a comparison running `wallet.py` function tests before this PR: ``` 2017-08-08 03:25:20.881000 TestFramework (INFO): Initializing test directory /var/folders/1v/8_69hby54nj2k3n6fywt44x80000gn/T/testq_ramjjr 2017-08-08 03:25:23.853000 TestFramework (INFO): Mining blocks... 2017-08-08 03:25:24.132000 TestFramework (INFO): test getmemoryinfo 2017-08-08 03:25:24.559000 TestFramework (INFO): test gettxout 2017-08-08 03:25:59.858000 TestFramework (INFO): check -rescan 2017-08-08 03:26:07.735000 TestFramework (INFO): check -reindex 2017-08-08 03:26:15.751000 TestFramework (INFO): check -zapwallettxes=1 2017-08-08 03:26:24.105000 TestFramework (INFO): check -zapwallettxes=2 2017-08-08 03:26:36.694000 TestFramework (INFO): Stopping nodes 2017-08-08 03:26:43.599000 TestFramework (INFO): Cleaning up 2017-08-08 03:26:43.612000 TestFramework (INFO): Tests successful ``` After: ``` 2017-08-08 03:24:04.319000 TestFramework (INFO): Initializing test directory /var/folders/1v/8_69hby54nj2k3n6fywt44x80000gn/T/testoqeyi50_ 2017-08-08 03:24:07.035000 TestFramework (INFO): Mining blocks... 2017-08-08 03:24:07.317000 TestFramework (INFO): test getmemoryinfo 2017-08-08 03:24:07.763000 TestFramework (INFO): test gettxout 2017-08-08 03:24:25.715000 TestFramework (INFO): check -rescan 2017-08-08 03:24:27.792000 TestFramework (INFO): check -reindex 2017-08-08 03:24:29.797000 TestFramework (INFO): check -zapwallettxes=1 2017-08-08 03:24:32.207000 TestFramework (INFO): check -zapwallettxes=2 2017-08-08 03:24:36.812000 TestFramework (INFO): Stopping nodes 2017-08-08 03:24:37.915000 TestFramework (INFO): Cleaning up 2017-08-08 03:24:37.927000 TestFramework (INFO): Tests successful ``` This largely improves the time spent in Travis (under evaluation). Tree-SHA512: 023012fb3f8a380addf5995a4bf865862fed712cdd1a648d82a710e6566bc3bd34b6c49f9f06d6cc6bd81ca859da50d30d7f786c816e702549ab642e3476426f
28479f9 qa: Test bitcond shutdown (João Barbosa) 8d3f46e http: Remove timeout to exit event loop (João Barbosa) e98a9ee http: Remove unnecessary event_base_loopexit call (João Barbosa) 6b13580 http: Unlisten sockets after all workers quit (João Barbosa) 18e9685 http: Send "Connection: close" header if shutdown is requested (João Barbosa) 02e1e4e rpc: Add wait argument to stop (João Barbosa) Pull request description: Fixes bitcoin#11777. Reverts bitcoin#11006. Replaces bitcoin#13501. With this change the HTTP server will exit gracefully, meaning that all requests will finish processing and sending the response, even if this means to wait more than 2 seconds (current time allowed to exit the event loop). Another small change is that connections are accepted even when the server is stopping, but HTTP requests are rejected. This can be improved later, especially if chunked replies are implemented. Briefly, before this PR, this is the order or events when a request arrives (RPC `stop`): 1. `bufferevent_disable(..., EV_READ)` 2. `StartShutdown()` 3. `evhttp_del_accept_socket(...)` 4. `ThreadHTTP` terminates (event loop exits) because there are no active or pending events thanks to 1. and 3. 5. client doesn't get the response thanks to 4. This can be verified by applying ```diff // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. StartShutdown(); + MilliSleep(2000); return "Bitcoin server stopping"; } ``` and checking the log output: ``` Received a POST request for / from 127.0.0.1:62443 ThreadRPCServer method=stop user=__cookie__ Interrupting HTTP server ** Exited http event loop Interrupting HTTP RPC server Interrupting RPC tor: Thread interrupt Shutdown: In progress... torcontrol thread exit Stopping HTTP RPC server addcon thread exit opencon thread exit Unregistering HTTP handler for / (exactmatch 1) Unregistering HTTP handler for /wallet/ (exactmatch 0) Stopping RPC RPC stopped. Stopping HTTP server Waiting for HTTP worker threads to exit msghand thread exit net thread exit ... sleep 2 seconds ... Waiting for HTTP event thread to exit Stopped HTTP server ``` For this reason point 3. is moved right after all HTTP workers quit. In that moment HTTP replies are queued in the event loop which keeps spinning util all connections are closed. In order to trigger the server side close with keep alive connections (implicit in HTTP/1.1) the header `Connection: close` is sent if shutdown was requested. This can be tested by ``` bitcoind -regtest nc localhost 18443 POST / HTTP/1.1 Authorization: Basic ... Content-Type: application/json Content-Length: 44 {"jsonrpc": "2.0","method":"stop","id":123} ``` Summing up, this PR: - removes explicit event loop exit — event loop exits once there are no active or pending events - changes the moment the listening sockets are removed — explained above - sends header `Connection: close` on active requests when shutdown was requested which is relevant when it's a persistent connection (default in HTTP 1.1) — libevent is aware of this header and closes the connection gracefully - removes event loop explicit break after 2 seconds timeout Tree-SHA512: 4dac1e86abe388697c1e2dedbf31fb36a394cfafe5e64eadbf6ed01d829542785a8c3b91d1ab680d3f03f912d14fc87176428041141441d25dcb6c98a1e069d8
6c10037 rpc: Fix data race (UB) in InterruptRPC() (practicalswift) Pull request description: Fix data race (UB) in `InterruptRPC()`. Before: ``` $ ./configure --with-sanitizers=thread $ make $ test/functional/test_runner.py feature_shutdown.py … SUMMARY: ThreadSanitizer: data race rpc/server.cpp:314 in InterruptRPC() … ALL | ✖ Failed | 2 s (accumulated) ``` After: ``` $ ./configure --with-sanitizers=thread $ make $ test/functional/test_runner.py feature_shutdown.py … ALL | ✓ Passed | 3 s (accumulated) ``` Tree-SHA512: b139ca1a0480258f8caa7730cabd7783a821d906630f51487750a6b15b7842675ed679747e1ff1bdade77d248807e9d77bae7bb88da54d1df84a179cd9b9b987
…raises_init_error 62c304e tests: Allow closed http server in assert_start_raises_init_error (Chun Kuan Lee) Pull request description: The rpc handler may be unregistered when http server haven't been closed yet. So it may be allowable to get -342 `non-JSON HTTP response with \'%i %s\' from server` (503 Service Unavailable) See https://ci.appveyor.com/project/DrahtBot/bitcoin/build/master.2001. It shows "Rejecting request while shutting down" between "RPC stopped" and "Stopped HTTP server" Tree-SHA512: e1f50ab9096cf23494ccc2850c01516c4a75f112c99108759157b22fce2011682a4b88216f206702f6a56e960182c00098053ad75f13aa0eafe27046adae63da
35c9b1e Fix data race (ale) Pull request description: Fix the data race in which a thread is trying to write `pcoinsTip.cacheSaplingAnchors`: `CWallet::BlockConnected -> pcoinsTip->GetSaplingAnchorAt(...)` At the same time another thread is trying to read `pcoinsTip.cacheSaplingAnchors`: `ActivateBestChain -> FlushStateToDisk -> pcoinsTip->DynamicMemoryUsage()`. The fix is trivial `CWallet::BlockConnected` was not locking the mutex `cs_main` that guards `pcoinsTip`. This data race was causing the functional test `wallet_basic.py` to randomly fail: To see this I created two test branches where I ran `wallet_basic.py` 50 times in parallel. One branch has this fix the other doesn't. The github action for the branch without this fix failed many times https://github.com/panleone/PIVX/actions/runs/8428460478 The github action for the branch with this fixed always passed instead https://github.com/panleone/PIVX/actions/runs/8428472901 ACKs for top commit: 35c9b1e Duddino: utACK 35c9b1e Liquid369: tACK 35c9b1e Fuzzbawls: ACK 35c9b1e Tree-SHA512: a5cea7f1b1478045f0e642c306f8f0427a10e92a13cc61e830bb214d4c5d1ba234cba1f628606ee02d0acc68c2719a981433f84fadbbd559af885e14e6353489
607ba81 test: add trivial validation invalid tests (ale) 2c87e78 test: add trivial validation tests (ale) 325a204 Verify match between type and tx version in GetValidatedTxPayload (ale) bf373bf Refactor special_tx_validation (ale) 78c7d26 Add SPECIAL_TX_TYPE member to payload classes (ale) 357f572 Add IsTriviallyValid for special payloads (ale) Pull request description: First 4 commits are a (almost) move only refactor: The self contained part of special transaction validation (i.e the part based only on specialtx info and not on full chain and masternode list) has been moved to functions `IsTriviallyValid`. Last 2 commits improve unit test coverage: I have added two files with a list of special txs which are either valid or invalid. Test consist in correctly deserializing the txs and verifying that valid/invalid ones are indeed considered valid/invalid. ACKs for top commit: 607ba81 Liquid369: tACK 607ba81 Duddino: utACK 607ba81 Fuzzbawls: ACK 607ba81 Tree-SHA512: 59a810d49c17943e2dfc10d95e7886922b42cfa2c91b6f8d77d64aa2a386fdebe137c11eb8f93188d79fcd0bae42aceeb2fc3d63af2a3abc1b2ac1cf103d31ea
471cae3 Do not sleep after staking and after sending pings (ale) Pull request description: `tiertwo_governance_sync_basic` is by far the slowest test that we have. Profiling showed that most of the time was spent in the function stake() which slept for a total of 2 seconds in each call. Those 2 seconds sleep were actually useless and have been removed. With this simple change the total running time of the test on my machine dropped from `15` minutes to only `6` minutes. To prove that indeed sleeping was useless I tried to run the same test in parallel 15 times, and it never failed, see here: https://github.com/panleone/PIVX/actions/runs/8420343441 ACKs for top commit: 471cae3 Duddino: utACK 471cae3 Liquid369: tACK 471cae3 Fuzzbawls: ACK 471cae3 Tree-SHA512: 6b00a18c7edd6dffaf9c5df3f53c8481c4312f7d8a63819010289c17b5ab7715aeafb901c8da885515fa59a093ab6e101c572972823ac2ca5e5c53139921f063
44116cf Separate Init from Start and Stop from Destroy functions for Chainlock handler (ale) 581eabe Add Interrupt step for signing_share thread and do NOT call StopWorkerThread twice (ale) 8e85ca2 promote chainLocksHandler to unique pointer (ale) 4f579eb promote quorumSigningManager to unique pointer (ale) 4820a63 promote quorumSigSharesManager to unique pointer (ale) Pull request description: Fix some issues in the LLMQ threads handling, that might be also causing some problems in some tests that randomly fail during the final `ShutDown()` phase. First 3 commits are just trivial refactor: Use unique pointers in place of normal pointers to be consistent with the style used for other llmq managers. Fourth commit: In `CSigSharesManager` the function `StopWorkerThread()` was being called twice. The wrong call was the one in the destructor which has been removed. The rest of the diff is basically a copy and paste from [net_masternodes.cpp](https://github.com/PIVX-Project/PIVX/blob/master/src/tiertwo/net_masternodes.cpp) (the variable `interruptNet` of that file plays the exact same role of the variable `interruptSigningShare` that I have added). This new version should be the right way to manage the thread Fifth commit: In the chainlock manager split the `Init` from the `Start` and the `Stop` from the `Destroy` phases, as we do for all other llmq managers. ACKs for top commit: 44116cf Duddino: utACK 44116cf Liquid369: tACK 44116cf Fuzzbawls: ACK 44116cf Tree-SHA512: 41a432781861753ce29904093fc6f8771d8745babd498526c5fa6890703de96078c6a242c42ebc292ddefa0c5c0488effb8f6f367771af229f066132a8004eba
05c30a8 Merge bitcoin#14413: tests: Allow closed rpc handler in assert_start_raises_init_error (MarcoFalke) cb65d0a Merge bitcoin#14993: rpc: Fix data race (UB) in InterruptRPC() (Wladimir J. van der Laan) cf6cb29 Merge bitcoin#14670: http: Fix HTTP server shutdown (Wladimir J. van der Laan) d19d9fa Call RPCNotifyBlockChange on RPC stopped (ale) af10755 Merge bitcoin#11006: Improve shutdown process (Wladimir J. van der Laan) Pull request description: The aim of this PR is fixing the functional test errors that we get sometimes during shutdown, for example the failure of this action https://github.com/PIVX-Project/PIVX/actions/runs/8429115630/job/23082877380 In a few words, on HTTP server shutdown, sometimes 2 seconds are not enough time to finish handling all the RPC requests. `2024-03-26T01:35:26Z HTTP event loop did not exit within allotted time, sending loopbreak` So after this point all existing RPC requests (in particular the `stop()` one) does not receive any answer... And this lead to functional tests failing at the end. For more info see the discussion in each backported PR. Backport bitcoin PRs bitcoin#11006 bitcoin#14670 bitcoin#14993 bitcoin#14413 ACKs for top commit: 05c30a8 Liquid369: tACK 05c30a8 Fuzzbawls: ACK 05c30a8 Tree-SHA512: 3fa2d7d9f11f851dbf3703a70e45a12905cb801156b32654f624487c386d76d4eebbc9d54418f4fbe0ac500afeed626636cda2dc942294ea991ff5cde47b07f4
A recent 3rd party server outage for the miniupnpc library has caused macOS 11 jobs to fail. While the homebrew package manager for macOS has fixed the issue on their end, GitHub has no clear intention to update their macOS 11 runner image due to macOS 11 runners already being deprecated. macOS 11 support will be removed from GA completely on June, 28 2024. Pre-emptively bump the os runner versions we use to macOS 12 and macOS 14 (the latter of which runs on Apple's new CPU architecture). Relevant and required build system changes have also been applied to support the new macOS 14 runner environment.
454fbfd [GA] Bump native macOS runner versions (Fuzzbawls) Pull request description: A recent 3rd party server outage for the miniupnpc library has caused macOS 11 jobs to fail. While the homebrew package manager for macOS has fixed the issue on their end, GitHub has no clear intention to update their macOS 11 runner image due to macOS 11 runners already being deprecated. macOS 11 support will be removed from GA completely on June, 28 2024. Pre-emptively bump the os runner versions we use to macOS 12 and macOS 14 (the latter of which runs on Apple's new CPU architecture). Relevant and required build system changes have also been applied to support the new macOS 14 runner environment. ACKs for top commit: 454fbfd panleone: utACK 454fbfd Liquid369: tACK 454fbfd Tree-SHA512: bb5b1965ae99b9e7eda0d464d07b1d8226a00a0aa27be84b2d7cf18f265ecdd759b5eaeba83316a75376100f8a37bb61fe96048a40746ed71febb3e2505cedd8
f9a6ebd fix: solve data race by making nTimeBestReceived atomic (ale) 4331c30 refactor: move nTimeBestReceived to CWallet (ale) Pull request description: First commit is a small refactor: the global variable `nTimeBestReceived` is now a private member of the `CWallet` class, as it was used only there. Second commit: fix the following data race by making `nTimeBestReceived` atomic ``` WARNING: ThreadSanitizer: data race (pid=18270) Write of size 8 at 0x7b6800000ed8 by thread T16: #0 CWallet::UpdatedBlockTip(CBlockIndex const*, CBlockIndex const*, bool) wallet/wallet.cpp:2094 (pivxd+0x5058c9) #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2ee1c0) #2 std::__invoke_result<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>::type std::__invoke<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2ee25a) #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>, std::_Placeholder<2>, std::_Placeholder<3>))(CBlockIndex const*, CBlockIndex const*, bool)>::__call<void, CBlockIndex const*&&, CBlockIndex const*&&, bool&&, 0ul, 1ul, 2ul, 3ul>(std::tuple<CBlockIndex const*&&, CBlockIndex const*&&, bool&&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/12/functional:484 (pivxd+0x2ee25a) ... Previous read of size 8 at 0x7b6800000ed8 by thread T35 (mutexes: write M134298, write M132880): #0 CWallet::ResendWalletTransactions(CConnman*) wallet/wallet.cpp:2065 (pivxd+0x515b25) #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2eef45) #2 std::__invoke_result<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>::type std::__invoke<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2eefb9) #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>))(CConnman*)>::__call<void, CConnman*&&, 0ul, 1ul>(std::tuple<CConnman*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/functional:484 (pivxd+0x2eefb9) ... ``` ACKs for top commit: f9a6ebd Duddino: utACK f9a6ebd Liquid369: tACK f9a6ebd Fuzzbawls: ACK f9a6ebd Tree-SHA512: c1de883956bf0958204b05c338831c27f694e6453e74b77e3347ae12a3cbd69df2fe1120bae3fca72dab14587c0c499c4f92d5c53943305855e66fc1275ab4fa
90b1892 build: AX_BOOST_UNIT_TEST_FRAMEWORK() serial 22 (Fuzzbawls) 92d478c build: AX_BOOST_THREAD() serial 33 (Fuzzbawls) 103b802 build: AX_BOOST_SYSTEM() serial 20 (Fuzzbawls) 1712d9a build: AX_BOOST_FILESYSTEM() serial 28 (Fuzzbawls) 583adf6 build: AX_BOOST_CHRONO() serial 5 (Fuzzbawls) 7d93e29 build: AX_BOOST_BASE() serial 54 (Fuzzbawls) 076fbf4 build: AX_PTHREAD() serial 31 (Fuzzbawls) 8813235 build: AX_GCC_FUNC_ATTRIBUTE() serial 13 (Fuzzbawls) 3596f0f build: AX_CXX_COMPILE_STDCXX() serial 18 (Fuzzbawls) 52a11ff build: AX_CHECK_PREPROC_FLAG() serial 6 (Fuzzbawls) 3200d3f build: AX_CHECK_LINK_FLAG() serial 6 (Fuzzbawls) fd78360 build: AX_CHECK_COMPILE_FLAG() serial 6 (Fuzzbawls) Pull request description: This is the first part in what will be a series of updates to our autotools build system. these commits are simply updating macro scripts from their upstream published versions. the only manual change that i've made is to ensure there is a new line at the bottom of each file. ACKs for top commit: 90b1892 panleone: utACK 90b1892, Liquid369: tACK 90b1892 Tree-SHA512: 273dec63811972835802ee890206e159494ec8934dac4e570bb74013473b4716022dcc933e593ab4634334a2acabf81e4e94df99d4d96b3aeaa6f328468bd7e1
ddbd4b0 fix BlockStateCatcher data races (ale) b01c3df let validationinterface take shared_ptr (ale) Pull request description: first commit: Partially backport bitcoin PR bitcoin#18338 which introduce the functions `RegisterSharedValidationInterface` and `UnregisterSharedValidationInterface`. See that PR why using normal pointers is problematic, and how `shared_ptr` solve the issue (In a few words the problem is that it can happen that the pointed memory is freed before all signals have been processed, while with shared pointer we are sure that memory will be freed after the last signal is handled) second commit: Utilize the new functions to the validation interface `BlockStateCatcher`. In order to keep everywhere the logic unchanged (Registering on creation and Unregistering when the object goes out of scope) I created the wrapper class `BlockStateCatcherWrapper` which contains a `shared_ptr` to `BlockStatecatcher`. Those two commits solve the following data race where a `BlockStateCatcher` pointer is dereferenced after the pointed memory is freed ``` WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) (pid=6423) Write of size 8 at 0x7fc3d7c2d570 by thread T20: #0 CValidationInterface::~CValidationInterface() validationinterface.h:75 (pivxd+0xeacac) #1 BlockStateCatcher::~BlockStateCatcher() util/blockstatecatcher.h:23 (pivxd+0xeacac) #2 generateBlocks(Consensus::Params const&, CWallet*, bool, int, int, int, CScript*) rpc/mining.cpp:78 (pivxd+0x1c1a98) #3 generate(JSONRPCRequest const&) rpc/mining.cpp:140 (pivxd+0x1c2215) ... Previous read of size 8 at 0x7fc3d7c2d570 by thread T35 (mutexes: write M133270, write M132847): #0 void std::__invoke_impl<void, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2eef14) #1 std::__invoke_result<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>::type std::__invoke<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2eefbb) #2 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>))(CConnman*)>::__call<void, CConnman*&&, 0ul, 1ul>(std::tuple<CConnman*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/functional:484 (pivxd+0x2eefbb) ... ``` ACKs for top commit: ddbd4b0 Duddino: utACK ddbd4b0 Liquid369: tACK ddbd4b0 Tree-SHA512: 94d55effc11f14f755f27c30ef1b6801a235bc314cc34312ebb14b75525824d3b11bb7812389d9977c30a0929d62fb745bd5c1a15249708014bd573132a5e55c
f5ba264 fix another possible deadlock (ale) f162f76 fix: fix data race and remove circular dependency (ale) 9207b8f fix data race: LOCK cs before accessing nodeState (ale) 58e5c8c fix: Solve possible deadlock (ale) Pull request description: This PR fixes more thread issues that ThreadSanitizer showed: first commit: Solve a possible 3-threads deadlock fixed by locking `mempool.cs` before `cs_inventory` (see the comment for the log) second commit: solve the trivial data race where `nodeState` is accessed without locking the corresponding mutex third commit: solve all the issues that the function `getQuorumNodes` had: - `it` iterator was accessed without having the lock on `cs_vPendingMasternodes` which was causing a possible data race - `connman->vNodes` was accessed without locking the mutex `cs_vNodes` (another possible data race) Solved by using the function `connman->ForEachNode` and removing the useless getter. fourth commit: Solve a possible deadlock: lock `cs_vPendingMasternodes` before calling `connmann->ForEachNode(...)` ACKs for top commit: f5ba264 Liquid369: tACK f5ba264 Fuzzbawls: ACK f5ba264 Tree-SHA512: 2fa61ec6b3ad395cbe968874a90f1ec16eb5ddec85d48ddeb438fc92f4ae281d4ff7c5b297b147c4b3bf60d6a4849c0619567e3beccb930a76f61d0815bea1d5
3ba861c [GA] Overhaul Github Actions Workflow (Fuzzbawls) Pull request description: This is a major overhaul of our GitHub Actions workflow file that introduces several cleanups and optimizations, as well as new jobs dedicated to running just the python functional test suite. Below are some of the more notable changes: - Split off functional tests to their own jobs. Often times we will see a trivial failure in one of the functional tests. Prior to this change, this meant needing to re-run the entire build job again. With this change, we can trigger a re-run of just the functional test suite without needing to re-compile the wallet again. This is handled by creating a minimal archive "artifact" in the build job, and passing it to the new functional test jobs. Credit goes to @Duddino for coming up with this idea and doing much of the initial research and testing! - Split old configure/build/unittest groupings into their own steps. Instead of exclusively relying on older group macros in a single step, I've created new more descriptive steps to improve readability and responsiveness of the actions report portal. This also helps in seeing where any significant bottle-neck is for a particular job configuration. - Unify the `apt_get` and `brew_install` matrix config params. These two params have been unified to just `packages` across all matrix configurations. - No longer cache the sapling params. Since the sapling params files are part of our source tree anyways, caching them in GitHub Actions cache files is redundant and only serves to waste valuable cache storage space. - Add some basic descriptive commentary for jobs. Self-explanatory. Note: The minimalist build artifacts are ONLY intended to be used within the GitHub Actions environment and expire after 5 days from their creation. Failing functional tests that are not resolved within this expiry time will necessitate a full re-run of the workflow. Any use of these artifacts outside of GitHub Actions is discouraged and unsupported. --- Reviewer's note: GitHub will report this PR as not having all the "required checks" as passing as the names/IDs of jobs have been changed. Once this PR is merged it will be then possible to update the repository settings to reflect the new job names/IDs in the "required checks" settings. ACKs for top commit: 3ba861c Duddino: ACK 3ba861c panleone: utACK 3ba861c Tree-SHA512: f5d7ff65c5dd37dc24c64dfa47038126923c92146a1fd61410e902cb452e350b49d65a9c8a55c1281b412713825ffc8d794d275519c632e4e2b541b0339cc54a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.