Skip to content

chore: Bump MIN_MASTERNODE_PROTO_VERSION for v19 - #5223

Merged
PastaPastaPasta merged 1 commit into
dashpay:developfrom
ogabrielides:v19_mn_min_protocol_version_bump
Feb 28, 2023
Merged

chore: Bump MIN_MASTERNODE_PROTO_VERSION for v19#5223
PastaPastaPasta merged 1 commit into
dashpay:developfrom
ogabrielides:v19_mn_min_protocol_version_bump

Conversation

@ogabrielides

@ogabrielides ogabrielides commented Feb 28, 2023 •
edited
Loading

Copy link
Copy Markdown

Issue being fixed or feature implemented

What was done?

MIN_MASTERNODE_PROTO_VERSION was bumped to match latest PROTOCOL_VERSION.

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@ogabrielides ogabrielides added this to the 19 milestone Feb 28, 2023
PastaPastaPasta
PastaPastaPasta approved these changes Feb 28, 2023
View reviewed changes

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta
PastaPastaPasta merged commit 1a810ca into dashpay:develop Feb 28, 2023
@PastaPastaPasta

PastaPastaPasta commented Feb 28, 2023

Copy link
Copy Markdown
Member

@UdjinM6 please review when you can.

@ogabrielides
ogabrielides deleted the v19_mn_min_protocol_version_bump branch February 28, 2023 17:20
UdjinM6
UdjinM6 reviewed Feb 28, 2023
View reviewed changes

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

makes sense assuming we want to get rid of (most of) pre-v19 MNs before v19 hf

UdjinM6
UdjinM6 reviewed Feb 28, 2023
View reviewed changes

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

post-merge utACK

@PastaPastaPasta PastaPastaPasta mentioned this pull request Sep 22, 2026
Merged
2 of 3 tasks
PastaPastaPasta added a commit that referenced this pull request Sep 22, 2026
@PastaPastaPasta
Merge #7724: chore: bump MIN_MASTERNODE_PROTO_VERSION to 70242 for v24
cd4ef90 
f764147 chore: bump MIN_MASTERNODE_PROTO_VERSION to 70242 for v24 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  For v24, `MIN_MASTERNODE_PROTO_VERSION` needs to match the current `PROTOCOL_VERSION`. This follows the bump done for every major release (#5223 for v19, #6458 for v22, #7138 for 23.1). On `develop` the constant is still 70240 while `PROTOCOL_VERSION` is 70242. Both values were checked on `develop` at e5de15a.

  v24.0.0-rc.1 will be tagged on `develop`, so the bump has to land there first. It is split from the version / testnet-params PR on purpose. The operator-facing effect (see Breaking Changes) differs from a version-string or chain-params change, so it is easier to review, and to revert if it has to be, as its own PR.

  v24 masternodes need DKG peers at 70242 or newer because the two protocol versions added in this cycle are both v24 features:

  - **70241 `COINJOIN_REBALANCE_VERSION`** (#7052): CoinJoin denomination promotion/demotion (rebalance) sessions. `dsa` gains a version-gated flags field, and rebalance DSTXes are withheld from peers below 70241 (`CanAnnounceDstxTo`). Older peers would drop those DSTXes and penalize the relayer. Masternodes are the CoinJoin servers, so a pre-70241 masternode cannot host rebalance sessions after V24 activates.
  - **70242 `ASSET_UNLOCK_INV_VERSION`** (#7639): the `MSG_ASSET_UNLOCK` inventory type. Version 2 asset unlocks are announced by instance hash, so re-signed instances of one withdrawal, which share a txid, still propagate. Masternodes sign those re-issued withdrawals and InstantSend-lock them by withdrawal index. A masternode that only understands txid-based announcements does not reliably see every re-signed instance.

  ## What was done?

  - `src/version.h`: `MIN_MASTERNODE_PROTO_VERSION` 70240 → 70242 (= `PROTOCOL_VERSION`).
  - `test/functional/feature_llmq_simplepose.py`: its local copy `MIN_MASTERNODE_PROTO_VERSION = 70238` → 70242.

  ### Consumers of `MIN_MASTERNODE_PROTO_VERSION`

  A grep over `src/` and `test/` finds exactly one C++ consumer and one test mirror:

  1. **`src/active/dkgsession.cpp` `ActiveDKGSession::VerifyConnectionAndMinProtoVersions()`**. This runs at the end of the DKG contribution phase on every participating masternode. For each other quorum member with a live MNAUTH-verified connection, it compares the peer's `nVersion` (the version the peer advertised; on non-mainnet networks this can be overridden with `-pushversion`) against the constant. If the peer's version is lower, the member is marked `badConnection`, and in the complaint phase this node votes it into `badMembers`. Once a member collects `dkgBadVotesThreshold` such votes (2 for the small test LLMQs, 7 for `llmq_test_dip0024`/`llmq_devnet*`, 40 for `llmq_50_60`, 48 for `llmq_60_75`, 300 for `llmq_400_*`), every honest member marks it `bad`. It is then left out of `validMembers` in the final commitment. When that commitment is mined, `HandleQuorumCommitment()` in `src/evo/specialtxman.cpp` applies `PoSePunish(CalcPenalty(66))` to the excluded member. If this repeats across DKG sessions, the masternode ends up PoSe-banned.
     Members that are not connected at all are not affected by this constant; they go through the separate `SPORK_21` all-connected rule.
  2. **`test/functional/feature_llmq_simplepose.py`** mirrors the constant (details under testing).

  No other code paths use it: net_processing, `masternode/`, `llmq/` signing, sync, InstantSend, ChainLocks, and governance all have their own version constants or none. `MIN_PEER_PROTO_VERSION` (70221), which controls general peer disconnects, is unchanged.

  ### When does this take effect?

  It does **not** depend on `DEPLOYMENT_V24` or any other deployment, and not on the network either. The check is gated only on `SPORK_23_QUORUM_POSE` through `IsQuorumPoseEnabled()`: spork value 0 enables it for all LLMQ types, value 1 enables it for all types except `llmq_100_67`, `llmq_400_60` and `llmq_400_85`, and any other value disables it. So as soon as a masternode runs this code and SPORK_23 is on for the LLMQ type involved, it votes against DKG members that advertise less than 70242. It does not wait for V24 activation. Masternodes still running 23.1.x (70240) become bad DKG members, and eventually PoSe-banned, once enough upgraded quorum members reach `dkgBadVotesThreshold` in a session. How much this matters depends on how quickly operators upgrade on testnet and mainnet. This PR does not change the gating.

  ## How Has This Been Tested?

  `feature_llmq_simplepose.py` was reviewed along with the constant. The `force_old_mn_proto` case restarts one masternode with `-pushversion={MIN_MASTERNODE_PROTO_VERSION - 1}`. With SPORK_23 on, it expects that masternode to be PoSe-punished and banned (`test_banning`). With `--disable-spork23` it expects no punishment (`test_no_banning`). With the stale 70238 the test pushed 70237, which is below both the old (70240) and the new (70242) minimum. The case still passed, but it no longer checked the actual threshold, and it would keep passing if the bump were reverted. With 70242 the test pushes 70241, which is valid under the old minimum. The case now passes only because of this bump, so it directly covers the new value. No other test in `test/functional/` hardcodes a protocol version tied to this constant. `test_framework/p2p.py`'s `P2P_VERSION = 70242` is already current, and `p2p_sendtxrcncl.py`'s 70234 is unrelated.

  Tested on macOS arm64 (depends build, `--enable-debug --enable-werror --without-gui`). What ran:

  - `make`: success.
  - `src/test/test_dash --run_test=net_tests`: no errors.
  - `test/functional/test_runner.py feature_llmq_simplepose.py "feature_llmq_simplepose.py --disable-spork23" feature_llmq_dkgerrors.py feature_llmq_signing.py`: all passed (`feature_llmq_simplepose.py`, `feature_llmq_simplepose.py --disable-spork23`, `feature_llmq_dkgerrors.py`, `feature_llmq_signing.py`, `feature_llmq_signing.py --spork21`).

  No wider unit or functional suites were run.

  ## Breaking Changes

  Yes, for masternode operators. Suggested release-notes text:

  > **Minimum masternode protocol version raised to 70242.** v24 masternodes treat quorum members that advertise a protocol version below 70242 (any release before v24, including 23.1.x) as bad DKG participants whenever `SPORK_23_QUORUM_POSE` is active. This applies as soon as v24 masternodes are running and does not wait for the v24 hard fork. Masternodes that have not upgraded will be excluded from the quorums they are selected for and PoSe-punished for each missed DKG, leading to a PoSe ban. Operators should upgrade to v24 promptly.

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

  🤖 Generated with [Claude Code](https://claude.com/claude-code)

Top commit has no ACKs.

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

Reviewers

@UdjinM6 UdjinM6 UdjinM6 left review comments
@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

19

Development

Successfully merging this pull request may close these issues.

3 participants

@ogabrielides @PastaPastaPasta @UdjinM6