From 04d6f00cb845767ef1a5ec8ce45b35d28c9a8c53 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Thu, 15 Oct 2026 18:20:34 +0700 Subject: [PATCH 1/2] fix(drive-abci): keep masternode list diffs applied as v0 and v1 always have The Core RPC client is changing how it reads `protx listdiff` (dashpay/rust-dashcore#1179): it starts decoding an updated masternode's `operatorPayoutAddress`, and `DMNState::apply_diff` stops clearing `PoSeRevivedHeight` when a diff does not print it. Every protocol version so far (1-15, through update_state_masternode_list v0 and v1) processed its blocks the old way: operator payouts came only from full masternode states, and a revived masternode's next diff cleared its revived height. The stored masternode lists, the operator identities' TRANSFER keys and the validator sets of every processed block follow from that, so v0 and v1 keep it: - the operator payouts of updated masternodes are dropped right after the fetch, the only place a decoded diff enters Platform, so neither the stored list nor the identity updates see them; - every diff is applied with the revived height replaced by the diff's (`apply_diff_replacing_revived_height`, used by v0's early-return check, the state accessor only v0 uses, and v1's stored-form apply). Both are in place, without a new generation: with the current pin they change nothing, and after the bump v0 and v1 see the same decoded fields and make the same state transitions as before. The revived height is re-imposed rather than left to the client because in v0 whether a block takes the full path decides whether validator-set members are refreshed from the fields a diff prints, so it can reach the validator sets. One input does change with the bump: a malformed non-empty `operatorPayoutAddress` now fails the whole diff instead of being ignored. Core cannot print one: it prints the field only for a destination it extracts from the payout script, which ProUpServTx validation limits to P2PKH or P2SH. Tests, at protocol versions 13 and latest: - a diff that sets an operator payout, clears one, or carries one with a new operator key leaves the operator identities' keys and the stored payouts as they were, and the new operator's identity gets the stored payout. Before the strip it fails at 13 (the old TRANSFER key is disabled and a new one added); after it passes. - a revived evonode's payment-only diff, alone in its block, clears the revived height in both masternode lists, and a revive diff sets it. It passes with the current pin. With the pin bumped to #1179 it fails without the re-imposed clearing (`Some(900)` kept) and when only v0's early-return check applies diffs the client's way, and passes with this commit; the core_based_updates, platform_state and masternode drive-abci tests (222) pass there too. - unit tests of the helper: an unprinted revived height is cleared, a printed one is taken. Co-Authored-By: Claude Opus 5.5 --- .../update_masternode_list/mod.rs | 349 +++++++++++++++++- .../update_state_masternode_list/mod.rs | 15 + .../update_state_masternode_list/v0/mod.rs | 16 +- .../update_state_masternode_list/v1/mod.rs | 16 +- .../src/platform_types/masternode/mod.rs | 63 +++- .../platform_state/accessors.rs | 12 +- 6 files changed, 457 insertions(+), 14 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/mod.rs index 4292296c059..4124842361f 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/mod.rs @@ -95,9 +95,9 @@ mod test { use dpp::bls::SecretKey; use dpp::core_types::validator_set::v0::ValidatorSetV0; use dpp::dashcore::hashes::Hash; - use dpp::dashcore::{Network, ProTxHash, PubkeyHash, QuorumHash}; + use dpp::dashcore::{Network, ProTxHash, PubkeyHash, QuorumHash, Txid}; use dpp::dashcore_rpc::json::{ - DMNStateDiff, MasternodeListDiff, MasternodeListItem, MasternodeType, + DMNState, DMNStateDiff, MasternodeListDiff, MasternodeListItem, MasternodeType, }; use dpp::identifier::MasternodeIdentifiers; use dpp::identity::accessors::IdentityGettersV0; @@ -1116,6 +1116,351 @@ mod test { (Some(expected.clone()), Some(expected)) ); } + + /// A Core diff with no field the client decodes set: the fields a test's diff carries are + /// added on top of this. + fn empty_state_diff() -> DMNStateDiff { + serde_json::from_str("{}").expect("an empty Core state diff") + } + + /// A Regular masternode named by `id` with an operator payout and an operator reward. + fn masternode_with_operator_payout(id: u8) -> MasternodeListItem { + MasternodeListItem { + node_type: MasternodeType::Regular, + pro_tx_hash: ProTxHash::from_byte_array([id; 32]), + collateral_hash: Txid::from_byte_array([id ^ 0x40; 32]), + collateral_index: 0, + collateral_address: Some([id ^ 0x50; 20]), + operator_reward: 10.0, + state: DMNState { + service: SocketAddr::from(([192, 0, 2, id], 9999)), + registered_height: 1000, + pose_revived_height: None, + pose_ban_height: None, + revocation_reason: 0, + owner_address: Some([id ^ 0x10; 20]), + voting_address: [id ^ 0x20; 20], + payout_address: Some([id ^ 0x30; 20]), + payouts: None, + pub_key_operator: vec![id; 48], + operator_payout_address: Some([id; 20]), + platform_node_id: None, + #[allow(deprecated)] + legacy_platform_p2p_port: None, + #[allow(deprecated)] + legacy_platform_http_port: None, + addresses: None, + }, + } + } + + /// Processes a mock Core's full list of `masternodes` at Core height 1000 and then `diff` at + /// 1001 on a platform at `platform_version`, calling `before_diff` in between. Returns the + /// platform, with the updates committed, and its platform state after the diff. + fn masternode_list_after_diff( + platform_version: &PlatformVersion, + masternodes: Vec, + diff: MasternodeListDiff, + before_diff: impl FnOnce(&TempPlatform, &Transaction), + ) -> (TempPlatform, PlatformState) { + let mut platform = TestPlatformBuilder::new() + .with_config(PlatformConfig::default()) + .with_initial_protocol_version(platform_version.protocol_version) + .build_with_mock_rpc() + .set_genesis_state(); + let full_list = MasternodeListDiff { + base_height: 0, + block_height: 1000, + added_mns: masternodes, + removed_mns: vec![], + updated_mns: vec![], + }; + platform + .core_rpc + .expect_get_protx_diff_with_masternodes() + .returning(move |_base_block, block| { + Ok(if block == 1000 { + full_list.clone() + } else { + diff.clone() + }) + }); + + let transaction = platform.drive.grove.start_transaction(); + let mut platform_state = platform.state.load().as_ref().clone(); + platform + .update_masternode_list( + None, + &mut platform_state, + 1000, + true, + &BlockInfo { + height: 1, + core_height: 1000, + time_ms: 1, + ..Default::default() + }, + &transaction, + platform_version, + ) + .expect("expected to process the full masternode list"); + before_diff(&platform, &transaction); + + let mut block_platform_state = platform_state.clone(); + platform + .update_masternode_list( + Some(&platform_state), + &mut block_platform_state, + 1001, + false, + &BlockInfo { + height: 2, + core_height: 1001, + time_ms: 2, + ..Default::default() + }, + &transaction, + platform_version, + ) + .expect("expected to process the masternode list diff"); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit"); + (platform, block_platform_state) + } + + /// The keys of a masternode's operator identity, with their purpose, data and disabled state. + fn operator_identity_keys( + platform: &TempPlatform, + transaction: Option<&Transaction>, + pro_tx_hash: &ProTxHash, + pub_key_operator: &[u8], + platform_version: &PlatformVersion, + ) -> BTreeMap, bool)> { + platform + .drive + .fetch_full_identity( + Identifier::create_operator_identifier( + pro_tx_hash.as_byte_array(), + pub_key_operator, + ) + .to_buffer(), + transaction, + platform_version, + ) + .expect("expected to fetch an identity") + .expect("expected the operator identity") + .public_keys() + .iter() + .map(|(key_id, key)| { + ( + *key_id, + (key.purpose(), key.data().to_vec(), key.is_disabled()), + ) + }) + .collect() + } + + /// The masternode list generations so far (v0 and v1) processed Core diffs without their + /// operator payout: operator payouts reached Platform only in full masternode states, and the + /// stored list and the operator identities' keys follow from that. A diff that sets or clears + /// an operator payout must leave both as they were, whether the operator key stays or + /// changes. + #[test] + fn should_keep_operator_payouts_and_keys_when_a_diff_carries_an_operator_payout() { + for platform_version in [ + PlatformVersion::get(13).expect("expected protocol version 13"), + PlatformVersion::latest(), + ] { + let context = format!("protocol version {}", platform_version.protocol_version); + let payout_set = masternode_with_operator_payout(0x61); + let payout_cleared = masternode_with_operator_payout(0x62); + let new_operator = masternode_with_operator_payout(0x63); + let new_pub_key_operator = vec![0x73; 48]; + let diff = MasternodeListDiff { + base_height: 1000, + block_height: 1001, + added_mns: vec![], + removed_mns: vec![], + updated_mns: vec![ + ( + payout_set.pro_tx_hash, + DMNStateDiff { + operator_payout_address: Some(Some([0x81; 20])), + ..empty_state_diff() + }, + ), + ( + payout_cleared.pro_tx_hash, + DMNStateDiff { + operator_payout_address: Some(None), + ..empty_state_diff() + }, + ), + ( + new_operator.pro_tx_hash, + DMNStateDiff { + pub_key_operator: Some(new_pub_key_operator.clone()), + operator_payout_address: Some(Some([0x83; 20])), + ..empty_state_diff() + }, + ), + ], + }; + let unchanged_operators = [&payout_set, &payout_cleared]; + let mut keys_before = Vec::new(); + let (platform, state) = masternode_list_after_diff( + platform_version, + vec![ + payout_set.clone(), + payout_cleared.clone(), + new_operator.clone(), + ], + diff, + |platform, transaction| { + for masternode in unchanged_operators { + keys_before.push(operator_identity_keys( + platform, + Some(transaction), + &masternode.pro_tx_hash, + &masternode.state.pub_key_operator, + platform_version, + )); + } + }, + ); + + for (masternode, keys_before) in unchanged_operators.into_iter().zip(keys_before) { + assert_eq!( + operator_identity_keys( + &platform, + None, + &masternode.pro_tx_hash, + &masternode.state.pub_key_operator, + platform_version, + ), + keys_before, + "{context}: {}", + masternode.pro_tx_hash + ); + } + // The new operator's identity gets the stored payout, not the diff's. + let new_operator_transfer_keys = operator_identity_keys( + &platform, + None, + &new_operator.pro_tx_hash, + &new_pub_key_operator, + platform_version, + ) + .into_values() + .filter(|(purpose, _, _)| *purpose == Purpose::TRANSFER) + .collect::>(); + assert_eq!( + new_operator_transfer_keys, + [(Purpose::TRANSFER, vec![0x63; 20], false)], + "{context}" + ); + for masternode in [&payout_set, &payout_cleared, &new_operator] { + assert_eq!( + state.full_masternode_list()[&masternode.pro_tx_hash] + .state + .operator_payout_address, + masternode.state.operator_payout_address, + "{context}: {}", + masternode.pro_tx_hash + ); + } + } + } + + /// Core prints `PoSeRevivedHeight` only in a diff that spans the revive. The masternode list + /// generations so far (v0 and v1) always applied a diff with the revived height replaced by + /// the diff's, so a revived masternode's next diff clears it; their stored lists and + /// validator sets follow from that, and they keep it whatever the Core RPC client's own diff + /// application does. The payment-only diff of a revived evonode, alone in its block, is what + /// decides v0's early return and reaches the evonode list as well as the full list. + #[test] + fn should_clear_a_revived_height_a_diff_does_not_print() { + for platform_version in [ + PlatformVersion::get(13).expect("expected protocol version 13"), + PlatformVersion::latest(), + ] { + let context = format!("protocol version {}", platform_version.protocol_version); + let pose_heights = |state: &PlatformState, pro_tx_hash: &ProTxHash| { + [state.full_masternode_list(), state.hpmn_masternode_list()].map(|list| { + let state = &list[pro_tx_hash].state; + (state.pose_revived_height, state.pose_ban_height) + }) + }; + + let mut revived = masternode_with_operator_payout(0x64); + revived.node_type = MasternodeType::Evo; + revived.state.platform_node_id = Some([0x64; 20]); + #[allow(deprecated)] + { + revived.state.legacy_platform_p2p_port = Some(26656); + revived.state.legacy_platform_http_port = Some(443); + } + revived.state.pose_revived_height = Some(900); + let (_platform, state) = masternode_list_after_diff( + platform_version, + vec![revived.clone()], + MasternodeListDiff { + base_height: 1000, + block_height: 1001, + added_mns: vec![], + removed_mns: vec![], + updated_mns: vec![( + revived.pro_tx_hash, + DMNStateDiff { + last_paid_height: Some(1001), + ..empty_state_diff() + }, + )], + }, + |_, _| {}, + ); + assert_eq!( + pose_heights(&state, &revived.pro_tx_hash), + [(None, None); 2], + "{context}" + ); + + let mut reviving = masternode_with_operator_payout(0x65); + reviving.state.pose_ban_height = Some(950); + let (_platform, state) = masternode_list_after_diff( + platform_version, + vec![reviving.clone()], + MasternodeListDiff { + base_height: 1000, + block_height: 1001, + added_mns: vec![], + removed_mns: vec![], + updated_mns: vec![( + reviving.pro_tx_hash, + DMNStateDiff { + pose_revived_height: Some(1001), + pose_ban_height: Some(None), + ..empty_state_diff() + }, + )], + }, + |_, _| {}, + ); + let reviving_state = &state.full_masternode_list()[&reviving.pro_tx_hash].state; + assert_eq!( + ( + reviving_state.pose_revived_height, + reviving_state.pose_ban_height + ), + (Some(1001), None), + "{context}" + ); + } + } } #[cfg(test)] diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs index d0ad9c9e79e..f589173d750 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs @@ -2,3 +2,18 @@ mod v0; mod v1; // Private helpers are selected by the update_masternode_list generation. + +use dpp::dashcore_rpc::dashcore_rpc_json::MasternodeListDiff; + +/// The Core diff without the operator payouts of its updated masternodes. +/// +/// The generations calling this take operator payouts only from full masternode states +/// (`added_mns`), never from an updated masternode's diff: the stored list and the operator +/// identities' TRANSFER keys of every block they processed follow from that, so a diff's +/// operator payout must not reach them. +fn without_operator_payout_changes(mut masternode_diff: MasternodeListDiff) -> MasternodeListDiff { + for (_, state_diff) in &mut masternode_diff.updated_mns { + state_diff.operator_payout_address = None; + } + masternode_diff +} diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs index e5518c423d3..8bb0abbf774 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs @@ -1,5 +1,7 @@ +use super::without_operator_payout_changes; use crate::error::Error; use crate::execution::types::update_state_masternode_list_outcome; +use crate::platform_types::masternode::apply_diff_replacing_revived_height; use crate::platform_types::platform::Platform; use crate::platform_types::platform_state::PlatformState; use crate::platform_types::platform_state::PlatformStateV0Methods; @@ -98,7 +100,9 @@ where updated_mns.iter().any(|(pro_tx_hash, state_diff)| { masternode_list.get(pro_tx_hash).is_some_and(|item| { let mut updated = item.state.clone(); - updated.apply_diff(state_diff.clone()); + // Inert for every protocol version selecting this generation: it has always + // applied diffs with the revived height replaced by the diff's. + apply_diff_replacing_revived_height(&mut updated, state_diff.clone()); updated != item.state }) }) @@ -123,9 +127,13 @@ where Some(state_core_height) }; - let masternode_diff = self - .core_rpc - .get_protx_diff_with_masternodes(previous_core_height, core_block_height)?; + // Inert for every protocol version selecting this generation: it processed its blocks + // without updated masternodes' operator payouts, so a diff's must not reach the stored + // list or the identity updates. + let masternode_diff = without_operator_payout_changes( + self.core_rpc + .get_protx_diff_with_masternodes(previous_core_height, core_block_height)?, + ); let MasternodeListDiff { added_mns, diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v1/mod.rs index af7b64bbd84..a5cfdabbca7 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v1/mod.rs @@ -1,5 +1,7 @@ +use super::without_operator_payout_changes; use crate::error::Error; use crate::execution::types::update_state_masternode_list_outcome; +use crate::platform_types::masternode::apply_diff_replacing_revived_height; use crate::platform_types::masternode::v0::MasternodeStateV0; use crate::platform_types::platform::Platform; use crate::platform_types::platform_state::PlatformState; @@ -125,9 +127,13 @@ where Some(state_core_height) }; - let masternode_diff = self - .core_rpc - .get_protx_diff_with_masternodes(previous_core_height, core_block_height)?; + // Inert for every protocol version selecting this generation: it processed its blocks + // without updated masternodes' operator payouts, so a diff's must not reach the stored + // list or the identity updates. + let masternode_diff = without_operator_payout_changes( + self.core_rpc + .get_protx_diff_with_masternodes(previous_core_height, core_block_height)?, + ); let MasternodeListDiff { added_mns, @@ -264,7 +270,9 @@ fn in_stored_form(mut state: DMNState) -> DMNState { fn apply_diff_in_stored_form(state: &DMNState, diff: &DMNStateDiff) -> DMNState { let mut updated = legacy_stored_form(state.clone()); - updated.apply_diff(diff.clone()); + // Inert for every protocol version selecting this generation: it has always applied diffs + // with the revived height replaced by the diff's. + apply_diff_replacing_revived_height(&mut updated, diff.clone()); in_stored_form(updated) } diff --git a/packages/rs-drive-abci/src/platform_types/masternode/mod.rs b/packages/rs-drive-abci/src/platform_types/masternode/mod.rs index 0b2b94b0586..32a091d242d 100644 --- a/packages/rs-drive-abci/src/platform_types/masternode/mod.rs +++ b/packages/rs-drive-abci/src/platform_types/masternode/mod.rs @@ -2,7 +2,7 @@ use crate::error::execution::ExecutionError; use crate::error::Error; use crate::platform_types::masternode::v0::MasternodeV0; use bincode::{Decode, Encode}; -use dpp::dashcore_rpc::json::MasternodeListItem; +use dpp::dashcore_rpc::json::{DMNState, DMNStateDiff, MasternodeListItem}; use dpp::version::{PlatformVersion, TryFromPlatformVersioned}; mod accessors; @@ -41,3 +41,64 @@ impl From for MasternodeListItem { } } } + +/// Applies a Core diff to a masternode state with the revived height replaced by the diff's, so +/// a diff that does not print `PoSeRevivedHeight` clears it. +/// +/// Core prints the revived height only in a diff that spans the revive, so this loses it on the +/// masternode's next diff. The masternode list generations calling this +/// (`update_state_masternode_list` v0 and v1) have always applied diffs this way, and their +/// stored lists and validator sets for every processed block follow from it, so they keep doing +/// so. A generation that keeps Core's revived height must apply diffs another way. +pub(crate) fn apply_diff_replacing_revived_height(state: &mut DMNState, diff: DMNStateDiff) { + let pose_revived_height = diff.pose_revived_height; + state.apply_diff(diff); + state.pose_revived_height = pose_revived_height; +} + +#[cfg(test)] +mod tests { + use super::apply_diff_replacing_revived_height; + use dpp::dashcore_rpc::json::{DMNState, DMNStateDiff}; + use serde_json::json; + + fn state_with_revived_height(pose_revived_height: Option) -> DMNState { + let mut state: DMNState = serde_json::from_value(json!({ + "service": "192.0.2.1:9999", + "registeredHeight": 1000, + "PoSeRevivedHeight": -1, + "PoSeBanHeight": -1, + "revocationReason": 0, + "ownerAddress": "yPxSLV3rNkiq4aNsvYHLb5RzgtwwnHPLWP", + "votingAddress": "yPxSLV3rNkiq4aNsvYHLb5RzgtwwnHPLWP", + "payoutAddress": "yPxSLV3rNkiq4aNsvYHLb5RzgtwwnHPLWP", + "pubKeyOperator": "00".repeat(48), + })) + .expect("expected a masternode state"); + state.pose_revived_height = pose_revived_height; + state + } + + fn diff(json: serde_json::Value) -> DMNStateDiff { + serde_json::from_value(json).expect("expected a Core state diff") + } + + #[test] + fn should_clear_a_revived_height_the_diff_does_not_print() { + let mut state = state_with_revived_height(Some(1100)); + apply_diff_replacing_revived_height(&mut state, diff(json!({"lastPaidHeight": 1200}))); + assert_eq!(state.pose_revived_height, None); + } + + #[test] + fn should_take_the_revived_height_the_diff_prints() { + for previous in [None, Some(1100)] { + let mut state = state_with_revived_height(previous); + apply_diff_replacing_revived_height( + &mut state, + diff(json!({"PoSeRevivedHeight": 1200, "PoSeBanHeight": -1})), + ); + assert_eq!(state.pose_revived_height, Some(1200), "from {previous:?}"); + } + } +} diff --git a/packages/rs-drive-abci/src/platform_types/platform_state/accessors.rs b/packages/rs-drive-abci/src/platform_types/platform_state/accessors.rs index 73a1990a4ea..94fa7759f2a 100644 --- a/packages/rs-drive-abci/src/platform_types/platform_state/accessors.rs +++ b/packages/rs-drive-abci/src/platform_types/platform_state/accessors.rs @@ -1,5 +1,6 @@ use crate::error::execution::ExecutionError; use crate::error::Error; +use crate::platform_types::masternode::apply_diff_replacing_revived_height; use crate::platform_types::platform_state::masternode_list_changes::MasternodeListChanges; use crate::platform_types::platform_state::PlatformState; use crate::platform_types::signature_verification_quorum_set::SignatureVerificationQuorumSet; @@ -182,7 +183,10 @@ pub trait PlatformStateV0Methods { /// Applies a Core state diff to a listed masternode in both lists, recording /// an entry change only when its stored state changes. Returns false, - /// changing nothing, when the masternode is not listed. + /// changing nothing, when the masternode is not listed. The revived height + /// is replaced by the diff's, as `update_state_masternode_list` v0, which + /// uses this, has always applied diffs; a generation that keeps Core's + /// revived height must not use this. fn apply_masternode_state_diff( &mut self, pro_tx_hash: &ProTxHash, @@ -578,10 +582,12 @@ impl PlatformStateV0Methods for PlatformState { return false; }; let previous_state = masternode.state.clone(); - masternode.state.apply_diff(state_diff.clone()); + // Inert for update_state_masternode_list v0, the only caller: it has always applied + // diffs with the revived height replaced by the diff's. + apply_diff_replacing_revived_height(&mut masternode.state, state_diff.clone()); let entry_changed = masternode.state != previous_state; if let Some(hpmn) = self.hpmn_masternode_list.get_mut(pro_tx_hash) { - hpmn.state.apply_diff(state_diff.clone()); + apply_diff_replacing_revived_height(&mut hpmn.state, state_diff.clone()); } self.heavy_fields_dirty = true; // A mixed Core diff can include payment-only updates alongside real From 3bcac43cf2f1f69192de680176117f6c273add43 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sun, 18 Oct 2026 12:18:02 +0700 Subject: [PATCH 2/2] test(drive-abci): pin masternode list diffs that carry a re-encoded key and an operator payout update_state_masternode_list v0 and v1 now drop updated masternodes' operator payouts and then the operator keys Core re-encoded from every fetched diff. These tests pin the two together: - a recorded Core v24 re-encode step with an operator payout added ends exactly as without it, at protocol versions 13 and latest: legacy key bytes, the listed operator payout and every operator identity key kept; - a diff whose only stored-field changes are the two leaves the platform state clean in v0 and v1 and hands the identity updates neither; - the two normalizations give the same diff in either order. Removing the payout strip from v0 or v1, or the key drop from v1, turns the tests red (checked by mutation). Co-Authored-By: Claude Opus 5.5 --- .../operator_key_encoding_tests.rs | 95 +++++++++++++ .../update_state_masternode_list/mod.rs | 133 +++++++++++++++++- 2 files changed, 225 insertions(+), 3 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/operator_key_encoding_tests.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/operator_key_encoding_tests.rs index 1665ce9b79c..09295aac711 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/operator_key_encoding_tests.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/operator_key_encoding_tests.rs @@ -560,6 +560,101 @@ fn should_reach_the_kept_operator_identity_after_a_re_encode_and_a_restart() { } } +/// One diff entry can carry a re-encoded operator key and an operator payout at once: MN_L2's +/// recorded ProUpRegTx step, here with an operator payout added. Platform drops both from the +/// entry. MN_L2 keeps its legacy key bytes, the operator payout of its full state, and its +/// operator identity with every key, the payout key included; and the blocks end exactly as they +/// do without the diff's operator payout. +#[test] +fn should_ignore_the_operator_payout_of_a_diff_that_re_encodes_the_operator_key() { + let mn_l2 = pro_tx_hash(MN_L2); + let diff_payout = [0x81; 20]; + let listed_payout = recorded_masternodes()[&mn_l2].state.operator_payout_address; + assert!(listed_payout.is_some_and(|payout| payout != diff_payout)); + for platform_version in [ + PlatformVersion::get(13).expect("expected protocol version 13"), + PlatformVersion::latest(), + ] { + let version = platform_version.protocol_version; + let mut outcomes = vec![]; + for with_diff_payout in [false, true] { + let mut list_diffs = recorded_steps(); + if with_diff_payout { + let (_, entry) = list_diffs + .get_mut(&(V24_ACTIVATION_HEIGHT + 2)) + .and_then(|diff| { + diff.updated_mns + .iter_mut() + .find(|(pro_tx_hash, _)| *pro_tx_hash == mn_l2) + }) + .expect("expected MN_L2's recorded re-encode"); + assert_eq!(entry.pub_key_operator, Some(bytes(MN_L2_BASIC_KEY))); + entry.operator_payout_address = Some(Some(diff_payout)); + } + let core_heights: Vec = list_diffs.keys().copied().collect(); + let platform = platform_with_core(platform_version, list_diffs, V24_ACTIVATION_HEIGHT); + let transaction = platform.drive.grove.start_transaction(); + let state = run_core_heights( + &platform, + &transaction, + platform_version, + core_heights, + None, + ); + let context = format!("PV{version}, diff payout: {with_diff_payout}"); + + let stored = &state.full_masternode_list()[&mn_l2].state; + assert_eq!( + stored.pub_key_operator, + bytes(MN_L2_LEGACY_KEY), + "{context}" + ); + assert_eq!(stored.operator_payout_address, listed_payout, "{context}"); + let operator_keys = identity_keys( + &platform, + &transaction, + platform_version, + operator_identifier(&mn_l2, &bytes(MN_L2_LEGACY_KEY)), + ) + .expect("expected MN_L2's operator identity"); + assert!( + operator_keys.iter().all(|(.., disabled)| !disabled), + "{context}: {operator_keys:?}" + ); + assert!( + operator_keys.iter().any(|(_, purpose, data, _)| { + *purpose == Purpose::TRANSFER + && listed_payout.is_some_and(|payout| data == &payout) + }), + "{context}: MN_L2's operator payout key is the listed payout: {operator_keys:?}" + ); + assert_eq!( + identity_keys( + &platform, + &transaction, + platform_version, + operator_identifier(&mn_l2, &bytes(MN_L2_BASIC_KEY)) + ), + None, + "{context}" + ); + outcomes.push(( + state.full_masternode_list().clone(), + platform + .drive + .grove + .root_hash(Some(&transaction), &platform_version.drive.grove_version) + .value + .expect("expected a root hash"), + )); + } + assert_eq!( + outcomes[0], outcomes[1], + "PV{version}: the diff's operator payout changes nothing" + ); + } +} + /// The masternode for the synthetic steps: MN_L1 as recorded, with operator key `key`. fn masternode_with_key(key: Vec) -> MasternodeListItem { let mut masternode = recorded_masternodes() diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs index 41611a5befb..ba98310d003 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/mod.rs @@ -129,7 +129,10 @@ where #[cfg(test)] mod tests { - use super::{is_basic_re_encoding_of_legacy_key, V24_COINBASE_PAYLOAD_VERSION}; + use super::{ + is_basic_re_encoding_of_legacy_key, without_operator_payout_changes, + V24_COINBASE_PAYLOAD_VERSION, + }; use crate::error::execution::ExecutionError; use crate::error::Error; use crate::execution::platform_events::core_based_updates::update_masternode_list::operator_key_encoding_tests::{ @@ -137,10 +140,13 @@ mod tests { MN_L1_BASIC_KEY, MN_L1_LEGACY_KEY, MN_L2, MN_L2_BASIC_KEY, MN_L2_LEGACY_KEY, MN_L3, MN_L3_NEW_KEY, MN_L4, }; + use crate::config::PlatformConfig; + use crate::platform_types::platform_state::{PlatformState, PlatformStateV0Methods}; use crate::test::helpers::setup::TestPlatformBuilder; - use dpp::dashcore::ProTxHash; - use dpp::dashcore_rpc::dashcore_rpc_json::DMNStateDiff; + use dpp::dashcore::{Network, ProTxHash}; + use dpp::dashcore_rpc::dashcore_rpc_json::{DMNStateDiff, MasternodeListDiff}; use dpp::dashcore_rpc::Error as CoreRpcError; + use dpp::version::PlatformVersion; use serde_json::json; use std::str::FromStr; @@ -357,4 +363,125 @@ mod tests { .expect("expected to normalize the diff"); assert_eq!(entries, reported); } + + /// v0 and v1 drop operator payouts from a fetched diff and then the operator keys Core + /// re-encoded. The first reads nothing and writes only `operator_payout_address`; the second + /// reads the stored list and `pub_key_operator`, asks Core only about entries it would clear, + /// and writes only `pub_key_operator`. So the diff they apply is the same whichever runs first, + /// also for entries that carry an operator payout and a re-encoded or unchanged key at once. + #[test] + fn should_normalize_a_diff_the_same_whichever_normalization_runs_first() { + let masternodes = recorded_masternodes(); + let reported = MasternodeListDiff { + base_height: 400, + block_height: 401, + added_mns: vec![], + removed_mns: vec![], + updated_mns: updated_mns() + .into_iter() + .map(|(pro_tx_hash, state_diff)| { + ( + pro_tx_hash, + DMNStateDiff { + operator_payout_address: Some(Some([0x81; 20])), + ..state_diff + }, + ) + }) + .collect(), + }; + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + platform + .core_rpc + .expect_get_coinbase_payload_version() + .times(2) + .returning(|_| Ok(Some(V24_COINBASE_PAYLOAD_VERSION))); + + let mut payouts_first = without_operator_payout_changes(reported.clone()); + platform + .drop_unchanged_operator_keys(&masternodes, 401, &mut payouts_first.updated_mns) + .expect("expected to normalize the diff"); + + let mut keys_first = reported; + platform + .drop_unchanged_operator_keys(&masternodes, 401, &mut keys_first.updated_mns) + .expect("expected to normalize the diff"); + let keys_first = without_operator_payout_changes(keys_first); + + let mut expected = updated_mns(); + expected[0].1.pub_key_operator = None; + expected[1].1.pub_key_operator = None; + assert_eq!(payouts_first.updated_mns, expected); + assert_eq!(keys_first.updated_mns, expected); + } + + /// When a re-encoded operator key and an operator payout are the only stored fields a diff + /// changes, v0 and v1 drop both before they check whether the diff changes the stored list. + /// So the block keeps the masternode lists as they were, leaves the platform state clean, and + /// hands the identity updates an entry with neither. + #[test] + fn should_leave_the_state_clean_when_a_diff_only_re_encodes_a_key_and_sets_an_operator_payout() + { + let mn_l2 = pro_tx_hash(MN_L2); + let listed = recorded_masternodes() + .remove(&mn_l2) + .expect("expected MN_L2 in the recording"); + let reported = DMNStateDiff { + operator_payout_address: Some(Some([0x81; 20])), + ..diff_entry(json!({ "pubKeyOperator": MN_L2_BASIC_KEY, "lastPaidHeight": 401 })) + }; + for (generation, platform_version) in [ + ( + 0, + PlatformVersion::get(13).expect("expected protocol version 13"), + ), + (1, PlatformVersion::latest()), + ] { + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + let reported = reported.clone(); + platform + .core_rpc + .expect_get_protx_diff_with_masternodes() + .times(1) + .returning(move |base_block, block| { + Ok(MasternodeListDiff { + base_height: base_block.unwrap_or_default(), + block_height: block, + added_mns: vec![], + removed_mns: vec![], + updated_mns: vec![(mn_l2, reported.clone())], + }) + }); + platform + .core_rpc + .expect_get_coinbase_payload_version() + .times(1) + .returning(|_| Ok(Some(V24_COINBASE_PAYLOAD_VERSION))); + let mut state = PlatformState::default_with_protocol_versions( + platform_version.protocol_version, + platform_version.protocol_version, + &PlatformConfig::default_for_network(Network::Testnet), + ) + .expect("expected a platform state"); + state.insert_masternode(listed.clone()); + state.mark_saved(); + let before = state.full_masternode_list().clone(); + + let outcome = if generation == 0 { + platform.update_state_masternode_list_v0(&mut state, 401, false) + } else { + platform.update_state_masternode_list_v1(&mut state, 401, false) + } + .expect("expected to update the masternode list"); + + assert!(!state.heavy_fields_dirty, "v{generation}"); + assert!(state.masternode_changes.is_empty(), "v{generation}"); + assert_eq!(state.full_masternode_list(), &before, "v{generation}"); + assert_eq!( + outcome.masternode_list_diff.updated_mns, + vec![(mn_l2, diff_entry(json!({ "lastPaidHeight": 401 })))], + "v{generation}" + ); + } + } }