Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: trivial 2024 10 23 pr10 #6353

Commits on Oct 24, 2024

  1. Merge bitcoin#24988: lint: Mention NONFATAL_UNREACHABLE in lint-asser…

    …tions.py
    
    fa82a1e lint: Mention NONFATAL_UNREACHABLE in lint-assertions.py (MacroFake)
    
    Pull request description:
    
      Follow up to commit b1c5991. Also remove empty newline added in that commit.
    
    ACKs for top commit:
      fanquake:
        ACK fa82a1e
    
    Tree-SHA512: cf398eceb135672137183bfa19ee57a82553a3dbcbce74db954c6fcd79f9606092cc0d8217610fe6cd67b7ef2d4f01d90329f0f568516d9b14aa2cd0f0715478
    fanquake authored and PastaPastaPasta committed Oct 24, 2024
    Configuration menu
    Copy the full SHA
    f39fcd1 View commit details
    Browse the repository at this point in the history
  2. Merge bitcoin#25092: doc: various developer notes updates

    6542842 Add clang lifetimebound section to developer notes (Jon Atack)
    e66b321 Add C++ functions and methods section to developer notes (Jon Atack)
    5fca70f Link in developer notes style to internal interface exception (Jon Atack)
    fc4cb85 Prefer Python for scripts in developer notes (Jon Atack)
    370120e Remove obsolete BDB ENABLE_WALLET section in developer notes (Jon Atack)
    
    Pull request description:
    
      A few updates noticed while working on a lifetimebound section.
    
      - Remove obsolete BDB ENABLE_WALLET section (only one file, src/wallet/bdb.h, still has a `db_cxx.h` BDB header)
      - Prefer Python for scripts in developer notes (and a few miscellaneous touch-ups)
      - In the code style section, add a link to the internal interface exception so that people are aware of it
      - Add a "C++ functions and methods" section
      - Add a Clang `lifetimebound` attribute section
    
    ACKs for top commit:
      laanwj:
        ACK 6542842
      jarolrod:
        code review ACK bitcoin@6542842
    
    Tree-SHA512: d2ae335cac899451d5c7bdc8e94fd82053a5f44ac1ba444bdde71abaaa24a519713c1078f3a8f07ec8466b181788a613fd3c68061e54b3fdc8cd6f3e3f9791ec
    MacroFake authored and PastaPastaPasta committed Oct 24, 2024
    Configuration menu
    Copy the full SHA
    c91f010 View commit details
    Browse the repository at this point in the history
  3. Merge bitcoin#24062: refactor: replace RecursiveMutex `m_most_recent_…

    …block_mutex` with Mutex
    
    83003ff refactor: replace RecursiveMutex `m_most_recent_block_mutex` with Mutex (Sebastian Falbesoner)
    8edd0d3 refactor: reduce scope of lock `m_most_recent_block_mutex` (Sebastian Falbesoner)
    
    Pull request description:
    
      This PR is related to bitcoin#19303 and gets rid of the RecursiveMutex `m_most_recent_block_mutex`. All of the critical sections (5 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex:
    
      https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1650-L1655
    
      https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1861-L1865
    
      https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3149-L3152
    
      https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3201-L3206
    
      https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L4763-L4769
    
      The scope of the last critical section is reduced in the first commit, in order to avoid calling the non-trivial method `CConnman::PushMessage` while the lock is held.
    
    ACKs for top commit:
      furszy:
        Code ACK 83003ff with a small comment.
      hebasto:
        ACK 83003ff
      w0xlt:
        ACK bitcoin@83003ff
    
    Tree-SHA512: 3df290cafd2f6c4d40afb9f14e822a77d9c1828e66f5e2233f3ac1deccc2b0a8290bc5fb8eb992f49d39e887b50bc0e9aad63e05db2d870791a8d409fb95695f
    MacroFake authored and PastaPastaPasta committed Oct 24, 2024
    Configuration menu
    Copy the full SHA
    de17997 View commit details
    Browse the repository at this point in the history

Commits on Oct 25, 2024

  1. Merge bitcoin#25486: test: fix failing test `interface_usdt_utxocache…

    ….py`
    
    f665c6e test: fix failing test interface_usdt_utxocache.py (Sebastian Falbesoner)
    
    Pull request description:
    
      The `from_node` argument doesn't exist anymore for `MiniWallet.create_self_transfer` since PR bitcoin#25435 (commit fa8421b), leading to an error on master:
    
      ```
      $ sudo ./test/functional/interface_usdt_utxocache.py
      2022-06-27T17:45:35.585000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_7s1djjo1
      2022-06-27T17:45:36.515000Z TestFramework (INFO): testing the utxocache:uncache tracepoint API
      2022-06-27T17:45:36.517000Z TestFramework (ERROR): Unexpected exception caught during testing
      Traceback (most recent call last):
        File "/home/honeybadger/bitcoin/test/functional/test_framework/test_framework.py", line 133, in main
          self.run_test()
        File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 149, in run_test
          self.test_uncache()
        File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 172, in test_uncache
          invalid_tx = self.wallet.create_self_transfer(
      TypeError: create_self_transfer() got an unexpected keyword argument 'from_node'
      2022-06-27T17:45:36.568000Z TestFramework (INFO): Stopping nodes
      [...]
      ```
      Fix this by removing the argument. (Unfortunately, the USDT tests don't seem to run on any CI target, I guess that's due to missing permissions to hook into the kernel.)
    
    ACKs for top commit:
      MarcoFalke:
        cr ACK f665c6e
    
    Tree-SHA512: 74f8e398739a25ab5518ff71b998d03d4e529a786ba5b424509de81a511ad3e2e1cd38a5b7bb9f1f5a21340391d6807f4951ff39fa3a2ad65a3b11b989eebea6
    MacroFake authored and PastaPastaPasta committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    1204dc0 View commit details
    Browse the repository at this point in the history
  2. Merge bitcoin#25599: build: Check for std::atomic::exchange rather th…

    …an std::atomic_exchange
    
    4de4221 build: Check for std::atomic::exchange rather than std::atomic_exchange (Andrew Chow)
    
    Pull request description:
    
      Our usage of std::atomic is with it's own exchange function, not std::atomic_exchange. So we should be looking specifically for that function.
    
      This removes the need for -latomic for riscv builds, which resolves a guix cross architecture reproducibility issue.
    
    ACKs for top commit:
      hebasto:
        ACK 4de4221
      fanquake:
        ACK 4de4221
    
    Tree-SHA512: dd8225fc9c6a335601f611700003d0249b9ef941efa502db39306129677929d013048e9221be1d6d7f0ea2d90313d4b87de239f441be21b25bea40a6c19a031e
    fanquake authored and PastaPastaPasta committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    b66eebe View commit details
    Browse the repository at this point in the history
  3. Merge bitcoin#23997: wallet: avoid rescans under assumed-valid blocks

    817326a wallet: avoid rescans if under the snapshot (James O'Beirne)
    
    Pull request description:
    
      This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: bitcoin#15606)
    
      ---
    
      Refuse to load a wallet if it requires a rescan lower than the height of assumed-valid blocks.
    
      Of course in live code right now, `BLOCK_ASSUMED_VALID` block index entries don't exist since they're a unique flag introduced by the use of UTXO snapshots, so this is prophylactic code exercised only by unittests.
    
    ACKs for top commit:
      achow101:
        ACK 817326a
      ryanofsky:
        Code review ACK 817326a. This seems like the simplest change we can make to avoid wallet problems when an assumeutxo snapshot is loaded.
    
    Tree-SHA512: cfa44b2eb33d1818d30df45210d0dde1e9b78cc9b7c88cb985054dc28427bba9e0905debe4196065d1d3a5ce7bca7e605e629d5ce5f0225b25395746e6d3d596
    achow101 authored and PastaPastaPasta committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    e2e8598 View commit details
    Browse the repository at this point in the history
  4. Merge bitcoin#26282: wallet: have prune error take precedence over as…

    …sumedvalid
    
    1c36baf wallet: have prune error take precedence over assumedvalid (James O'Beirne)
    
    Pull request description:
    
      Fixes bitcoin#23997 (comment).
    
      From Russ Yanofsky:
    
      > Agree with all of Marco's points here and think this should be updated
      >
      > If havePrune and hasAssumedValidChain are both true, better to show havePrune error message.  Assumed-valid error message is vague and not very actionable.  Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}"
    
    ACKs for top commit:
      MarcoFalke:
        ACK 1c36baf
      aureleoules:
        ACK 1c36baf
    
    Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
    fanquake authored and PastaPastaPasta committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    c59cb15 View commit details
    Browse the repository at this point in the history
  5. Merge bitcoin#28304: doc: Remove confusing assert linter

    fa6e6a3 doc: Remove confusing assert linter (MarcoFalke)
    
    Pull request description:
    
      The `assert()` documentation and linter are redundant and confusing:
    
      * The source code already refuses to compile with `assert()` disabled.
      * They violate the assumptions about `Assert()`, which *requires* side effects.
      * The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects.
    
      Fix all issues by removing the docs and the linter. See also bitcoin#26684 (comment)
    
      Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit.
    
    ACKs for top commit:
      hebasto:
        ACK fa6e6a3, I have reviewed the code and it looks OK.
      theStack:
        ACK fa6e6a3
    
    Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236
    fanquake authored and PastaPastaPasta committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    4101fea View commit details
    Browse the repository at this point in the history