39252 Commits

Author SHA1 Message Date
Greg Sanders
a0626ccdad fuzz: allow reaching MempoolAcceptResult::ResultType::DIFFERENT_WITNESS in tx_package_eval target 2023-11-09 09:07:03 -05:00
fanquake
21d985784f
Merge bitcoin/bitcoin#28788: test: bugfix CheckPackageMempoolAcceptResult return all error strings
5380f055136ea99f76cd3df2c2add081852d35d0 test: bugfix CheckPackageMempoolAcceptResult return all error strings (Greg Sanders)

Pull request description:

  Noticed on follow-up testing work https://github.com/bitcoin/bitcoin/pull/28764/files#r1382150706

ACKs for top commit:
  glozow:
    utACK 5380f055136ea99f76cd3df2c2add081852d35d0
  dergoegge:
    utACK 5380f055136ea99f76cd3df2c2add081852d35d0

Tree-SHA512: abe122b5d702aaa0d731b45f6ba0f2f8ff9ecc14398cc087df56ee0980b5b483fc159d38ec36fe6360f82e5443673cfa80afc1c9cee6b840c95703b7dc8a8d3f
2023-11-06 14:25:47 +00:00
fanquake
f2cc718e69
Merge bitcoin/bitcoin#28798: build: Drop no longer needed MSVC warning suppressions
33223f9d550b66885a15e2fe45147d629cc359d2 build: Drop no longer needed MSVC warning suppressions (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  maflcko:
    lgtm ACK 33223f9d550b66885a15e2fe45147d629cc359d2

Tree-SHA512: 3757fce0a6900f6edc50a1b1298c5448059b100060e8a200bfc12d034852138084db12d100caefd2a19f170faa22bf4be9050094fd64f1273312541df7c81ef4
2023-11-06 09:54:40 +00:00
fanquake
953d302a24
Merge bitcoin/bitcoin#28735: depends: Bump to capnproto-c++-1.0.1
3333f14efac815ba3c885398a6795c7e8ce68d08 depends: Bump to capnproto-c++-1.0.1 (MarcoFalke)

Pull request description:

  Reasons:
  * Debian is starting to ship this version in Trixie (https://packages.debian.org/trixie/capnproto), which will likely become the version shipped with Ubuntu 24.04 LTS. So testing with this version will help to find any issues before real users start to use those distro packages.
  * The feature is currently experimental, so bumping the version shouldn't cause any production issues.
  * With multiprocess begin a priority project for 27.0, it seems better to do build system changes/bumps early, rather than later, to allow for more time testing them.

ACKs for top commit:
  TheCharlatan:
    Re-ACK 3333f14efac815ba3c885398a6795c7e8ce68d08
  fanquake:
    ACK 3333f14efac815ba3c885398a6795c7e8ce68d08 - the response from upstream is that [if we submit a PR, they can take a look](https://github.com/capnproto/capnproto/issues/1833#issuecomment-1792582206), so if anyone would like this to work for Windows, I'd suggest sending a patch.
  ryanofsky:
    Code review ACK 3333f14efac815ba3c885398a6795c7e8ce68d08

Tree-SHA512: 7d53ad1536f042ab43dbc7847126b826e7fc76694f173c348b835fd1067b8f3dd682c5bcb4887f09ee85bab69130721cd7f8fb96b2e82053d4e28bd5c38bdc5f
2023-11-05 18:22:36 +00:00
fanquake
d2d53b4ac8
Merge bitcoin/bitcoin#28796: ci: Drop no longer needed "Fix Visual Studio installation" step
5bd1b8d4f1ab4dd947c5e93712ba47e14a0fe2da ci: Drop no longer needed "Fix Visual Studio installation" step (Hennadii Stepanov)

Pull request description:

  The underlying issue has been [fixed](https://github.com/actions/runner-images/pull/8686) in the image version 20231029.

ACKs for top commit:
  maflcko:
    lgtm ACK 5bd1b8d4f1ab4dd947c5e93712ba47e14a0fe2da

Tree-SHA512: d0efef3086a147d863c9b5f45ba1142c6e7cc65e47d685b2094211e58036315fb7562253b7d7172b527fa1ded4b5a86634ba7c151e761ec20fe948145aff83fe
2023-11-05 18:15:38 +00:00
Hennadii Stepanov
33223f9d55
build: Drop no longer needed MSVC warning suppressions 2023-11-05 17:34:30 +00:00
Hennadii Stepanov
5bd1b8d4f1
ci: Drop no longer needed "Fix Visual Studio installation" step
The underlying issue has been fixed in the image version 20231029.
2023-11-05 10:01:56 +00:00
Greg Sanders
5380f05513 test: bugfix CheckPackageMempoolAcceptResult return all error strings 2023-11-03 16:05:55 -04:00
Andrew Chow
d9007f51a7
Merge bitcoin/bitcoin#28762: MiniMiner changes for package linearization
d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9 [test] MiniMiner::Linearize and manual construction (glozow)
dfd6a3788c35be121eba1ad84f20effadcb7e7dc [refactor] unify fee amounts in miniminer_tests (glozow)
f4b1b24a3bcd0199a6d2e828ad46033e1368336e [MiniMiner] track inclusion order and add Linearize() function (glozow)
004075963f12f17c3c399d81e3b48d6a8151aebd [test] add case for MiniMiner working with negative fee txns (glozow)
fe6332c0badf07e99044b00084fc6b07f735a051 [MiniMiner] make target_feerate optional (glozow)
5a83f55c96661a886dd6f5231920b2f730cf6773 [MiniMiner] allow manual construction with non-mempool txns (glozow)
e3b2e630b219ca15fe0b2640ca422712c86ac33d [refactor] change MiniMinerMempoolEntry ctor to take values, update includes (glozow)
4aa98b79b266cd526efa577762b0bcfccbdeda11 [lint] update expected boost includes (glozow)

Pull request description:

  This is part of #27463. It splits off the `MiniMiner`-specific changes from #26711 for ease of review, as suggested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253.

  - Allow using `MiniMiner` on transactions that aren't in the mempool.
  - Make `target_feerate` param of `BuildMockTemplate` optional, meaning "don't stop building the template until all the transactions have been selected."
    - Add clarification for how this is different from `target_feerate=0` (https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1377019133)
  - Track the order in which transactions are included in the template to get the "linearization order" of the transactions.
  - Tests

  Reviewers can take a look at #26711 to see how these functions are used to linearize the `AncestorPackage` there.

ACKs for top commit:
  TheCharlatan:
    ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9
  kevkevinpal:
    reACK [d9cc99d](d9cc99d04e)
  achow101:
    re-ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

Tree-SHA512: 32b80064b6679536ac573d674825c5ca0cd6245e49c2fd5eaf260dc535335a57683c74ddd7ce1f249b5b12b2683de4362a7b0f1fc0814c3b3b9f14c682665583
2023-11-03 10:50:50 -04:00
fanquake
0fd7ca4838
Merge bitcoin/bitcoin#28778: depends: drop -O1 workaround from arm64 apple Qt build
664c87354f9ec5df95346eab72a034296b83914d depends: drop -O1 workaround from arm64 apple Qt build (fanquake)

Pull request description:

  Drop the workaround of setting optimization flags to -O1 for the `arm64-apple-darwin` builds. I no-longer see reproducibility issues when building across `x86_64` and `aarch64`:
  ```bash
  real7m21.192s
  user67m41.047s
  sys5m8.596s
  fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  fedora-32gb-hel1-1 bitcoin]# uname -m
  aarch64
  ```

  ```bash
  real18m10.759s
  user108m21.656s
  sys6m12.930s
  ubuntu-32gb-hel1-1:~/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  ubuntu-32gb-hel1-1:~/bitcoin# uname -m
  x86_64
  ```

ACKs for top commit:
  hebasto:
    ACK 664c87354f9ec5df95346eab72a034296b83914d.
  TheCharlatan:
    ACK 664c87354f9ec5df95346eab72a034296b83914d

Tree-SHA512: 79527df4181eb0a0c42fe526581479abcdeba8fb09e1faf52265d697d39a8f3a3532ee3c573579b9af00b7330a401e4b6f1686636f9bac6bf9839be8381a2033
2023-11-03 14:43:37 +00:00
fanquake
5d9f45082b
Merge bitcoin/bitcoin#28758: refactors for subpackage evaluation
b5a60abe8783852f5b31bc1e63b5836530410e65 MOVEONLY: CleanupTemporaryCoins into its own function (glozow)
10c0a8678cd28e7f0715e6cfa3e651903e4ad4aa [test util] CreateValidTransaction multi-in/out, configurable feerate, signal BIP125 (glozow)
6ff647a7e0d85040a6033047c5cf84f8f22b1c65 scripted-diff: rename CheckPackage to IsWellFormedPackage (glozow)
da9aceba217bbded6909f06144eaa1e1a4ebcb69 [refactor] move package checks into helper functions (glozow)

Pull request description:

  This is part of #27463. It splits off the more trivial changes from #26711 for ease of review, as requested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253.

  - Split package sanitization in policy/packages.h into helper functions
    - Add some tests for its quirks (https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1340521597)
  - Rename `CheckPackage` to `IsPackageWellFormed`
  - Improve the `CreateValidTransaction` unit test utility to:
    - Configure the target feerate and return the fee paid
    - Signal BIP125 on transactions to enable RBF tests
    - Allow the specification of multiple inputs and outputs
  - Move `CleanupTemporaryCoins` into its own function to be reused later without duplication

ACKs for top commit:
  dergoegge:
    Code review ACK b5a60abe8783852f5b31bc1e63b5836530410e65
  instagibbs:
    ACK b5a60abe8783852f5b31bc1e63b5836530410e65

Tree-SHA512: 39d67a5f0041e381f0d0f802a98ccffbff11e44daa3a49611189d6306b03f18613d5ff16c618898d490c97a216753e99e0db231ff14d327f92c17ae4d269cfec
2023-11-03 14:41:17 +00:00
glozow
f23ac10ca5
Merge bitcoin/bitcoin#28764: Fuzz: Check individual and package transaction invariants
fcb3069fa307942cf7f3edabcda1be96d615c91f Use CheckPackageMempoolAcceptResult for package evaluation fuzzing (Greg Sanders)
34088d6c9ed4ed99bb6b7fc83795da01ec9f3c97 [test util] CheckPackageMempoolAcceptResult for sanity-checking results (glozow)
651fa404e454e31f8e9d830aa292eb3b456b54fb fuzz: tx_pool checks ATMP result invariants (Greg Sanders)

Pull request description:

  Poached from https://github.com/bitcoin/bitcoin/pull/26711 since that PR is being split apart, and modified to match current behavior.

ACKs for top commit:
  glozow:
    reACK fcb3069fa307942cf7f3edabcda1be96d615c91f, only whitespace changes
  dergoegge:
    ACK fcb3069fa307942cf7f3edabcda1be96d615c91f

Tree-SHA512: abd687e526d8dfc8d65b3a873ece8ca35fdcbd6b0f7b93da6a723ef4e47cf85612de819e6f2b8631bdf897e1aba27cdd86f89b7bd85fc3356e74be275dcdf8cc
2023-11-03 13:51:12 +00:00
MarcoFalke
3333f14efa
depends: Bump to capnproto-c++-1.0.1 2023-11-03 12:48:42 +01:00
glozow
d9cc99d04e [test] MiniMiner::Linearize and manual construction 2023-11-03 10:39:29 +00:00
glozow
dfd6a3788c [refactor] unify fee amounts in miniminer_tests
Name {low,med,high}_fee constants for reuse across file.
2023-11-03 10:17:41 +00:00
glozow
f4b1b24a3b [MiniMiner] track inclusion order and add Linearize() function
Sometimes we are just interested in the order in which transactions
would be included in a block (we want to "linearize" the transactions).
Track and store this information.

This doesn't change any of the bump fee calculations.
2023-11-03 10:17:41 +00:00
glozow
004075963f [test] add case for MiniMiner working with negative fee txns 2023-11-03 10:17:41 +00:00
glozow
fe6332c0ba [MiniMiner] make target_feerate optional
Add an option to keep building the template regardless of feerate. We
can't just use target_feerate=0 because it's possible for transactions
to have negative modified feerates.

No behavior change for users that pass in a target_feerate.
2023-11-03 10:17:41 +00:00
glozow
5a83f55c96 [MiniMiner] allow manual construction with non-mempool txns
This is primarily intended for linearizing a package of transactions
prior to submitting them to mempool. Note that, if this ctor is used,
bump fees will not be calculated because we haven't instructed MiniMiner
which outpoints for which we want bump fees to be calculated.
2023-11-03 10:17:41 +00:00
glozow
e3b2e630b2 [refactor] change MiniMinerMempoolEntry ctor to take values, update includes
No behavior change. All we are doing is copying out these values before
passing them into the ctor instead of within the ctor.

This makes it possible to use the MiniMiner algorithms to analyze
transactions that haven't been submitted to the mempool yet.

It also iwyu's the mini_miner includes.
2023-11-03 10:17:41 +00:00
glozow
4aa98b79b2 [lint] update expected boost includes 2023-11-03 10:17:41 +00:00
Andrew Chow
9b68c9b85e
Merge bitcoin/bitcoin#28172: refactor: use string_view for passing string literals to Parse{Hash,Hex}
bb91131d545d986aab81c4bb13676c4520169259 doc: remove out-of-date external link in src/util/strencodings.h (Jon Atack)
7d494a48ddf4248ef3b1753b6e7f2eeab3a8ecb7 refactor: use string_view to pass string literals to Parse{Hash,Hex} (Jon Atack)

Pull request description:

  as `string_view` is optimized to be trivially copiable, whereas the current code creates a `std::string` copy at each call.

  These utility methods are called by quite a few RPCs and tests, as well as by each other.

  ```
  $ git grep "ParseHashV\|ParseHashO\|ParseHexV\|ParseHexO" | wc -l
  61
  ```

  Also remove an out-of-date external link.

ACKs for top commit:
  jonatack:
    Rebased per `git range-diff c9273f6 b94581a bb91131` for an include header from the merge of https://github.com/bitcoin/bitcoin/pull/28230. Should be trivial to re-ACK.
  maflcko:
    lgtm ACK bb91131d545d986aab81c4bb13676c4520169259
  ns-xvrn:
    ACK bb91131d54
  achow101:
    ACK bb91131d545d986aab81c4bb13676c4520169259
  brunoerg:
    crACK bb91131d545d986aab81c4bb13676c4520169259

Tree-SHA512: 9734fe022c9e43fd93c23a917770d332dbbd3132c80a234059714c32faa6469391e59349954749fc86c4ef0b18d5fd99bf8f4b7b82d9f799943799c1253272ae
2023-11-02 15:45:13 -04:00
Andrew Chow
5f88622191
Merge bitcoin/bitcoin#27852: test: add coverage to rpc_blockchain.py
376dc2cfb32806a8aa450589effe4d384e648398 test: add coverage to rpc_blockchain.py (kevkevin)

Pull request description:

  Included a test that checks the functionality of setting
  the first param of getnetworkhashps to negative value returns
  the average network hashes per second from the last difficulty change.

ACKs for top commit:
  jlopp:
    tACK 376dc2cfb3
  achow101:
    ACK 376dc2cfb32806a8aa450589effe4d384e648398
  ismaelsadeeq:
    Tested ACK 376dc2cfb32806a8aa450589effe4d384e648398
  pablomartin4btc:
    tACK 376dc2cfb32806a8aa450589effe4d384e648398

Tree-SHA512: 02d52f622e9cb7a1240c5d124510dd75d03f696f119b2625b0befd60b004ec50ff1a2d5515e0e227601adeecd837e0778ed131ee2a8c5f75f1b824be711213a7
2023-11-02 15:19:37 -04:00
Andrew Chow
0857f2935f
Merge bitcoin/bitcoin#24097: Replace RecursiveMutex m_cs_banned with Mutex, and rename it
37d150d8c5ffcb2bddcd99951a739e97571194c7 refactor: Add more negative `!m_banned_mutex` thread safety annotations (Hennadii Stepanov)
0fb29087080a4e60d7c709ff5edf14e830ef3a69 refactor: replace RecursiveMutex m_banned_mutex with Mutex (w0xlt)
784c316f9cb664c9577cbfed1873bae573efd1b4 scripted-diff: rename m_cs_banned -> m_banned_mutex (w0xlt)
46709c5f27bf6cbc8eba1298b04bd079da2cdded refactor: Get rid of `BanMan::SetBannedSetDirty()` (Hennadii Stepanov)
d88c0d8440cf640ef4f2c7a40b8b8b31bfd38f23 refactor: Get rid of `BanMan::BannedSetIsDirty()` (Hennadii Stepanov)

Pull request description:

  This PR is an alternative to bitcoin/bitcoin#24092. Last two commit have been cherry-picked from the latter.

ACKs for top commit:
  maflcko:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7 🎾
  achow101:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
  theStack:
    Code-review ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
  vasild:
    ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7

Tree-SHA512: 5e9d40101a09af6e0645a6ede67432ea68631a1b960f9e6af0ad07415ca7718a30fcc1aad5182d1d5265dc54c26aba2008fc9973840255c09adbab8fedf10075
2023-11-02 14:09:27 -04:00
fanquake
664c87354f
depends: drop -O1 workaround from arm64 apple Qt build 2023-11-02 15:59:46 +00:00
Greg Sanders
fcb3069fa3 Use CheckPackageMempoolAcceptResult for package evaluation fuzzing 2023-11-02 09:33:47 -04:00
glozow
34088d6c9e [test util] CheckPackageMempoolAcceptResult for sanity-checking results 2023-11-02 09:33:47 -04:00
glozow
2e9454a633
Merge bitcoin/bitcoin#21161: Fee estimation: extend bucket ranges consistently
a5e39d325da4eeb9273fb7c919fcbfbc721ed75d Fee estimation: extend bucket ranges consistently (Anthony Towns)

Pull request description:

  When calculating a median fee for a confirmation target at a particular threshold, we analyse buckets in ranges rather than individually in case some buckets have very little data. This patch ensures the breaks between ranges are independent of the the confirmation target.

  Fixes #20725

ACKs for top commit:
  ismaelsadeeq:
    Code review ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d
  glozow:
    btw what I meant by [this](https://github.com/bitcoin/bitcoin/pull/21161#pullrequestreview-1350258467) was ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d
  jonatack:
    Initial ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d

Tree-SHA512: 0edf4e56717c4ab8d4ab0bc0f1d7ab36a13b99de12f689e55c9142c6b81691367ffd8df2e8260c5e14335310b1a51770c6c22995db31109976239befcb558ef8
2023-11-02 11:25:50 +00:00
glozow
023418a140
Merge bitcoin/bitcoin#28530: tests, bug fix: DisconnectedBlockTransactions rewrite followups
9b3da70bd06b45482e7211aa95637a72bd115553 [test] DisconnectedBlockTransactions::DynamicMemoryUsage (glozow)
b2d04479647af64ad7cf5ebfb6175251b2f6b72e bugfix: correct DisconnectedBlockTransactions memory usage (stickies-v)
f4254e209801d6a790b5f0c251c0b32154a4e3cc assume duplicate transactions are not added to `iters_by_txid` (ismaelsadeeq)
29eb219c1247993378fce06c8f71aab20736c237 move only: move implementation code to disconnected_transactions.cpp (ismaelsadeeq)
81dfeddea70ae5feeaf79062585c2ff9f33c0ca3 refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytes (ismaelsadeeq)

Pull request description:

  This PR is a follow-up to fix review comments and a bugfix from #28385

  The PR

  - Updated `DisconnectedBlockTransactions`'s `MAX_DISCONNECTED_TX_POOL` from kb to bytes.
  - Moved `DisconnectedBlockTransactions` implementation code to `kernel/disconnected_transactions.cpp`.
  - `AddTransactionsFromBlock` now assume duplicate transactions are not passed by asserting after inserting each transaction to `iters_by_txid`.
  - Included a Bug fix: In the current master we are underestimating the memory usage of `DisconnectedBlockTransactions`.

      * When adding and subtracting `cachedInnerUsage` we call `RecursiveDynamicUsage` with `CTransaction` which invokes this [`RecursiveDynamicUsage(const CTransaction& tx)`](6e721c923c/src/core_memusage.h (L32)) version of `RecursiveDynamicUsage`, the output of that call only account for the memory usage of the inputs and outputs of the `CTransaction`, this omits the memory usage of the `CTransaction` object and the control block.
      * This PR fixes this bug by calling `RecursiveDynamicUsage` with `CTransactionRef` when adding and subtracting `cachedInnerUsage` which invokes [`RecursiveDynamicUsage(const std::shared_ptr<X>& p)`](6e721c923c/src/core_memusage.h (L67)) version of `RecursiveDynamicUsage` the output of the calculation accounts for the` CTransaction` object, the control blocks, inputs and outputs memory usage.
      * see  [comment ](https://github.com/bitcoin/bitcoin/pull/28385#discussion_r1322948452)
  - Added test for DisconnectedBlockTransactions memory limit.

ACKs for top commit:
  stickies-v:
    ACK 9b3da70bd06b45482e7211aa95637a72bd115553 - nice work!
  BrandonOdiwuor:
    re ACK 9b3da70bd06b45482e7211aa95637a72bd115553
  glozow:
    ACK 9b3da70bd06b45482e7211aa95637a72bd115553

Tree-SHA512: 69b9595d09f4d0209038f97081d790cea92ccf63efb94e9e372749979fcbe527f7f17a8e454720cedd12021be0c8e11cf99874625d3dafd9ec602b12dbeb4098
2023-11-02 11:12:17 +00:00
fanquake
023e8c2001
Merge bitcoin/bitcoin#28769: build: Update qt package up to 5.15.11
8047bb6feaa9ee5d6c1edb7640baaf228450bc6b build: Update `qt` package up to 5.15.11 (Hennadii Stepanov)

Pull request description:

  In the light of https://github.com/bitcoin/bitcoin/pull/28622, we probably have to patch Qt. It seems reasonable to update it up to the latest available version before doing that.

ACKs for top commit:
  TheCharlatan:
    ACK 8047bb6feaa9ee5d6c1edb7640baaf228450bc6b

Tree-SHA512: b4d7df2ff059b8f58c3202d913237c0d39a962748658f1ce853884dca095fbda5f56d4d68f73a1bc8da2f295e96a20927306e148b41a9f4afc42c8edb11c3729
2023-11-02 09:53:01 +00:00
fanquake
b2240f6522
Merge bitcoin/bitcoin#28770: refactor: Remove unused circular include dependency from validation.cpp
fa7d31910ab181c7e0e5f1fa1e23a49e208aec2a refactor: Remove unused circular include dependency from validation.cpp (MarcoFalke)

Pull request description:

  Also, sort includes

ACKs for top commit:
  ismaelsadeeq:
    ACK fa7d31910ab181c7e0e5f1fa1e23a49e208aec2a
  hebasto:
    ACK fa7d31910ab181c7e0e5f1fa1e23a49e208aec2a
  TheCharlatan:
    ACK fa7d31910ab181c7e0e5f1fa1e23a49e208aec2a

Tree-SHA512: ea5e0001644d70ecfbccf87e27b393786a0eda79af4923ff68a0096d4d5b910cf6eeed8667ecbf55f3a164f500d3f5aeaf9d81bb190296c30ce0cc93c165717d
2023-11-02 09:47:47 +00:00
glozow
b5a60abe87 MOVEONLY: CleanupTemporaryCoins into its own function
Avoid duplicate code. This will be used at the end of every
AcceptSubPackage and after PreChecks loop in AcceptPackage.
2023-11-01 17:21:54 +00:00
glozow
10c0a8678c [test util] CreateValidTransaction multi-in/out, configurable feerate, signal BIP125
Support the creation of a transaction with multiple specified inputs or
outputs. Also accept a target feerate and return the fee paid.

Also, signal BIP125 by default - a subsequent commit needs to RBF
something.

Co-authored-by: Andrew Chow <achow101@gmail.com>
2023-11-01 17:21:54 +00:00
glozow
6ff647a7e0 scripted-diff: rename CheckPackage to IsWellFormedPackage
-BEGIN VERIFY SCRIPT-
sed -i 's/CheckPackage(/IsWellFormedPackage(/g' $(git grep -l CheckPackage)
-END VERIFY SCRIPT-
2023-11-01 17:21:54 +00:00
glozow
da9aceba21 [refactor] move package checks into helper functions
This allows IsSorted() and IsConsistent() to be used by themselves.
IsSorted() with a precomputed set is used so that we don't create this
set multiple times.
2023-11-01 17:21:54 +00:00
MarcoFalke
fa7d31910a
refactor: Remove unused circular include dependency from validation.cpp 2023-11-01 17:45:48 +01:00
Hennadii Stepanov
8047bb6fea
build: Update qt package up to 5.15.11 2023-11-01 15:20:20 +00:00
fanquake
eca2e430ac
Merge bitcoin/bitcoin#28632: test: make python p2p not send getaddr on incoming connections
9cfc1c94407359379f10906affd2b837851c1b84 test: check that we don't send a getaddr msg to an inbound peer (Martin Zumsande)
88c33c6748da3c4fdadc554ebca43ce7267e9178 test: make python p2p not send getaddr messages when it's being connected to (Martin Zumsande)

Pull request description:

  `bitcoind` nodes send `getaddr` messages only to outbound nodes (and ignore `getaddr` received by outgoing connections).
  The python p2p node should mirror this behavior by not sending a `getaddr` message when it is not the initiator of the connection.
  This is currently causing several unnecessary messages being sent and then ignored (`Ignoring "getaddr" from outbound-full-relay connection.`) in tests like `p2p_add_connections.py`.

ACKs for top commit:
  pinheadmz:
    concept ACK 9cfc1c9440
  pablomartin4btc:
    re ACK 9cfc1c94407359379f10906affd2b837851c1b84
  BrandonOdiwuor:
    re ACK 9cfc1c94407359379f10906affd2b837851c1b84

Tree-SHA512: 812bec5d8a4828b4384d4cdd4362d6eec09acb2363e888f2b3e3bf8b925e0e17f15e13dc297d6b616c68b93ace9ede7245b07b405d3f5f8eada98350f74230dc
2023-11-01 10:39:48 +00:00
fanquake
4733de3242
Merge bitcoin/bitcoin#28729: addrman: log AS only when using asmap
02a4f1a3859ed7e865641b35ca1bc9ce711e696f addrman: log AS only when using asmap (brunoerg)

Pull request description:

  This PR changes the log to just print the ASN when using asmap, same logic presented in other logs:

  afa081a39b/src/net_processing.cpp (L3552-L3556)

  afa081a39b/src/net_processing.cpp (L3598-L3604)

ACKs for top commit:
  naumenkogs:
    ACK 02a4f1a3859ed7e865641b35ca1bc9ce711e696f
  mzumsande:
    Code Review ACK 02a4f1a3859ed7e865641b35ca1bc9ce711e696f

Tree-SHA512: adad5904ab163660d47554b32dc2dc3dfdff8dd64b94e5320ad11706381264d1e338654fa8239430eed4ccbebc8f6670698b4278895794055c37fc4bcefe71bc
2023-11-01 10:25:53 +00:00
fanquake
9d594ed1d8
Merge bitcoin/bitcoin#28755: build: remove duplicate -lminiupnpc linking
b74e449ffa9965f18037f0322ea178e2fe1dbc18 build: remove potential for duplciate natpmp linking (fanquake)
4e9509695206bd952da00a2b1c90a92db31a0fe7 build: remove duplicate -lminiupnpc linking (fanquake)

Pull request description:

  Having the link check in the header check loop means we get `-lminiupnpc -lminiupnpc -lminiupnpc` on the link line.
  This is unnecessary, and results in warnings, i.e:
  ```bash
  ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
  ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
  ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
  ```

  These warnings have been occurring since the new macOS linker released with Xcode 15, and also came up in https://github.com/hebasto/bitcoin/pull/34.

  There are other duplicate lib issues, i.e with `-levent` + `-levent_pthreads -levent`, but those are less straight forward to solve, and won't be included here.

ACKs for top commit:
  jonatack:
    ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18
  hebasto:
    ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18, it fixes one issue mentioned in https://github.com/hebasto/bitcoin/pull/34#issuecomment-1782914787.
  TheCharlatan:
    ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18
  theuni:
    ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18

Tree-SHA512: 987a56ef17cbaf273cb672c41016f3f615b16889317325a9e88135d0c41f01af3840ad44a6f811a7df97f5873c9cd957e60aaa1b99bd408b17b4b1ffe2c68f36
2023-11-01 10:08:58 +00:00
Hennadii Stepanov
04bfe8c9c3
Merge bitcoin-core/gui#774: Fix crash on selecting "Mask values" in transaction view
e26e665f9f64a962dd56053be817cc953e714847 gui: fix crash on selecting "Mask values" in transaction view (Sebastian Falbesoner)

Pull request description:

  This PR fixes a crash bug that can be caused with the following steps:
  - change to the "Transactions" view
  - right-click on an arbitrary transaction -> "Show transaction details"
  - close the transaction detail window again
  - select menu item "Settings" -> "Mask values"

  The problem is that the list of opened dialogs, tracked in the member variable `m_opened_dialogs` (introduced in https://github.com/bitcoin-core/gui/pull/708, commit 4492de1be11f4131811f504a037342c78f480e20), is only ever appended with newly opened transaction detail dialog pointers, but never removed. This leads to dangling pointers in the list, and if the "Mask values" menu item is selected, a crash is caused in the course of trying to close the opened transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this by removing a pointer of the list if the corresponding widget is destroyed.

ACKs for top commit:
  achow101:
    ACK e26e665f9f64a962dd56053be817cc953e714847
  pablomartin4btc:
    tACK e26e665f9f64a962dd56053be817cc953e714847
  furszy:
    utACK e26e665f9
  hebasto:
    ACK e26e665f9f64a962dd56053be817cc953e714847, tested on Ubuntu 22.04.

Tree-SHA512: 37885c22abae0ab065b4878bae46fd362f41b09609d081fd59e26bb05474f427b98771ee73f5480526afaef04e016c5ba62c956e0e85a57b6a0f44a905b68a83
2023-11-01 09:57:17 +00:00
Greg Sanders
651fa404e4 fuzz: tx_pool checks ATMP result invariants 2023-10-31 14:52:45 -04:00
fanquake
3c0b66c2ec
Merge bitcoin/bitcoin#28759: guix: update signapple to latest master
79539fbfbf4d09a8b4861ddcba5b194297bc1b65 guix: update signapple (fanquake)

Pull request description:

  Fixes #28449, and removes the need to boostrap Rust, by avoiding the `python-requests` dependency.

  Comparing a `--no-substitutes` build of this PR, to master, signapple requires ~1350 _less_ packages to boostrap:
  Master derivation - https://gist.github.com/fanquake/dbf69a62c9a78b7ae8c183a160e6d58d
  PR derivation - https://gist.github.com/fanquake/0aa2d8eddaba861ba489ed3d936f727d

ACKs for top commit:
  achow101:
    ACK 79539fbfbf4d09a8b4861ddcba5b194297bc1b65

Tree-SHA512: 341ddcae27e53c31d114465cb5173573dcc9e1c0874ee160715630f686da6f69255f6080ec0181ffeffc26efbdb545599d667784b1cd17dfa7e3da0998ec9bd6
2023-10-31 17:09:36 +00:00
fanquake
697ded943c
Merge bitcoin/bitcoin#28757: guix: Zip needs to include all files and set time to SOURCE_DATE_EPOCH
f6f18eeaa88784e487e9bca8c5ace6c66bd721cc guix: Zip needs to include all files with time as SOURCE_DATE_EPOCH (Andrew Chow)

Pull request description:

  The zip for codesigned MacOS distribution needs to have all files included and have their timestamps set to the same value (`SOURCE_DATE_EPOCH`).

  This uses the same pattern for zip as is done for the other zip files produced by guix.

ACKs for top commit:
  hebasto:
    ACK f6f18eeaa88784e487e9bca8c5ace6c66bd721cc.
  TheCharlatan:
    ACK f6f18eeaa88784e487e9bca8c5ace6c66bd721cc

Tree-SHA512: 569ff0d8bfe76b9b111a2454478523eeb514b44b691be8b57b61415db88356c683582550ea67ebd5fb392b4f486be170a925067b507979090535ca41cbc7351b
2023-10-31 16:45:30 +00:00
Andrew Chow
f6f18eeaa8 guix: Zip needs to include all files with time as SOURCE_DATE_EPOCH
The zip for codesigned MacOS distribution needs to have all files have
the same timestamp. These files also need to be included in the zip as
zip is not automatically recursive. We use the same pattern for zip as
is done for the other zip files produced by guix.
2023-10-31 11:24:21 -04:00
fanquake
79539fbfbf
guix: update signapple
Fixes #28449
2023-10-31 15:14:33 +00:00
fanquake
b74e449ffa
build: remove potential for duplciate natpmp linking
Consolidate the lib checking logic to be the same as miniupnpc.
2023-10-31 11:12:28 +00:00
fanquake
4e95096952
build: remove duplicate -lminiupnpc linking
Having the link check in the header check loop means we get `-lminiupnpc
-lminiupnpc -lminiupnpc` on the link line. This is unnecessary, and
results in warnings, i.e:
```bash
ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc'
```

These warnings have been occurring since the new linker released with
Xcode 15, and also came up in https://github.com/hebasto/bitcoin/pull/34.
2023-10-31 11:12:24 +00:00
fanquake
d51fb9caa6
Merge bitcoin/bitcoin#28503: refactor: Remove WithParams serialization helper, use SER_PARAMS_OPFUNC
99990194ce26af89308fab5ad0b1c8c26e45f80c Remove WithParams serialization helper (MarcoFalke)
ffffb4af83a47979a0ecc84247bc1167abc2fbf6 scripted-diff: Use ser params operator (MarcoFalke)
fae9054793ff2a15db1a645cce3df749e0de2f39 test: Use SER_PARAMS_OPFUNC in serialize_tests.cpp (MarcoFalke)

Pull request description:

  Every serialization parameter struct already has the `SER_PARAMS_OPFUNC`, except for one in the tests.

  For consistency, and to remove verbose code, convert the test to `SER_PARAMS_OPFUNC`, and use it everywhere, then remove the `WithParams` helper.

ACKs for top commit:
  ajtowns:
    reACK 99990194ce26af89308fab5ad0b1c8c26e45f80c
  TheCharlatan:
    Re-ACK 99990194ce26af89308fab5ad0b1c8c26e45f80c

Tree-SHA512: be9cae4225a502486fe8d552aaf4b2cd2904a9f73cce9d931c6b7c757594ff1982fcc2c30d00d012cd12b0a9531fd609f8bcd7c94b811e965ac087eb8a3589d3
2023-10-31 11:11:25 +00:00
Sebastian Falbesoner
e26e665f9f gui: fix crash on selecting "Mask values" in transaction view
This commits fixes a crash bug that can be caused with the following steps:
- change to the "Transactions" view
- right-click on an arbitrary transaction -> "Show transaction details"
- close the transaction detail window again
- select "Settings" -> "Mask values"

The problem is that the list of opened dialogs, tracked in the member
variable `m_opened_dialogs`, is only ever appended with newly opened
transaction detail dialog pointers, but never removed. This leads to
dangling pointers in the list, and if the "Mask values" menu item is
selected, a crash is caused in the course of trying to close the opened
transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this
by removing the pointer from the list if the corresponding widget is
destroyed.
2023-10-31 00:27:26 +01:00