pivx-core:master

Last commit made on 2024-04-06
Get this branch:
git clone -b master https://git.launchpad.net/pivx-core

Branch merges

Branch information

Name:
master
Repository:
lp:pivx-core

Recent commits

db78a7e... by Fuzzbawls

Merge #2922: [GA] Bump native macOS runner versions

454fbfd858f282858b372361b660ac79d35d327b [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: 454fbfd858f282858b372361b660ac79d35d327b
  panleone:
    utACK 454fbfd858f282858b372361b660ac79d35d327b
  Liquid369:
    tACK 454fbfd858f282858b372361b660ac79d35d327b

Tree-SHA512: bb5b1965ae99b9e7eda0d464d07b1d8226a00a0aa27be84b2d7cf18f265ecdd759b5eaeba83316a75376100f8a37bb61fe96048a40746ed71febb3e2505cedd8

454fbfd... by Fuzzbawls

[GA] Bump native macOS runner versions

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.

f428a74... by Fuzzbawls

Merge #2920: [Http] [Test] Fix test failures on shutdown

05c30a84fb834db3a6945585fad9994607cc5610 Merge #14413: tests: Allow closed rpc handler in assert_start_raises_init_error (MarcoFalke)
cb65d0af5492d917d394e0aab70668764a35dd3d Merge #14993: rpc: Fix data race (UB) in InterruptRPC() (Wladimir J. van der Laan)
cf6cb299c165c491994a75dc984ae6d3362278ab Merge #14670: http: Fix HTTP server shutdown (Wladimir J. van der Laan)
d19d9faad595fa54a03a244eb7efa1219bb3a926 Call RPCNotifyBlockChange on RPC stopped (ale)
af10755fcd0795524de6321a20239b8189cd52b5 Merge #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 #11006 #14670 #14993 #14413

ACKs for top commit: 05c30a84fb834db3a6945585fad9994607cc5610
  Liquid369:
    tACK 05c30a84fb834db3a6945585fad9994607cc5610
  Fuzzbawls:
    ACK 05c30a84fb834db3a6945585fad9994607cc5610

Tree-SHA512: 3fa2d7d9f11f851dbf3703a70e45a12905cb801156b32654f624487c386d76d4eebbc9d54418f4fbe0ac500afeed626636cda2dc942294ea991ff5cde47b07f4

7954d0a... by Fuzzbawls

Merge #2917: [LLMQ] Improve thread handling

44116cf9f0eaf9855d10aeb179990b72dd79b556 Separate Init from Start and Stop from Destroy functions for Chainlock handler (ale)
581eabebe97e463543e9e69aa436c0ee853d7664 Add Interrupt step for signing_share thread and do NOT call StopWorkerThread twice (ale)
8e85ca21ea94a8a25f4567fea487c695b8700fd1 promote chainLocksHandler to unique pointer (ale)
4f579eb29177a6e5d269c48dbe94987d27e32d20 promote quorumSigningManager to unique pointer (ale)
4820a6338ba15c2f41099af3508519d97f60a016 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: 44116cf9f0eaf9855d10aeb179990b72dd79b556
  Duddino:
    utACK 44116cf9f0eaf9855d10aeb179990b72dd79b556
  Liquid369:
    tACK 44116cf9f0eaf9855d10aeb179990b72dd79b556
  Fuzzbawls:
    ACK 44116cf9f0eaf9855d10aeb179990b72dd79b556

Tree-SHA512: 41a432781861753ce29904093fc6f8771d8745babd498526c5fa6890703de96078c6a242c42ebc292ddefa0c5c0488effb8f6f367771af229f066132a8004eba

e4f61cd... by Fuzzbawls

Merge #2916: [Test] speed un tiertwo_governance_sync_basic

471cae3838f4a0c133fff7800162df816bf90567 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: 471cae3838f4a0c133fff7800162df816bf90567
  Duddino:
    utACK 471cae3838f4a0c133fff7800162df816bf90567
  Liquid369:
    tACK 471cae3838f4a0c133fff7800162df816bf90567
  Fuzzbawls:
    ACK 471cae3838f4a0c133fff7800162df816bf90567

Tree-SHA512: 6b00a18c7edd6dffaf9c5df3f53c8481c4312f7d8a63819010289c17b5ab7715aeafb901c8da885515fa59a093ab6e101c572972823ac2ca5e5c53139921f063

d75c161... by Fuzzbawls

Merge #2915: [Test] [DMNs] Split special transaction validation

607ba813f69139ecc7246431855be7c5b8b657d0 test: add trivial validation invalid tests (ale)
2c87e7863fa2360a4b022629da4371946aef3c19 test: add trivial validation tests (ale)
325a204e97238ca13352d43e8f7cfc1680f589f0 Verify match between type and tx version in GetValidatedTxPayload (ale)
bf373bf1aee415a954b5a18aa410580c3a8f23cc Refactor special_tx_validation (ale)
78c7d266f7e8c041fd12adbe9200c169241a3a7a Add SPECIAL_TX_TYPE member to payload classes (ale)
357f572d1b0694bdc6832d5dd5390e861c36a63b 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: 607ba813f69139ecc7246431855be7c5b8b657d0
  Liquid369:
    tACK 607ba813f69139ecc7246431855be7c5b8b657d0
  Duddino:
    utACK 607ba813f69139ecc7246431855be7c5b8b657d0
  Fuzzbawls:
    ACK 607ba813f69139ecc7246431855be7c5b8b657d0

Tree-SHA512: 59a810d49c17943e2dfc10d95e7886922b42cfa2c91b6f8d77d64aa2a386fdebe137c11eb8f93188d79fcd0bae42aceeb2fc3d63af2a3abc1b2ac1cf103d31ea

75ad914... by Fuzzbawls

Merge #2919: [Bug] Fix data race

35c9b1efd3edcccde1b67bd0358e90ae1f9a80e3 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: 35c9b1efd3edcccde1b67bd0358e90ae1f9a80e3
  Duddino:
    utACK 35c9b1efd3edcccde1b67bd0358e90ae1f9a80e3
  Liquid369:
    tACK 35c9b1efd3edcccde1b67bd0358e90ae1f9a80e3
  Fuzzbawls:
    ACK 35c9b1efd3edcccde1b67bd0358e90ae1f9a80e3

Tree-SHA512: a5cea7f1b1478045f0e642c306f8f0427a10e92a13cc61e830bb214d4c5d1ba234cba1f628606ee02d0acc68c2719a981433f84fadbbd559af885e14e6353489

05c30a8... by Bitcoin Core

Merge #14413: tests: Allow closed rpc handler in assert_start_raises_init_error

62c304ea48 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

cb65d0a... by Wladimir J. van der Laan

Merge #14993: rpc: Fix data race (UB) in InterruptRPC()

6c10037f72073eecc674c313580ef50a4f1e1e44 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

cf6cb29... by Wladimir J. van der Laan

Merge #14670: http: Fix HTTP server shutdown

28479f926f21f2a91bec5a06671c60e5b0c55532 qa: Test bitcond shutdown (João Barbosa)
8d3f46ec3938e2ba17654fecacd1d2629f9915fd http: Remove timeout to exit event loop (João Barbosa)
e98a9eede2fb48ff33a020acc888cbcd83e24bbf http: Remove unnecessary event_base_loopexit call (João Barbosa)
6b13580f4e3842c11abd9b8bee7255fb2472b6fe http: Unlisten sockets after all workers quit (João Barbosa)
18e968581697078c36a3c3818f8906cf134ccadd http: Send "Connection: close" header if shutdown is requested (João Barbosa)
02e1e4eff6cda0bfc24b455a7c1583394cbff6eb rpc: Add wait argument to stop (João Barbosa)

Pull request description:

  Fixes #11777. Reverts #11006. Replaces #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