Commit Graph

27137 Commits

Author SHA1 Message Date
Konstantin Akimov
96c9b469ca
fix: getblock for withdrawal transaction if verbosity level is 2 2024-10-22 21:11:29 +07:00
pasta
0946eec692
Merge #6327: backport: merge bitcoin#23046, #24138, #24812, #24626, #21726, #25123, #25074, #24832, #26215, #24858, #26417, #16981 (index backports)
7d9ff96091 merge bitcoin#16981: Improve runtime performance of --reindex (Kittywhiskers Van Gogh)
e531dff5f7 merge bitcoin#26417: fix intermittent failure in feature_index_prune.py (Kittywhiskers Van Gogh)
b04b71a957 merge bitcoin#24858: incorrect blk file size calculation during reindex results in recoverable blk file corruption (Kittywhiskers Van Gogh)
9e75b99c53 merge bitcoin#26215: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability (Kittywhiskers Van Gogh)
3bd584c845 merge bitcoin#24832: Verify the block filter hash when reading the filter from disk (Kittywhiskers Van Gogh)
e507a51323 fix: avoid `mandatory-script-verify-flag-failed` crash in bench test (Kittywhiskers Van Gogh)
a86109a017 merge bitcoin#25074: During sync, commit best block after indexing (Kittywhiskers Van Gogh)
e6867a35ce merge bitcoin#25123: Fix race condition in index prune test (Kittywhiskers Van Gogh)
baf6e26eed merge bitcoin#21726: Improve Indices on pruned nodes via prune blockers (Kittywhiskers Van Gogh)
c65ec190c5 merge bitcoin#24626: disallow reindex-chainstate when pruning (Kittywhiskers Van Gogh)
bcd24a25e3 fix: push activation height for forks ahead, fix `feature_pruning.py` (Kittywhiskers Van Gogh)
10203560f5 merge bitcoin#24812: Add CHECK_NONFATAL identity function and NONFATAL_UNREACHABLE macro (Kittywhiskers Van Gogh)
1caaa85716 merge bitcoin#24138: Commit MuHash and best block together for coinstatsindex (Kittywhiskers Van Gogh)
b218f123b7 merge bitcoin#23046: Add txindex migration test (Kittywhiskers Van Gogh)
ebae59eedf fix: make sure we flush our committed best block in no-upgrade cases (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * When backporting [bitcoin#23046](https://github.com/bitcoin/bitcoin/pull/23046), it was discovered that there has been a longstanding bug in `CDeterministicMNManager::MigrateDBIfNeeded`(`2`)`()` that flagged a database taken from an older version for failing its "previous migration attempt", requiring the database to be fully rebuilt through a reindex.

    This occurred because the older database would be read pre-DIP3 in `MigrateDBIfNeeded()`, which then caused the migration logic to write the new best block ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1236-L1241))) (the legacy best block is erased before the DIP3 condition is checked, [source](3f0c2ff324/src/evo/deterministicmns.cpp (L1233))) but while it completed the transaction ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1240))), critically, it didn't write it to disk (example of writing to disk, [here](3f0c2ff324/src/evo/deterministicmns.cpp (L1288-L1292))).

    This meant that when it was read again by `MigrateDBIfNeeded2()`, it saw three things a) there is no new best block (because it didn't get written), b) there is no legacy best block (because it gets erased before the new best block is written) and c) that the chain height is greater than 1 (since this isn't a new datadir and the chain has already advanced), it concludes that it was a botched migration attempt and fails ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1337-L1343))).

    This bug affects v19 to `develop` (`3f0c2ff3` as of this writing) and prevents `feature_txindex_compatibility.py` from working as expected as it would migrate legacy databases to newer versions to test txindex migration code and get stuck at unhappy EvoDB migration logic, to allow the test to function properly when testing against the latest version of the client, this bug has been fixed as part of this PR.

  * In [bitcoin#23046](https://github.com/bitcoin/bitcoin/pull/23046), version v0.17 was used as the last version to support legacy txindex as the updated txindex format was introduced in [dash#4178](https://github.com/dashpay/dash/pull/4178) (i.e. after v0.17) and the version selected for having migration code in it (note, migration code was removed in [dash#6296](https://github.com/dashpay/dash/pull/6296), so far not included as part of any release) was v18.2.2 despite the range being v18.x to v21.x was a) due to the bug mentioned above affecting v19.x onwards and b) v18.2.2 being the latest release in the v18.x lifecycle.

    * The specific version number used for v0.17 is `170003` as the binaries corresponding to `170000` are not populated in `releases/`, which causes a CI failure ([source](https://gitlab.com/dashpay/dash/-/jobs/8073041955#L380))

  * As of `develop` (`3f0c2ff3` as of this writing), `feature_pruning.py` was broken due to changes in Core that were not adjusted for, namely:
    * The enforcement of `MAX_STANDARD_TX_SIZE` ([source](3f0c2ff324/src/policy/policy.h (L23-L24))) from DIP1 onwards ([source](3f0c2ff324/src/validation.cpp (L299-L301)))  resulting in `bad-txns-oversize` errors in blocks generated for the test as the transactions generated are ~9.5x larger than the now-enforced limit ([source](3f0c2ff324/test/functional/feature_pruning.py (L48C51-L48C57))), this is resolved by pushing the DIP1 activation height upwards to `2000` (the same activation height used for DIP3 and DIP8).
    * Change in subsidy logic in v20 ([source](3f0c2ff324/src/validation.cpp (L1073-L1082))) that results in `bad-cb-amount` errors, this is resolved by pushing the v20 activation height upwards.

    Additionally, an inopportune implicit post-`generate` sync ([source](3f0c2ff324/test/functional/feature_pruning.py (L215))) also causes the test to fail. Alongside the above, they have been resolved in this PR.

  * As of `develop` (`3f0c2ff3` as of this writing), `bench_dash` crashes when running the `AssembleBlock` benchmark. The regression was traced back to [bitcoin#21840](https://github.com/bitcoin/bitcoin/pull/21840) (5d10b41) in [dash#6152](https://github.com/dashpay/dash/pull/6152) due to the switch to `P2SH_OP_TRUE`.

    This has been resolved by reverting this particular change.

    <details>

    <summary>Pre-fix test failure:</summary>

    ```
    $ ./src/bench/bench_dash
    Warning, results might be unstable:
    * CPU governor is '' but should be 'performance'
    * Turbo is enabled, CPU frequency will fluctuate

    Recommendations
    * Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf

    |               ns/op |                op/s |    err% |          ins/op |         bra/op |   miss% |     total | benchmark
    |--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:----------
    |       17,647,657.00 |               56.66 |    0.1% |  231,718,349.00 |  42,246,265.00 |    0.1% |      0.20 | `AddrManAdd`
    |       42,201,861.00 |               23.70 |    0.1% |  544,366,811.00 | 102,569,244.00 |    0.0% |      0.46 | `AddrManAddThenGood`
    |          189,697.83 |            5,271.54 |    0.1% |    1,763,991.40 |     356,189.40 |    0.3% |      0.01 | `AddrManGetAddr`
    |              454.38 |        2,200,808.04 |    0.6% |        6,229.11 |       1,343.92 |    0.1% |      0.01 | `AddrManSelect`
    |        1,066,471.00 |              937.67 |   67.6% |   13,350,463.00 |   3,150,465.00 |    0.4% |      0.01 | 〰️ `AddrManSelectByNetwork` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
    |        1,181,774.50 |              846.19 |   49.0% |   18,358,489.50 |   4,224,642.50 |    0.0% |      0.02 | 〰️ `AddrManSelectFromAlmostEmpty` (Unstable with ~1.1 iters. Increase `minEpochIterations` to e.g. 11)
    bench_dash: bench/block_assemble.cpp:46: void AssembleBlock(benchmark::Bench &): Assertion `res.m_result_type == MempoolAcceptResult::ResultType::VALID' failed.
    [1]    2343746 IOT instruction (core dumped)  ./src/bench/bench_dash
    ```
    </details>

  ## Breaking changes

  None expected

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 7d9ff96091
  PastaPastaPasta:
    utACK 7d9ff96091

Tree-SHA512: e2f1e58abb0a0368c4f1d5e488520957e042e6207b7d2d68a15eb18662405a3cdac91c5ff8e93c8a94c0fdab9b1412bd608d055f196230506c1640439939c25d
2024-10-21 11:32:29 -05:00
pasta
b7d543061d
Merge #6279: fix: new fork 'withdrawals' and fixes for asset unlock signature verification
d690309513 fix: limit amount of attempts for test `test_withdrawal_fork` (Konstantin Akimov)
ecd0a96f63 test: fix test_withdrawal_fork (UdjinM6)
6fbd128947 chore: clang format/reword/typos (UdjinM6)
5fd7b07ddc chore: test new validation of asset unlocks tests after fork 'withdrawals' (Konstantin Akimov)
27040030e9 fix: using proper quorums for asset unlock validation (Konstantin Akimov)
0253438503 feat: new fork WITHDRAWALS introduced (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  https://github.com/dashpay/dash-issues/issues/83

  ## What was done?
  Introduces new fork "withdrawals" which let Asset Unlock be valid for any active quorum + 1 extra inactive (in opposite of hard-coded 2 of them).

  ## How Has This Been Tested?
  See new test section `test_withdrawals_fork` in functional test feature_asset_locks.py

  ## Breaking Changes
   - new fork "withdrawals"
   - new logic of validation of Asset Unlock transactions's signature. Even no asset-unlock is mined yet, previous version of clients will refuse blocks which is fine for current system

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK d690309513
  PastaPastaPasta:
    utACK d690309513

Tree-SHA512: cac28cf974e97d4a4100c22d5aa07b398b15413235383c68a0f4ff005005b28892c7ac8e6b703582c126cb6e0ff80a950f190dd32268655fac534ed6f2a90d03
2024-10-21 11:17:37 -05:00
pasta
40dc6d9e83
Merge #6256: backport: bitcoin#23590, 23061, 23694
7d170bd1cc Merge bitcoin/bitcoin#23694: doc: Add missing optional to MempoolEntryDescription (MarcoFalke)
ed7ad7af74 Merge bitcoin/bitcoin#23061: Fix (inverse) meaning of -persistmempool (merge-script)
f5f69ae982 Merge bitcoin/bitcoin#23590: Crash debug builds when mempool ConsensusScriptChecks fails (MarcoFalke)

Pull request description:

  more bitcoin backports

ACKs for top commit:
  UdjinM6:
    utACK 7d170bd1cc
  PastaPastaPasta:
    utACK 7d170bd1cc

Tree-SHA512: 0332461598c9aa17476b591e98a2b3a952fcea0b7abbda604668b13745bf73268e0faba25a9338eabb8d27fe594c26804f8cc653c30c4f53e6b88f0f7fced287
2024-10-21 09:30:48 -05:00
pasta
7701b8dcea
Merge #6319: backport: bitcoin#19668, #21598, #19979, #28774 - lock annotations
1db9e6ac76 Merge #19979: Replace LockAssertion with AssertLockHeld, remove LockAssertion (MarcoFalke)
d0a4198166 Merge #21598: refactor: Remove negative lock annotations from globals (MarcoFalke)
55114a682e Merge #19668: Do not hide compile-time thread safety warnings (MarcoFalke)
898282d620 Merge bitcoin/bitcoin#28774: wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it (Ava Chow)
6d452845dc fix: add missing lock annotation for ContextualCheckBlock and related TODO (Konstantin Akimov)
846ebab6e0 fix: fixes for orphange's locks and cs_main (Konstantin Akimov)
002580c42d fix: add ignoring safety-annotation for llmq/signing_shares due to template lambdas (Konstantin Akimov)
502d6ae8ef fix: add multiple missing annotation about locks for dash specific code (Konstantin Akimov)
a219a8be15 partial Merge #29040: refactor: Remove pre-C++20 code, fs::path cleanup (Konstantin Akimov)
042e8a4101 fix: drop unneded lock assert in net.cpp for m_nodes_mutex (Konstantin Akimov)

Pull request description:

  ## What was done?
  These 4 backports improve noticeable implementation of thread-safety analysis by moving many possible warnings to compilation level.
  There's a lot of fixes for annotations after that!

  ## How Has This Been Tested?
  Build with clang:
  ```
  CC=clang CXX=clang++ ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings   --enable-debug  --enable-stacktraces --enable-werror --enable-crash-hooks --enable-maintainer-mode
  ```

  ## Breaking Changes
  n/a

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 1db9e6ac76
  PastaPastaPasta:
    utACK 1db9e6ac76

Tree-SHA512: 42a73e423f311689babc366b58a92f024556cf63af93357cfa0715c471efc1491d02ed9e7ba19b23efcdf423ab5bd051a24da2ad64c32e4b6db132a05f6669c1
2024-10-21 09:08:38 -05:00
Konstantin Akimov
d690309513
fix: limit amount of attempts for test test_withdrawal_fork 2024-10-17 23:10:37 +07:00
MarcoFalke
7d170bd1cc
Merge bitcoin/bitcoin#23694: doc: Add missing optional to MempoolEntryDescription
fa1571b156dfa757d0ce0fddd53c25256196522b doc: Add missing optional to MempoolEntryDescription (MarcoFalke)

Pull request description:

  Needed for https://github.com/bitcoin/bitcoin/pull/23083.

  Can be reviewed with `--word-diff-regex=.`.

ACKs for top commit:
  josibake:
    ACK fa1571b156
  shaavan:
    ACK fa1571b156dfa757d0ce0fddd53c25256196522b

Tree-SHA512: b4370003d2aeadce438778e15bd9a0d6a7fef4711acbe8471a50a9d72bbf74e1705fecbaae6f7eb367ece7c795a816c4b8b6583ed6c8f91b35621ca30fd95c18
2024-10-17 20:25:29 +05:30
merge-script
ed7ad7af74
Merge bitcoin/bitcoin#23061: Fix (inverse) meaning of -persistmempool
faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a doc: Add 23061 release notes (MarcoFalke)
faff17bbde6dcb1482a6210bc48b3192603a446f Fix (inverse) meaning of -persistmempool (MarcoFalke)

Pull request description:

  Passing `-persistmempool` is currently treated as `-nopersistmempool`

ACKs for top commit:
  jnewbery:
    reACK faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a
  hebasto:
    ACK faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: f34a89a07745dabe340eb845b2a348b79c093e9056f7a21c17e1ba2e278177c9b4cf30e8095791fd645a7f90eb34850b2eee0c869b4f6ec02bf749c73b0e52ee
2024-10-17 20:25:27 +05:30
MarcoFalke
f5f69ae982
Merge bitcoin/bitcoin#23590: Crash debug builds when mempool ConsensusScriptChecks fails
faad05c6d2ad1840c26f6fdea7d358bf9ba6efea Crash debug builds when mempool ConsensusScriptChecks fails (MarcoFalke)

Pull request description:

  Currently a bug in the function might sneak around our testing infrastructure.

  Fix that by turning bugs into crashes during tests.

ACKs for top commit:
  glozow:
    utACK faad05c6d2ad1840c26f6fdea7d358bf9ba6efea, there's something seriously wrong with the code if this returns false, good to throw in debug mode

Tree-SHA512: dfea1cd9ce3f1c303f49cca1417cd5c77c6ed12849aaff7b6ab1b6060f2f0c9cf5d4689017355d11f66639bab35823f65f848e6979042fa875181509dfd5d3d7
2024-10-17 20:19:46 +05:30
UdjinM6
ecd0a96f63
test: fix test_withdrawal_fork 2024-10-17 03:01:39 +07:00
pasta
5903fb7898
Merge #6324: backport: merge bitcoin#28577, #28588, #28634, #28849, #28805, #28951, #29058, #29239, partial bitcoin#28331, #28452 (BIP324 backports: part 2)
5dd60c4875 merge bitcoin#29239: Make v2transport default for addnode RPC when enabled (Kittywhiskers Van Gogh)
b2ac426a24 merge bitcoin#29058: use v2transport for manual/addrfetch connections, add to -netinfo (Kittywhiskers Van Gogh)
92e862acb2 merge bitcoin#28951: damage ciphertext/aad in full byte range (Kittywhiskers Van Gogh)
4e96e261e0 merge bitcoin#28805: Make existing functional tests compatible with --v2transport (Kittywhiskers Van Gogh)
9371e2e9a4 merge bitcoin#28849: fix node index bug when comparing peerinfo (Kittywhiskers Van Gogh)
400c9ddd67 merge bitcoin#28634: add check for missing garbage terminator detection (Kittywhiskers Van Gogh)
65eb194d82 merge bitcoin#28588: add checks for v1 prefix matching / wrong network magic detection (Kittywhiskers Van Gogh)
6074974849 merge bitcoin#28577: raise V1_PREFIX_LEN from 12 to 16 (Kittywhiskers Van Gogh)
ff92d1adae partial bitcoin#28331: BIP324 integration (Kittywhiskers Van Gogh)
f9f8805c72 fix: drop `CConnman::mapNodesWithDataToSend`, use transport data (UdjinM6)
d39d8a4fa7 partial bitcoin#28452: Do not use std::vector = {} to release memory (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on https://github.com/dashpay/dash/pull/6280

  * Dependent on https://github.com/dashpay/dash/pull/6276

  * Dependency for https://github.com/dashpay/dash/pull/6329

  * [bitcoin#28452](https://github.com/bitcoin/bitcoin/pull/28452) was backported as the behavior it introduces is required for backports like [bitcoin#28331](https://github.com/bitcoin/bitcoin/pull/28331). As it pre-dates earlier BIP324 backports, the `ClearShrink` usage from those backports have also been incorporated into this backport.

  * When backporting [bitcoin#28331](https://github.com/bitcoin/bitcoin/pull/28331), in TestFramework's `add_nodes()`, `extra_args[i]` is not converted to a `list` like it is done upstream and avoiding duplication of `-v2transport=1` is instead done by an additional check. This is done to prevent test failures in `feature_mnehf.py`.

  * The local variable `args` is removed in [bitcoin#28805](https://github.com/bitcoin/bitcoin/pull/28805) as it does nothing (the argument appending logic is removed as part of the backport) and upstream removes it anyways.

  Special thanks to UdjinM6 for significant contributions to this and dependent PRs! 🎉

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    light ACK 5dd60c4875
  PastaPastaPasta:
    utACK 5dd60c4875

Tree-SHA512: 7f3d0e54e1c96fc99b2d6b1e64485110aa73aeec0e4e4245ed4e6dc0529a7608e9c39103af636d5945d289775c40d3d15d7d4a75bea82462194dbf355fca48dc
2024-10-16 11:48:52 -05:00
pasta
1fd0160914
Merge #6328: backport: bitcoin/bitcoin#30131, #23258, #30504 - fix bild for Ubuntu 24.10 + clang
e994691e2d Merge bitcoin/bitcoin#30504: doc: use proper doxygen formatting for CTxMemPool::cs (merge-script)
a3e6378108 Merge bitcoin/bitcoin#23258: doc: Fix outdated comments referring to ::ChainActive() (fanquake)
dcbf671551 Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning in PollutePubKey (merge-script)

Pull request description:

  ## Issue being fixed or feature implemented
  It fixes build with clang 19.1.1 (default clang version on Ubuntu 24.10)

  ## What was done?
  Backport 30131, 30504 to fix compilation error and warning; 23258 to reduce conflicts.
  See commit messages for details

  ## How Has This Been Tested?
  Build and succeed
  ```
  CC=clang CXX=clang++ ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings   --enable-debug  --enable-stacktraces --enable-werror --enable-crash-hooks --enable-maintainer-mode --enable-multiprocess
  ```

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK e994691e2d
  UdjinM6:
    utACK e994691e2d

Tree-SHA512: 82623030c164c0852d87e8497a59157630f87a385050b6c58d79bf5a8f32462fb26cb02e61b1062afdf9f835e10b9baf4590c326c0fb41bbdd79c82e9d105513
2024-10-16 11:41:00 -05:00
Kittywhiskers Van Gogh
5dd60c4875
merge bitcoin#29239: Make v2transport default for addnode RPC when enabled 2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
b2ac426a24
merge bitcoin#29058: use v2transport for manual/addrfetch connections, add to -netinfo 2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
92e862acb2
merge bitcoin#28951: damage ciphertext/aad in full byte range 2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
4e96e261e0
merge bitcoin#28805: Make existing functional tests compatible with --v2transport
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
9371e2e9a4
merge bitcoin#28849: fix node index bug when comparing peerinfo 2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
400c9ddd67
merge bitcoin#28634: add check for missing garbage terminator detection 2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
65eb194d82
merge bitcoin#28588: add checks for v1 prefix matching / wrong network magic detection
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-10-15 15:34:12 +00:00
Kittywhiskers Van Gogh
6074974849
merge bitcoin#28577: raise V1_PREFIX_LEN from 12 to 16 2024-10-15 15:34:11 +00:00
Kittywhiskers Van Gogh
ff92d1adae
partial bitcoin#28331: BIP324 integration
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

excludes:
- changes to `src/rpc/util.cpp`
2024-10-15 15:34:11 +00:00
UdjinM6
f9f8805c72
fix: drop CConnman::mapNodesWithDataToSend, use transport data 2024-10-15 15:33:34 +00:00
Kittywhiskers Van Gogh
d39d8a4fa7
partial bitcoin#28452: Do not use std::vector = {} to release memory
Does not include changes to `headerssync.cpp` as it does not exist yet
2024-10-15 15:33:34 +00:00
pasta
d62fa7c0d9
Merge #6322: backport: Merge bitcoin/bitcoin#25034: test: add missing stop_node calls to feature_coinstatsindex and feature_prune
766c2c2757 Merge bitcoin/bitcoin#25034: test: add missing stop_node calls to feature_coinstatsindex and feature_prune (MacroFake)

Pull request description:

  ## Issue being fixed or feature implemented
  #6316 follow-up

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    ACK 766c2c2757
  kwvg:
    utACK 766c2c2757
  PastaPastaPasta:
    utACK 766c2c2757

Tree-SHA512: f2ea80f427ae7fbff0fec570e5a34c98da165dff50a1012398d60d6253b4a2defbe74a7c35ebe49d086724e590d5d684bd1ecd3cd988a5639cfa88606f4f9975
2024-10-15 09:31:41 -05:00
pasta
5318d68213
Merge #6302: feat: remove DIP0020 requirement for OP_ since it is activated long time ago
c8fd37d7ab docs: added a comment about removed SCRIPT_ENABLE_DIP0020_OPCODES (Konstantin Akimov)
61bc3002b0 feat: drop SCRIPT_ENABLE_DIP0020_OPCODES, make opcodes available from genesis block (Konstantin Akimov)
0e55abd0f2 feat: remove feature_dip0020_activationl.py functional test and related code (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  it's alternate solution for https://github.com/dashpay/dash/pull/6299 to fix a crash reported https://github.com/dashpay/dash/pull/6256

  ## What was done?
  Removed code related to DIP0020 activation for various OP codes. DIP0020 is activated long time ago and no any historical blocks are violating rules, removing it's backwards compatible.

  ## How Has This Been Tested?
  Run unit and functional tests. See also changes in data for unit tests and removed functional test.
  It also re-index mainnet and testnet successfully
  ```
  src/qt/dash-qt  -reindex -assumevalid=0
  src/qt/dash-qt -testnet -reindex -assumevalid=0
  ```

  Also extra test is done with bitcoin/bitcoin#23590 - no crash with it in `feature_dip0020_activation.py` [modified assuming it is always activated]

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK c8fd37d7ab
  PastaPastaPasta:
    utACK c8fd37d7ab

Tree-SHA512: 05ddda4e8fb66305995e91c8a04fbda690aef8fb82acb23b7d62f302da60b5ec7e7a97bd988efd2523dbd9cafde9f4b65cae2db9e4b5257464ce1c8fcca6a40f
2024-10-15 09:17:19 -05:00
pasta
dc02da1b28
Merge #6283: fix: do not use extra_args for enforced DashTestFramework arguments
6d4a782756 refactor: make dash specific args `sporkkey` and `dip3params` resilient for dashd restart (Konstantin Akimov)
7eaa0cf9ca refactor: simplify extra arguments wallet_mnemonicbits.py since usehd=1 is default option (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  DashTestFramework requires 2 arguments for dashd which are lost every dashd restart: `sporkkey` and `dip3params`.
  Without this PR you need to pass this arguments manually every time when you restart dashd in functional tests.

  ## What was done?
  Make dash specific args `sporkkey` and `dip3params` resilient for dashd restart

  It makes workarounds such as [these](c28b05c5ca) no more needed:
  ```
  self.restart_node(1, self.extra_args[1] + ["-checkaddrman=1"])
  ```

  Also there's some cleanup for `wallet_mnemonicbits.py` to remove `usehd=1` which is not required, and it has been discovered during revising all node's restarts related code.

  ## How Has This Been Tested?
  Removed workaround from `rpc_net.py` and run functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 6d4a782756
  PastaPastaPasta:
    utACK 6d4a782756

Tree-SHA512: 638b2dfe45aa35d7a9c9b4e527c3211b47e8f2fc97caf130eae09ee348d539b4c73007be0e3949ac978e306d394d9ead1d63bda3f4b515335cc62c32d2635e62
2024-10-15 08:53:35 -05:00
UdjinM6
6fbd128947
chore: clang format/reword/typos 2024-10-15 20:39:10 +07:00
Kittywhiskers Van Gogh
7d9ff96091
merge bitcoin#16981: Improve runtime performance of --reindex 2024-10-15 06:58:52 +00:00
Kittywhiskers Van Gogh
e531dff5f7
merge bitcoin#26417: fix intermittent failure in feature_index_prune.py 2024-10-15 06:58:52 +00:00
Kittywhiskers Van Gogh
b04b71a957
merge bitcoin#24858: incorrect blk file size calculation during reindex results in recoverable blk file corruption 2024-10-15 06:58:52 +00:00
Kittywhiskers Van Gogh
9e75b99c53
merge bitcoin#26215: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
3bd584c845
merge bitcoin#24832: Verify the block filter hash when reading the filter from disk 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
e507a51323
fix: avoid mandatory-script-verify-flag-failed crash in bench test
Aberrant behaviour was first introduced in 5d10b413 (dash#6152), changes
meant for SegWit made it in, reverting them fixes the problem.
2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
a86109a017
merge bitcoin#25074: During sync, commit best block after indexing 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
e6867a35ce
merge bitcoin#25123: Fix race condition in index prune test 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
baf6e26eed
merge bitcoin#21726: Improve Indices on pruned nodes via prune blockers 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
c65ec190c5
merge bitcoin#24626: disallow reindex-chainstate when pruning 2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
bcd24a25e3
fix: push activation height for forks ahead, fix feature_pruning.py
We need to push DIP-0001's activation height ahead as post-DIP-0001
consensus rejects transactions greater than `MAX_STANDARD_TX_SIZE`
(`bad-txns-oversize`) and this test generates transactions nearly 9.5x
that.

We also need to push v20 activation ahead to avoid different subsidy
calculation logic (`bad-cb-amount`).
2024-10-15 06:58:51 +00:00
Kittywhiskers Van Gogh
10203560f5
merge bitcoin#24812: Add CHECK_NONFATAL identity function and NONFATAL_UNREACHABLE macro 2024-10-15 06:58:50 +00:00
Kittywhiskers Van Gogh
1caaa85716
merge bitcoin#24138: Commit MuHash and best block together for coinstatsindex
Continuation of c21f23db from dash#5501
2024-10-15 06:58:50 +00:00
Kittywhiskers Van Gogh
b218f123b7
merge bitcoin#23046: Add txindex migration test 2024-10-15 06:58:49 +00:00
merge-script
e994691e2d
Merge bitcoin/bitcoin#30504: doc: use proper doxygen formatting for CTxMemPool::cs
6a5e9e40e1dd3d397020703feb9aa0b6f4577c98 doc: use proper doxygen formatting for CTxMemPool::cs (Vasil Dimov)

Pull request description:

  Having `@par title` followed by an empty line renders improperly in Doxygen - it results in a paragraph with a title but without a body.

  https://www.doxygen.nl/manual/commands.html#cmdpar

  This also results in a compiler warning (or error) with Clang 19:

  ```
  ./txmempool.h:368:34: error: empty paragraph passed to '@par' command [-Werror,-Wdocumentation]
    368 |      * @par Consistency guarantees
        |        ~~~~~~~~~~~~~~~~~~~~~~~~~~^
  1 error generated.
  ```

ACKs for top commit:
  maflcko:
    review ACK 6a5e9e40e1dd3d397020703feb9aa0b6f4577c98
  tdb3:
    ACK 6a5e9e40e1dd3d397020703feb9aa0b6f4577c98

Tree-SHA512: 2c4c9e5fd4bd44754800a9bcfff74df101afc060b84451c45aa098e4ceb05a47f28a36f8473b31222552fad6339b752a148e6b1c7d41c2003f515b3eb4060902
2024-10-15 13:52:49 +07:00
fanquake
a3e6378108
Merge bitcoin/bitcoin#23258: doc: Fix outdated comments referring to ::ChainActive()
BACKPORT NOTE:
changes for TestLockPointValidity in src/validation.cpp are applied to the same function but in src/txmempool.cpp

a0efe529e4fd053b890450413b9ca5e1bcd8f2c2 Fix outdated comments referring to ::ChainActive() (Samuel Dobson)

Pull request description:

  After #21866 there are a few outdated comments referring to `::ChainActive()`, which should instead refer to `ChainstateManager::ActiveChain()`.

ACKs for top commit:
  jamesob:
    ACK a0efe529e4

Tree-SHA512: 80da19c105ed29ac247e6df4c8e916c3bf3f37230b63f07302114eef9c115add673e9649f0bbe237295be0c6da7b1030b5b93e14daf6768f17ce5de7cf2c9ff2
2024-10-15 13:52:49 +07:00
merge-script
dcbf671551
Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning in PollutePubKey
2289d4524053ab71c0d9133987cb36412797c1a2 wallet, tests: Avoid stringop-overflow warning in PollutePubKey (Ava Chow)

Pull request description:

  Fixes  #30114

ACKs for top commit:
  maflcko:
    ACK 2289d4524053ab71c0d9133987cb36412797c1a2 with g++ 14.1.1 🦄
  theStack:
    utACK 2289d4524053ab71c0d9133987cb36412797c1a2
  laanwj:
    ACK 2289d4524053ab71c0d9133987cb36412797c1a2

Tree-SHA512: 173c3c299bdd890f73e8a67a37880dbf816265e8b3c8298557ef2fc4670f5447005c0d2d81726f9bc43f6a69d677365d90a604354b3cbab0e3c52c4526d0407e
2024-10-15 13:52:48 +07:00
Kittywhiskers Van Gogh
ebae59eedf
fix: make sure we flush our committed best block in no-upgrade cases
This was spotted when working on `feature_txindex_compatibility.py`,
attempting to load old databases would fail because `MigrateDBIfNeeded()`
would delete `DB_OLD_BEST_BLOCK`, write `EVODB_BEST_BLOCK`, commit it
but never flush it.

So when `MigrateDBIfNeeded2()` would read it again, it would note the lack
of `DB_OLD_BEST_BLOCK` and conclude it was a failed run and exit. This
is solved by actually flushing our new best block, which would prevent
`MigrateDBIfNeeded2` from raising an objection.
2024-10-13 15:45:14 +00:00
MarcoFalke
1db9e6ac76
Merge #19979: Replace LockAssertion with AssertLockHeld, remove LockAssertion
0bd1184adf6610c0bd14f4e9a25c0a200e65ae25 Remove unused LockAssertion struct (Hennadii Stepanov)
ab2a44297fd0796bf5797ae2a477e8e56d9c3c12 Replace LockAssertion with a proper thread safety annotations (Hennadii Stepanov)
73f71e19965e07534eb47701f2b23c9ed59ef475 refactor: Use explicit function type instead of template (Hennadii Stepanov)

Pull request description:

  This PR replaces `LockAssertion` with `AssertLockHeld`, and removes `LockAssertion`.

  This PR is compared with alternatives in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/AssertLockHeld-PRs

ACKs for top commit:
  MarcoFalke:
    ACK 0bd1184adf6610c0bd14f4e9a25c0a200e65ae25
  ajtowns:
    ACK 0bd1184adf6610c0bd14f4e9a25c0a200e65ae25
  vasild:
    ACK 0bd1184ad

Tree-SHA512: ef7780dd689faf0bb479fdb97c49bc652e2dd10c148234bb95502dfbb676442d8565ee37864d923ca21a25f9dc2a335bf46ee82c095e387b59a664ab05c0ae41
2024-10-12 20:06:29 +07:00
MarcoFalke
d0a4198166
Merge #21598: refactor: Remove negative lock annotations from globals
fa5eabe72117f6e3704858e8d5b2c57a120258ed refactor: Remove negative lock annotations from globals (MarcoFalke)

Pull request description:

  They only make sense for mutexes that are private members. Until cs_main is a private member the negative annotations should be replaced by excluded annotations, which are optional.

ACKs for top commit:
  sipa:
    utACK fa5eabe72117f6e3704858e8d5b2c57a120258ed
  ajtowns:
    ACK fa5eabe72117f6e3704858e8d5b2c57a120258ed
  hebasto:
    ACK fa5eabe72117f6e3704858e8d5b2c57a120258ed
  vasild:
    ACK fa5eabe72117f6e3704858e8d5b2c57a120258ed

Tree-SHA512: 06f8a200304f81533010efcc42d9f59b8c4d0ae355920c0a28efb6fa161a3e3e68f2dfffb0c009afd9c2501e6a293c6e5a419a64d718f1f4e79668ab2ab1fcdc
2024-10-12 20:06:28 +07:00
MarcoFalke
55114a682e
Merge #19668: Do not hide compile-time thread safety warnings
ea74e10acf17903e44c85e3678853414653dd4e1 doc: Add best practice for annotating/asserting locks (Hennadii Stepanov)
2ee7743fe723227f2ea1b031eddb14fc6863f4c8 sync.h: Make runtime lock checks require compile-time lock checks (Anthony Towns)
23d71d171e6e22ba5e4a909d597a54595b2a2c1f Do not hide compile-time thread safety warnings (Hennadii Stepanov)
3ddc150857178bfb1c854c05bf9b526777876f56 Add missed thread safety annotations (Hennadii Stepanov)
af9ea55a72c94678b343f5dd98dc78f3a3ac58cb Use LockAssertion utility class instead of AssertLockHeld() (Hennadii Stepanov)

Pull request description:

  On the way of transit from `RecursiveMutex` to `Mutex` (see #19303) it is crucial to have run-time `AssertLockHeld()` assertion that does _not_ hide compile-time Clang Thread Safety Analysis warnings.

  On master (65e4ecabd5b4252154640c7bac38c92a3f3a7018) using `AssertLockHeld()` could hide Clang Thread Safety Analysis warnings, e.g., with the following patch applied:
  ```diff
  --- a/src/txmempool.h
  +++ b/src/txmempool.h
  @@ -607,7 +607,7 @@ public:
       void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);

       void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
  -    void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
  +    void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
       void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
       void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);

  ```
  Clang compiles the code without any thread safety warnings.

  See "Add missed thread safety annotations" commit for the actual thread safety warnings that are fixed in this PR.

ACKs for top commit:
  MarcoFalke:
    ACK ea74e10acf 🎙
  jnewbery:
    ACK ea74e10acf17903e44c85e3678853414653dd4e1
  ajtowns:
    ACK ea74e10acf17903e44c85e3678853414653dd4e1

Tree-SHA512: 8cba996e526751a1cb0e613c0cc1b10f027a3e9945fbfb4bd30f6355fd36b9f9c2e1e95ed3183fc254b42df7c30223278e18e5bdb5e1ef85db7fef067595d447
2024-10-12 20:06:28 +07:00
Ava Chow
898282d620
Merge bitcoin/bitcoin#28774: wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it
32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it (Vasil Dimov)

Pull request description:

  `CWallet::GetEncryptionKey()` would return a reference to the internal
  `CWallet::vMasterKey`, guarded by `CWallet::cs_wallet`, which is unsafe.

  Returning a copy would be a shorter solution, but could have security
  implications of the master key remaining somewhere in the memory even
  after `CWallet::Lock()` (the current calls to
  `CWallet::GetEncryptionKey()` are safe, but that is not future proof).

  So, instead of `EncryptSecret(m_storage.GetEncryptionKey(), ...)`
  change the `GetEncryptionKey()` method to provide the encryption
  key to a given callback:
  `m_storage.WithEncryptionKey([](const CKeyingMaterial& k) { EncryptSecret(k, ...); })`

  This silences the following (clang 18):

  ```
  wallet/wallet.cpp:3520:12: error: returning variable 'vMasterKey' by reference requires holding mutex 'cs_wallet' [-Werror,-Wthread-safety-reference-return]
   3520 |     return vMasterKey;
        |            ^
  ```

  ---
  _Previously this PR modified both ArgsManager and wallet code. But the ArgsManager commit 856c88776f was merged in https://github.com/bitcoin/bitcoin/pull/29040 so now this only affects wallet code. The previous PR description was:_

  Avoid this unsafe pattern from `ArgsManager` and `CWallet`:

  ```cpp
  class A
  {
      Mutex mutex;
      Foo member GUARDED_BY(mutex);
      const Foo& Get()
      {
          LOCK(mutex);
          return member;
      } // callers of `Get()` will have access to `member` without owning the mutex.
  ```

ACKs for top commit:
  achow101:
    ACK 32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b
  ryanofsky:
    Code review ACK 32a9f13cb805ecf6aebb5cf4e5d92b8a47c4548b. This seems like a potentially real race condition, and the fix here is pretty simple.
  furszy:
    ACK 32a9f13c

Tree-SHA512: 133da84691642afc1a73cf14ad004a7266cb4be1a6a3ec634d131dca5dbcdef52522c1d5eb04f5b6c4e06e1fc3e6ac57315f8fe1e207b464ca025c2b4edefdc1
2024-10-12 20:06:28 +07:00
Konstantin Akimov
6d452845dc
fix: add missing lock annotation for ContextualCheckBlock and related TODO 2024-10-12 20:06:27 +07:00