fix(drive-abci): apply masternode list diffs as v0 and v1 always have

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<MasternodeListItem>,
+        diff: MasternodeListDiff,
+        before_diff: impl FnOnce(&TempPlatform<MockCoreRPCLike>, &Transaction),
+    ) -> (TempPlatform<MockCoreRPCLike>, 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<MockCoreRPCLike>,
+        transaction: Option<&Transaction>,
+        pro_tx_hash: &ProTxHash,
+        pub_key_operator: &[u8],
+        platform_version: &PlatformVersion,
+    ) -> BTreeMap<KeyID, (Purpose, Vec<u8>, 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::<Vec<_>>();
+            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)]

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
+}