Skip to content

Dip0001-related adjustments (inc. constants) #1621

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
UdjinM6 merged 3 commits into from
Sep 14, 2017
Merged

Dip0001-related adjustments (inc. constants) #1621

UdjinM6 merged 3 commits into from
Sep 14, 2017

Conversation

@the-schnibble
Copy link

@the-schnibble the-schnibble commented Sep 13, 2017

No description provided.

UdjinM6
UdjinM6 reviewed Sep 13, 2017
View reviewed changes
src/validation.h Outdated
static const unsigned int BLOCKFILE_CHUNK_SIZE = 0x2000000; // 32 MiB
/** The pre-allocation chunk size for rev?????.dat files (since 0.8) */
static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
static const unsigned int UNDOFILE_CHUNK_SIZE = 0x200000; // 2 MiB
Copy link

@UdjinM6 UdjinM6 Sep 13, 2017

Choose a reason for hiding this comment

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

Are these 3 above actually DIP0001-related? IMO storing blocks on disk is kind of perpendicular to consensus rules.

Copy link
Author

@the-schnibble the-schnibble Sep 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Agree. We shouldn't scale storing blocks on disk. Thus MIN_DISK_SPACE_FOR_BLOCK_FILES should be > 941 MB

src/net.h
static const unsigned int MAX_ADDR_TO_SEND = 1000;
/** Maximum length of incoming protocol messages (no message over 2 MiB is currently acceptable). */
static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 2 * 1024 * 1024;
static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 3 * 1024 * 1024;
Copy link

@UdjinM6 UdjinM6 Sep 13, 2017

Choose a reason for hiding this comment

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

The comment above should be fixed as well.

Copy link
Author

@the-schnibble the-schnibble Sep 13, 2017

Choose a reason for hiding this comment

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

fixed

@UdjinM6 UdjinM6 added this to the 12.2 milestone Sep 13, 2017
@UdjinM6 UdjinM6 changed the title Dip0001: block size related constants adjusted Dip0001-related adjustments (inc. constants) Sep 13, 2017
UdjinM6
UdjinM6 requested changes Sep 13, 2017
View reviewed changes
src/validation.cpp Outdated
ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos)
{
LOCK(cs_main);
AssertLockHeld(cs_main);
Copy link

@UdjinM6 UdjinM6 Sep 13, 2017

Choose a reason for hiding this comment

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

While I agree that AssertLockHeld should be enough here, it feels rather unrelated to DIP0001 specifically, so I think its better to keep it as is for now. Revert this pls.

Copy link
Author

@the-schnibble the-schnibble Sep 14, 2017

Choose a reason for hiding this comment

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

fixed (moved this change to a separate PR)

UdjinM6
UdjinM6 approved these changes Sep 14, 2017
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.

utACK

@UdjinM6 UdjinM6 merged commit 6a6b31b into dashpay:v0.12.2.x Sep 14, 2017
andvgal added a commit to energicryptocurrency/gen2-energi that referenced this pull request Oct 4, 2019
@andvgal
CHANGED: dashpay#1621 - extend migration snapshot
5174dee
@kwvg kwvg mentioned this pull request Oct 21, 2025
Merged
5 tasks
PastaPastaPasta added a commit that referenced this pull request Nov 4, 2025
@PastaPastaPasta
Merge #6901: backport: merge bitcoin#23345, bitcoin#24410, bitcoin#15936
e8c71ce 
, bitcoin#25748, bitcoin#25863, bitcoin#24051, bitcoin#25315, bitcoin#26624, bitcoin#26894, bitcoin#26673, bitcoin#24462, bitcoin#25815, bitcoin#22949, bitcoin#26723, bitcoin#23395, bitcoin#27016, bitcoin#27189, bitcoin#27317 (auxiliary backports: part 28)

2eef89c merge bitcoin#27317: Check that the timestamp string is non-empty to avoid undefined behavior (Kittywhiskers Van Gogh)
de6c0ae merge bitcoin#27189: Use steady clock in SeedStrengthen, FindBestImplementation, FlushStateToDisk (Kittywhiskers Van Gogh)
5bed6ea merge bitcoin#27016: require miniupnpc API version 17 or later (Kittywhiskers Van Gogh)
51a9f24 merge bitcoin#23395: Add -shutdownnotify option (Kittywhiskers Van Gogh)
af0e14d merge bitcoin#26723: call `keypoolrefill` with priv key disabled should throw an error (Kittywhiskers Van Gogh)
b91c5b9 merge bitcoin#22949: Round up fee calculation to avoid a lower than expected feerate (Kittywhiskers Van Gogh)
05fb900 merge bitcoin#25815: Use existing {Chainstate,Block}Man (Kittywhiskers Van Gogh)
945798c merge bitcoin#24462: For descriptor pubkey parse errors, include context information (Kittywhiskers Van Gogh)
033e060 merge bitcoin#26673: Remove confusing getBool method (Kittywhiskers Van Gogh)
37ca4b4 merge bitcoin#26894: Remove redundant key_to_p2pkh call (Kittywhiskers Van Gogh)
2b3f925 merge bitcoin#26624: Rename local variable to distinguish it from type alias (Kittywhiskers Van Gogh)
3a58533 merge bitcoin#25315: Add warning on first startup if free disk space is less than necessary (Kittywhiskers Van Gogh)
adeebb0 merge bitcoin#24051: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ (Kittywhiskers Van Gogh)
55a474f merge bitcoin#25863: remove unused norm_prv parameter in `descriptor_tests.cpp` (Kittywhiskers Van Gogh)
65299a0 merge bitcoin#25748: Avoid copies in FlatSigningProvider Merge (Kittywhiskers Van Gogh)
5d69d18 merge bitcoin#15936: Expose settings.json methods to GUI (Kittywhiskers Van Gogh)
bdf5e92 merge bitcoin#24410: Split hashing/index `GetUTXOStats` codepaths, decouple from `coinstatsindex` (Kittywhiskers Van Gogh)
43e2a19 merge bitcoin#23345: Drop unneeded dependencies for bitcoin-wallet tool (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * The minimum acceptable prune height in [bitcoin#25315](bitcoin#25315) is higher than upstream (`945` vs `550`) because minimum height is calculated using `MIN_DISK_SPACE_FOR_BLOCK_FILES` which was bumped in [dash#1621](#1621), this has been reflected in functional tests.

  * [bitcoin#27016](bitcoin#27016) raises the minimum miniupnpc version to v2.1. This is acceptable as depends already uses version v2.2.2 ([source](https://github.com/dashpay/dash/blob/f7dad69eab573c179060ff9eb1bbaccb317de6d3/depends/packages/miniupnpc.mk#L2)) and our minimum supported target based on glibc version (see description of [dash#6382](#6382)), Ubuntu 20.04 LTS ([source](https://launchpad.net/ubuntu/+source/miniupnpc/2.1.20190824-0ubuntu2)) and RHEL 9 ([source](https://rpmfind.net/linux/RPM/epel/9/x86_64/Packages/m/miniupnpc-2.2.4-2.el9.x86_64.html)) ship with the minimum required miniupnpc version or higher.

  * [bitcoin#24462](bitcoin#24462) includes changes that SegWit and Taproot-oriented that have been omitted from the backport.

  * In [bitcoin#22949](bitcoin#22949), `rpc_fundrawtransaction.py`'s `restart_node()` needs to be passed an empty set of `extra_args` to avoid the following test failure (see below)

    ```
    $ python3 ./test/functional/rpc_fundrawtransaction.py --descriptors
    2025-11-03T11:19:02.448000Z TestFramework (INFO): PRNG seed is: 4253671634984506297
    2025-11-03T11:19:02.448000Z TestFramework (INFO): Initializing test directory /var/folders/gt/rf6wpfx963x__7wg283kwnxc0000gp/T/dash_func_test_gylr91av
    2025-11-03T11:19:04.236000Z TestFramework (INFO): Connect nodes, set fees, generate blocks, and sync
    [...]
    2025-11-03T11:19:24.065000Z TestFramework (INFO): Test issue 22670 ApproximateBestSubset bug
    2025-11-03T11:20:02.261000Z TestFramework (ERROR): Unexpected exception caught during testing
    Traceback (most recent call last):
      File "/src/dash/test/functional/test_framework/test_framework.py", line 163, in main
        self.run_test()
      File "/src/dash/./test/functional/rpc_fundrawtransaction.py", line 134, in run_test
        self.test_22670()
      File "/src/dash/./test/functional/rpc_fundrawtransaction.py", line 1084, in test_22670
        self.restart_node(0)
      File "/src/dash/test/functional/test_framework/test_framework.py", line 697, in restart_node
        self.start_node(i, extra_args)
      File "/src/dash/test/functional/test_framework/test_framework.py", line 655, in start_node
        node.wait_for_rpc_connection()
      File "/src/dash/test/functional/test_framework/test_node.py", line 271, in wait_for_rpc_connection
        raise FailedToStartError(self._node_msg(
    test_framework.test_node.FailedToStartError: [node 0] dashd exited with status 1 during initialization. Error: Error loading default_wallet: You can't disable HD on an already existing HD wallet
    ************************
    ```

  * ~~To deal with frequent functional test failures due to `rpc_mempool_entry_fee_fields_deprecation.py` experiencing port conflicts ([build](https://github.com/dashpay/dash/actions/runs/19031966785/job/54350070090#step:6:1117), [build](https://github.com/dashpay/dash/actions/runs/19031966785/job/54350107927#step:6:1128)) retries will modify the port seed to try again with a different set of likely non-conflicting ports.~~ Superseded by [dash#6937](#6937).

  ## Breaking Changes

  Building with UPnP enabled requires miniupnpc API version 17 (raising the minimum version from v1.9 to v2.1).

  ## 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:
    utACK 2eef89c

Tree-SHA512: 8814ff6b01ac096107d0a78c89806ea1902653c0444db84c476698af4ff5cf1c7fc42b09fce4c3c62f16adce26f1e10cec15ca4c96078c58cdcce7b640a5163d
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

12.2

Development

Successfully merging this pull request may close these issues.

2 participants

@the-schnibble @UdjinM6