fix: correct masternode operator key BLS scheme handling (desync + cross-scheme uniqueness)

[UdjinM6]

Issue being fixed or feature implemented

Two independent bugs in how a masternode's operator BLS key relates to its state
version. Both exist on develop today and are reproduced by test; neither is
introduced by any open PR.

(A) Post-v24 operator-key scheme desync → consensus split.
CDeterministicMNState derives its operator key's serialization from nVersion
(CBLSLazyPublicKeyVersionWrapper(key, nVersion == LegacyBLS)). But after v24 a
masternode's nVersion could rise from LegacyBLS (v1) to BasicBLS (v2) while the
stored key kept the legacy scheme flag:

• ProUpServTx carries no operator key, yet set newState->nVersion from the
  payload.
• ProUpRegTx re-submitting the same key basic-encoded left operator_changed
  false, because CBLSLazyPublicKey::operator== compares the public key and ignores
  the scheme, so the re-encode was skipped while the version still rose.

Once a state is nVersion == 2 with a legacy-flagged key, a list rebuilt from block
diffs and the same list reloaded from disk hash that key differently, so
mnUniquePropertyMap diverges. Nodes then disagree on HasUniqueProperty() and
therefore on bad-protx-dup-key — a chain split determined purely by restart
history. update_service is the routine operation that plants this: any legacy
masternode updating its service address after v24 would trigger it, no attacker
required.

(B) Operator-key uniqueness enforced per encoding, not per key (live today).
mnUniquePropertyMap is keyed by GetUniquePropertyHash(), which serializes its
argument, and a BLS key serializes differently under the two schemes, so
H(K, legacy) != H(K, basic). CheckProRegTx's duplicate check consults that map,
so it does not see an operator key an existing masternode holds under the other
encoding. A ProRegTx never proves ownership of the operator key, so anyone can
re-register an existing masternode's operator public key for the price of a
collateral. This is exploitable on mainnet now; it is closed here for post-v24.

What was done?

The approach is migrate, don't force rotation (adopting the maintainer's
preferred model from #7472), while keeping the cross-scheme uniqueness guards that
make the re-encode safe. Every new consensus rule is gated on DEPLOYMENT_V24.

A legacy masternode keeps the same operator private key across the migration; only
the serialized encoding of the public key changes. On a version bump,
SetStateVersion() re-encodes the stored key to the scheme its version implies
(Set(Get(), …), not SetLegacy(), so the cached serialization actually changes),
and UpdateUniqueProperty() re-keys the scheme-dependent unique-property map when
the encoding changes (it now compares GetUniquePropertyHash() rather than the
scheme-blind operator==).

Re-encoding is only safe if the target slot is free: UpdateMN() reports a duplicate
by throwing, and that throw escapes BlockAssembler::CreateNewBlock, stalling block
production. Because bug B leaves the per-encoding registration hole open, a squatter
can already hold the key under the other encoding — so the migration is guarded at
every layer, and only when the key actually changes or the version crosses the
legacy→basic boundary (a grandfathered cross-scheme pair's non-migrating routine
update is not blocked).

The unique-property map is deliberately not made canonical. It is derived, not
serialized: CDeterministicMNList::Unserialize clears it and rebuilds via AddMN. A
canonical hash would apply retroactively to all history and, if any cross-scheme
duplicate pair already exists, make AddMN throw and nodes fail to sync past it.
Cross-scheme pairs created before activation are therefore tolerated (nothing
rehashes the map) and remain upgradable.

The change is split into two source commits, a test commit, and a release-notes
commit; each source commit builds on its own.

Commit 1 — enforce uniqueness across schemes. A helper
HasOperatorKeyUnderAnyScheme probes both encodings (two O(1) lookups, not a scan),
wired in at every point a key can be claimed:

• CheckProRegTx and CheckProUpRegTx, against the previous block's list.
• RebuildListFromBlock, against the list as rebuilt so far (same-block pairs).
• AcceptToMemoryPool. This probe is deliberately not v24-gated: block assembly
  does not revalidate special transactions cumulatively, so a pair admitted before
  activation is never evicted and would poison every template afterwards. Keeping the
  pair out of the mempool is what actually closes that, and mempool policy is allowed
  to be stricter than consensus (a node rejecting the second transaction still accepts
  a block containing it, so no chain can split over it).

The registrar probes run only when the operator key is actually changing: an update
that keeps its own key cannot create a duplicate, and probing it anyway would let a
pre-activation cross-scheme pair permanently block the affected masternode's
registrar updates.

Commit 2 — allow legacy→basic migration without key rotation. Handles bug A by
migrating in place rather than forcing a rotation (adopting #7472's re-encode model):

• SetStateVersion() re-encodes the stored operator key to the scheme its version
  implies (Set(Get(), …), not SetLegacy(), so the cached serialization actually
  changes), keeping the invariant that a stored key's encoding always matches its
  nVersion; UpdateUniqueProperty() re-keys the scheme-dependent map on an encoding
  change (comparing GetUniquePropertyHash() rather than the scheme-blind
  operator==).
• The migration is guarded against collisions via HasOperatorKeyUnderAnyScheme in
  CheckProUpServTx, CheckProUpRegTx and RebuildListFromBlock — the last against
  the list as rebuilt so far, since per-transaction checks run against pindexPrev
  and are blind to an earlier transaction in the same block — so a re-encode that
  would collide with another masternode's key cannot throw out of block assembly. The
  guard fires only when the operator key changes or the version crosses the
  legacy→basic boundary (a shared IsSchemeMigration predicate), so a grandfathered
  cross-scheme pair's non-migrating update is not blocked.
• CDeterministicMNStateDiff now compares the scheme-dependent hash for the operator
  key field. Its comparison used operator== (scheme-blind), so a same-key migration
  produced a diff that omitted the key — a node reconstructing the list from evoDB
  diffs kept the old encoding while an online-built list had the new one, a
  reconstruction split that full-snapshot serialization does not reveal.
• RPC (rpc/evo.cpp): protx update_service / protx update_registrar build a
  BasicBLS migration payload for a legacy masternode (keeping the same key, or
  supplying a new one), instead of erroring or forcing a rotation. The legacy-BLS RPC
  variants keep the masternode on the legacy scheme.

Commit 3 — tests. Unit tests in evo_deterministicmns_tests.cpp and the
functional-test extension in feature_dip3_v19.py covering both fixes (enumerated
below).

Commit 4 — release notes. doc/release-notes-7473.md documenting the post-v24
per-key uniqueness enforcement and the in-place migration behavior of
protx update_service / protx update_registrar.

How Has This Been Tested?

Built with --enable-werror; the full unit suite (./src/test/test_dash), the
touched functional test, and lints all pass. The two source commits build on their
own; the test commit adds the coverage below, and the full evo_dip3_activation_tests
suite passes.

Unit tests (src/test/evo_deterministicmns_tests.cpp) — each rejection test was
watched fail first (by flipping the v24 activation height):

• proupserv_migrates_legacy, proupreg_migrates_legacy_same_key — the happy-path
  migrations: a legacy masternode raises its version keeping the same operator key,
  the stored key re-encodes to basic, and the masternode is not PoSe-banned.
• migration_rejected_when_key_squatted — a squatter already holds the key under the
  other encoding; the migration is rejected bad-protx-dup-key at the per-tx layer
  rather than throwing.
• same_mn_same_block_migration_consistent,
  same_mn_same_block_version_crossing_key_rotation — same-masternode transitions
  within one block reconstruct identically.
• same_block_cross_scheme_key_pair_rejected, mempool_rejects_cross_scheme_key_race,
  pre_v24_cross_scheme_pair_cannot_become_resident — the same-block, mempool, and
  activation-boundary pairings that motivated the rebuild-time and ungated mempool
  probes.
• proregtx_rejects_cross_scheme_key_reuse, proupreg_rejects_cross_scheme_key_reuse
  — bug B at the registration and registrar paths, both directions, with fresh-key and
  self-key non-false-positive cases.
• statediff_captures_operator_key_reencoding — the evoDB-diff reconstruction path:
  a same-key migration must emit the re-encoded key in the diff.
• has_operator_key_under_any_scheme — the two-encoding lookup helper.
• stale_special_tx_does_not_poison_template — a resident-but-invalid special tx
  seeded via addUnchecked; without the package recheck CreateNewBlock throws.
• pre_v24_behaviour_unchanged — the non-retroactivity guard: asserts the new rules
  are inert before v24, so nothing on live mainnet/testnet changes.

Functional test (test/functional/feature_dip3_v19.py): extended to activate v24 and
assert that a legacy masternode's update_service migrates it to the basic scheme in
place (v2 payload, state.version == 2, same operator key re-encoded to a different
hex, PoSeBanHeight == -1), and that the list reloads from disk identically after the
migration.

Breaking Changes

All consensus rules here are gated on DEPLOYMENT_V24, which is NEVER_ACTIVE on
mainnet and testnet, so there is no consensus change on any live network;
pre_v24_behaviour_unchanged asserts this. After v24 activates:

• A LegacyBLS masternode migrates to the basic scheme in place on its next version
  bump (update_service or update_registrar), keeping the same operator key; the
  stored public key is re-encoded legacy→basic. No forced key rotation, no PoSe ban.
• Registering or updating to an operator public key already held by another masternode
  is rejected regardless of which BLS encoding either side uses, and a migration that
  would collide with such a key is rejected bad-protx-dup-key rather than stalling
  block assembly.

The AcceptToMemoryPool cross-scheme check is mempool policy (ungated) rather than
consensus, so a node may reject a second in-flight transaction that another node's
mempool accepted; both still accept a block containing it.

Not addressed here, and worth stating: cross-scheme duplicate pairs created before
activation are tolerated, not resolved; and bug B is only closed post-v24, so the
registration hole remains open on mainnet until v24 activates.

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 (release notes)
• I have assigned this pull request to a milestone

[github-actions]

Potential PR merge conflicts

This is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order.

If this PR merges first

These open PRs will likely need a rebase:

• #7302: fix!: use the same version along all protx special transactions Changed files: src/evo/dmnstate.h, src/evo/specialtxman.cpp, src/rpc/evo.cpp, src/test/evo_deterministicmns_tests.cpp, src/validation.cpp, test/functional/feature_dip3_v19.py.
• #7437: feat!: implement Decentralized Masternode Shares DIP Changed files: src/evo/dmnstate.h, src/evo/specialtxman.cpp, src/rpc/evo.cpp, src/txmempool.cpp, src/validation.cpp.
• #7472: fix: proper migration for legacy bls keys to the protx v>=2 Changed files: src/evo/deterministicmns.h, src/evo/dmnstate.h, src/evo/specialtxman.cpp, src/rpc/evo.cpp, src/test/evo_deterministicmns_tests.cpp, src/validation.cpp, test/functional/feature_dip3_v19.py.

[thepastaclaw]

⛔ Blockers found — Sonnet deferred (commit 3d2d102)
Canonical validated blockers: 1

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8b1e5fbc0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Reject cross-scheme conflicts within packages

In the package path this only checks conflicts against transactions that are already resident in the mempool; AcceptMultipleTransactions runs PreChecks for every package member before any of them are finalized into the mempool, so two low-fee ProRegTx parents that claim the same operator key under opposite encodings can both pass here and then be submitted together. After that, block assembly will see neither key at the tip, select both, and BuildNewListFromBlock will reject the template with bad-protx-dup-key, recreating the miner-stall this policy check is meant to prevent for package submissions.

Useful? React with 👍 / 👎.

[UdjinM6]

Good catch, and the analysis is correct: existsProviderTxCrossSchemeConflict only inspects
mapTx, so within a single AcceptMultipleTransactions package two members can each pass
PreChecks against a pool that does not yet contain the other, both get finalized, and then stall
template construction — the same failure mode the ungated single-tx policy is meant to prevent.

To be precise about what this PR does and doesn't do here:

• The package-admission blind spot itself predates this PR. existsProviderTxConflict immediately
  above (validation.cpp:991) is equally package-blind — it also only sees mapTx, so two package
  members claiming the same-encoding duplicate operator key (or duplicate owner/voting key, or
  netInfo entry) already slip through today for the same reason.
• What this PR adds is a new conflict category — cross-scheme operator-key duplicates — that
  inherits that same blindness. So within a package the new policy can be evaded, exactly as the
  existing per-encoding checks already can. The mechanism is shared; the category is new.

That said, it is not reachable outside regtest. AcceptMultipleTransactions is reached only via
ProcessNewPackage, which has exactly two callers, both RPC:

• testmempoolaccept (rpc/mempool.cpp:184) passes test_accept=true, so it never inserts into the
  mempool and cannot create the resident pair this finding needs.
• submitpackage (rpc/mempool.cpp:841) is the only one that actually submits, and it is -regtest
  only (if (!Params().IsMockableChain()) throw "submitpackage is for regression testing (-regtest mode) only").

There is no p2p package-relay path (net_processing.cpp references neither ProcessNewPackage nor
AcceptMultipleTransactions), so a package cannot be assembled by a peer on any real network.

A proper fix is package-local key-claim tracking inside AcceptMultipleTransactions, and to be
consistent it should cover the existing per-encoding provider conflicts too, not just the new
cross-scheme one. That is a broader mempool/package-policy change that reaches beyond this PR's
scope (v24 operator-key scheme handling), so given the regtest-only reachability I'd prefer to
track it as a follow-up covering both the old and new checks rather than fold it into this change.

[coderabbitai]

Walkthrough

The change adds cross-scheme BLS operator-key detection to deterministic masternode lists, consensus validation, and mempool admission. V24 rules reject duplicate keys across encodings and support legacy-BLS upgrades without key rotation. Block assembly revalidates special transactions against current chain state. RPC handling preserves legacy transition behavior. Unit and functional tests cover activation boundaries, duplicate races, stale transactions, state diffs, and list consistency.

Sequence Diagram(s)

sequenceDiagram
  participant MemPoolAccept
  participant CTxMemPool
  participant BlockAssembler
  participant SpecialTxManager
  MemPoolAccept->>CTxMemPool: Check cross-scheme provider conflict
  CTxMemPool-->>MemPoolAccept: Accept or reject transaction
  BlockAssembler->>SpecialTxManager: Revalidate special transaction
  SpecialTxManager-->>BlockAssembler: Include or exclude transaction

Loading

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• dashpay/dash#7212: Also modifies the block assembler’s special-transaction package validation path.
• dashpay/dash#7460: Adjusts shared v24 activation fixtures used by deterministic masternode tests.
• dashpay/dash#7472: Modifies related legacy-to-basic operator-key re-encoding and uniqueness handling.

Suggested reviewers: kwvg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.36% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main fix: BLS scheme handling for operator keys, including desync and cross-scheme uniqueness.
Description check	✅ Passed	The description is detailed and directly matches the implemented fixes, tests, and v24-gated behavior.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/evo_deterministicmns_tests.cpp`:
- Around line 1840-1867: Strengthen the transaction assertions around
ProcessTransaction so tx_b is rejected specifically with the protx-dup reason,
while confirming tx_a remains in the mempool after the attempted conflicting
submission. After v24 activation, inspect the block template produced by
make_template and assert that tx_a is selected, preserving the existing
template-build checks.

In `@src/txmempool.h`:
- Around line 830-840: Update the documentation for
existsProviderTxCrossSchemeConflict() to reflect that its caller,
MemPoolAccept::PreChecks(), invokes it unconditionally. Remove the requirement
that callers gate it on a deployment, and describe the intended
activation-boundary behavior while preserving the explanation of cross-scheme
key comparison.

In `@src/validation.cpp`:
- Around line 995-1008: Update AcceptMultipleTransactions() to track
operator-key claims across all staged package members, normalizing BLS encodings
so opposite schemes conflict; reject a transaction when its claim conflicts with
an earlier staged member before finalizing or inserting the package, while
preserving the existing mempool conflict check for transactions already in
m_pool.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 81458661-845d-45e2-a112-e73e1f5de4c7

📥 Commits

Reviewing files that changed from the base of the PR and between ca2912d and b8b1e5f.

📒 Files selected for processing (10)

• src/evo/deterministicmns.h
• src/evo/specialtxman.cpp
• src/node/miner.cpp
• src/node/miner.h
• src/rpc/evo.cpp
• src/test/evo_deterministicmns_tests.cpp
• src/txmempool.cpp
• src/txmempool.h
• src/validation.cpp
• test/functional/feature_dip3_v19.py

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9366bffff1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[thepastaclaw]

Preliminary review — Codex only

The exact checkout matches head 9366bff, and the commit history evidence is accurate. The package-admission finding is confirmed: package members are prechecked before provider indexes are updated, then inserted without rerunning the new cross-scheme conflict check, allowing a pair that makes CreateNewBlock fail cumulative block validation.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (failed), gpt-5.6-sol — dash-core-commit-history (failed), gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/validation.cpp`:
- [BLOCKING] src/validation.cpp:1006-1008: Package submission bypasses the cross-scheme mempool check
  AcceptMultipleTransactions runs PreChecks for every package member before inserting any member into the real mempool. PackageAddTransaction only exposes package outputs through the temporary coins view; it does not update mapProTxBlsPubKeyHashes, so two package members claiming the same operator key under different encodings cannot detect each other here. AcceptPackage can reach this path when a parent fails individual mempool-fee policy and its CPFP child initially has missing inputs. SubmitPackage then finalizes the validated members sequentially without rerunning provider-conflict checks. The currently exposed submitting RPC is restricted to mockable chains, but the package-admission API still violates the invariant introduced by this PR: both transactions become resident, TestPackageTransactions validates each independently against the chain tip, both are selected as an ancestor package, and cumulative deterministic-list rebuilding rejects the second claim during TestBlockValidity, causing CreateNewBlock to throw. Validate provider conflicts cumulatively across the package, or rerun the checks before each Finalize call after earlier members have updated the mempool indexes.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/evo_deterministicmns_tests.cpp`:
- Around line 1858-1907: Update the exception handling around
RebuildListFromBlock so any thrown exception fails the regression test instead
of only populating thrown and skipping assertions. Keep the diagnostic message
if useful, but require successful completion before proceeding with the existing
rebuilt-state checks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e80fc521-562d-40d4-aed6-cd801bd50cea

📥 Commits

Reviewing files that changed from the base of the PR and between 9366bff and a693a86.

📒 Files selected for processing (11)

• src/evo/deterministicmns.h
• src/evo/dmnstate.h
• src/evo/specialtxman.cpp
• src/node/miner.cpp
• src/node/miner.h
• src/rpc/evo.cpp
• src/test/evo_deterministicmns_tests.cpp
• src/txmempool.cpp
• src/txmempool.h
• src/validation.cpp
• test/functional/feature_dip3_v19.py

🚧 Files skipped from review as they are similar to previous changes (5)

• src/txmempool.h
• src/node/miner.cpp
• src/node/miner.h
• src/validation.cpp
• src/txmempool.cpp

[thepastaclaw]

Preliminary review — Codex only

The prior blocking finding c83910ef32bd remains valid at a693a86: package members are prechecked against the unchanged real mempool, allowing cross-scheme operator-key conflicts within one package to bypass the new policy check. The commit-history suggestion is not actionable because the adjacent commits remain coherent, tested design iterations rather than a broken intermediate state.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (failed), gpt-5.6-sol — dash-core-commit-history (failed), gpt-5.6-sol — general (failed), gpt-5.6-sol — dash-core-commit-history (failed), gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/rpc/evo.cpp`:
- Around line 1259-1275: Add an explicit validation in the update-registrar
flow, before the existing dmn->pdmnState->nVersion == ProTxVersion::LegacyBLS
block, to reject use_legacy when the masternode version is BasicBLS or newer.
Return the established RPC validation error for this invalid request and leave
the existing legacy-key handling unchanged for LegacyBLS masternodes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 162df6bd-a361-4c18-aca8-9a0684a6da3e

📥 Commits

Reviewing files that changed from the base of the PR and between a693a86 and 3d2d102.

📒 Files selected for processing (12)

• doc/release-notes-7473.md
• src/evo/deterministicmns.h
• src/evo/dmnstate.h
• src/evo/specialtxman.cpp
• src/node/miner.cpp
• src/node/miner.h
• src/rpc/evo.cpp
• src/test/evo_deterministicmns_tests.cpp
• src/txmempool.cpp
• src/txmempool.h
• src/validation.cpp
• test/functional/feature_dip3_v19.py

🚧 Files skipped from review as they are similar to previous changes (10)

• src/node/miner.h
• src/node/miner.cpp
• src/evo/dmnstate.h
• src/validation.cpp
• src/txmempool.h
• src/evo/deterministicmns.h
• test/functional/feature_dip3_v19.py
• src/txmempool.cpp
• src/evo/specialtxman.cpp
• src/test/evo_deterministicmns_tests.cpp

[thepastaclaw]

Preliminary review — Codex only

Prior finding c83910ef32bd is STILL_VALID: package submission can admit two cross-scheme operator-key claims because all package members complete PreChecks before any member updates the real mempool provider indexes. The author's reachability reply correctly limits current non-test insertion to the regtest-only submitpackage RPC, but the defect remains in scope because this PR adds the shared conflict check to prevent resident transactions from poisoning block templates, and the reachable package path can still make CreateNewBlock throw. The CodeRabbit report is not actionable because it misreads the payload version selection and otherwise concerns pre-existing legacy-RPC behavior.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (failed), gpt-5.6-sol — dash-core-commit-history (failed), gpt-5.6-sol — general (failed), gpt-5.6-sol — dash-core-commit-history (failed), gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.