Skip to content

backport: bitcoin#15584, #16060, #16465, #16477, #16680, #16714, #16941, #16991, #17453, #17696, #19058 (last batch for v19) #5255

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Apr 26, 2023
Merged

backport: bitcoin#15584, #16060, #16465, #16477, #16680, #16714, #16941, #16991, #17453, #17696, #19058 (last batch for v19) #5255

merged 13 commits into from
Apr 26, 2023

Conversation

@knst
Copy link
Collaborator

@knst knst commented Mar 15, 2023 •
edited
Loading

What was done?

Last batch of missing backports from v18-v19 and related changes.

To follow-up backport bitcoin#16060 there were buried also 2 dash's specific deployments:

  • DIP0001
  • DIP0008

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

No breaking changes

Checklist:

  • I have performed a self-review of my own code
  • I have assigned this pull request to a milestone

@knst knst changed the title backport: bitcoin (last batch for v19) backport: bitcoin#15584, #16477, #16680, #16714, #16941, #16991, #19058 (last batch for v19) Mar 15, 2023
@knst knst added this to the 20 milestone Mar 15, 2023
@knst knst force-pushed the bc-bp-missing-4 branch from 5153ee1 to 3cd94ef Compare March 28, 2023 09:40
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

This pull request has conflicts, please rebase.

@knst knst force-pushed the bc-bp-missing-4 branch from 3cd94ef to dbeee3b Compare April 7, 2023 04:38
@github-actions
Copy link

github-actions bot commented Apr 15, 2023

This pull request has conflicts, please rebase.

1 similar comment
@github-actions
Copy link

github-actions bot commented Apr 15, 2023

This pull request has conflicts, please rebase.

@knst knst force-pushed the bc-bp-missing-4 branch 2 times, most recently from 57ce683 to a3855fa Compare April 25, 2023 09:42
@knst knst force-pushed the bc-bp-missing-4 branch from a3855fa to 2952e2c Compare April 25, 2023 10:24
@knst knst changed the title backport: bitcoin#15584, #16477, #16680, #16714, #16941, #16991, #19058 (last batch for v19) backport: bitcoin#15584, #16060, #16465, #16477, #16680, #16714, #16941, #16991, #17453, #17696, #19058 (last batch for v19) Apr 25, 2023
@knst knst force-pushed the bc-bp-missing-4 branch 2 times, most recently from cbfb81e to ccb94c2 Compare April 25, 2023 10:59
@knst knst marked this pull request as ready for review April 25, 2023 15:12
@knst knst requested review from PastaPastaPasta and UdjinM6 April 25, 2023 15:12
@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Apr 25, 2023
UdjinM6
UdjinM6 approved these changes Apr 25, 2023
View reviewed changes
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.

lightly tested ACK

@UdjinM6 UdjinM6 mentioned this pull request Apr 25, 2023
Merged
5 tasks
PastaPastaPasta
PastaPastaPasta approved these changes Apr 26, 2023
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merging via merge commit; however a subsequent PR should be done to bury the other hardened hard forks

src/chainparams.cpp
consensus.vDeployments[Consensus::DEPLOYMENT_DIP0008].nStartTime = 1557878400; // May 15th, 2019
consensus.vDeployments[Consensus::DEPLOYMENT_DIP0008].nTimeout = 1589500800; // May 15th, 2020
consensus.vDeployments[Consensus::DEPLOYMENT_DIP0008].nWindowSize = 4032;
consensus.vDeployments[Consensus::DEPLOYMENT_DIP0008].nThresholdStart = 3226; // 80% of 4032
Copy link
Member

@PastaPastaPasta PastaPastaPasta Apr 26, 2023

Choose a reason for hiding this comment

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

other deployments have been buried as well no? Should do those in another PR at least

fanquake and others added 6 commits April 25, 2023 23:41
@fanquake @PastaPastaPasta
Merge bitcoin#16477: build: skip deploying plugins we dont use in mac…
a778ddd 
…deployqtplus

1ac7b7f scripts: filter more qt plugins we don't use in macdeployqtplus (fanquake)
57cdd06 scripts: misc cleanups in macdeployqtplus (fanquake)
51729a4 scripts: use format() in macdeployqtplus (fanquake)
1c37e81 scripts: add type annotations to macdeployqtplus (fanquake)

Pull request description:

  I frequently run `make deploy` while testing on macOS to get a properly light themed .app. With a brew installed Qt, this currently results in a pretty bloated executable:

  | branch | .app size | .dmg size | `make deploy` time |
  | ------- | --------- | --------- | --------------------- |
  | master (febf3a8) | 235mb | 86mb | 38s |
  | This PR (da98f6d) | 51mb | 21mb | 22s |

  Similar change to dd367ff.

  ```diff
                           'QtGui.framework'],
    'pluginPath': '/usr/local/opt/qt/plugins',
    'qtPath': '/usr/local/opt/qt'}
  -[('platforminputcontexts', 'libqtvirtualkeyboardplugin.dylib'),
  - ('geoservices', 'libqtgeoservices_esri.dylib'),
  - ('geoservices', 'libqtgeoservices_mapboxgl.dylib'),
  - ('geoservices', 'libqtgeoservices_nokia.dylib'),
  - ('geoservices', 'libqtgeoservices_itemsoverlay.dylib'),
  - ('geoservices', 'libqtgeoservices_osm.dylib'),
  - ('geoservices', 'libqtgeoservices_mapbox.dylib'),
  - ('sceneparsers', 'libgltfsceneexport.dylib'),
  - ('sceneparsers', 'libgltfsceneimport.dylib'),
  - ('platforms', 'libqwebgl.dylib'),
  +[('platforms', 'libqwebgl.dylib'),
    ('platforms', 'libqoffscreen.dylib'),
    ('platforms', 'libqminimal.dylib'),
    ('platforms', 'libqcocoa.dylib'),
    ('platformthemes', 'libqxdgdesktopportal.dylib'),
  - ('printsupport', 'libcocoaprintersupport.dylib'),
  - ('webview', 'libqtwebview_webengine.dylib'),
  - ('webview', 'libqtwebview_darwin.dylib'),
  - ('geometryloaders', 'libdefaultgeometryloader.dylib'),
  - ('geometryloaders', 'libgltfgeometryloader.dylib'),
    ('styles', 'libqmacstyle.dylib'),
  - ('canbus', 'libqttinycanbus.dylib'),
  - ('canbus', 'libqtpassthrucanbus.dylib'),
  - ('canbus', 'libqtvirtualcanbus.dylib'),
  - ('canbus', 'libqtpeakcanbus.dylib'),
    ('bearer', 'libqgenericbearer.dylib'),
  - ('imageformats', 'libqgif.dylib'),
  - ('imageformats', 'libqwbmp.dylib'),
  - ('imageformats', 'libqwebp.dylib'),
  - ('imageformats', 'libqico.dylib'),
  - ('imageformats', 'libqmacheif.dylib'),
  - ('imageformats', 'libqjpeg.dylib'),
  - ('imageformats', 'libqtiff.dylib'),
  - ('imageformats', 'libqicns.dylib'),
  - ('imageformats', 'libqtga.dylib'),
  - ('imageformats', 'libqmacjp2.dylib'),
  - ('texttospeech', 'libqtexttospeech_speechosx.dylib'),
  - ('generic', 'libqtuiotouchplugin.dylib'),
  - ('renderplugins', 'libscene2d.dylib'),
  - ('gamepads', 'libdarwingamepad.dylib'),
  - ('virtualkeyboard', 'libqtvirtualkeyboard_thai.dylib'),
  - ('virtualkeyboard', 'libqtvirtualkeyboard_openwnn.dylib'),
  - ('virtualkeyboard', 'libqtvirtualkeyboard_hangul.dylib'),
  - ('virtualkeyboard', 'libqtvirtualkeyboard_pinyin.dylib'),
  - ('virtualkeyboard', 'libqtvirtualkeyboard_tcime.dylib')]
  + ('generic', 'libqtuiotouchplugin.dylib')]
  ```

ACKs for top commit:
  laanwj:
    ACK 1ac7b7f (purely Python code review and the fact that this passes travis, cannot run this on a mac)
  dongcarl:
    tested ACK 1ac7b7f

Tree-SHA512: 5974eeaf7229bb5bde2b283c1331ec57ee87f624db146401f6b77dee4ee5502e0bd669958a46205f10398a371f8e6c91ddacb9f0e1943f9f7d042fb6de7957a8
@PastaPastaPasta
Merge bitcoin#16680: Preparations for more testchains
d5d3c3e 
3bf9d8c Testchains: Qt: Simplify network/chain styles (Jorge Timón)
052c54e Testchains: Generic selection with -chain=<str> in addition of -testnet and -regtest (Jorge Timón)

Pull request description:

  Separated from bitcoin#8994 as suggested by MarcoFalke and Sjors in bitcoin#8994 (comment)

  You can't really test the qt changes on their own, so to test them, use bitcoin#8994 .

ACKs for top commit:
  MarcoFalke:
    ACK 3bf9d8c

Tree-SHA512: 5b5e6083ebc0a44505a507fac633e7af18037c85e5e73f5d1e6f7e730575d3297ba8a31d1c2441df623b273f061c32d8fa324f4aa6bead01d23e88582029b568
@jonasschnelli @PastaPastaPasta
Merge bitcoin#16714: gui: add prune to intro screen with smart default
6ea9349 
9924bce [gui] intro: enable pruning by default unless disk is big (Sjors Provoost)
c8de347 [gui] intro: add prune preference (Sjors Provoost)
1bbc49d [gui] intro: inform caller if intro was shown (Sjors Provoost)
1957103 [gui] add explicit prune setter (Sjors Provoost)
1bccf6a [node] add forceSetArg to interface (Sjors Provoost)

Pull request description:

  This adds a checkbox to the intro screen to enable pruning from the get go.

  If the user has plenty of space, it's unchecked by default:

  <img width="671" alt="big" src="https://user-images.githubusercontent.com/10217/63641289-10339000-c6ac-11e9-98d7-caf64dff0da6.png">

  If the user has insufficient space it's checked by default:
  <img width="897" alt="low" src="https://user-images.githubusercontent.com/10217/63641276-d4002f80-c6ab-11e9-9f5b-a53472f814ff.png">

  When the user has barely enough space and is likely to need pruning in the near future, this is shown in yellow and we also check the prune box:

  <img width="662" alt="medium" src="https://user-images.githubusercontent.com/10217/63641294-1c1f5200-c6ac-11e9-8ecb-6b69e42b1ece.png">

  The cut-off for this 10 GB above `m_assumed_blockchain_size` (`=240` in `chainparams.cpp`).

  If the user launches the first time with `-prune=...` then we disable the check box and display the correct size (rounded to GB):
  <img width="658" alt="Schermafbeelding 2019-08-24 om 20 23 14" src="https://user-images.githubusercontent.com/10217/63641351-09594d00-c6ad-11e9-94fe-fe5ed562e109.png">

  The 2 GB default matches the settings default. The user can't change it in the intro screen, but can change it later. I'm tempted to increase that default to 10 GB, and then have the intro screen reduce it if space is really tight.

  Tips for testing:
  * move your existing data dir elsewhere
  * wipe data dir at every restart (behavior is different if it exists)
  * launch with `bitcoin-qt -resetguisettings -lang=en` (there's some space issues in different languages)
  * fake your free space by changing `intro.cpp` line 90: `freeBytesAvailable = 5000000000; // 5 GB`
  * try both testnet and mainnet, because settings are seperate. In particular note how step 7 in `GuiMain` switches where `QTSettings settings` points to; this had me thoroughly confused on testnet, because I was setting them too early.

ACKs for top commit:
  jonasschnelli:
    Tested ACK 9924bce
  ryanofsky:
    utACK 9924bce. The changes are very logical, and implement the feature in a clean that way that doesn't add a lot of complication and shouldn't interfere with future improvements. I looked at Luke's branch too, and I think there's also a lot of great stuff there that seems fully compatible with this change.

Tree-SHA512: 9523961451c53aebd347716976bc3a4a398f989dc21e9bbbd357060bd11a8f46c435f068bd421bb31ccb08e55445ef67bc347d8d19a4fb8fde9d6d3f9a3bcbb0
@fanquake @PastaPastaPasta
Merge bitcoin#17696: qt: Force set nPruneSize in QSettings after the …
e75d5b7 
…intro dialog

af112ab qt: Rename SetPrune() to InitializePruneSetting() (Hennadii Stepanov)
b0bfbe5 refactor: Drop `bool force' parameter (Hennadii Stepanov)
68c9bbe qt: Force set nPruneSize in QSettings after intro (Hennadii Stepanov)
a82bd8f util: Replace magics with DEFAULT_PRUNE_TARGET_GB (Hennadii Stepanov)

Pull request description:

  On master (5622d8f), having `QSettings` set already
  ```
  $ grep nPruneSize ~/.config/Bitcoin/Bitcoin-Qt-testnet.conf
  nPruneSize=6
  ```

  enabling prune option in the intro dialog

  ```
  $ ./src/qt/bitcoin-qt -choosedatadir -testnet
  ```
  ![DeepinScreenshot_select-area_20191208120425](https://user-images.githubusercontent.com/32963518/70388183-eed68580-19b6-11ea-9aa1-f9ad9aaa68a6.png)

  has no effect:
  ```
  $ grep Prune ~/.bitcoin/testnet3/debug.log
  2019-12-08T10:04:41Z Prune configured to target 5722 MiB on disk for block and undo files.
  ```

  ---

  With this PR:
  ```
  $ grep Prune ~/.bitcoin/testnet3/debug.log
  2019-12-08T10:20:35Z Prune configured to target 1907 MiB on disk for block and undo files.
  ```

  This PR has been split of bitcoin#17453 (the first two commits) as it fixes an orthogonal bug.

  Refs:
  - bitcoin#17453 (comment)
  - bitcoin#17453 (comment)

ACKs for top commit:
  Sjors:
    Code review re-ACK af112ab
  ryanofsky:
    Code review ACK af112ab. Just suggested changes since last review (thanks!)
  promag:
    Tested ACK af112ab. Latest suggestions and changes look good to me.

Tree-SHA512: 8ddad34b30dcc2cdcad6678ba8a0b36fa176e4e3465862ef6eee9be0f98d8146705138c9c7995dd8c0990af41078ca743fef1a90ed9240081f052f32ddec72b9
@jonasschnelli @PastaPastaPasta
Merge bitcoin#17453: gui: Fix intro dialog labels when the prune butt…
78e475a 
…on is toggled

4f7127d gui: Make Intro consistent with prune checkbox (Hennadii Stepanov)
4824a7d gui: Add Intro::UpdateFreeSpaceLabel() (Hennadii Stepanov)
daa3f3f refactor: Add Intro::UpdatePruneLabels() (Hennadii Stepanov)
e4caa82 refactor: Replace static variable with data member (Hennadii Stepanov)
2bede28 util: Add PruneGBtoMiB() function (Hennadii Stepanov)
e35e4b2 util: Add PruneMiBtoGB() function (Hennadii Stepanov)

Pull request description:

  On master (a6f6333) and on 0.19.0.1 the intro dialog with prune enabled (checkbox "Discard blocks..." is checked) provides a user with wrong info about the required disk space:

  ![DeepinScreenshot_bitcoin-qt_20191208112228](https://user-images.githubusercontent.com/32963518/70387510-8daab400-19ae-11ea-9338-29add9c31118.png)

  Also the paragraph "If you have chosen to limit..." is missed.

  ---

  With this PR when prune checkbox is toggled, the related text labels and the amount of required space shown are updated (previously they were only updated when the data directory was updated):
  ![Screenshot from 2019-12-08 11-34-53](https://user-images.githubusercontent.com/32963518/70387542-eed28780-19ae-11ea-9565-49d8a64b2f33.png)

  ---

  This PR is an alternative to bitcoin#17035.

  **ryanofsky**'s [suggestion](bitcoin#17035 (comment)) also has been implemented.

ACKs for top commit:
  emilengler:
    ACK 4f7127d
  Sjors:
    tACK 4f7127d
  ryanofsky:
    Code review ACK 4f7127d. It seems like there are a few visible changes here:
  jonasschnelli:
    utACK 4f7127d

Tree-SHA512: fa0bbdcfafde97d7906cda066cbd4608b936a71cae1b4cda3ee3aa2eed3a9795f279f14c6b1b4997278e094db891c7d3bb695368ba0882347aa42165a86e5172
@laanwj @PastaPastaPasta
Merge bitcoin#15584: build: disable BIP70 support by default
d4bc451 
BIP70 is removed in "merge bitcoin#17165: Remove BIP70 support  (dashpay#4023)".
So, this commit contains only some follow-ups to unify codebase

e09913f doc: specify protobuf as optional in build docs (fanquake)
376f492 build: disable BIP70 support by default (fanquake)

Pull request description:

  Disable BIP70 support in the GUI by default for `0.19.0` (for eventual removal in `0.20.0`?).

  Users who want to compile with BIP70 support enabled can pass `--enable-bip70` to `./configure`.

  I've inverted the current `--disable-bip70` test to instead pass `--enable-bip70`.

  Tested configurations on `macOS` (`protobuf` installed with `brew`).
  Protobuf available and `./configure`:
  ```
  Options used to compile and link:
    with wallet   = yes
    with gui / qt = yes
      with bip70  = no
  ```

  Protobuf available and `./configure --enable-bip70`:
  ```
  Options used to compile and link:
    with wallet   = yes
    with gui / qt = yes
      with bip70  = yes
  ```

  Protobuf not available (i.e `brew unlink protobuf`) and `./configure`:
  ```
  Options used to compile and link:
    with wallet   = yes
    with gui / qt = yes
      with bip70  = no
  ```

  Protobuf not available and `./configure --enable-bip70`:
  ```
  checking whether to build test_bitcoin-qt... yes
  checking whether to build BIP70 support... configure: error: protobuf missing
  ```

  TODO:
  - [x] Remove `protobuf` from other Travis builds
  - [ ] Documentation updates (mention that `protobuf` is now optional)?
  - [ ] Could split release notes into GUI and build

ACKs for top commit:
  laanwj:
    ACK e09913f
  elichai:
    ACK e09913f Read the autotools changes. awesome that this removes the protobuf requirement.
  practicalswift:
    ACK e09913f -- diff looks correct

Tree-SHA512: 7bf87ae8555e24db2da2e89cc4d4e90d09be27499ad386ad65879d05df8f96d9a1384379891ac8963d17728c90e55961560813df97e849e631e2de8c08e210c8
fanquake and others added 7 commits April 25, 2023 23:41
@fanquake @PastaPastaPasta
Merge bitcoin#19058: doc: Drop protobuf stuff
3e0160b 
ea9fcfd doc: Drop protobuf stuff (Hennadii Stepanov)

Pull request description:

  This is a follow-up to bitcoin#17165.

ACKs for top commit:
  fanquake:
    ACK ea9fcfd -  clicked the links and they seem to work.

Tree-SHA512: 0861bbac3a3ff781a413e15f5ed02c624bc15d572a001a53cd2fb9f7683456175f69e9d666b72f260abbb5114b67cefca9fada4d179c62384c90479534ae63d5
@laanwj @PastaPastaPasta
Merge bitcoin#16941: travis: Disable feature_block in tsan run due to…
dc5da97 
… OOM

faa079d travis: Disable feature_block in tsan run (MarcoFalke)

Pull request description:

  The `feature_block` test causes the travis machine to OOM, when run with the thread sanitizer.

  The stderr says:

  ```
  ==27237==ERROR: ThreadSanitizer failed to allocate 0xf6000 (1007616) bytes of LargeMmapAllocator (error code: 12)
  ...
  FATAL: ThreadSanitizer CHECK failed: /build/llvm-toolchain-3.8-_PD09B/llvm-toolchain-3.8-3.8/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:183 "((0 && "unable to mmap")) != (0)" (0x0, 0x0)

  ERROR: Failed to mmap
  ```
  (from https://travis-ci.org/bitcoin/bitcoin/jobs/588194563#L10505)

  Fix this by disabling `feature_block` on travis. Longer term, I'd like to move away from travis, but I'll leave this for a follow-up.

ACKs for top commit:
  fanquake:
    ACK faa079d

Tree-SHA512: c0dc2272853aac53f68eb9e110c8500c4a92211ba89d856660bacdf6e959d875477e422b3280b743d85fc8a65e083bf9153911f12039d026e2501f426540dac4
@laanwj @PastaPastaPasta
Merge bitcoin#16991: qa: Fix service flag comparison check in rpc_net…
2307f97 
… test (luke-jr)

9c23ebd qa: Fix service flag comparison check in rpc_net test (Luke Dashjr)

Pull request description:

  Rebase of bitcoin#16936

ACKs for top commit:
  darosior:
    ACK 9c23ebd

Tree-SHA512: 74f287740403da1040ab1e235ef6eba4e304f3ee5d57a3b25d1e2e1f2f982d256528d398a4d6cb24ba393798e680a8f46cd7dae54ed84ab2c747e96288f1f884
@PastaPastaPasta
Merge bitcoin#16465: test: Test p2sh-witness and bech32 in wallet_imp…
3e1c8a6 
…ort_rescan

fa3c657 lint: Add false positive to python dead code linter (MarcoFalke)
fa25668 test: Test p2sh-witness and bech32 in wallet_import_rescan (MarcoFalke)
fa79af2 test: Replace fragile "rng" with call to random() (MarcoFalke)
fac3dcf test: Generate one block for each send in wallet_import_rescan (MarcoFalke)

Pull request description:

  This adds test coverage for segwit in the `wallet_import_rescan` test, among other cleanups.

ACKs for top commit:
  jnewbery:
    ACK fa3c657

Tree-SHA512: 877741763c62c1bf9d868864a1e3f0699857e8c028e9fcd65c7eeb73600c22cbe97b7b51093737743d9e87bcb991c1fe1086f673e18765aef0fcfe27951402f0
@PastaPastaPasta
Merge bitcoin#16060: Bury bip9 deployments
6ad9bdf 
e78aaf4 [docs] Add release notes for burying bip 9 soft fork deployments (John Newbery)
8319e73 [tests] Add coverage for the content of getblockchaininfo.softforks (James O'Beirne)
0328dcd [Consensus] Bury segwit deployment (John Newbery)
1c93b9b [Consensus] Bury CSV deployment height (John Newbery)
3862e47 [rpc] Tidy up reporting of buried and ongoing softforks (John Newbery)

Pull request description:

  This hardcodes CSV and segwit activation heights, similar to the BIP 90 buried deployments for BIPs 34, 65 and 66.

  CSV and segwit have been active for over 18 months. Hardcoding the activation height is a code simplification, makes it easier to understand segwit activation status, and reduces technical debt.

  This was originally attempted by jl2012 in bitcoin#11398 and again by me in bitcoin#12360.

ACKs for top commit:
  ajtowns:
    ACK e78aaf4 ; checked diff to previous acked commit, checked tests still work
  ariard:
    ACK e78aaf4, check diff, run the tests again and successfully activated csv/segwit heights on mainnet as expected.
  MarcoFalke:
    ACK e78aaf4 (still didn't check if the mainnet block heights are correct, but the code looks good now)

Tree-SHA512: 7e951829106e21a81725f7d3e236eddbb59349189740907bb47e33f5dbf95c43753ac1231f47ae7bee85c8c81b2146afcdfdc11deb1503947f23093a9c399912
@knst @PastaPastaPasta
feat: burry DIP0001 deployment to follow-up bitcoin#16060
d26b07a
@knst @PastaPastaPasta
feat: burry DIP0008 deployment to follow-up bitcoin#16060
8457336
@PastaPastaPasta PastaPastaPasta merged commit e697554 into dashpay:develop Apr 26, 2023
PastaPastaPasta pushed a commit that referenced this pull request Apr 26, 2023
@UdjinM6
fix(qt): provide correct data into intro dlg (#5348)
6c5d6c1 
## Issue being fixed or feature implemented
we failed to backport 13216 correctly in #4359

noticed this while reviewing/testing #5255 

## What was done?
fix it

## How Has This Been Tested?
run qt with `-resetguisetting` and check info with and without the patch
on testnet for example (or tweak regtest params and test there)

## Breaking Changes
n/a

## Checklist:
- [x] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
@UdjinM6 UdjinM6 mentioned this pull request Apr 27, 2023
Merged
5 tasks
PastaPastaPasta pushed a commit that referenced this pull request May 2, 2023
@UdjinM6
fix(qt): fix few gui issues (#5351)
4cd7e9a 
## Issue being fixed or feature implemented
- red `#800000` looks ugly in Dark theme, use TS_ERROR like we did prior
to 17453 backport (via #5255), also introduce TS_WARNING for `#999900`
to unify things
before:
<img width="454" alt="Screenshot 2023-04-26 at 22 40 54"
src="https://user-images.githubusercontent.com/1935069/234685172-129bc88e-bfaf-45d9-ad93-f2c5139b545a.png">
after:
<img width="432" alt="Screenshot 2023-04-26 at 22 41 30"
src="https://user-images.githubusercontent.com/1935069/234685179-2c304b67-fb48-4f36-b8e7-869cb3400c74.png">

using TS_WARNING in sendcoinsdialog:
<img width="582" alt="Screenshot 2023-05-01 at 20 38 12"
src="https://user-images.githubusercontent.com/1935069/235508898-cdb0f875-21bc-451e-b19f-2e6465f14e38.png">
<img width="419" alt="Screenshot 2023-05-01 at 21 38 20"
src="https://user-images.githubusercontent.com/1935069/235508909-0c999cb9-445d-4b5d-96aa-5f8a094df114.png">


- there are ugly borders in statusbar when running on windows, fix them
via css

before:

![dc_before](https://user-images.githubusercontent.com/1935069/234683392-e0d97760-dd81-4e87-bb37-c67d8a03e4de.png)

after:

![dc_after](https://user-images.githubusercontent.com/1935069/234683382-d9d9d1a0-d136-4018-b470-c091e4285979.png)

## What was done?


## How Has This Been Tested?
run `dash-qt --regtest --nowallet --resetguisettings` 

## Breaking Changes
n/a

## Checklist:
- [x] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 22, 2023
@UdjinM6 @PastaPastaPasta
fix(qt): provide correct data into intro dlg (dashpay#5348)
a7a69df 
## Issue being fixed or feature implemented
we failed to backport 13216 correctly in dashpay#4359

noticed this while reviewing/testing dashpay#5255 

## What was done?
fix it

## How Has This Been Tested?
run qt with `-resetguisetting` and check info with and without the patch
on testnet for example (or tweak regtest params and test there)

## Breaking Changes
n/a

## Checklist:
- [x] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
@kwvg kwvg mentioned this pull request Oct 27, 2024
Closed
5 tasks
This was referenced Oct 21, 2025
Merged
Draft
Merged
@kwvg kwvg mentioned this pull request Oct 28, 2025
Merged
5 tasks
PastaPastaPasta added a commit that referenced this pull request Oct 29, 2025
@PastaPastaPasta
Merge #6929: fix: repair makeseeds.py, `getblockchaininfo[softforks…
ec10c05 
…]` help text, drop extra `generate`s from test, resolve macOS GID issue

39e847c fix: do not create group if GID is already taken (Kittywhiskers Van Gogh)
36db83a fix: annotate `getblockchaininfo[softforks]` as `OBJ_DYN` (Kittywhiskers Van Gogh)
e4ace44 fix: repair IP extraction in `makeseeds.py`, read from `core_p2p` arr (Kittywhiskers Van Gogh)
9ef8c53 test: drop extra `generate`s from `wallet_signrawtransactionwithwallet` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * The help text for `getblockchaininfo['softforks']` was introduced in [bitcoin#16060](bitcoin#16060) (backported in [dash#5255](#5255)) _but_ the `RPCResult` help text was introduced in [bitcoin#17809](bitcoin#17809), backported _before_ [bitcoin#16060](bitcoin#16060). This resulted in the help text for `softforks` deviating from upstream without the output to match (which is aligned with upstream).

    This has since been resolved as the output structure hasn't changed, only the help text.

  * [bitcoin#22818](bitcoin#22818) (backported in [dash#6214](#6214)) activated many softforks at height 1, which rendered the block generation calls in `wallet_signrawtransactionwithwallet.py` ([source](https://github.com/dashpay/dash/blob/3e6e8f57c45fb24107816a10854ed8f349c4aa20/test/functional/wallet_signrawtransactionwithwallet.py#L132-L133)) no longer necessary, additionally [bitcoin#22550](bitcoin#22550) (backported in [dash#6189](#6189)) added assertions to validate CSV/CLTV activation that were not included in the backport.
    This has since been resolved, replacing the `generate` calls with assertions.

    * The locktime height was dropped from `1000` to `100` (matching with upstream, [source](https://github.com/bitcoin/bitcoin/blob/24.x/test/functional/wallet_signrawtransactionwithwallet.py#L219)) as we are no longer generating enough blocks to reach that height anymore.

  * `makeseeds.py` is currently mangled because [dash#6666](#6666) updated the script to look for the `core_p2p` purpose even though purpose code support was spun-off into [dash#6674](#6674). This meant, in between [dash#6666](#6666) and [dash#6674](#6674 merger, `makeseeds.py` was broken.

    This necessitated a fix that was introduced as e714c06 ([dash#6858](#6858)) but meant that after  [dash#6674](#6674), `makeseeds.py` is broken again. This has been resolved as part of this PR.

  * macOS users who want to utilise the `develop` container ([source](https://github.com/dashpay/dash/blob/b82450aa7dce2307b13e6fe282f7c4f84b48cbbf/contrib/containers/develop/Dockerfile)) need to set the `USER_ID` and `GROUP_ID` `args` to avoid permissions issues. This creates a slight problem as the GID for `staff` is 20 on macOS but Linux uses GID 20 for `dialout`, so creating a group with GID 20 will _fail_.

    As we only care about the GID, it is safe to skip the group creation step and assign the user to the group anyways as we do it by GID and _not_ name (the user will be presented as a member of the `dialout` group). This should resolve issues for macOS users trying to build the `develop` container.

  ## 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 **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 39e847c
  knst:
    utACK 39e847c
  UdjinM6:
    utACK 39e847c

Tree-SHA512: 0ecb8bb930d54d4b9024a6e94a2fe3b49c7334666e5f67d38920ac58f54332fb8b4f62bd883bd949969d60b6b5183c7a20a3593f24c2e6bbbb0de143b3de8fe1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Nov 7, 2025
@PastaPastaPasta
Merge dashpay#6929: fix: repair makeseeds.py, `getblockchaininfo[so…
3f30664 
…ftforks]` help text, drop extra `generate`s from test, resolve macOS GID issue

39e847c fix: do not create group if GID is already taken (Kittywhiskers Van Gogh)
36db83a fix: annotate `getblockchaininfo[softforks]` as `OBJ_DYN` (Kittywhiskers Van Gogh)
e4ace44 fix: repair IP extraction in `makeseeds.py`, read from `core_p2p` arr (Kittywhiskers Van Gogh)
9ef8c53 test: drop extra `generate`s from `wallet_signrawtransactionwithwallet` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * The help text for `getblockchaininfo['softforks']` was introduced in [bitcoin#16060](bitcoin#16060) (backported in [dash#5255](dashpay#5255)) _but_ the `RPCResult` help text was introduced in [bitcoin#17809](bitcoin#17809), backported _before_ [bitcoin#16060](bitcoin#16060). This resulted in the help text for `softforks` deviating from upstream without the output to match (which is aligned with upstream).

    This has since been resolved as the output structure hasn't changed, only the help text.

  * [bitcoin#22818](bitcoin#22818) (backported in [dash#6214](dashpay#6214)) activated many softforks at height 1, which rendered the block generation calls in `wallet_signrawtransactionwithwallet.py` ([source](https://github.com/dashpay/dash/blob/3e6e8f57c45fb24107816a10854ed8f349c4aa20/test/functional/wallet_signrawtransactionwithwallet.py#L132-L133)) no longer necessary, additionally [bitcoin#22550](bitcoin#22550) (backported in [dash#6189](dashpay#6189)) added assertions to validate CSV/CLTV activation that were not included in the backport.
    This has since been resolved, replacing the `generate` calls with assertions.

    * The locktime height was dropped from `1000` to `100` (matching with upstream, [source](https://github.com/bitcoin/bitcoin/blob/24.x/test/functional/wallet_signrawtransactionwithwallet.py#L219)) as we are no longer generating enough blocks to reach that height anymore.

  * `makeseeds.py` is currently mangled because [dash#6666](dashpay#6666) updated the script to look for the `core_p2p` purpose even though purpose code support was spun-off into [dash#6674](dashpay#6674). This meant, in between [dash#6666](dashpay#6666) and [dash#6674](dashpay#6674 merger, `makeseeds.py` was broken.

    This necessitated a fix that was introduced as e714c06 ([dash#6858](dashpay#6858)) but meant that after  [dash#6674](dashpay#6674), `makeseeds.py` is broken again. This has been resolved as part of this PR.

  * macOS users who want to utilise the `develop` container ([source](https://github.com/dashpay/dash/blob/b82450aa7dce2307b13e6fe282f7c4f84b48cbbf/contrib/containers/develop/Dockerfile)) need to set the `USER_ID` and `GROUP_ID` `args` to avoid permissions issues. This creates a slight problem as the GID for `staff` is 20 on macOS but Linux uses GID 20 for `dialout`, so creating a group with GID 20 will _fail_.

    As we only care about the GID, it is safe to skip the group creation step and assign the user to the group anyways as we do it by GID and _not_ name (the user will be presented as a member of the `dialout` group). This should resolve issues for macOS users trying to build the `develop` container.

  ## 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 **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 39e847c
  knst:
    utACK 39e847c
  UdjinM6:
    utACK 39e847c

Tree-SHA512: 0ecb8bb930d54d4b9024a6e94a2fe3b49c7334666e5f67d38920ac58f54332fb8b4f62bd883bd949969d60b6b5183c7a20a3593f24c2e6bbbb0de143b3de8fe1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

RPC Some notable changes to RPC params/behaviour/descriptions

Projects

None yet

Milestone

20

Development

Successfully merging this pull request may close these issues.

6 participants

@knst @UdjinM6 @PastaPastaPasta @fanquake @jonasschnelli @laanwj