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 03 22 #5951

Merged
10 commits merged into from
Mar 26, 2024

Conversation

PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Mar 23, 2024

Issue being fixed or feature implemented

Batch of backports

What was done?

Trivial batch of backports

How Has This Been Tested?

CI looks good

Breaking Changes

None

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)

MarcoFalke and others added 4 commits March 22, 2024 11:20
…block

4444128 test: Fix intermittent issue in wallet_listsinceblock (MarcoFalke)

Pull request description:

ACKs for top commit:
  Empact:
    Code Review ACK bitcoin@4444128

Tree-SHA512: 86d47b1e3c8681dd479654589c894016ac81a3c96a34c3b4a75278b2af85054ea8c6f768e518a5322a4928d82d5e99105bbce0f4fa6a7a18c40e3e0799f9ab54
fada2df test: Fix wallet_multiwallet issue on windows (MarcoFalke)

Pull request description:

  The error message on windows:

  > 2020-11-30T18:10:47.536032Z ListWalletDir: Error scanning C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink: boost::filesystem::status: The name of the file cannot be resolved by the system: "C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink\wallet.dat"

ACKs for top commit:
  promag:
    Code review ACK fada2df. Although it could ignore (don't log) directories that lead to no permission error.
  fanquake:
    ACK fada2df

Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72
173ef89 build: Small libxcb.mk improvements (Hennadii Stepanov)
5129b36 build: Clean remnants of QTBUG-34748 fix (Hennadii Stepanov)

Pull request description:

  Hope, this PR will make [transit](bitcoin#21376) to Qt 5.12.10 neater.

  A fix for [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was introduced in dashpay#5915 (v0.11.0, Qt 5.2.1).

  [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was [fixed](qt/qtbase@b19b080) in Qt 5.3.0.

  The separated [`fix-xcb-include-order.patch`](https://github.com/theuni/bitcoin/blob/bb44d9e7546e6118cd91db5bbe471a3ce2ee7fcd/depends/patches/qt/fix-xcb-include-order.patch), provided by dashpay#5915, was dropped in bitcoin#12971 while bumping Qt to 5.9.4 (5.9.6). But `libxcb.mk` remained unchanged.

  This PR reverts dashpay#5915 for `libxcb.mk` as well.

ACKs for top commit:
  practicalswift:
    cr ACK 173ef89: patch looks correct
  fanquake:
    ACK 173ef89

Tree-SHA512: 9815a7e532ff4aa08f9623ded8d5708eca1c9c73ac7a2684419a18c125da7627b44ac3191f2e7978946942c8d0580e73b1a93df624986fb2a13791a68ce1e025
… blank wallet checkbox

915e341 qt: fix issue when disabling the auto-enabled blank wallet checkbox (Jarol Rodriguez)

Pull request description:

  As detailed by dashpay#151, On `master` a user can create the confusing scenario where you have a disabled `Encrypt Wallet` checkbox and a selected `Disable Private Keys` checkbox after unselecting the auto-enabled `Blank Wallet` checkbox.

  This commit makes it so that when the `Blank Wallet` checkbox is auto-selected after the user selects `Disable Private keys`, unselecting it will also unselect the `Disable Private Keys` checkbox, which in turn re-enables the `Encrypt Wallet` checkbox.

  Below are screenshots comparing the behavior of selecting `Disable Private Keys` then unselecting the `Blank Wallet` between `master` and this `PR`:

  **Master:**
  | Select `Disable Private Keys` | Unselect `Blank Wallet` |
  | ----------------------------- | ------------------------ |
  | ![Screen Shot 2021-03-09 at 7 57 14 PM](https://user-images.githubusercontent.com/23396902/110560141-77405a80-8113-11eb-9285-5acba6241dcf.png) |   ![Screen Shot 2021-03-09 at 7 57 31 PM](https://user-images.githubusercontent.com/23396902/110560159-81faef80-8113-11eb-9b37-086aa39ecb9f.png)    |

  **PR:**
  | Select `Disable Private Keys` | Unselect `Blank Wallet` |
  | ----------------------------- | ------------------------ |
  | ![Screen Shot 2021-03-09 at 7 34 12 PM](https://user-images.githubusercontent.com/23396902/110560379-e3bb5980-8113-11eb-899a-3a4c6a1bc115.png) | ![Screen Shot 2021-03-09 at 7 34 20 PM](https://user-images.githubusercontent.com/23396902/110560412-f170df00-8113-11eb-8bd0-f7fe6fc0d739.png) |

ACKs for top commit:
  hebasto:
    ACK 915e341
  Talkless:
    ACK 915e341

Tree-SHA512: ce6ecbc35b94a08cabf0b8a24dbdfc874d82cc8918cc8623dce8172c7fc9c75d63a13b036bae5f7ab2c090f8d020574a542285d1651600813faf5d91e2506a8d
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-03-22 branch from b469d98 to 06c5ae7 Compare March 24, 2024 00:27
@PastaPastaPasta PastaPastaPasta changed the title [WIP] backport: trivial 2024 03 22 backport: trivial 2024 03 22 Mar 24, 2024
@PastaPastaPasta PastaPastaPasta added this to the 21 milestone Mar 24, 2024
@PastaPastaPasta PastaPastaPasta marked this pull request as ready for review March 24, 2024 03:33
knst
knst previously approved these changes Mar 25, 2024
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

test/sanitizer_suppressions/tsan Outdated Show resolved Hide resolved
Comment on lines +226 to +230
if not final_psbt["complete"]:
node.log.info(f'final_psbt={final_psbt}')
for w in node.listwallets():
wrpc = node.get_wallet_rpc(w)
node.log.info(f'listunspent={wrpc.listunspent()}')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

22149: NOTE: this code was later removed in 22308

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but 22308 is not backported yet - so, that's fine, isn't it?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I just made a note to myself while reviewing it cause it says "temporary" :)

fanquake and others added 6 commits March 25, 2024 11:21
…_tests suppression

fab1987 test: Document race:validation_chainstatemanager_tests suppression (MarcoFalke)

Pull request description:

ACKs for top commit:
  jamesob:
    ACK bitcoin@fab1987
  practicalswift:
    ACK fab1987

Tree-SHA512: 3f1838b4cf11eba768ce06826cd4b57c9065669b61a5530af44216fc96535ebf37124b47a8de8f72aedf32345157a72d2208cd63214481a9cb56c063f05db5dd
faa9496 test: Add temporary logging to debug bitcoin#20975 (MarcoFalke)

Pull request description:

  to be reverted after a fix

ACKs for top commit:
  laanwj:
    Code review ACK bitcoin@faa9496

Tree-SHA512: 1f3103fcf4cad0af54e26c4d257bd824b128b5f2d2b81c302e861a829fd55d6a099fa476b79b30a71fe98975ae604b9e3ff31fd48a51d442389a9bd515e60ba0
fa34cb8 cli: Avoid truncating -rpcwaittimeout (MarcoFalke)

Pull request description:

  `seconds` is not enough precision to "exactly" store a timestamp n seconds into the future. Improve the precision by using `microseconds`. Fixes bitcoin#22325

  Also, use chrono literals.

ACKs for top commit:
  jonatack:
    ACK fa34cb8 review, debug-built, tested
  theStack:
    Tested ACK fa34cb8

Tree-SHA512: 7158da8545f9998a82bcc8636e04564efdb1e1be43b4288298c151b4df29ad47a2760259eefadd4a01db92ea18a1e017f3febc1cd8c69a4b28c86180229d8c90
…false in case of a blank hd wallet.

8733a8e the result of CWallet::IsHDEnabled() was initialized with true. (Saibato)

Pull request description:

  the result of CWallet::IsHDEnabled() was initialized with true.

  But in case of no keys or a blank hd wallet the iterator would be skipped
  and not set to false but true, since the loop would be not entered.

  That had resulted in a wrong return and subsequent false HD and watch-only
  icon display in GUi when reloading a wallet after closing.

ACKs for top commit:
  achow101:
    ACK 8733a8e
  hebasto:
    ACK 8733a8e
  theStack:
    utACK 8733a8e
  meshcollider:
    utACK 8733a8e

Tree-SHA512: 79b976594f7174d05c29fe3819037ead59aaef27498d95415ceba74d633a8e035f6b03b521000ac3370684a8cb09319d8be1a443ce2d29b3ff4089e399f6b719
0bd882b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner)

Pull request description:

  The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member:

  https://github.com/bitcoin/bitcoin/blob/83daf47898f8a79cb20d20316c64becd564cf54c/src/validation.cpp#L2973-L2976

  ~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~

  ~~This is related to bitcoin#19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR dashpay#3370, commit 75f51f2) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7), probably also because the compilers didn't support them properly earlier.~~

  At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.

ACKs for top commit:
  Zero-1729:
    ACK 0bd882b
  promag:
    Code review ACK 0bd882b.
  hebasto:
    ACK 0bd882b

Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
…eaders-only Boost.Test

81738d2 test: Remove suppression no longer needed with headers-only Boost.Test (Hennadii Stepanov)

Pull request description:

  It appears, that moving to [headers-only](bitcoin#24301) Boost.Test makes the removed suppression unneeded even without [bumping](bitcoin#24383) boost version.

ACKs for top commit:
  MarcoFalke:
    cr ACK 81738d2

Tree-SHA512: e60443f79a2e38cc78fceeff5c2956d622e8a10730129f9c27c14aef59bc6fa0894b8011e6191530443bf3165f78da978bc08ad04248ddb65e2da373264afa6a
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@PastaPastaPasta PastaPastaPasta closed this pull request by merging all changes into dashpay:develop in f2a42a0 Mar 26, 2024
@PastaPastaPasta PastaPastaPasta deleted the develop-trivial-2024-03-22 branch March 26, 2024 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants