From e762ac466dfc34cb024359d8553c3a15c836901b Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 28 Oct 2026 15:20:53 +0700 Subject: [PATCH 01/10] feat(drive-abci)!: emit version 2 asset unlocks once Core V24 is active (PV15) At protocol version 15, build_untied_withdrawal_transactions_from_documents v1 (selected only by DRIVE_ABCI_METHOD_VERSIONS_V11) gives a withdrawal it pools asset unlock payload version 2 when the coinbase of the Core block at the Platform block's chain locked height has payload version 4 or higher (Core requires that exactly while V24 is active) and the withdrawal's whole amount is not below Core's dust threshold; otherwise version 1. Core is asked once per pooling block, and a missing answer fails the block rather than defaulting. The version byte is frozen in the stored untied bytes, so every re-signed instance of a v2 withdrawal keeps Core's stable txid, and withdrawals pooled before the switch stay v1. Dust stays v1, so the FAILED path only ever ends v1 withdrawals, which Core never InstantSend-locks. PV14 and earlier keep generation 0 and replay byte-identically. Also: V24_COINBASE_PAYLOAD_VERSION moves to rpc/core.rs (pure move); the mock Core answers coinbase payload version 3 by default, and tests that set their own answer now clear the defaults first. Tests written first; red before the change, green after: - version choice from the coinbase at the chain locked height (3/4/5), one Core read per block: FAIL before, PASS after - Ok(None) / RPC error fail the block without pooling: FAIL / PASS - dust stays v1 (P2PKH and P2SH boundaries, cross-checked against the FAILED predicate): FAIL / PASS - PV14 never asks Core and emits v1: FAIL (PV15 half) / PASS - strategy: v2 txid kept across expiry and re-sign: FAIL / PASS - strategy: switch at the first chain locked height with V24: FAIL / PASS DIP-0027 txid vectors through dequeue and re-sign pin existing transforms (green from the start; a lock_time mutation makes them fail). Refs #4908 Co-Authored-By: Claude Opus 5.5 --- .../operator_key_encoding_tests.rs | 2 + .../update_state_masternode_list/mod.rs | 25 +- .../mod.rs | 13 +- .../v0/mod.rs | 2 +- .../v1/mod.rs | 593 ++++++++++++++++++ .../v0/mod.rs | 175 ++++++ .../v2/mod.rs | 51 +- .../v2/mod.rs | 5 + .../src/platform_types/platform/mock.rs | 7 + packages/rs-drive-abci/src/rpc/core.rs | 7 + .../test_cases/withdrawal_tests.rs | 515 ++++++++++++++- .../drive_abci_method_versions/v11.rs | 16 +- .../src/version/system_limits/mod.rs | 5 +- .../rs-platform-version/src/version/v15.rs | 18 +- 14 files changed, 1408 insertions(+), 26 deletions(-) create mode 100644 packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs 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 09295aac711..1fd9324bd58 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 @@ -131,6 +131,8 @@ fn platform_with_core( .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() .set_genesis_state(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_protx_diff_with_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 ba98310d003..4024e7420de 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 @@ -7,7 +7,7 @@ mod v1; use crate::error::execution::ExecutionError; use crate::error::Error; use crate::platform_types::platform::Platform; -use crate::rpc::core::CoreRPCLike; +use crate::rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}; use dpp::bls::PublicKey as BlsPublicKey; use dpp::dashcore::ProTxHash; use dpp::dashcore_rpc::dashcore_rpc_json::{DMNStateDiff, MasternodeListDiff, MasternodeListItem}; @@ -26,11 +26,6 @@ fn without_operator_payout_changes(mut masternode_diff: MasternodeListDiff) -> M masternode_diff } -/// The coinbase payload version Dash Core requires while DEPLOYMENT_V24 is active -/// (`MERKLE_ROOT_ASSETUNLOCKS`). Core requires it exactly at the blocks where V24 and V20 are -/// active and refuses it at every other block; V20 is active on every chain Platform runs on. -const V24_COINBASE_PAYLOAD_VERSION: u16 = 4; - /// The first byte of the point at infinity, which has the same 48 bytes in both schemes. const BLS_INFINITY_FIRST_BYTE: u8 = 0xc0; /// The legacy scheme's only flag: bit 7 of the first byte is the sign of y. Bits 5 and 6 are @@ -129,10 +124,7 @@ where #[cfg(test)] mod tests { - use super::{ - is_basic_re_encoding_of_legacy_key, without_operator_payout_changes, - V24_COINBASE_PAYLOAD_VERSION, - }; + use super::{is_basic_re_encoding_of_legacy_key, without_operator_payout_changes}; use crate::error::execution::ExecutionError; use crate::error::Error; use crate::execution::platform_events::core_based_updates::update_masternode_list::operator_key_encoding_tests::{ @@ -142,6 +134,7 @@ mod tests { }; use crate::config::PlatformConfig; use crate::platform_types::platform_state::{PlatformState, PlatformStateV0Methods}; + use crate::rpc::core::V24_COINBASE_PAYLOAD_VERSION; use crate::test::helpers::setup::TestPlatformBuilder; use dpp::dashcore::{Network, ProTxHash}; use dpp::dashcore_rpc::dashcore_rpc_json::{DMNStateDiff, MasternodeListDiff}; @@ -282,6 +275,8 @@ mod tests { (V24_COINBASE_PAYLOAD_VERSION, true), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_coinbase_payload_version() @@ -314,6 +309,8 @@ mod tests { Ok(None), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); let expect_none = answer.is_ok(); platform .core_rpc @@ -341,7 +338,9 @@ mod tests { #[test] fn should_keep_other_entries_without_asking_core() { let masternodes = recorded_masternodes(); - let platform = TestPlatformBuilder::new().build_with_mock_rpc(); + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, so asking Core fails the test. + platform.core_rpc.checkpoint(); let unlisted = ProTxHash::from_str(&"11".repeat(32)).expect("expected a pro tx hash"); let mut entries = vec![ ( @@ -391,6 +390,8 @@ mod tests { .collect(), }; let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_coinbase_payload_version() @@ -438,6 +439,8 @@ mod tests { (1, PlatformVersion::latest()), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); let reported = reported.clone(); platform .core_rpc diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs index d161ca1a45d..25af7c3340b 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs @@ -9,6 +9,7 @@ use dpp::version::PlatformVersion; use dpp::withdrawal::{WithdrawalTransactionIndex, WithdrawalTransactionIndexAndBytes}; mod v0; +mod v1; impl Platform where @@ -23,7 +24,9 @@ where /// /// * `documents` - A mutable reference to a vector of `Document` representing the withdrawal requests. /// * `start_index` - The starting index for the transaction, of type `WithdrawalTransactionIndex`. - /// * `block_info` - A reference to the `BlockInfo`, which provides the current block's timestamp. + /// * `block_info` - A reference to the `BlockInfo`, which provides the current block's timestamp + /// and, from version 1, the chain locked height whose Core block decides the asset unlock + /// payload version. /// * `platform_version` - A reference to the `PlatformVersion` that specifies the version of the platform being used. /// /// # Returns @@ -49,9 +52,15 @@ where block_info, platform_version, ), + 1 => self.build_untied_withdrawal_transactions_from_documents_v1( + documents, + start_index, + block_info, + platform_version, + ), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "build_untied_withdrawal_transactions_from_documents".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v0/mod.rs index 6cb975441ed..75f46b1f77e 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v0/mod.rs @@ -110,7 +110,7 @@ mod tests { #[test] fn test_build() { - let platform_version = PlatformVersion::latest(); + let platform_version = PlatformVersion::get(14).expect("expected protocol version 14"); let platform = TestPlatformBuilder::new() .build_with_mock_rpc() .set_initial_state_structure(); diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs new file mode 100644 index 00000000000..452377f8ca5 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -0,0 +1,593 @@ +use dpp::block::block_info::BlockInfo; +use dpp::dashcore::consensus::Encodable; +use dpp::dashcore::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; +use dpp::dashcore::ScriptBuf; +use dpp::data_contracts::withdrawals_contract; +use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; +use dpp::document::document_methods::DocumentMethodsV0; +use dpp::document::{Document, DocumentV0Getters, DocumentV0Setters}; +use dpp::fee::Credits; +use dpp::identity::convert_credits_to_duffs; +use dpp::platform_value::btreemap_extensions::BTreeValueMapHelper; +use dpp::withdrawal::{ + core_dust_threshold_duffs, WithdrawalTransactionIndex, WithdrawalTransactionIndexAndBytes, +}; + +use crate::{ + error::{execution::ExecutionError, Error}, + platform_types::platform::Platform, + rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}, +}; +use dpp::version::PlatformVersion; + +impl Platform +where + C: CoreRPCLike, +{ + /// Build list of Core transactions from withdrawal documents. + /// + /// Version 1 changes on version 0 by emitting asset unlock payload version 2 once Dash + /// Core's V24 is active. The txid of a version 2 asset unlock leaves out the quorum signing + /// info, so every instance Platform signs of one withdrawal shares one txid, and Core may + /// InstantSend-lock it before it is mined. Core refuses version 2 before V24, so the + /// version follows Core's fork state at the block's chain locked height, which every node + /// reads alike: V24 is active there exactly when the coinbase of that Core block has + /// payload version 4 or higher. A withdrawal whose whole amount is below Core's dust + /// threshold stays at version 1 (see `withdrawal_amount_is_core_dust`). + /// + /// The version is decided here once and kept in the untied transaction bytes, which every + /// later signing reuses unchanged. Nothing may rewrite those bytes, nor change how a + /// signed transaction is built from them, for a withdrawal pooled as version 2: an + /// InstantSend lock pins its txid, so an instance with another txid could never be mined + /// while the lock holds. + pub(super) fn build_untied_withdrawal_transactions_from_documents_v1( + &self, + documents: &mut [Document], + start_index: WithdrawalTransactionIndex, + block_info: &BlockInfo, + platform_version: &PlatformVersion, + ) -> Result<(Vec, Credits), Error> { + if documents.is_empty() { + return Ok((Vec::new(), 0)); + } + + let dust_relay_fee_per_kb = platform_version + .system_limits + .core_dust_relay_fee_per_kb + .ok_or(Error::Execution(ExecutionError::CorruptedCodeExecution( + "build_untied_withdrawal_transactions_from_documents v1 requires a Core dust relay fee", + )))?; + + // Every block at a Core height Platform follows carries a coinbase payload. + let core_height = block_info.core_height; + let coinbase_payload_version = self + .core_rpc + .get_coinbase_payload_version(core_height)? + .ok_or_else(|| { + ExecutionError::DashCoreBadResponseError(format!( + "the coinbase of the block at core height {core_height} has no payload" + )) + })?; + let v24_active = coinbase_payload_version >= V24_COINBASE_PAYLOAD_VERSION; + + documents.iter_mut().enumerate().try_fold( + (Vec::new(), 0u64), // Start with an empty vector for transactions and 0 for total amount. + |(mut transactions, mut total_amount), (i, document)| { + // Calculate the transaction index. + let transaction_index = start_index + i as WithdrawalTransactionIndex; + + // Convert the document into the withdrawal transaction information. + let mut withdrawal_transaction = document + .try_into_asset_unlock_base_transaction_info( + transaction_index, + platform_version, + )?; + + if v24_active && !withdrawal_amount_is_core_dust(document, dust_relay_fee_per_kb)? { + withdrawal_transaction.base_payload.version = + AssetUnlockPayload::STABLE_TXID_VERSION; + } + + // Create a buffer to hold the encoded transaction. + let mut transaction_buffer: Vec = vec![]; + + // Get the withdrawal amount from the document properties. + let amount: u64 = document + .properties() + .get_integer(withdrawal::properties::AMOUNT)?; + + // Add the amount to the total, checking for overflow. + total_amount = total_amount.checked_add(amount).ok_or_else(|| { + Error::Execution(ExecutionError::Overflow( + "Overflow while calculating total amount", + )) + })?; + + // Consensus encode the withdrawal transaction into the buffer. + withdrawal_transaction + .consensus_encode(&mut transaction_buffer) + .map_err(|_| { + Error::Execution(ExecutionError::CorruptedCodeExecution( + "Can't consensus encode a withdrawal transaction", + )) + })?; + + // Update the document properties. + document.set_u64(withdrawal::properties::TRANSACTION_INDEX, transaction_index); + document.set_u8( + withdrawal::properties::STATUS, + withdrawals_contract::WithdrawalStatus::POOLED as u8, + ); + document.set_updated_at(Some(block_info.time_ms)); + + // Increment the document revision, handle error if it fails. + document.increment_revision().map_err(|_| { + Error::Execution(ExecutionError::Overflow( + "Overflow when adding to document revision for withdrawals", + )) + })?; + + // Add the transaction index and encoded transaction to the result. + transactions.push((transaction_index, transaction_buffer)); + + // Return the updated accumulator. + Ok((transactions, total_amount)) + }, + ) + } +} + +/// Whether the whole withdrawal amount is below the dust threshold Core's mempool applies to +/// the withdrawal's output script. +/// +/// It is the predicate on which `rebroadcast_expired_withdrawal_documents` 2 marks an expired +/// withdrawal FAILED and stops re-signing it, with the same `core_dust_relay_fee_per_kb`. +/// Platform must keep re-signing a version 2 withdrawal until it is mined, as an InstantSend +/// lock may already rely on it, so such a withdrawal is kept at version 1, which Core never +/// InstantSend-locks. A protocol version that changes the dust relay fee, or the predicate +/// FAILED uses, must keep FAILED reaching only version 1 withdrawals. +/// +/// Core's mempool tests the payout, the amount less the fee, rather than the amount. The +/// amount is tested here because what must hold is agreement with FAILED's predicate. +fn withdrawal_amount_is_core_dust( + document: &Document, + dust_relay_fee_per_kb: u64, +) -> Result { + let amount: u64 = document + .properties() + .get_integer(withdrawal::properties::AMOUNT)?; + + let output_script_bytes = document + .properties() + .get_bytes(withdrawal::properties::OUTPUT_SCRIPT)?; + + let amount_duffs = convert_credits_to_duffs(amount).map_err(Error::Protocol)?; + + let output_script = ScriptBuf::from_bytes(output_script_bytes); + + Ok(amount_duffs < core_dust_threshold_duffs(&output_script, dust_relay_fee_per_kb)) +} + +#[cfg(test)] +mod tests { + use crate::error::execution::ExecutionError; + use crate::error::Error; + use crate::rpc::core::MockCoreRPCLike; + use crate::test::helpers::setup::TestPlatformBuilder; + use dpp::balances::credits::CREDITS_PER_DUFF; + use dpp::block::block_info::BlockInfo; + use dpp::block::epoch::Epoch; + use dpp::dashcore::consensus::Decodable; + use dpp::dashcore::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBaseTransactionInfo; + use dpp::dashcore_rpc::Error as CoreRpcError; + use dpp::data_contract::accessors::v0::DataContractV0Getters; + use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; + use dpp::data_contracts::withdrawals_contract::WithdrawalStatus; + use dpp::data_contracts::SystemDataContract; + use dpp::document::Document; + use dpp::fee::Credits; + use dpp::identity::core_script::CoreScript; + use dpp::platform_value::platform_value; + use dpp::prelude::Identifier; + use dpp::system_data_contracts::load_system_data_contract; + use dpp::tests::fixtures::get_withdrawal_document_fixture; + use dpp::version::PlatformVersion; + use dpp::withdrawal::{ + core_dust_threshold_duffs, Pooling, WithdrawalTransactionIndex, + WithdrawalTransactionIndexAndBytes, + }; + use drive::config::DEFAULT_QUERY_LIMIT; + use drive::drive::identity::withdrawals::paths::get_withdrawal_transactions_broadcasted_path; + use drive::grovedb::Element; + use drive::util::test_helpers::setup::{setup_document, setup_system_data_contract}; + + /// The chain locked height of the Platform block that pools the withdrawals. + const CORE_HEIGHT: u32 = 2_000; + + const START_INDEX: WithdrawalTransactionIndex = 50; + + fn block_info() -> BlockInfo { + BlockInfo { + time_ms: 1_700_000_000_000, + height: 10, + core_height: CORE_HEIGHT, + epoch: Epoch::default(), + } + } + + fn p2pkh() -> CoreScript { + CoreScript::new_p2pkh([0x11; 20]) + } + + fn p2sh() -> CoreScript { + CoreScript::new_p2sh([0x22; 20]) + } + + fn withdrawal_document( + amount: Credits, + output_script: &CoreScript, + status: WithdrawalStatus, + transaction_index: Option, + platform_version: &PlatformVersion, + ) -> Document { + let data_contract = + load_system_data_contract(SystemDataContract::Withdrawals, platform_version) + .expect("to load system data contract"); + let output_script = output_script.clone(); + let properties = match transaction_index { + None => platform_value!({ + "amount": amount, + "coreFeePerByte": 1u32, + "pooling": Pooling::Never as u8, + "outputScript": output_script, + "status": status as u8, + }), + Some(transaction_index) => platform_value!({ + "amount": amount, + "coreFeePerByte": 1u32, + "pooling": Pooling::Never as u8, + "outputScript": output_script, + "status": status as u8, + "transactionIndex": transaction_index, + "transactionSignHeight": 1_000u64, + }), + }; + get_withdrawal_document_fixture( + &data_contract, + Identifier::new([7u8; 32]), + properties, + None, + platform_version.protocol_version, + ) + .expect("expected withdrawal document") + } + + /// Withdrawal documents as the pooling step hands them over: queued, without an index. + fn queued_withdrawals( + amounts: &[Credits], + platform_version: &PlatformVersion, + ) -> Vec { + amounts + .iter() + .map(|amount| { + withdrawal_document( + *amount, + &p2pkh(), + WithdrawalStatus::QUEUED, + None, + platform_version, + ) + }) + .collect() + } + + /// Pools `documents` through the dispatcher at `platform_version`, against `core_rpc`. + /// Returns the untied transactions, their total amount and the pooled documents. + fn build_untied( + core_rpc: MockCoreRPCLike, + mut documents: Vec, + platform_version: &PlatformVersion, + ) -> ( + Vec, + Credits, + Vec, + ) { + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + platform.core_rpc = core_rpc; + let (transactions, total_amount) = platform + .build_untied_withdrawal_transactions_from_documents( + &mut documents, + START_INDEX, + &block_info(), + platform_version, + ) + .expect("expected to build the untied withdrawal transactions"); + (transactions, total_amount, documents) + } + + /// A Core whose coinbase at `CORE_HEIGHT` has `payload_version`, asked exactly once. + fn core_with_coinbase_payload_version(payload_version: u16) -> MockCoreRPCLike { + let mut core_rpc = MockCoreRPCLike::new(); + core_rpc + .expect_get_coinbase_payload_version() + .withf(|core_height| *core_height == CORE_HEIGHT) + .times(1) + .returning(move |_| Ok(Some(payload_version))); + core_rpc + } + + fn decode(untied_transaction_bytes: &[u8]) -> AssetUnlockBaseTransactionInfo { + AssetUnlockBaseTransactionInfo::consensus_decode(&mut &untied_transaction_bytes[..]) + .expect("expected an untied asset unlock transaction") + } + + /// Core requires coinbase payload version 4 or higher exactly at the blocks where V24 is + /// active, so that is when a withdrawal pooled at that chain locked height gets payload + /// version 2. Core is asked once per block, however many withdrawals the block pools, and + /// apart from the payload version the transactions, their total and the pooled documents + /// are those version 0 builds. + #[test] + fn should_choose_the_asset_unlock_payload_version_from_the_coinbase_at_the_blocks_chain_locked_height( + ) { + let platform_version = PlatformVersion::latest(); + let documents = queued_withdrawals(&[1_000_000, 2_500_000, 70_000_000], platform_version); + + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + platform.core_rpc = MockCoreRPCLike::new(); + let mut expected_documents = documents.clone(); + let (expected_transactions, expected_total_amount) = platform + .build_untied_withdrawal_transactions_from_documents_v0( + &mut expected_documents, + START_INDEX, + &block_info(), + platform_version, + ) + .expect("expected to build the untied withdrawal transactions"); + + for (coinbase_payload_version, payload_version) in [(3, 1), (4, 2), (5, 2)] { + let (transactions, total_amount, pooled_documents) = build_untied( + core_with_coinbase_payload_version(coinbase_payload_version), + documents.clone(), + platform_version, + ); + + assert_eq!(total_amount, expected_total_amount); + assert_eq!(pooled_documents, expected_documents); + assert_eq!(transactions.len(), expected_transactions.len()); + for ((index, bytes), (expected_index, expected_bytes)) in + transactions.iter().zip(&expected_transactions) + { + assert_eq!(index, expected_index); + let mut transaction = decode(bytes); + assert_eq!( + transaction.base_payload.version, payload_version, + "coinbase payload version {coinbase_payload_version}" + ); + transaction.base_payload.version = 1; + assert_eq!(transaction, decode(expected_bytes)); + } + if payload_version == 1 { + assert_eq!( + transactions, expected_transactions, + "before V24 the bytes are version 0's" + ); + } + } + } + + /// Protocol version 14 emits payload version 1 whatever Core's fork state, without asking + /// Core, so the blocks committed under it replay byte for byte; protocol version 15 emits + /// version 2 for the same withdrawals once V24 is active. + #[test] + fn should_keep_emitting_version_1_asset_unlocks_at_protocol_version_14_even_when_v24_is_active() + { + let platform_version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); + let documents = queued_withdrawals(&[1_000_000, 2_500_000], platform_version_14); + + let mut core_rpc = MockCoreRPCLike::new(); + core_rpc + .expect_get_coinbase_payload_version() + .times(0) + .returning(|_| Ok(Some(4))); + let (transactions, _, _) = build_untied(core_rpc, documents.clone(), platform_version_14); + assert!(transactions + .iter() + .all(|(_, bytes)| decode(bytes).base_payload.version == 1)); + + let (transactions, _, _) = build_untied( + core_with_coinbase_payload_version(4), + documents, + PlatformVersion::latest(), + ); + assert!(transactions + .iter() + .all(|(_, bytes)| decode(bytes).base_payload.version == 2)); + } + + #[test] + fn should_not_ask_core_when_no_withdrawal_is_pooled() { + let mut core_rpc = MockCoreRPCLike::new(); + core_rpc.expect_get_coinbase_payload_version().times(0); + + let (transactions, total_amount, _) = + build_untied(core_rpc, vec![], PlatformVersion::latest()); + + assert!(transactions.is_empty()); + assert_eq!(total_amount, 0); + } + + /// A Core error, or a coinbase without a payload, fails the block instead of being read as + /// V24 not being active, before any document is pooled. + #[test] + fn should_fail_rather_than_guess_when_core_cannot_tell_whether_v24_is_active() { + let platform_version = PlatformVersion::latest(); + let documents = queued_withdrawals(&[1_000_000], platform_version); + + for answer in [ + Ok(None), + Err(CoreRpcError::UnexpectedStructure("refused".to_string())), + ] { + let no_payload = answer.is_ok(); + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + let mut core_rpc = MockCoreRPCLike::new(); + core_rpc + .expect_get_coinbase_payload_version() + .times(1) + .return_once(move |_| answer); + platform.core_rpc = core_rpc; + + let mut pooled_documents = documents.clone(); + let result = platform.build_untied_withdrawal_transactions_from_documents( + &mut pooled_documents, + START_INDEX, + &block_info(), + platform_version, + ); + + if no_payload { + assert!( + matches!( + result, + Err(Error::Execution(ExecutionError::DashCoreBadResponseError( + _ + ))) + ), + "{result:?}" + ); + } else { + assert!(matches!(result, Err(Error::CoreRpc(_))), "{result:?}"); + } + assert_eq!(pooled_documents, documents, "no document may be pooled"); + } + } + + /// The status the expiry step leaves an expired withdrawal of `amount` credits to + /// `output_script` in, at `platform_version`: FAILED when it gives up on the withdrawal, + /// BROADCASTED when it re-signs it. + fn status_after_expiry( + amount: Credits, + output_script: &CoreScript, + platform_version: &PlatformVersion, + ) -> WithdrawalStatus { + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_initial_state_structure(); + let transaction = platform.drive.grove.start_transaction(); + + let data_contract = + load_system_data_contract(SystemDataContract::Withdrawals, platform_version) + .expect("to load system data contract"); + setup_system_data_contract(&platform.drive, &data_contract, Some(&transaction)); + let document = withdrawal_document( + amount, + output_script, + WithdrawalStatus::EXPIRED, + Some(START_INDEX), + platform_version, + ); + setup_document( + &platform.drive, + &document, + &data_contract, + data_contract + .document_type_for_name(withdrawal::NAME) + .expect("expected to get document type"), + Some(&transaction), + ); + platform + .drive + .grove_insert_if_not_exists( + get_withdrawal_transactions_broadcasted_path() + .as_slice() + .into(), + &START_INDEX.to_be_bytes(), + Element::Item(vec![0xAB; 64], None), + Some(&transaction), + None, + &platform_version.drive, + ) + .expect("expected to store the untied transaction bytes"); + + platform + .rebroadcast_expired_withdrawal_documents( + &block_info(), + &platform.state.load(), + &transaction, + platform_version, + ) + .expect("expected to process expired withdrawals"); + + let statuses = [WithdrawalStatus::FAILED, WithdrawalStatus::BROADCASTED] + .into_iter() + .filter(|status| { + !platform + .drive + .fetch_oldest_withdrawal_documents_by_status( + (*status).into(), + DEFAULT_QUERY_LIMIT, + Some(&transaction), + platform_version, + ) + .expect("expected to fetch withdrawal documents by status") + .is_empty() + }) + .collect::>(); + assert_eq!(statuses.len(), 1, "{statuses:?}"); + statuses[0] + } + + /// A withdrawal whose whole amount is below Core's dust threshold for its output script + /// stays at version 1 while V24 is active. That is the threshold at which an expired + /// withdrawal is marked FAILED instead of re-signed, so FAILED only ever ends a version 1 + /// withdrawal, which Core never InstantSend-locks. + #[test] + fn should_keep_a_dust_withdrawal_at_version_1_even_when_v24_is_active() { + let platform_version = PlatformVersion::latest(); + let dust_relay_fee_per_kb = platform_version + .system_limits + .core_dust_relay_fee_per_kb + .expect("expected a Core dust relay fee"); + + for (output_script, expected_threshold_duffs) in [(p2pkh(), 546), (p2sh(), 540)] { + let threshold_duffs = core_dust_threshold_duffs(&output_script, dust_relay_fee_per_kb); + assert_eq!(threshold_duffs, expected_threshold_duffs); + let dust = (threshold_duffs - 1) * CREDITS_PER_DUFF; + let viable = threshold_duffs * CREDITS_PER_DUFF; + + let (transactions, _, _) = build_untied( + core_with_coinbase_payload_version(4), + [dust, viable] + .iter() + .map(|amount| { + withdrawal_document( + *amount, + &output_script, + WithdrawalStatus::QUEUED, + None, + platform_version, + ) + }) + .collect(), + platform_version, + ); + let payload_versions = transactions + .iter() + .map(|(_, bytes)| decode(bytes).base_payload.version) + .collect::>(); + assert_eq!( + payload_versions, + vec![1, 2], + "{threshold_duffs} duff threshold" + ); + + assert_eq!( + status_after_expiry(dust, &output_script, platform_version), + WithdrawalStatus::FAILED + ); + assert_eq!( + status_after_expiry(viable, &output_script, platform_version), + WithdrawalStatus::BROADCASTED + ); + } + } +} diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/dequeue_and_build_unsigned_withdrawal_transactions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/dequeue_and_build_unsigned_withdrawal_transactions/v0/mod.rs index 40e4ac903c3..3d977cfae89 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/dequeue_and_build_unsigned_withdrawal_transactions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/dequeue_and_build_unsigned_withdrawal_transactions/v0/mod.rs @@ -198,8 +198,183 @@ fn build_unsigned_transaction( #[cfg(test)] mod tests { use super::*; + use crate::rpc::core::MockCoreRPCLike; use crate::test::helpers::setup::TestPlatformBuilder; use dpp::block::epoch::Epoch; + use dpp::dashcore::consensus; + use dpp::dashcore::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::{ + AssetUnlockBasePayload, AssetUnlockBaseTransactionInfo, + }; + use dpp::dashcore::transaction::special_transaction::TransactionPayload::AssetUnlockPayloadType; + use dpp::dashcore::{PubkeyHash, ScriptBuf, TxOut, Txid}; + use dpp::system_data_contracts::{load_system_data_contract, SystemDataContract}; + use drive::util::test_helpers::setup::setup_system_data_contract; + use std::str::FromStr; + + /// The untied transaction of the DIP-0027 worked examples at `index`: a version 2 asset + /// unlock paying 1 Dash to the P2PKH script of `[0x11; 20]` with a 70,000 duff fee. The + /// pooling step only builds fees that are multiples of 190 duffs, so it is queued as is. + fn dip_0027_untied_transaction(index: WithdrawalTransactionIndex) -> Vec { + consensus::serialize(&AssetUnlockBaseTransactionInfo { + version: 1, + lock_time: 0, + output: vec![TxOut { + value: 100_000_000, + script_pubkey: ScriptBuf::new_p2pkh(&PubkeyHash::from_byte_array([0x11; 20])), + }], + base_payload: AssetUnlockBasePayload { + version: 2, + index, + fee: 70_000, + }, + }) + } + + /// Signs what the queue holds at `core_height` with the validator set `quorum_hash`. + fn dequeue( + platform: &Platform, + quorum_hash: [u8; 32], + core_height: CoreHeight, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Vec { + let block_info = BlockInfo { + time_ms: 1, + height: 1, + core_height, + epoch: Epoch::default(), + }; + platform + .dequeue_and_build_unsigned_withdrawal_transactions( + quorum_hash, + &block_info, + transaction, + platform_version, + ) + .expect("expected to sign the queued withdrawals") + .into_iter() + .collect() + } + + /// Every instance Platform signs of a version 2 withdrawal must have Core's stable txid: + /// an InstantSend lock on the withdrawal pins that txid, so an instance with another one + /// could never be mined. The DIP-0027 txids are pinned through the whole signing path, + /// both when the withdrawal is first signed and when the expiry step queues the same bytes + /// again and it is signed at a later height by another quorum. This guards the transforms + /// from untied bytes to the signed transaction, in Platform and in rust-dashcore, against + /// any change that would move the txid. + #[test] + fn should_give_a_version_2_withdrawal_cores_stable_txid_through_the_signing_pipeline() { + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_initial_state_structure(); + let transaction = platform.drive.grove.start_transaction(); + let data_contract = + load_system_data_contract(SystemDataContract::Withdrawals, platform_version) + .expect("to load system data contract"); + setup_system_data_contract(&platform.drive, &data_contract, Some(&transaction)); + + let dip_0027_txids = [ + ( + 101, + "3c4db73c8356407a5d7c78df5045bd280f2dc4fd644b06c4bfbdead3d5ae41cf", + ), + ( + 123_456_789, + "a67e1107ae6e04b813bc8e81348266f5206d1ca93d305dc4323940e18cdbaf34", + ), + ] + .map(|(index, txid)| { + ( + index, + Txid::from_str(txid).expect("expected a txid as Core prints it"), + ) + }); + let indices = dip_0027_txids.map(|(index, _)| index).to_vec(); + + let mut drive_operations = vec![]; + platform + .drive + .add_enqueue_untied_withdrawal_transaction_operations( + indices + .iter() + .map(|index| (*index, dip_0027_untied_transaction(*index))) + .collect(), + 0, + &mut drive_operations, + platform_version, + ) + .expect("expected to queue the withdrawals"); + platform + .drive + .apply_drive_operations( + drive_operations, + true, + &BlockInfo::default(), + Some(&transaction), + platform_version, + None, + ) + .expect("expected to queue the withdrawals"); + + let first = dequeue( + &platform, + [1; 32], + 500, + Some(&transaction), + platform_version, + ); + + let mut drive_operations = vec![]; + platform + .drive + .move_broadcasted_withdrawal_transactions_back_to_queue_operations( + indices, + &mut drive_operations, + platform_version, + ) + .expect("expected to queue the withdrawals again"); + platform + .drive + .apply_drive_operations( + drive_operations, + true, + &BlockInfo::default(), + Some(&transaction), + platform_version, + None, + ) + .expect("expected to queue the withdrawals again"); + + let re_signed = dequeue( + &platform, + [2; 32], + 549, + Some(&transaction), + platform_version, + ); + + assert_eq!(first.len(), dip_0027_txids.len()); + assert_eq!(re_signed.len(), dip_0027_txids.len()); + for (((index, txid), first), re_signed) in dip_0027_txids.iter().zip(&first).zip(&re_signed) + { + for (instance, request_height) in [(first, 500), (re_signed, 549)] { + let Some(AssetUnlockPayloadType(payload)) = &instance.special_transaction_payload + else { + panic!("expected an asset unlock"); + }; + assert_eq!(payload.base.index, *index); + assert_eq!(payload.request_info.request_height, request_height); + assert_eq!(&instance.txid(), txid, "index {index}"); + } + assert_ne!( + first.instance_hash(), + re_signed.instance_hash(), + "index {index}: a re-signed instance is relayed under a new instance hash" + ); + } + } /// When the withdrawal queue is empty, the function must return an empty /// `UnsignedWithdrawalTxs` without performing any further work. This diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs index 8bd5a66bad4..ac42b43accf 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs @@ -94,9 +94,14 @@ mod tests { use std::sync::Arc; /// Pools two queued withdrawals of `amount` credits each through the dispatcher, at - /// `platform_version`, against a Core whose credit pool holds `pool_duffs` at every height. - /// Returns how many were pooled and how many credit pool balances were asked of Core. - fn pool(platform_version: &PlatformVersion, amount: u64, pool_duffs: u64) -> (usize, usize) { + /// `platform_version`, against a Core whose credit pool holds `pool_duffs` at every height + /// and where V24 is not active. Returns how many were pooled, how many credit pool balances + /// were asked of Core and how many coinbase payload versions. + fn pool( + platform_version: &PlatformVersion, + amount: u64, + pool_duffs: u64, + ) -> (usize, usize, usize) { let mut platform = TestPlatformBuilder::new() .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() @@ -111,6 +116,14 @@ mod tests { reads.fetch_add(1, Ordering::SeqCst); Ok(pool_duffs) }); + let coinbase_reads = Arc::new(AtomicUsize::new(0)); + let reads = coinbase_reads.clone(); + core_rpc + .expect_get_coinbase_payload_version() + .returning(move |_| { + reads.fetch_add(1, Ordering::SeqCst); + Ok(Some(3)) + }); platform.core_rpc = core_rpc; let transaction = platform.drive.grove.start_transaction(); @@ -175,7 +188,11 @@ mod tests { ) .expect("to fetch withdrawal documents") .len(); - (pooled, balance_reads.load(Ordering::SeqCst)) + ( + pooled, + balance_reads.load(Ordering::SeqCst), + coinbase_reads.load(Ordering::SeqCst), + ) } #[test] @@ -200,7 +217,7 @@ mod tests { #[test] fn should_hold_back_on_cores_credit_pool_only_from_protocol_version_14() { let version_13 = PlatformVersion::get(13).expect("expected protocol version 13"); - assert_eq!(pool(version_13, 1000, 1), (2, 0)); + assert_eq!(pool(version_13, 1000, 1), (2, 0, 0)); assert_eq!(pool(PlatformVersion::latest(), 1000, 1).0, 1); } @@ -217,11 +234,31 @@ mod tests { dash_to_credits!(3000), 1_000_000_000_000 ), - (0, 50) + (0, 50, 0) ); assert_eq!( pool(PlatformVersion::latest(), 1000, 1_000_000_000_000), - (2, 50) + (2, 50, 1) + ); + } + + /// From protocol version 15 the block asks Core whether V24 is active once when it pools + /// withdrawals, and not at all when it pools none, so a block that pools nothing does not + /// depend on Core answering. Protocol version 14 never asks. + #[test] + fn should_ask_core_whether_v24_is_active_only_in_blocks_that_pool_withdrawals() { + let coinbase_reads = |(_, _, coinbase_reads): (usize, usize, usize)| coinbase_reads; + let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); + let latest = PlatformVersion::latest(); + + assert_eq!(coinbase_reads(pool(version_14, 1000, 1_000_000_000_000)), 0); + assert_eq!(coinbase_reads(pool(latest, 1000, 1_000_000_000_000)), 1); + // Nothing pools when Core's credit pool admits nothing, nor when the oldest withdrawal + // does not fit Platform's own limit. + assert_eq!(coinbase_reads(pool(latest, 1000, 0)), 0); + assert_eq!( + coinbase_reads(pool(latest, dash_to_credits!(3000), 1_000_000_000_000)), + 0 ); } } diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs index e4a36a09045..4e48208e1e0 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs @@ -167,6 +167,11 @@ where /// Whether the whole withdrawal amount is below the dust threshold Core's mempool applies /// to the withdrawal's output script, so no asset unlock built from this document can /// ever be relayed. +/// +/// `build_untied_withdrawal_transactions_from_documents` 1 keeps such a withdrawal at asset +/// unlock payload version 1 with the same predicate, so this step never gives up on a version 2 +/// withdrawal, which an InstantSend lock may rely on. A generation that changes this predicate +/// must keep that so. fn withdrawal_amount_is_core_dust( document: &Document, dust_relay_fee_per_kb: u64, diff --git a/packages/rs-drive-abci/src/platform_types/platform/mock.rs b/packages/rs-drive-abci/src/platform_types/platform/mock.rs index 8b5fb08caaf..02d596d788e 100644 --- a/packages/rs-drive-abci/src/platform_types/platform/mock.rs +++ b/packages/rs-drive-abci/src/platform_types/platform/mock.rs @@ -44,6 +44,13 @@ impl Platform { .expect_get_credit_pool_balance() .returning(|_| Ok(1_000_000_000_000_000)); + // A coinbase of payload version 3 at every height, a Core where V24 is not active, so + // withdrawals keep asset unlock payload version 1. The same rule as above applies to an + // answer a test needs instead. + core_rpc_mock + .expect_get_coinbase_payload_version() + .returning(|_| Ok(Some(3))); + Self::open_with_client(path, config, core_rpc_mock, initial_protocol_version) } diff --git a/packages/rs-drive-abci/src/rpc/core.rs b/packages/rs-drive-abci/src/rpc/core.rs index e16e4e79863..2208254390d 100644 --- a/packages/rs-drive-abci/src/rpc/core.rs +++ b/packages/rs-drive-abci/src/rpc/core.rs @@ -58,6 +58,13 @@ pub(crate) fn credit_pool_balance_from_coinbase(coinbase: &[u8]) -> Result, + chain_locked_height: u32, + /// The first core height whose coinbase has payload version 4, which Core requires + /// exactly while V24 is active. + v24_activation_height: u32, + /// Every asset unlock Platform sent to Core, in order. + broadcast: Vec, + /// Every core height Platform asked the coinbase payload version of, in order. + coinbase_reads: Vec, + } + + impl V24CoreState { + fn new(v24_activation_height: u32) -> Arc> { + Arc::new(Mutex::new(Self { + asset_unlock_statuses: BTreeMap::new(), + chain_locked_height: 1, + v24_activation_height, + broadcast: vec![], + coinbase_reads: vec![], + })) + } + + /// The asset unlocks broadcast for `index`, in order. + fn broadcast_for(&self, index: WithdrawalTransactionIndex) -> Vec { + self.broadcast + .iter() + .filter(|transaction| asset_unlock_index(transaction) == index) + .cloned() + .collect() + } + } + + fn asset_unlock_payload(tx: &Transaction) -> &AssetUnlockPayload { + let Some(AssetUnlockPayloadType(ref payload)) = tx.special_transaction_payload else { + panic!("expected to get AssetUnlockPayloadType"); + }; + payload + } + + fn v24_test_config() -> PlatformConfig { + PlatformConfig { + validator_set: ValidatorSetConfig::default_100_67(), + chain_lock: ChainLockConfig::default_100_67(), + instant_lock: InstantLockConfig::default_100_67(), + execution: ExecutionConfig { + verify_sum_trees: true, + ..Default::default() + }, + block_spacing_ms: 1000 * 60 * 60, + testing_configs: PlatformTestConfig::default_minimal_verifications(), + ..Default::default() + } + } + + /// A platform at the latest protocol version whose Core is `core_state`. Core's credit pool + /// is large enough that the Core-anchored withdrawal limit never binds. + fn platform_with_v24_core( + config: &PlatformConfig, + core_state: &Arc>, + ) -> TempPlatform { + let mut platform = TestPlatformBuilder::new() + .with_latest_protocol_version() + .with_config(config.clone()) + .build_with_mock_rpc(); + + // The default answers would shadow the ones below. + platform.core_rpc.checkpoint(); + platform + .core_rpc + .expect_get_block_hash() + .returning(|_| Ok(BlockHash::all_zeros())); + platform + .core_rpc + .expect_get_block_json() + .returning(|_| Ok(serde_json::json!({ "tx": [] }))); + // 10 million Dash, in duffs. + platform + .core_rpc + .expect_get_credit_pool_balance() + .returning(|_| Ok(1_000_000_000_000_000)); + + let state = core_state.clone(); + platform + .core_rpc + .expect_get_coinbase_payload_version() + .returning(move |core_height| { + let mut state = state.lock().unwrap(); + state.coinbase_reads.push(core_height); + let v24_active = core_height >= state.v24_activation_height; + Ok(Some(if v24_active { 4 } else { 3 })) + }); + + let state = core_state.clone(); + platform + .core_rpc + .expect_send_raw_transaction() + .returning(move |bytes| { + let transaction: Transaction = + consensus::deserialize(bytes).expect("expected a transaction"); + let txid = transaction.txid(); + state.lock().unwrap().broadcast.push(transaction); + Ok(txid) + }); + + let state = core_state.clone(); + platform + .core_rpc + .expect_get_asset_unlock_statuses() + .returning(move |indices, _| { + let state = state.lock().unwrap(); + Ok(indices + .iter() + .map(|index| AssetUnlockStatusResult { + index: *index, + status: state + .asset_unlock_statuses + .get(index) + .cloned() + .unwrap_or(AssetUnlockStatus::Unknown), + }) + .collect()) + }); + + let state = core_state.clone(); + platform + .core_rpc + .expect_get_best_chain_lock() + .returning(move || { + Ok(ChainLock { + block_height: state.lock().unwrap().chain_locked_height, + block_hash: BlockHash::from_byte_array([1; 32]), + signature: BLSSignature::from([2; 96]), + }) + }); + + platform + } + + fn v24_test_strategy( + operations: Vec, + signer: Option, + ) -> NetworkStrategy { + NetworkStrategy { + strategy: Strategy { + start_contracts: vec![], + operations, + start_identities: StartIdentities::default(), + start_addresses: StartAddresses::default(), + identity_inserts: if signer.is_none() { + IdentityInsertInfo { + frequency: Frequency { + times_per_block_range: 1..2, + chance_per_block: None, + }, + start_keys: 3, + extra_keys: [( + Purpose::TRANSFER, + [(SecurityLevel::CRITICAL, vec![KeyType::ECDSA_SECP256K1])].into(), + )] + .into(), + start_balance_range: dash_to_duffs!(1)..=dash_to_duffs!(1), + } + } else { + IdentityInsertInfo::default() + }, + identity_contract_nonce_gaps: None, + signer, + }, + total_hpmns: 100, + extra_normal_mns: 0, + validator_quorum_count: 24, + chain_lock_quorum_count: 24, + upgrading_info: None, + proposer_strategy: Default::default(), + rotate_quorums: false, + failure_testing: None, + query_testing: None, + verify_state_transition_results: true, + ..Default::default() + } + } + + /// Runs blocks 1 and 2, which create the identities that later withdraw. Returns the + /// outcome and the strategies of a block with one withdrawal and of an empty block. + async fn run_identity_blocks<'a>( + platform: &'a mut TempPlatform, + config: &PlatformConfig, + ) -> (ChainExecutionOutcome<'a>, NetworkStrategy, NetworkStrategy) { + let outcome = run_chain_for_strategy( + platform, + 2, + v24_test_strategy( + vec![Operation { + op_type: OperationType::IdentityTopUp(dash_to_duffs!(10)..=dash_to_duffs!(10)), + frequency: Frequency { + times_per_block_range: 1..4, + chance_per_block: None, + }, + }], + None, + ), + config.clone(), + 1, + &mut None, + &mut None, + ) + .await; + + let withdraw = v24_test_strategy( + vec![Operation { + op_type: OperationType::IdentityWithdrawal( + dash_to_credits!(0.1)..=dash_to_credits!(0.1), + ), + frequency: Frequency { + times_per_block_range: 1..2, + chance_per_block: None, + }, + }], + Some(outcome.signer.clone()), + ); + let idle = v24_test_strategy(vec![], Some(outcome.signer.clone())); + + (outcome, withdraw, idle) + } + + /// Runs block `block`, the one after `outcome`, with `strategy`, and checks that every state + /// transition in it succeeded. + async fn next_block<'a>( + outcome: ChainExecutionOutcome<'a>, + block: u64, + strategy: &NetworkStrategy, + config: &PlatformConfig, + ) -> ChainExecutionOutcome<'a> { + let ChainExecutionOutcome { + abci_app, + proposers, + validator_quorums, + current_validator_quorum_hash, + current_proposer_versions, + end_time_ms, + identity_nonce_counter, + identity_contract_nonce_counter, + instant_lock_quorums, + identities, + addresses_with_balance, + .. + } = outcome; + + let outcome = continue_chain_for_strategy( + abci_app, + ChainExecutionParameters { + block_start: block, + core_height_start: 1, + block_count: 1, + proposers, + validator_quorums, + current_validator_quorum_hash, + current_proposer_versions: Some(current_proposer_versions), + current_identity_nonce_counter: identity_nonce_counter, + current_identity_contract_nonce_counter: identity_contract_nonce_counter, + current_votes: BTreeMap::default(), + start_time_ms: GENESIS_TIME_MS, + current_time_ms: end_time_ms, + instant_lock_quorums, + current_identities: identities, + current_addresses_with_balance: addresses_with_balance, + }, + strategy.clone(), + config.clone(), + StrategyRandomness::SeedEntropy(block), + ) + .await; + + for tx_results_per_block in outcome.state_transition_results_per_block.values() { + for (state_transition, result) in tx_results_per_block { + assert_eq!( + result.code, 0, + "state transition got code {} : {:?}", + result.code, state_transition + ); + } + } + + outcome + } + + fn withdrawal_documents( + outcome: &ChainExecutionOutcome, + status: withdrawals_contract::WithdrawalStatus, + ) -> Vec { + outcome + .abci_app + .platform + .drive + .fetch_oldest_withdrawal_documents_by_status( + status.into(), + DEFAULT_QUERY_LIMIT, + None, + PlatformVersion::latest(), + ) + .expect("expected to fetch withdrawal documents") + } + + /// Core mines every withdrawal it was sent, and the next block marks them complete. + async fn mine_all_and_complete<'a>( + outcome: ChainExecutionOutcome<'a>, + block: u64, + idle: &NetworkStrategy, + config: &PlatformConfig, + core_state: &Arc>, + ) -> ChainExecutionOutcome<'a> { + { + let mut state = core_state.lock().unwrap(); + let indices = state + .broadcast + .iter() + .map(asset_unlock_index) + .collect::>(); + for index in indices { + state + .asset_unlock_statuses + .insert(index, AssetUnlockStatus::Chainlocked); + } + state.chain_locked_height += 1; + } + + let outcome = next_block(outcome, block, idle, config).await; + + for status in [ + withdrawals_contract::WithdrawalStatus::QUEUED, + withdrawals_contract::WithdrawalStatus::POOLED, + withdrawals_contract::WithdrawalStatus::BROADCASTED, + withdrawals_contract::WithdrawalStatus::EXPIRED, + withdrawals_contract::WithdrawalStatus::FAILED, + ] { + assert!( + withdrawal_documents(&outcome, status).is_empty(), + "no withdrawal may remain {status}" + ); + } + + outcome + } + + /// Core may InstantSend-lock a version 2 withdrawal under its txid before mining it, and a + /// child spending its output then relies on that txid, so the withdrawal must keep it when + /// it expires unmined and is re-signed. The re-signed instance differs only in its signing + /// info: a later sign height and a new signature over a new message, so Core relays it + /// under a new instance hash and replaces the expired one in place. + #[tokio::test] + async fn should_keep_the_txid_of_a_version_2_withdrawal_when_it_is_re_signed_after_expiry() { + let config = v24_test_config(); + // V24 is active from the first core height. + let core_state = V24CoreState::new(1); + let mut platform = platform_with_v24_core(&config, &core_state); + let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + + // Block 3 pools a withdrawal at core height 1; block 4 signs it there. + let outcome = next_block(outcome, 3, &withdraw, &config).await; + let outcome = next_block(outcome, 4, &idle, &config).await; + let index = asset_unlock_index(outcome.withdrawals.first().expect("signed in block 4")); + + // Core reaches core height 50 without mining it: 49 blocks after its sign height it + // expires in block 5, and block 6 signs it again. + core_state.lock().unwrap().chain_locked_height = 50; + let outcome = next_block(outcome, 5, &idle, &config).await; + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::EXPIRED).len(), + 1 + ); + let outcome = next_block(outcome, 6, &idle, &config).await; + + let broadcast = core_state.lock().unwrap().broadcast_for(index); + let [first, re_signed] = broadcast.as_slice() else { + panic!("expected two instances, got {broadcast:?}"); + }; + let (first_payload, re_signed_payload) = + (asset_unlock_payload(first), asset_unlock_payload(re_signed)); + assert_eq!(first_payload.base.version, 2); + assert_eq!(re_signed_payload.base, first_payload.base); + assert_eq!(first.output, re_signed.output); + + assert_eq!(re_signed.txid(), first.txid()); + assert_ne!(re_signed.instance_hash(), first.instance_hash()); + + // Signed again at the first chain locked height where it counts as expired, past the + // first instance's last minable height, so no earlier instance can still be mined. + let first_height = first_payload.request_info.request_height; + let re_signed_height = re_signed_payload.request_info.request_height; + assert_eq!(first_height, 1); + assert_eq!(re_signed_height, 50); + let broadcasted = withdrawal_documents( + &outcome, + withdrawals_contract::WithdrawalStatus::BROADCASTED, + ); + let [document] = broadcasted.as_slice() else { + panic!("expected the re-signed withdrawal to be broadcast"); + }; + assert_eq!( + document + .properties() + .get_integer::(withdrawal::properties::TRANSACTION_SIGN_HEIGHT) + .expect("expected a sign height"), + re_signed_height as u64 + ); + + let outcome = mine_all_and_complete(outcome, 7, &idle, &config, &core_state).await; + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::COMPLETE).len(), + 1 + ); + } + + /// Platform emits version 2 from the first block whose chain locked height has V24 active, + /// and only for withdrawals pooled from then on. One pooled before the switch stays version + /// 1 when it is first signed after the switch, and when it expires and is re-signed after + /// it, with a new txid each time, as Core keeps accepting version 1. Core is asked about the + /// chain locked height of each block that pools a withdrawal, and of no other block. + #[tokio::test] + async fn should_switch_to_version_2_asset_unlocks_at_the_first_chain_locked_height_where_v24_is_active( + ) { + const V24_ACTIVATION_HEIGHT: u32 = 50; + let config = v24_test_config(); + let core_state = V24CoreState::new(V24_ACTIVATION_HEIGHT); + let mut platform = platform_with_v24_core(&config, &core_state); + let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + + // Block 3 pools a withdrawal at core height 1, block 4 signs it. + let outcome = next_block(outcome, 3, &withdraw, &config).await; + let outcome = next_block(outcome, 4, &idle, &config).await; + let pooled_long_before = asset_unlock_index(outcome.withdrawals.first().expect("signed")); + + // Block 5 pools one at the last core height before V24. + core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT - 1; + let outcome = next_block(outcome, 5, &withdraw, &config).await; + + // Block 6, at the first core height with V24 active, signs it, and the first one + // expires, 49 core blocks after it was signed. + core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT; + let outcome = next_block(outcome, 6, &idle, &config).await; + let pooled_just_before = asset_unlock_index(outcome.withdrawals.first().expect("signed")); + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::EXPIRED).len(), + 1 + ); + + // Block 7 re-signs the expired one and pools one at the activation height; block 8 + // signs that. + let outcome = next_block(outcome, 7, &withdraw, &config).await; + let outcome = next_block(outcome, 8, &idle, &config).await; + let pooled_at_activation = asset_unlock_index(outcome.withdrawals.first().expect("signed")); + + { + let state = core_state.lock().unwrap(); + let payload_versions_and_heights = |index| { + state + .broadcast_for(index) + .iter() + .map(|transaction| { + let payload = asset_unlock_payload(transaction); + (payload.base.version, payload.request_info.request_height) + }) + .collect::>() + }; + assert_eq!( + payload_versions_and_heights(pooled_long_before), + vec![(1, 1), (1, V24_ACTIVATION_HEIGHT)] + ); + assert_eq!( + payload_versions_and_heights(pooled_just_before), + vec![(1, V24_ACTIVATION_HEIGHT)] + ); + assert_eq!( + payload_versions_and_heights(pooled_at_activation), + vec![(2, V24_ACTIVATION_HEIGHT)] + ); + + let re_signed_version_1 = state.broadcast_for(pooled_long_before); + assert_ne!( + re_signed_version_1[0].txid(), + re_signed_version_1[1].txid(), + "a version 1 txid covers the signing info" + ); + + // Blocks 3, 5 and 7 pooled; blocks 4, 6 and 8 did not. + assert_eq!( + state.coinbase_reads, + vec![1, V24_ACTIVATION_HEIGHT - 1, V24_ACTIVATION_HEIGHT] + ); + } + + let outcome = mine_all_and_complete(outcome, 9, &idle, &config, &core_state).await; + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::COMPLETE).len(), + 3 + ); + } } diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs index c232a2170be..7fbbb5c01de 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs @@ -1,6 +1,7 @@ use super::v10::DRIVE_ABCI_METHOD_VERSIONS_V10; use super::{ - DkgMiningSchedule, DriveAbciCoreChainLockMethodVersionsAndConstants, DriveAbciMethodVersions, + DkgMiningSchedule, DriveAbciCoreChainLockMethodVersionsAndConstants, + DriveAbciIdentityCreditWithdrawalMethodVersions, DriveAbciMethodVersions, }; /// Dash Core's DKG schedule (`llmq/params.h`) of the non-rotating quorum types, the ones chain @@ -87,7 +88,9 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ ]; /// Drive ABCI method versions 11, introduced in protocol v15. Differs from v10 in the chain lock -/// verification, which only works as a whole: +/// verification, which only works as a whole, and in the asset unlocks it emits for withdrawals. +/// +/// The chain lock verification: /// - `core_chain_lock.verify_chain_lock_locally` is 1 rather than 0: a chain lock is rejected /// locally only when Platform holds exactly the quorums active at its signing height, using /// Dash Core's DKG mining schedule in `core_chain_lock.dkg_mining_schedules`. When a quorum may @@ -98,6 +101,11 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ /// `submitchainlock`, which skips that check for a height Core already has a chain lock at or /// above. `verify_chain_lock_locally` 1 leaves to Core chain locks that 0 rejected, so it is /// only safe together with this check. +/// +/// The asset unlocks: `withdrawals.build_untied_withdrawal_transactions_from_documents` is 1 +/// rather than 0: a withdrawal pooled at a chain locked height where Dash Core's V24 is active +/// (the coinbase of that Core block has payload version 4 or higher) gets asset unlock payload +/// version 2, whose txid every re-signed instance shares, unless its amount is Core dust. pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMethodVersions { core_chain_lock: DriveAbciCoreChainLockMethodVersionsAndConstants { verify_chain_lock_locally: 1, @@ -105,5 +113,9 @@ pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMet dkg_mining_schedules: Some(DKG_MINING_SCHEDULES_V1), ..DRIVE_ABCI_METHOD_VERSIONS_V10.core_chain_lock }, + withdrawals: DriveAbciIdentityCreditWithdrawalMethodVersions { + build_untied_withdrawal_transactions_from_documents: 1, // changed in v15: asset unlock payload version 2 once Core's V24 is active at the chain locked height + ..DRIVE_ABCI_METHOD_VERSIONS_V10.withdrawals + }, ..DRIVE_ABCI_METHOD_VERSIONS_V10 }; diff --git a/packages/rs-platform-version/src/version/system_limits/mod.rs b/packages/rs-platform-version/src/version/system_limits/mod.rs index af6da317d5a..0bf7a0d46a1 100644 --- a/packages/rs-platform-version/src/version/system_limits/mod.rs +++ b/packages/rs-platform-version/src/version/system_limits/mod.rs @@ -220,7 +220,10 @@ pub struct SystemLimits { /// a P2PKH output at the default 3000 duffs/kB). From protocol version 14 an expired /// withdrawal whose whole amount is below the threshold of its output script is marked /// FAILED instead of being re-signed forever (`rebroadcast_expired_withdrawal_documents` - /// method version 2). `None` for the protocol versions that predate the rule. + /// method version 2). From protocol version 15 a withdrawal whose whole amount is below + /// that threshold also keeps asset unlock payload version 1 when it is pooled + /// (`build_untied_withdrawal_transactions_from_documents` method version 1), so FAILED only + /// ever ends a version 1 withdrawal. `None` for the protocol versions that predate the rule. pub core_dust_relay_fee_per_kb: Option, /// Maximum Core transaction fee rate, in duffs per byte, accepted for a withdrawal. /// `None` preserves the behavior of protocol versions that predate this limit. diff --git a/packages/rs-platform-version/src/version/v15.rs b/packages/rs-platform-version/src/version/v15.rs index 1a12bfacc9d..9ca4547d0e2 100644 --- a/packages/rs-platform-version/src/version/v15.rs +++ b/packages/rs-platform-version/src/version/v15.rs @@ -188,12 +188,28 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// time fields at their widest: a bound on what a block measures. /// Protocol v14 and earlier keep format version 1 (0 before v9) and /// `apply_contract_with_serialization` 0. +/// +/// 9. **Asset unlocks with a stable txid once Core's V24 is active**: +/// `build_untied_withdrawal_transactions_from_documents` 1 +/// (`DRIVE_ABCI_METHOD_VERSIONS_V11`) gives a withdrawal it pools asset unlock payload +/// version 2 when the coinbase of the Core block at the Platform block's chain locked height +/// has payload version 4 or higher, which Core requires exactly while V24 is active, and the +/// withdrawal's whole amount is not below Core's dust threshold for its output script +/// (`core_dust_relay_fee_per_kb`); otherwise version 1. The txid of a version 2 asset +/// unlock leaves out the requested height, quorum hash and signature, so a withdrawal +/// re-signed after it expires keeps its txid and Core can InstantSend-lock it before it is +/// mined. The version is kept in the untied transaction bytes the queue stores, so +/// withdrawals pooled before the switch stay version 1 when they are re-signed, and a dust +/// withdrawal, the only kind the expiry step marks FAILED, is never version 2. Pooling +/// blocks ask Core for that coinbase, and fail when Core cannot answer. Protocol v14 and +/// earlier keep `build_untied_withdrawal_transactions_from_documents` 0, which emits version +/// 1 without asking Core. pub const PLATFORM_V15: PlatformVersion = PlatformVersion { protocol_version: PROTOCOL_VERSION_15, drive: DRIVE_VERSION_V10, // changed: apply_contract_with_serialization 1 writes nothing for a contract whose format version 1 bytes match the stored one's (note 8) drive_abci: DriveAbciVersion { structs: DRIVE_ABCI_STRUCTURE_VERSIONS_V2, - methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock + methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock; build_untied_withdrawal_transactions_from_documents 1 emits asset unlock payload version 2 once Core's V24 is active (note 9) validation_and_processing: DRIVE_ABCI_VALIDATION_VERSIONS_V11, // changed: identity update advanced_structure 1 refuses unpaid an identity update whose added key's proof of possession does not verify (note 2); create_domain_data_trigger 2 (note 3); contract update transform_into_action 1 holds an update to the compiled size cap in check_tx (note 8) withdrawal_constants: DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V3, query: DRIVE_ABCI_QUERY_VERSIONS_V2, From 4dd07b998918c6d395d8f8940aec8998ecdf81cf Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 28 Oct 2026 15:21:00 +0700 Subject: [PATCH 02/10] fix(drive-abci): record no rejection for stale or already mined asset unlock instances Core 24 holds one mempool instance per withdrawal index and refuses another as assetunlock-stale-instance; it refuses an index its credit pool already paid out as txn-already-known. Both are expected on nodes that replay or catch up, since finalize re-broadcasts committed instances, so they are now logged at debug and no longer written to the rejection directory. In place in append_signatures_and_broadcast_withdrawal_transactions v0 (every protocol version): the method writes no Drive state and the new arm only decides the log level and whether this node writes a file under its own rejections_path, so consensus cannot change. Test at protocol version 14: no rejection file for either reason (FAIL before: 1 file written; PASS after); other reasons still write one. Refs #4908 Co-Authored-By: Claude Opus 5.5 --- .../v0/mod.rs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs index 257fb8d3148..c5b14668ff9 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs @@ -23,6 +23,13 @@ const CORE_RPC_ERROR_ASSET_UNLOCK_TOO_OLD_QUORUM: &str = "bad-assetunlock-too-ol const CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED: &str = "bad-assetunlock-too-late"; +// Since Core 24 the mempool holds one instance per withdrawal index. It refuses another one unless +// that one's requested height is above the held one's and it can be mined at Core's tip +const CORE_RPC_ERROR_ASSET_UNLOCK_STALE_INSTANCE: &str = "assetunlock-stale-instance"; + +// Core 24 refuses an asset unlock whose withdrawal index its credit pool already paid out +const CORE_RPC_ERROR_ASSET_UNLOCK_ALREADY_MINED: &str = "txn-already-known"; + impl Platform where C: CoreRPCLike, @@ -93,6 +100,24 @@ where { // Transaction already in chain; no action needed } + // Core keeps the instance of this withdrawal it already holds, or has mined the + // withdrawal. A node replaying or catching up re-broadcasts instances committed + // long ago, and a Core whose tip is behind the requested height refuses a fresh + // instance until it catches up. Neither is a rejection worth keeping. This only + // decides what is logged and which rejection files this node writes, so it is the + // same at every protocol version. + Err(CoreRPCError::JsonRpc(jsonrpc::error::Error::Rpc(e))) + if e.message == CORE_RPC_ERROR_ASSET_UNLOCK_STALE_INSTANCE + || e.message == CORE_RPC_ERROR_ASSET_UNLOCK_ALREADY_MINED => + { + tracing::debug!( + tx_id = transaction.txid().to_string(), + tx_index, + error = ?e, + "Core keeps the instance it holds of asset unlock transaction with index {tx_index}, or has mined it: {}", + e.message + ); + } Err(CoreRPCError::JsonRpc(jsonrpc::error::Error::Rpc(e))) if e.message == CORE_RPC_ERROR_ASSET_UNLOCK_NO_ACTIVE_QUORUM || e.message == CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED @@ -181,3 +206,80 @@ fn store_transaction_failures( Ok(()) } + +#[cfg(test)] +mod tests { + use crate::config::PlatformConfig; + use crate::test::helpers::setup::TestPlatformBuilder; + use crate::test::helpers::withdrawals::unsigned_withdrawal_transactions; + use dpp::dashcore_rpc::jsonrpc; + use dpp::dashcore_rpc::Error as CoreRPCError; + use dpp::version::PlatformVersion; + use std::collections::BTreeMap; + use std::fs; + use tenderdash_abci::proto::types::VoteExtension; + + /// Core's code for a transaction its mempool refuses (`RPC_TRANSACTION_REJECTED`), with the + /// reject reason as the message. + const CORE_RPC_TRANSACTION_REJECTED: i32 = -26; + + /// Broadcasts one signed withdrawal at the last shipped protocol version to a Core that + /// refuses it with `reason`, and returns how many rejections were recorded. + fn rejections_recorded_for(reason: &'static str) -> usize { + let rejections = tempfile::tempdir().expect("expected a temporary directory"); + let mut platform = TestPlatformBuilder::new() + .with_config(PlatformConfig { + rejections_path: Some(rejections.path().to_path_buf()), + ..Default::default() + }) + .build_with_mock_rpc(); + platform + .core_rpc + .expect_send_raw_transaction() + .times(1) + .returning(move |_| { + Err(CoreRPCError::JsonRpc(jsonrpc::error::Error::Rpc( + jsonrpc::error::RpcError { + code: CORE_RPC_TRANSACTION_REJECTED, + message: reason.to_string(), + data: None, + }, + ))) + }); + + let transactions = unsigned_withdrawal_transactions(500); + let transaction = transactions + .first() + .expect("expected a withdrawal transaction"); + let vote_extension = VoteExtension { + signature: vec![0; 96], + ..Default::default() + }; + + platform + .append_signatures_and_broadcast_withdrawal_transactions( + BTreeMap::from([(transaction, &vote_extension)]), + PlatformVersion::get(14).expect("expected protocol version 14"), + ) + .expect("a refused broadcast does not fail the block"); + + fs::read_dir(rejections.path()) + .map(|entries| entries.count()) + .unwrap_or(0) + } + + /// Core refuses an instance it already holds a fresher one of, and a withdrawal it already + /// mined. Both are expected on a node that catches up or re-broadcasts what was committed + /// before, and neither is a rejection worth keeping. + #[test] + fn should_not_record_a_rejection_when_core_holds_a_fresher_instance_or_mined_the_withdrawal() { + assert_eq!(rejections_recorded_for("assetunlock-stale-instance"), 0); + assert_eq!(rejections_recorded_for("txn-already-known"), 0); + } + + #[test] + fn should_still_record_rejections_for_other_reasons() { + assert_eq!(rejections_recorded_for("bad-assetunlock-too-late"), 1); + assert_eq!(rejections_recorded_for("bad-assetunlocktx-version-2"), 1); + } +} From bb5403f0dcfe0500d835da098b9fdbe5850afef6 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 28 Oct 2026 15:33:01 +0700 Subject: [PATCH 03/10] test(drive-abci): document the asset unlock version test helpers Comment-only; no behaviour change, so no test. Co-Authored-By: Claude Opus 5.5 --- .../v0/mod.rs | 2 ++ .../v1/mod.rs | 5 +++++ .../tests/strategy_tests/test_cases/withdrawal_tests.rs | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs index c5b14668ff9..ae073e375d7 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs @@ -277,6 +277,8 @@ mod tests { assert_eq!(rejections_recorded_for("txn-already-known"), 0); } + /// An expired instance, and one Core refuses for a reason Platform does not expect, are + /// still recorded. #[test] fn should_still_record_rejections_for_other_reasons() { assert_eq!(rejections_recorded_for("bad-assetunlock-too-late"), 1); diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs index 452377f8ca5..df42474a53f 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -206,6 +206,7 @@ mod tests { const START_INDEX: WithdrawalTransactionIndex = 50; + /// The Platform block that pools the withdrawals, at `CORE_HEIGHT`. fn block_info() -> BlockInfo { BlockInfo { time_ms: 1_700_000_000_000, @@ -223,6 +224,8 @@ mod tests { CoreScript::new_p2sh([0x22; 20]) } + /// A withdrawal document of `amount` credits to `output_script` in `status`, with a + /// transaction index and a sign height once it has been pooled. fn withdrawal_document( amount: Credits, output_script: &CoreScript, @@ -316,6 +319,7 @@ mod tests { core_rpc } + /// Decodes the untied transaction bytes the pooling step stores. fn decode(untied_transaction_bytes: &[u8]) -> AssetUnlockBaseTransactionInfo { AssetUnlockBaseTransactionInfo::consensus_decode(&mut &untied_transaction_bytes[..]) .expect("expected an untied asset unlock transaction") @@ -404,6 +408,7 @@ mod tests { .all(|(_, bytes)| decode(bytes).base_payload.version == 2)); } + /// A block that pools nothing does not depend on Core answering. #[test] fn should_not_ask_core_when_no_withdrawal_is_pooled() { let mut core_rpc = MockCoreRPCLike::new(); diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs index df3cd7da95a..3b5a9f03f3c 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs @@ -4024,6 +4024,7 @@ mod tests { } } + /// The asset unlock payload of `tx`, which must be an asset unlock. fn asset_unlock_payload(tx: &Transaction) -> &AssetUnlockPayload { let Some(AssetUnlockPayloadType(ref payload)) = tx.special_transaction_payload else { panic!("expected to get AssetUnlockPayloadType"); @@ -4031,6 +4032,7 @@ mod tests { payload } + /// One block an hour, with the minimal verifications the withdrawal strategy tests use. fn v24_test_config() -> PlatformConfig { PlatformConfig { validator_set: ValidatorSetConfig::default_100_67(), @@ -4130,6 +4132,8 @@ mod tests { platform } + /// A strategy running `operations` on 100 evonodes. Without a `signer` it also creates one + /// identity of 1 Dash per block, the identities the later blocks withdraw from. fn v24_test_strategy( operations: Vec, signer: Option, @@ -4278,6 +4282,7 @@ mod tests { outcome } + /// The withdrawal documents in `status` after `outcome`, oldest first. fn withdrawal_documents( outcome: &ChainExecutionOutcome, status: withdrawals_contract::WithdrawalStatus, From be5ae99820553a6fe50d096647e9a98e923259e1 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 28 Oct 2026 22:40:49 +0700 Subject: [PATCH 04/10] refactor(drive-abci): share the Core dust predicate between pooling and expiry build_untied_withdrawal_transactions_from_documents v1 keeps a dust withdrawal at asset unlock payload version 1, and rebroadcast_expired_withdrawal_documents v2 marks an expired dust withdrawal FAILED. Each had its own private copy of withdrawal_amount_is_core_dust, and the safety of v2 asset unlocks rests on the two agreeing: FAILED must never reach a version 2 withdrawal, which an InstantSend lock may rely on. The predicate now lives once in withdrawals/mod.rs and both methods call it. In-place change to a shipped generation: rebroadcast v2 is selected by PV14 and PV15. The moved body is unchanged and its call site untouched, so the output is identical. The shared function's doc says an edit there changes a shipped generation. should_only_treat_amounts_below_the_script_threshold_as_dust now also runs at PlatformVersion::get(14). It passed before and after the move. Mutating the shared `<` to `<=` fails it and the pooling-side should_keep_a_dust_withdrawal_at_version_1_even_when_v24_is_active, so both callers keep boundary coverage. Addresses the thepastaclaw review on #5634. Co-Authored-By: Claude Opus 5.5 --- .../v1/mod.rs | 38 +----- .../platform_events/withdrawals/mod.rs | 43 +++++++ .../v2/mod.rs | 116 ++++++++---------- 3 files changed, 93 insertions(+), 104 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs index df42474a53f..c69ed0bf956 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -1,20 +1,17 @@ use dpp::block::block_info::BlockInfo; use dpp::dashcore::consensus::Encodable; use dpp::dashcore::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; -use dpp::dashcore::ScriptBuf; use dpp::data_contracts::withdrawals_contract; use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; use dpp::document::document_methods::DocumentMethodsV0; use dpp::document::{Document, DocumentV0Getters, DocumentV0Setters}; use dpp::fee::Credits; -use dpp::identity::convert_credits_to_duffs; use dpp::platform_value::btreemap_extensions::BTreeValueMapHelper; -use dpp::withdrawal::{ - core_dust_threshold_duffs, WithdrawalTransactionIndex, WithdrawalTransactionIndexAndBytes, -}; +use dpp::withdrawal::{WithdrawalTransactionIndex, WithdrawalTransactionIndexAndBytes}; use crate::{ error::{execution::ExecutionError, Error}, + execution::platform_events::withdrawals::withdrawal_amount_is_core_dust, platform_types::platform::Platform, rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}, }; @@ -137,37 +134,6 @@ where } } -/// Whether the whole withdrawal amount is below the dust threshold Core's mempool applies to -/// the withdrawal's output script. -/// -/// It is the predicate on which `rebroadcast_expired_withdrawal_documents` 2 marks an expired -/// withdrawal FAILED and stops re-signing it, with the same `core_dust_relay_fee_per_kb`. -/// Platform must keep re-signing a version 2 withdrawal until it is mined, as an InstantSend -/// lock may already rely on it, so such a withdrawal is kept at version 1, which Core never -/// InstantSend-locks. A protocol version that changes the dust relay fee, or the predicate -/// FAILED uses, must keep FAILED reaching only version 1 withdrawals. -/// -/// Core's mempool tests the payout, the amount less the fee, rather than the amount. The -/// amount is tested here because what must hold is agreement with FAILED's predicate. -fn withdrawal_amount_is_core_dust( - document: &Document, - dust_relay_fee_per_kb: u64, -) -> Result { - let amount: u64 = document - .properties() - .get_integer(withdrawal::properties::AMOUNT)?; - - let output_script_bytes = document - .properties() - .get_bytes(withdrawal::properties::OUTPUT_SCRIPT)?; - - let amount_duffs = convert_credits_to_duffs(amount).map_err(Error::Protocol)?; - - let output_script = ScriptBuf::from_bytes(output_script_bytes); - - Ok(amount_duffs < core_dust_threshold_duffs(&output_script, dust_relay_fee_per_kb)) -} - #[cfg(test)] mod tests { use crate::error::execution::ExecutionError; diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/mod.rs index 3d7b93ab02f..5a1eb4f749f 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/mod.rs @@ -11,3 +11,46 @@ pub(in crate::execution) mod record_credit_inflows_for_withdrawals; pub(in crate::execution) mod record_total_credits_history_for_withdrawals; pub(in crate::execution) mod scan_core_blocks_for_withdrawals; pub(in crate::execution) mod update_broadcasted_withdrawal_statuses; + +use crate::error::Error; +use dpp::dashcore::ScriptBuf; +use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; +use dpp::document::{Document, DocumentV0Getters}; +use dpp::identity::convert_credits_to_duffs; +use dpp::platform_value::btreemap_extensions::BTreeValueMapHelper; +use dpp::withdrawal::core_dust_threshold_duffs; + +/// Whether the whole withdrawal amount is below the dust threshold Core's mempool applies +/// to the withdrawal's output script, so no asset unlock built from this document can +/// ever be relayed. +/// +/// `rebroadcast_expired_withdrawal_documents` 2 marks an expired withdrawal FAILED, and +/// stops re-signing it, on this predicate. `build_untied_withdrawal_transactions_from_documents` +/// 1 keeps such a withdrawal at asset unlock payload version 1 on it: Platform must keep +/// re-signing a version 2 withdrawal until it is mined, as an InstantSend lock may already +/// rely on it, so FAILED may only ever reach a version 1 withdrawal, which Core never +/// InstantSend-locks. Both read this one predicate, so at the same dust relay fee they agree. +/// +/// Protocol version 14 selects `rebroadcast_expired_withdrawal_documents` 2, so an edit here +/// changes a shipped generation. A withdrawal can be pooled under one protocol version and +/// expire under a later one, so a protocol version that raises `core_dust_relay_fee_per_kb`, +/// or needs another predicate, must keep FAILED reaching only version 1 withdrawals, +/// including those pooled before it. +fn withdrawal_amount_is_core_dust( + document: &Document, + dust_relay_fee_per_kb: u64, +) -> Result { + let amount: u64 = document + .properties() + .get_integer(withdrawal::properties::AMOUNT)?; + + let output_script_bytes = document + .properties() + .get_bytes(withdrawal::properties::OUTPUT_SCRIPT)?; + + let amount_duffs = convert_credits_to_duffs(amount).map_err(Error::Protocol)?; + + let output_script = ScriptBuf::from_bytes(output_script_bytes); + + Ok(amount_duffs < core_dust_threshold_duffs(&output_script, dust_relay_fee_per_kb)) +} diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs index 4e48208e1e0..dc216a94a6c 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v2/mod.rs @@ -1,18 +1,17 @@ use dpp::block::block_info::BlockInfo; -use dpp::dashcore::ScriptBuf; use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::data_contracts::withdrawals_contract::WithdrawalStatus; use dpp::document::document_methods::DocumentMethodsV0; -use dpp::document::{Document, DocumentV0Getters, DocumentV0Setters}; -use dpp::identity::convert_credits_to_duffs; +use dpp::document::{DocumentV0Getters, DocumentV0Setters}; use dpp::platform_value::btreemap_extensions::BTreeValueMapHelper; use dpp::system_data_contracts::withdrawals_contract::v1::document_types::withdrawal; use dpp::version::PlatformVersion; -use dpp::withdrawal::{core_dust_threshold_duffs, WithdrawalTransactionIndex}; +use dpp::withdrawal::WithdrawalTransactionIndex; use std::collections::BTreeSet; use crate::{ error::{execution::ExecutionError, Error}, + execution::platform_events::withdrawals::withdrawal_amount_is_core_dust, platform_types::platform::Platform, rpc::core::CoreRPCLike, }; @@ -164,39 +163,13 @@ where } } -/// Whether the whole withdrawal amount is below the dust threshold Core's mempool applies -/// to the withdrawal's output script, so no asset unlock built from this document can -/// ever be relayed. -/// -/// `build_untied_withdrawal_transactions_from_documents` 1 keeps such a withdrawal at asset -/// unlock payload version 1 with the same predicate, so this step never gives up on a version 2 -/// withdrawal, which an InstantSend lock may rely on. A generation that changes this predicate -/// must keep that so. -fn withdrawal_amount_is_core_dust( - document: &Document, - dust_relay_fee_per_kb: u64, -) -> Result { - let amount: u64 = document - .properties() - .get_integer(withdrawal::properties::AMOUNT)?; - - let output_script_bytes = document - .properties() - .get_bytes(withdrawal::properties::OUTPUT_SCRIPT)?; - - let amount_duffs = convert_credits_to_duffs(amount).map_err(Error::Protocol)?; - - let output_script = ScriptBuf::from_bytes(output_script_bytes); - - Ok(amount_duffs < core_dust_threshold_duffs(&output_script, dust_relay_fee_per_kb)) -} - #[cfg(test)] mod tests { use super::*; use crate::test::helpers::setup::TestPlatformBuilder; use dpp::block::epoch::Epoch; use dpp::data_contracts::SystemDataContract; + use dpp::document::Document; use dpp::identity::core_script::CoreScript; use dpp::platform_value::platform_value; use dpp::prelude::Identifier; @@ -511,46 +484,53 @@ mod tests { )); } + /// Protocol version 14 selects this generation, so the verdicts are pinned there as well + /// as at the latest protocol version. #[test] fn should_only_treat_amounts_below_the_script_threshold_as_dust() { - let platform_version = PlatformVersion::latest(); - let data_contract = - load_system_data_contract(SystemDataContract::Withdrawals, platform_version) - .expect("to load system data contract"); - let dust_relay_fee_per_kb = platform_version - .system_limits - .core_dust_relay_fee_per_kb - .expect("v14 sets the dust relay fee"); - - for (amount, expected) in [ - (DUST_AMOUNT_CREDITS, true), - (545_000u64, true), - (546_000u64, false), - (VIABLE_AMOUNT_CREDITS, false), + for platform_version in [ + PlatformVersion::get(14).expect("expected platform version 14"), + PlatformVersion::latest(), ] { - let document = get_withdrawal_document_fixture( - &data_contract, - Identifier::new([7u8; 32]), - platform_value!({ - "amount": amount, - "coreFeePerByte": 1u32, - "pooling": Pooling::Never as u8, - "outputScript": p2pkh_output_script(), - "status": WithdrawalStatus::EXPIRED as u8, - "transactionIndex": 1u64, - "transactionSignHeight": 1u64, - }), - None, - platform_version.protocol_version, - ) - .expect("expected withdrawal document"); - - assert_eq!( - withdrawal_amount_is_core_dust(&document, dust_relay_fee_per_kb) - .expect("expected a dust verdict"), - expected, - "{amount} credits" - ); + let data_contract = + load_system_data_contract(SystemDataContract::Withdrawals, platform_version) + .expect("to load system data contract"); + let dust_relay_fee_per_kb = platform_version + .system_limits + .core_dust_relay_fee_per_kb + .expect("v14 sets the dust relay fee"); + + for (amount, expected) in [ + (DUST_AMOUNT_CREDITS, true), + (545_000u64, true), + (546_000u64, false), + (VIABLE_AMOUNT_CREDITS, false), + ] { + let document = get_withdrawal_document_fixture( + &data_contract, + Identifier::new([7u8; 32]), + platform_value!({ + "amount": amount, + "coreFeePerByte": 1u32, + "pooling": Pooling::Never as u8, + "outputScript": p2pkh_output_script(), + "status": WithdrawalStatus::EXPIRED as u8, + "transactionIndex": 1u64, + "transactionSignHeight": 1u64, + }), + None, + platform_version.protocol_version, + ) + .expect("expected withdrawal document"); + + assert_eq!( + withdrawal_amount_is_core_dust(&document, dust_relay_fee_per_kb) + .expect("expected a dust verdict"), + expected, + "{amount} credits at protocol version {}", + platform_version.protocol_version + ); + } } } } From 1119492d7c75315bfc87b008955f7a41f0634dcf Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 30 Oct 2026 16:12:44 +0700 Subject: [PATCH 05/10] feat(drive-abci)!: record Core fork activation in state (PV15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drive asked its own Core whether a Core fork is active at each place that needed to know, and the asset unlock version 2 change would ask in every block that pools a withdrawal, even at an unchanged chain locked height, where a Core outage then stops the proposer. Protocol 15 keeps one consensus record instead. update_core_info v1, in blocks that move the chain locked height c and at init chain, reads the payload version of the coinbase of Core block c until V24 is recorded: Core requires version 4 exactly while V24 is active. The first block that sees it active writes c once to Misc at [b'F', 0] (big endian u32, no flags), covered by the app hash; Core is never asked about V24 again. A Core error or a coinbase without a payload fails the block. Reads and writes go only through the block transaction, so a thrown-away round takes its write with it; nothing is cached on the node. - rs-drive: CoreFork (append only), the key, fetch/store core_fork_activation_height (OptionalFeatureVersion, Some(0) in DRIVE_VERSION_V10, None before), the misc.core_fork_v24 structure node, its fixture and the regenerated grovedb-structure.json. - rs-platform-version: update_core_info 1 in DRIVE_ABCI_METHOD_VERSIONS_V11, PLATFORM_V15 note 11. - drive-abci: V24_COINBASE_PAYLOAD_VERSION moves unchanged to rpc/core.rs; the mock Core answers payload version 3 (V24 not active) by default. A protocol 15 block that keeps c records nothing, so after the 14 -> 15 upgrade V24 is recorded at the first block that moves c (accepted gap). The operator key logic keeps its own Core call at protocol 15. Tests would have caught this in CI: the rs-drive read-back and refuse-twice tests failed against stub methods (✖) and pass after (✔); four update_core_info v1 unit tests failed against v1 without the probe (✖) and pass after (✔); the three strategy tests fail with protocol 15's update_core_info set back to 0 (✖) and pass with 1 (✔). Co-Authored-By: Claude Opus 5.5 --- .../update_core_info/mod.rs | 18 +- .../update_core_info/v0/mod.rs | 6 +- .../update_core_info/v1/mod.rs | 594 ++++++++++++++ .../operator_key_encoding_tests.rs | 2 + .../update_state_masternode_list/mod.rs | 25 +- .../src/platform_types/platform/mock.rs | 7 + packages/rs-drive-abci/src/rpc/core.rs | 7 + .../test_cases/core_fork_activation_tests.rs | 733 ++++++++++++++++++ .../tests/strategy_tests/test_cases/mod.rs | 1 + .../test_cases/withdrawal_tests.rs | 5 + packages/rs-drive/grovedb-structure.json | 20 +- .../fetch_core_fork_activation_height/mod.rs | 51 ++ .../v0/mod.rs | 38 + .../drive/system/core_fork_activations/mod.rs | 195 +++++ .../store_core_fork_activation_height/mod.rs | 55 ++ .../v0/mod.rs | 34 + packages/rs-drive/src/drive/system/mod.rs | 2 + .../rs-drive/src/drive/system/structure.rs | 25 +- packages/rs-drive/src/structure/tests.rs | 12 + .../drive_abci_method_versions/v11.rs | 15 +- .../src/version/drive_versions/mod.rs | 7 + .../src/version/drive_versions/v1.rs | 2 + .../src/version/drive_versions/v10.rs | 12 +- .../src/version/drive_versions/v2.rs | 2 + .../src/version/drive_versions/v3.rs | 2 + .../src/version/drive_versions/v4.rs | 2 + .../src/version/drive_versions/v5.rs | 2 + .../src/version/drive_versions/v6.rs | 2 + .../src/version/drive_versions/v7.rs | 2 + .../src/version/drive_versions/v8.rs | 2 + .../src/version/drive_versions/v9.rs | 2 + .../src/version/mocks/v2_test.rs | 2 + .../rs-platform-version/src/version/v15.rs | 18 +- 33 files changed, 1877 insertions(+), 25 deletions(-) create mode 100644 packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v1/mod.rs create mode 100644 packages/rs-drive-abci/tests/strategy_tests/test_cases/core_fork_activation_tests.rs create mode 100644 packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/mod.rs create mode 100644 packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/v0/mod.rs create mode 100644 packages/rs-drive/src/drive/system/core_fork_activations/mod.rs create mode 100644 packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/mod.rs create mode 100644 packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/v0/mod.rs diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/mod.rs index 1196d4ebe92..9ef1f1ce0e6 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/mod.rs @@ -1,4 +1,5 @@ mod v0; +mod v1; use crate::error::execution::ExecutionError; use crate::error::Error; @@ -17,7 +18,8 @@ where /// /// This function updates both the masternode list and the quorum information in the platform /// state. It calls the update_masternode_list and update_quorum_info functions to perform - /// the respective updates. + /// the respective updates. From version 1 it first records in Drive, for later steps to + /// read, the Core forks it sees active for the first time. /// /// # Arguments /// @@ -32,7 +34,8 @@ where /// # Returns /// /// * Result<(), Error> - Returns Ok(()) if the update is successful. Returns an error if - /// there is a problem updating the masternode list, quorum information, or the state. + /// there is a problem updating the masternode list, quorum information, or the state, and + /// from version 1 if Core cannot tell whether a fork is active. #[allow(clippy::too_many_arguments)] pub(crate) fn update_core_info( &self, @@ -59,9 +62,18 @@ where transaction, platform_version, ), + 1 => self.update_core_info_v1( + platform_state, + block_platform_state, + core_block_height, + is_init_chain, + block_info, + transaction, + platform_version, + ), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "update_core_info".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v0/mod.rs index 91564d2db1c..ee446fe6a69 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v0/mod.rs @@ -82,8 +82,9 @@ mod tests { /// fire RPC calls we haven't set up in the mock. #[test] fn v0_returns_ok_when_same_core_height_and_not_init() { - let platform_version = PlatformVersion::latest(); + let platform_version = PlatformVersion::get(14).expect("expected protocol version 14"); let platform = TestPlatformBuilder::new() + .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() .set_genesis_state(); @@ -125,8 +126,9 @@ mod tests { fn v0_short_circuit_is_idempotent_when_not_init_chain() { // Re-confirm that both conditions together are required by running // the short-circuit case twice; idempotency guards against flakiness. - let platform_version = PlatformVersion::latest(); + let platform_version = PlatformVersion::get(14).expect("expected protocol version 14"); let platform = TestPlatformBuilder::new() + .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() .set_genesis_state(); diff --git a/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v1/mod.rs new file mode 100644 index 00000000000..3900d715a64 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_core_info/v1/mod.rs @@ -0,0 +1,594 @@ +use crate::error::execution::ExecutionError; +use crate::error::Error; +use crate::platform_types::platform::Platform; +use crate::platform_types::platform_state::PlatformState; +use crate::platform_types::platform_state::PlatformStateV0Methods; +use crate::rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}; +use dpp::block::block_info::BlockInfo; +use dpp::version::PlatformVersion; +use drive::drive::system::core_fork_activations::CoreFork; +use drive::grovedb::Transaction; + +impl Platform +where + C: CoreRPCLike, +{ + /// Updates the core information in the platform state based on the given core block height. + /// + /// Version 0's steps, after first recording the Core forks seen active for the first time + /// at `core_block_height` (`record_core_fork_activations_v1`). Like the other steps, the + /// recording runs only at init chain and in a block that moves the chain locked height. + /// + /// # Arguments + /// + /// * platform_state - A reference to the platform state before execution of current block. + /// * block_platform_state - A mutable reference to the current platform state in the block + /// execution context to be updated. + /// * core_block_height - The current block height in the Dash Core. + /// * is_init_chain - A boolean indicating if the chain is being initialized. + /// * block_info - A reference to the block information. + /// * transaction - The current groveDB transaction. + /// + /// # Returns + /// + /// * Result<(), Error> - Returns Ok(()) if the update is successful. Returns an error if + /// Core cannot tell whether a fork is active, or if there is a problem updating the + /// masternode list, quorum information, or the state. + #[inline(always)] + #[allow(clippy::too_many_arguments)] + pub(super) fn update_core_info_v1( + &self, + platform_state: Option<&PlatformState>, + block_platform_state: &mut PlatformState, + core_block_height: u32, + is_init_chain: bool, + block_info: &BlockInfo, + transaction: &Transaction, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + // the core height of the block platform state is the last committed + if !is_init_chain && block_platform_state.last_committed_core_height() == core_block_height + { + // if we get the same height that we know we do not need to update core info + return Ok(()); + } + + self.record_core_fork_activations_v1(core_block_height, transaction, platform_version)?; + + self.update_masternode_list( + platform_state, + block_platform_state, + core_block_height, + is_init_chain, + block_info, + transaction, + platform_version, + )?; + + self.update_quorum_info( + platform_state, + block_platform_state, + core_block_height, + false, + platform_version, + ) + } + + /// Records `core_block_height` as the height at which Core's V24 is active, if Core's V24 + /// is active there and nothing is recorded yet. This generation tracks V24 only; tracking + /// another fork needs a new generation with that fork's signal. + /// + /// V24's signal is the payload version of the coinbase of the Core block at + /// `core_block_height`: Core requires version 4 exactly while V24 is active, so every node + /// gets the same answer at a chain locked height. Forks never switch off, so once V24 is + /// recorded Core is not asked again. A Core error or a coinbase without a payload fails the + /// block rather than read as "not active". + /// + /// The record is read and written only through the block's transaction, so a round that is + /// thrown away takes its write with it. + fn record_core_fork_activations_v1( + &self, + core_block_height: u32, + transaction: &Transaction, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + if self + .drive + .fetch_core_fork_activation_height(CoreFork::V24, Some(transaction), platform_version)? + .is_some() + { + return Ok(()); + } + // Every block at a Core height Platform follows carries a coinbase payload. + let coinbase_payload_version = self + .core_rpc + .get_coinbase_payload_version(core_block_height) + .inspect_err(|error| { + tracing::error!( + core_fork = ?CoreFork::V24, + core_block_height, + ?error, + "could not read the coinbase that tells whether a core fork is active" + ) + })? + .ok_or_else(|| { + ExecutionError::DashCoreBadResponseError(format!( + "the coinbase of the block at core height {core_block_height} has no payload" + )) + })?; + if coinbase_payload_version < V24_COINBASE_PAYLOAD_VERSION { + tracing::debug!( + core_fork = ?CoreFork::V24, + core_block_height, + coinbase_payload_version, + "core fork not active at this chain locked core height" + ); + return Ok(()); + } + self.drive.store_core_fork_activation_height( + CoreFork::V24, + core_block_height, + Some(transaction), + platform_version, + )?; + tracing::info!( + core_fork = ?CoreFork::V24, + core_block_height, + coinbase_payload_version, + "core fork active at this chain locked core height, recorded in the block being executed" + ); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::error::execution::ExecutionError; + use crate::error::Error; + use crate::platform_types::platform_state::{PlatformState, PlatformStateV0Methods}; + use crate::rpc::core::MockCoreRPCLike; + use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; + use dpp::block::block_info::BlockInfo; + use dpp::block::extended_block_info::v0::ExtendedBlockInfoV0; + use dpp::dashcore_rpc::dashcore_rpc_json::{ExtendedQuorumListResult, MasternodeListDiff}; + use dpp::dashcore_rpc::Error as CoreRpcError; + use dpp::version::PlatformVersion; + use drive::drive::system::core_fork_activations::CoreFork; + use drive::grovedb::Transaction; + use std::collections::HashMap; + use std::sync::{Arc, Mutex}; + + /// The Core heights Platform asked the coinbase payload version of, in order + type CoinbaseReads = Arc>>; + + /// A platform at `platform_version` whose Core reports no masternode list changes and no + /// quorums, and `coinbase` for the coinbase payload version at each height. Returns the + /// heights Platform reads the coinbase at. + fn platform_with_core( + platform_version: &PlatformVersion, + coinbase: impl Fn(u32) -> Result, CoreRpcError> + Send + 'static, + ) -> (TempPlatform, CoinbaseReads) { + let mut platform = TestPlatformBuilder::new() + .with_initial_protocol_version(platform_version.protocol_version) + .build_with_mock_rpc() + .set_genesis_state(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); + platform + .core_rpc + .expect_get_protx_diff_with_masternodes() + .returning(|base_height, block_height| { + Ok(MasternodeListDiff { + base_height: base_height.unwrap_or_default(), + block_height, + added_mns: vec![], + removed_mns: vec![], + updated_mns: vec![], + }) + }); + let quorum_types = [ + platform.config.validator_set.quorum_type, + platform.config.chain_lock.quorum_type, + platform.config.instant_lock.quorum_type, + ]; + platform + .core_rpc + .expect_get_quorum_listextended() + .returning(move |_| { + Ok(ExtendedQuorumListResult { + quorums_by_type: quorum_types + .iter() + .map(|quorum_type| (*quorum_type, Default::default())) + .collect::>(), + }) + }); + let reads = CoinbaseReads::default(); + let recorded_reads = Arc::clone(&reads); + platform + .core_rpc + .expect_get_coinbase_payload_version() + .returning(move |core_height| { + recorded_reads + .lock() + .expect("expected the coinbase reads") + .push(core_height); + coinbase(core_height) + }); + (platform, reads) + } + + /// A Core whose coinbase has payload version 4 from `activation_height` on, as Core requires + /// while V24 is active, and 3 before. + fn v24_from(activation_height: u32) -> impl Fn(u32) -> Result, CoreRpcError> { + move |core_height| { + Ok(Some(if core_height >= activation_height { + 4 + } else { + 3 + })) + } + } + + /// Updates the core info of a block at chain locked height `core_height` whose previous + /// block was committed at `previous_core_height`, as `run_block_proposal` does. + fn run_block( + platform: &TempPlatform, + block_state: &mut PlatformState, + previous_core_height: u32, + core_height: u32, + transaction: &Transaction, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + let height = block_state.last_committed_block_height() + 1; + block_state.set_last_committed_block_info(Some( + ExtendedBlockInfoV0 { + basic_info: BlockInfo { + height: height - 1, + core_height: previous_core_height, + ..Default::default() + }, + app_hash: [0; 32], + quorum_hash: [0; 32], + block_id_hash: [0; 32], + proposer_pro_tx_hash: [0; 32], + signature: [0; 96], + round: 0, + } + .into(), + )); + platform.update_core_info( + None, + block_state, + core_height, + false, + &BlockInfo { + height, + core_height, + ..Default::default() + }, + transaction, + platform_version, + ) + } + + fn recorded_v24( + platform: &TempPlatform, + transaction: &Transaction, + platform_version: &PlatformVersion, + ) -> Option { + platform + .drive + .fetch_core_fork_activation_height(CoreFork::V24, Some(transaction), platform_version) + .expect("expected to read the V24 record") + } + + /// Core requires coinbase payload version 4 exactly while V24 is active, and refuses it + /// otherwise; a later version keeps V24's fields, so it means V24 too. + #[test] + fn should_record_v24_where_the_coinbase_has_payload_version_4_or_more() { + let platform_version = PlatformVersion::latest(); + for (payload_version, expected) in [(3, None), (4, Some(101)), (5, Some(101))] { + let (platform, reads) = + platform_with_core(platform_version, move |_| Ok(Some(payload_version))); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + + run_block( + &platform, + &mut block_state, + 100, + 101, + &transaction, + platform_version, + ) + .expect("expected to update the core info"); + + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + expected, + "coinbase payload version {payload_version}" + ); + assert_eq!(*reads.lock().unwrap(), vec![101]); + } + } + + /// Forks never switch off, so once V24 is recorded no block depends on Core about it. + #[test] + fn should_never_ask_core_about_v24_once_it_is_recorded() { + let platform_version = PlatformVersion::latest(); + let (platform, reads) = platform_with_core(platform_version, |core_height| { + panic!("Core asked about the coinbase at {core_height} after V24 was recorded") + }); + let transaction = platform.drive.grove.start_transaction(); + platform + .drive + .store_core_fork_activation_height( + CoreFork::V24, + 90, + Some(&transaction), + platform_version, + ) + .expect("expected to record V24"); + let mut block_state = platform.state.load().as_ref().clone(); + + for (previous_core_height, core_height) in [(100, 101), (101, 150)] { + run_block( + &platform, + &mut block_state, + previous_core_height, + core_height, + &transaction, + platform_version, + ) + .expect("expected to update the core info"); + } + + assert!(reads.lock().unwrap().is_empty()); + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + Some(90) + ); + } + + /// A Core error or a coinbase without a payload is never read as "not active": the block + /// fails and nothing is recorded. + #[test] + fn should_fail_without_recording_when_core_cannot_tell() { + let platform_version = PlatformVersion::latest(); + + let (platform, _) = platform_with_core(platform_version, |_| Ok(None)); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + let result = run_block( + &platform, + &mut block_state, + 100, + 101, + &transaction, + platform_version, + ); + assert!( + matches!( + result, + Err(Error::Execution(ExecutionError::DashCoreBadResponseError( + _ + ))) + ), + "a coinbase without a payload must fail the block, got {result:?}" + ); + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + None + ); + + let (platform, _) = platform_with_core(platform_version, |_| { + Err(CoreRpcError::UnexpectedStructure( + "Core is unreachable".to_string(), + )) + }); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + let result = run_block( + &platform, + &mut block_state, + 100, + 101, + &transaction, + platform_version, + ); + assert!( + matches!(result, Err(Error::CoreRpc(_))), + "a Core error must fail the block, got {result:?}" + ); + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + None + ); + } + + /// A block that keeps the chain locked height makes no Core call, so a Core outage there + /// does not stop Platform. + #[test] + fn should_not_ask_core_about_forks_at_an_unchanged_chain_locked_height() { + let platform_version = PlatformVersion::latest(); + let (platform, reads) = platform_with_core(platform_version, v24_from(0)); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + + run_block( + &platform, + &mut block_state, + 100, + 100, + &transaction, + platform_version, + ) + .expect("expected to skip the core info update"); + + assert!(reads.lock().unwrap().is_empty()); + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + None + ); + } + + /// At init chain the core info is updated from scratch at the genesis core height, although + /// the state already holds that height as its last committed one, so a chain born at this + /// protocol version records a V24 active there and asks Core about one that is not. + #[test] + fn should_ask_core_about_v24_at_init_chain() { + const GENESIS_CORE_HEIGHT: u32 = 1_000; + let platform_version = PlatformVersion::latest(); + for (activation_height, expected) in [ + (GENESIS_CORE_HEIGHT, Some(GENESIS_CORE_HEIGHT)), + (GENESIS_CORE_HEIGHT + 1, None), + ] { + let (platform, reads) = + platform_with_core(platform_version, v24_from(activation_height)); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + block_state.set_genesis_block_info(Some(BlockInfo { + core_height: GENESIS_CORE_HEIGHT, + ..Default::default() + })); + assert_eq!( + block_state.last_committed_core_height(), + GENESIS_CORE_HEIGHT + ); + + platform + .update_core_info( + None, + &mut block_state, + GENESIS_CORE_HEIGHT, + true, + &BlockInfo { + core_height: GENESIS_CORE_HEIGHT, + ..Default::default() + }, + &transaction, + platform_version, + ) + .expect("expected to update the core info"); + + assert_eq!(*reads.lock().unwrap(), vec![GENESIS_CORE_HEIGHT]); + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + expected, + "V24 active from {activation_height}" + ); + } + } + + /// The record is a cache of Core's answer, not a new rule: after every block that moves the + /// chain locked height, "recorded" equals Core's answer at that height, whatever the heights + /// and however far they jump past the activation, and Core is asked only until it is + /// recorded. + #[test] + fn should_agree_with_core_after_every_block_that_moves_the_chain_locked_height() { + let platform_version = PlatformVersion::latest(); + let cases: [(u32, &[u32], Option); 6] = [ + // Activation inside the run, one height at a time. + (103, &[101, 102, 103, 104, 105], Some(103)), + // A jump over the activation height records the height jumped to. + (103, &[101, 101, 102, 110, 110, 111], Some(110)), + // Unchanged heights before and after the activation. + (102, &[100, 100, 102, 102, 102, 103], Some(102)), + // Active before the run; the first block keeps the committed height, 100. + (50, &[100, 101, 101, 140], Some(101)), + // Never active in the run. + (1_000, &[101, 120, 120, 999], None), + // Activation exactly at a height reached by a jump. + (120, &[101, 120, 121], Some(120)), + ]; + for (activation_height, core_heights, expected) in cases { + let core = v24_from(activation_height); + let core_says_active = |core_height| core(core_height).unwrap() == Some(4); + let (platform, reads) = + platform_with_core(platform_version, v24_from(activation_height)); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + let mut previous_core_height = 100; + let mut expected_reads = vec![]; + let mut moved = false; + for &core_height in core_heights { + let recorded_before = recorded_v24(&platform, &transaction, platform_version); + run_block( + &platform, + &mut block_state, + previous_core_height, + core_height, + &transaction, + platform_version, + ) + .expect("expected to update the core info"); + if core_height != previous_core_height { + moved = true; + if recorded_before.is_none() { + expected_reads.push(core_height); + } + } + if moved { + assert_eq!( + recorded_v24(&platform, &transaction, platform_version).is_some(), + core_says_active(core_height), + "activation {activation_height}, heights {core_heights:?}, at {core_height}" + ); + } + previous_core_height = core_height; + } + assert_eq!( + recorded_v24(&platform, &transaction, platform_version), + expected, + "activation {activation_height}, heights {core_heights:?}" + ); + assert_eq!( + *reads.lock().unwrap(), + expected_reads, + "Core must be asked at each height the chain locked height moves to, until V24 \ + is recorded, and nowhere else" + ); + } + } + + /// Protocol version 14 shipped without the record: its blocks neither ask Core about the + /// coinbase nor write anything for a Core fork. + #[test] + fn should_neither_ask_core_nor_record_at_protocol_version_14() { + let platform_version = PlatformVersion::get(14).expect("expected protocol version 14"); + let (platform, reads) = platform_with_core(platform_version, v24_from(0)); + let transaction = platform.drive.grove.start_transaction(); + let mut block_state = platform.state.load().as_ref().clone(); + let root_hash = |transaction: &Transaction| { + platform + .drive + .grove + .root_hash(Some(transaction), &platform_version.drive.grove_version) + .unwrap() + .expect("expected a root hash") + }; + let root_before = root_hash(&transaction); + + run_block( + &platform, + &mut block_state, + 100, + 101, + &transaction, + platform_version, + ) + .expect("expected to update the core info"); + + assert!(reads.lock().unwrap().is_empty()); + assert_eq!(root_hash(&transaction), root_before); + assert!(platform + .drive + .fetch_core_fork_activation_height( + CoreFork::V24, + Some(&transaction), + PlatformVersion::latest() + ) + .expect("expected to read the V24 record") + .is_none()); + } +} 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 09295aac711..1fd9324bd58 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 @@ -131,6 +131,8 @@ fn platform_with_core( .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() .set_genesis_state(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_protx_diff_with_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 ba98310d003..4024e7420de 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 @@ -7,7 +7,7 @@ mod v1; use crate::error::execution::ExecutionError; use crate::error::Error; use crate::platform_types::platform::Platform; -use crate::rpc::core::CoreRPCLike; +use crate::rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}; use dpp::bls::PublicKey as BlsPublicKey; use dpp::dashcore::ProTxHash; use dpp::dashcore_rpc::dashcore_rpc_json::{DMNStateDiff, MasternodeListDiff, MasternodeListItem}; @@ -26,11 +26,6 @@ fn without_operator_payout_changes(mut masternode_diff: MasternodeListDiff) -> M masternode_diff } -/// The coinbase payload version Dash Core requires while DEPLOYMENT_V24 is active -/// (`MERKLE_ROOT_ASSETUNLOCKS`). Core requires it exactly at the blocks where V24 and V20 are -/// active and refuses it at every other block; V20 is active on every chain Platform runs on. -const V24_COINBASE_PAYLOAD_VERSION: u16 = 4; - /// The first byte of the point at infinity, which has the same 48 bytes in both schemes. const BLS_INFINITY_FIRST_BYTE: u8 = 0xc0; /// The legacy scheme's only flag: bit 7 of the first byte is the sign of y. Bits 5 and 6 are @@ -129,10 +124,7 @@ where #[cfg(test)] mod tests { - use super::{ - is_basic_re_encoding_of_legacy_key, without_operator_payout_changes, - V24_COINBASE_PAYLOAD_VERSION, - }; + use super::{is_basic_re_encoding_of_legacy_key, without_operator_payout_changes}; use crate::error::execution::ExecutionError; use crate::error::Error; use crate::execution::platform_events::core_based_updates::update_masternode_list::operator_key_encoding_tests::{ @@ -142,6 +134,7 @@ mod tests { }; use crate::config::PlatformConfig; use crate::platform_types::platform_state::{PlatformState, PlatformStateV0Methods}; + use crate::rpc::core::V24_COINBASE_PAYLOAD_VERSION; use crate::test::helpers::setup::TestPlatformBuilder; use dpp::dashcore::{Network, ProTxHash}; use dpp::dashcore_rpc::dashcore_rpc_json::{DMNStateDiff, MasternodeListDiff}; @@ -282,6 +275,8 @@ mod tests { (V24_COINBASE_PAYLOAD_VERSION, true), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_coinbase_payload_version() @@ -314,6 +309,8 @@ mod tests { Ok(None), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); let expect_none = answer.is_ok(); platform .core_rpc @@ -341,7 +338,9 @@ mod tests { #[test] fn should_keep_other_entries_without_asking_core() { let masternodes = recorded_masternodes(); - let platform = TestPlatformBuilder::new().build_with_mock_rpc(); + let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, so asking Core fails the test. + platform.core_rpc.checkpoint(); let unlisted = ProTxHash::from_str(&"11".repeat(32)).expect("expected a pro tx hash"); let mut entries = vec![ ( @@ -391,6 +390,8 @@ mod tests { .collect(), }; let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); platform .core_rpc .expect_get_coinbase_payload_version() @@ -438,6 +439,8 @@ mod tests { (1, PlatformVersion::latest()), ] { let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); + // Without the default answers, which would shadow the ones set here. + platform.core_rpc.checkpoint(); let reported = reported.clone(); platform .core_rpc diff --git a/packages/rs-drive-abci/src/platform_types/platform/mock.rs b/packages/rs-drive-abci/src/platform_types/platform/mock.rs index 8b5fb08caaf..539d84a480b 100644 --- a/packages/rs-drive-abci/src/platform_types/platform/mock.rs +++ b/packages/rs-drive-abci/src/platform_types/platform/mock.rs @@ -44,6 +44,13 @@ impl Platform { .expect_get_credit_pool_balance() .returning(|_| Ok(1_000_000_000_000_000)); + // A coinbase of payload version 3 at every height, a Core where V24 is not active, so + // no block records V24's activation. The same rule as above applies to an answer a test + // needs instead. + core_rpc_mock + .expect_get_coinbase_payload_version() + .returning(|_| Ok(Some(3))); + Self::open_with_client(path, config, core_rpc_mock, initial_protocol_version) } diff --git a/packages/rs-drive-abci/src/rpc/core.rs b/packages/rs-drive-abci/src/rpc/core.rs index e16e4e79863..2208254390d 100644 --- a/packages/rs-drive-abci/src/rpc/core.rs +++ b/packages/rs-drive-abci/src/rpc/core.rs @@ -58,6 +58,13 @@ pub(crate) fn credit_pool_balance_from_coinbase(coinbase: &[u8]) -> Result>, + } + + impl Core { + fn new(v24_activation_height: u32) -> Arc { + Arc::new(Self { + chain_locked_height: AtomicU32::new(1), + v24_activation_height, + coinbase_down: AtomicBool::new(false), + coinbase_reads: Mutex::new(vec![]), + }) + } + + fn chain_lock_at(&self, core_height: u32) { + self.chain_locked_height + .store(core_height, Ordering::SeqCst); + } + + fn coinbase_reads(&self) -> Vec { + self.coinbase_reads.lock().unwrap().clone() + } + } + + fn strategy(upgrading_info: Option) -> NetworkStrategy { + NetworkStrategy { + total_hpmns: 50, + validator_quorum_count: 24, + chain_lock_quorum_count: 24, + upgrading_info, + // Mock Core block hashes for every height these tests chain lock; `Core` reports + // which height is chain locked. + core_height_increase: CoreHeightIncrease::KnownCoreHeightIncreases( + (1..=MAX_CORE_HEIGHT).collect(), + ), + ..Default::default() + } + } + + fn config() -> PlatformConfig { + PlatformConfig { + validator_set: ValidatorSetConfig { + quorum_size: 30, + ..Default::default() + }, + chain_lock: ChainLockConfig::default_100_67(), + instant_lock: InstantLockConfig::default_100_67(), + execution: ExecutionConfig { + verify_sum_trees: true, + epoch_time_length_s: BLOCKS_PER_EPOCH, + ..Default::default() + }, + block_spacing_ms: BLOCK_SPACING_MS, + testing_configs: PlatformTestConfig { + store_platform_state: true, + ..PlatformTestConfig::default_minimal_verifications() + }, + ..Default::default() + } + } + + /// A platform at `protocol_version` whose Core is `core`. Core's credit pool is large + /// enough that the Core-anchored withdrawal limit never binds. + fn platform_with_core( + protocol_version: ProtocolVersion, + core: &Arc, + ) -> TempPlatform { + let mut platform = TestPlatformBuilder::new() + .with_config(config()) + .with_initial_protocol_version(protocol_version) + .build_with_mock_rpc(); + + // The default answers would shadow the ones below. + platform.core_rpc.checkpoint(); + platform + .core_rpc + .expect_get_block_hash() + .returning(|_| Ok(BlockHash::all_zeros())); + platform + .core_rpc + .expect_get_block_json() + .returning(|_| Ok(serde_json::json!({ "tx": [] }))); + // 10 million Dash, in duffs. + platform + .core_rpc + .expect_get_credit_pool_balance() + .returning(|_| Ok(1_000_000_000_000_000)); + + let state = Arc::clone(core); + platform + .core_rpc + .expect_get_coinbase_payload_version() + .returning(move |core_height| { + state.coinbase_reads.lock().unwrap().push(core_height); + if state.coinbase_down.load(Ordering::SeqCst) { + return Err(CoreRpcError::UnexpectedStructure( + "Core is down".to_string(), + )); + } + let v24_active = core_height >= state.v24_activation_height; + Ok(Some(if v24_active { 4 } else { 3 })) + }); + + let state = Arc::clone(core); + platform + .core_rpc + .expect_get_best_chain_lock() + .returning(move || { + Ok(ChainLock { + block_height: state.chain_locked_height.load(Ordering::SeqCst), + block_hash: BlockHash::from_byte_array([1; 32]), + signature: [2; 96].into(), + }) + }); + + platform + } + + /// The node restarted from its data directory, with the same Core + fn restarted(platform: TempPlatform) -> TempPlatform { + let TempPlatform { + platform: mut stopped, + tempdir, + } = platform; + let core_rpc = std::mem::take(&mut stopped.core_rpc); + drop(stopped); + let mut platform = TempPlatform::open_with_tempdir(tempdir, config()); + platform.platform.core_rpc = core_rpc; + platform + } + + fn continuation(outcome: &ChainExecutionOutcome, block_count: u64) -> ChainExecutionParameters { + ChainExecutionParameters { + block_start: outcome + .abci_app + .platform + .state + .load() + .last_committed_block_height() + + 1, + core_height_start: 1, + block_count, + proposers: outcome.proposers.clone(), + validator_quorums: outcome.validator_quorums.clone(), + current_validator_quorum_hash: outcome.current_validator_quorum_hash, + current_proposer_versions: Some(outcome.current_proposer_versions.clone()), + current_identity_nonce_counter: outcome.identity_nonce_counter.clone(), + current_identity_contract_nonce_counter: outcome + .identity_contract_nonce_counter + .clone(), + current_votes: BTreeMap::default(), + start_time_ms: GENESIS_TIME_MS, + current_time_ms: outcome.end_time_ms, + instant_lock_quorums: outcome.instant_lock_quorums.clone(), + current_identities: Vec::new(), + current_addresses_with_balance: AddressesWithBalance::default(), + } + } + + /// Commits `block_count` blocks after `outcome` at the height `core` chain locks + async fn continue_chain<'a>( + platform: &'a Platform, + outcome: &ChainExecutionOutcome<'_>, + block_count: u64, + upgrading_info: Option, + seed: u64, + ) -> ChainExecutionOutcome<'a> { + continue_chain_for_strategy( + FullAbciApplication::new(platform), + continuation(outcome, block_count), + strategy(upgrading_info), + config(), + StrategyRandomness::SeedEntropy(seed), + ) + .await + } + + /// The committed V24 record + fn committed_v24(platform: &Platform) -> Option { + platform + .drive + .fetch_core_fork_activation_height(CoreFork::V24, None, PlatformVersion::latest()) + .expect("expected to read the V24 record") + } + + /// Once V24 is recorded Platform never asks Core about it again, also after a restart from + /// disk, so nothing node-local stands in for the record: a node whose Core then refuses to + /// tell the coinbase payload version commits the same chain as a node whose Core answers. + /// Only the V24 question fails here; the withdrawal scan still reads Core's credit pool in + /// the blocks that move the chain locked height, as it does at protocol version 14. + #[stack_size(4 * 1024 * 1024)] + #[test] + async fn should_never_ask_core_about_v24_again_once_it_is_recorded_even_after_a_restart() { + let mut app_hashes = vec![]; + for core_down_after_a_restart in [false, true] { + let core = Core::new(3); + let mut platform = + platform_with_core(PlatformVersion::latest().protocol_version, &core); + + let outcome = run_chain_for_strategy( + &mut platform, + 3, + strategy(None), + config(), + 40, + &mut None, + &mut None, + ) + .await; + assert_eq!( + core.coinbase_reads(), + vec![1], + "only init chain asks, at the genesis core height" + ); + assert_eq!(committed_v24(outcome.abci_app.platform), None); + + core.chain_lock_at(3); + let recorded = continue_chain(outcome.abci_app.platform, &outcome, 2, None, 41).await; + drop(outcome); + assert_eq!(committed_v24(recorded.abci_app.platform), Some(3)); + assert_eq!(core.coinbase_reads(), vec![1, 3]); + let parameters = continuation(&recorded, 2); + drop(recorded); + + if core_down_after_a_restart { + platform = restarted(platform); + core.coinbase_down.store(true, Ordering::SeqCst); + } + + let mut parameters = parameters; + for (core_height, seed) in [(4, 42), (6, 43)] { + core.chain_lock_at(core_height); + let outcome = continue_chain_for_strategy( + FullAbciApplication::new(&platform), + parameters, + strategy(None), + config(), + StrategyRandomness::SeedEntropy(seed), + ) + .await; + assert_eq!( + outcome + .abci_app + .platform + .state + .load() + .last_committed_core_height(), + core_height + ); + parameters = continuation(&outcome, 2); + } + + assert_eq!(committed_v24(&platform), Some(3)); + assert_eq!( + core.coinbase_reads(), + vec![1, 3], + "Core must not be asked about V24 once it is recorded" + ); + let state = platform.state.load(); + assert_eq!(state.last_committed_block_height(), 9); + app_hashes.push(state.last_committed_block_app_hash()); + } + assert_eq!( + app_hashes[0], app_hashes[1], + "the node whose Core went down after a restart must commit the same chain" + ); + } + + /// The upgrade block from protocol version 14 asks Core nothing when it keeps the chain + /// locked height, so V24 is recorded at the first protocol version 15 block that moves it. + /// A node reopened from disk right after the upgrade block keeps its last committed core + /// height, 2 rather than the genesis core height, asks nothing in the next block either, and + /// commits the same chain as a warm node. When the upgrade block itself moves the height, it + /// records V24 there. + #[stack_size(4 * 1024 * 1024)] + #[test] + async fn should_record_v24_at_the_first_protocol_version_15_block_that_moves_the_chain_locked_height( + ) { + let previous_version = PlatformVersion::get(14).expect("expected protocol version 14"); + let latest_version = PlatformVersion::latest(); + let upgrading_info = Some(UpgradingInfo { + current_protocol_version: previous_version.protocol_version, + proposed_protocol_versions_with_weight: vec![(latest_version.protocol_version, 1)], + upgrade_three_quarters_life: 0.0, + }); + let mut app_hashes = vec![]; + for (activation_block_moves_core_height, reopen_after_activation) in + [(false, false), (false, true), (true, false)] + { + // V24 is active at every Core height these blocks chain lock. + let core = Core::new(1); + let mut platform = platform_with_core(previous_version.protocol_version, &core); + + let epoch_1 = run_chain_for_strategy( + &mut platform, + 2 * BLOCKS_PER_EPOCH - 1, + strategy(upgrading_info.clone()), + config(), + 13, + &mut None, + &mut None, + ) + .await; + // The last protocol version 14 block moves the chain locked height. + core.chain_lock_at(2); + let before_activation = continue_chain( + epoch_1.abci_app.platform, + &epoch_1, + 1, + upgrading_info.clone(), + 17, + ) + .await; + drop(epoch_1); + { + let state = before_activation.abci_app.platform.state.load(); + assert_eq!(state.current_protocol_version_in_consensus(), 14); + assert_eq!( + state.next_epoch_protocol_version(), + latest_version.protocol_version + ); + assert_eq!(state.last_committed_core_height(), 2); + } + assert!( + core.coinbase_reads().is_empty(), + "protocol version 14 never asks Core about V24" + ); + assert_eq!(committed_v24(before_activation.abci_app.platform), None); + + if activation_block_moves_core_height { + core.chain_lock_at(3); + } + let activation = continue_chain( + before_activation.abci_app.platform, + &before_activation, + 1, + upgrading_info.clone(), + 18, + ) + .await; + drop(before_activation); + assert_eq!( + activation + .abci_app + .platform + .state + .load() + .current_protocol_version_in_consensus(), + latest_version.protocol_version + ); + + if activation_block_moves_core_height { + assert_eq!(committed_v24(activation.abci_app.platform), Some(3)); + assert_eq!(core.coinbase_reads(), vec![3]); + continue; + } + + // The window: protocol version 15, V24 active in Core, not recorded yet. + assert_eq!(committed_v24(activation.abci_app.platform), None); + assert!(core.coinbase_reads().is_empty()); + let parameters = continuation(&activation, 1); + drop(activation); + + if reopen_after_activation { + platform = restarted(platform); + assert_eq!(platform.state.load().last_committed_core_height(), 2); + } + + let window = continue_chain_for_strategy( + FullAbciApplication::new(&platform), + parameters, + strategy(upgrading_info.clone()), + config(), + StrategyRandomness::SeedEntropy(19), + ) + .await; + assert_eq!(committed_v24(window.abci_app.platform), None); + assert!(core.coinbase_reads().is_empty()); + + core.chain_lock_at(3); + let recorded = continue_chain( + window.abci_app.platform, + &window, + 1, + upgrading_info.clone(), + 20, + ) + .await; + drop(window); + assert_eq!(committed_v24(recorded.abci_app.platform), Some(3)); + assert_eq!(core.coinbase_reads(), vec![3]); + app_hashes.push( + recorded + .abci_app + .platform + .state + .load() + .last_committed_block_app_hash(), + ); + } + assert_eq!( + app_hashes[0], app_hashes[1], + "a node reopened inside the window must commit the same chain as a warm node" + ); + } + + /// The proposal of `round` for the next height, at chain locked `core_chain_locked_height` + fn proposal( + outcome: &ChainExecutionOutcome, + round: u32, + core_chain_locked_height: u32, + ) -> RequestProcessProposal { + let platform_state = outcome.abci_app.platform.state.load(); + let height = platform_state.last_committed_block_height() + 1; + let time_ms = outcome.end_time_ms + BLOCK_SPACING_MS + round as u64 * 1000; + let proposer = outcome.proposers[round as usize].pro_tx_hash(); + + RequestProcessProposal { + txs: vec![], + proposed_last_commit: None, + misbehavior: vec![], + hash: [0xA0 + round as u8; 32].to_vec(), + height: height as i64, + time: Some(Timestamp::from_millis(time_ms).expect("expected a block time")), + next_validators_hash: [0u8; 32].to_vec(), + round: round as i32, + core_chain_locked_height, + core_chain_lock_update: None, + proposer_pro_tx_hash: proposer.to_byte_array().to_vec(), + proposed_app_version: PlatformVersion::latest().protocol_version as u64, + version: Some(Consensus { + block: 0, + app: PlatformVersion::latest().protocol_version as u64, + }), + quorum_hash: outcome + .current_quorum() + .quorum_hash + .to_byte_array() + .to_vec(), + } + } + + /// The request Tenderdash finalizes `proposal` with once it is committed in its own round, + /// with `app_hash` in the block header + fn finalize_request( + proposal: &RequestProcessProposal, + app_hash: Vec, + ) -> RequestFinalizeBlock { + RequestFinalizeBlock { + commit: Some(CommitInfo { + round: proposal.round, + quorum_hash: proposal.quorum_hash.clone(), + block_signature: vec![0u8; 96], + threshold_vote_extensions: vec![], + }), + misbehavior: vec![], + hash: proposal.hash.clone(), + height: proposal.height, + round: proposal.round, + block: Some(Block { + header: Some(Header { + version: proposal.version, + chain_id: CHAIN_ID.to_string(), + height: proposal.height, + time: proposal.time, + last_block_id: None, + last_commit_hash: vec![], + data_hash: vec![0u8; 32], + validators_hash: proposal.quorum_hash.clone(), + next_validators_hash: proposal.quorum_hash.clone(), + consensus_hash: vec![0u8; 32], + next_consensus_hash: vec![0u8; 32], + app_hash, + results_hash: vec![0u8; 32], + evidence_hash: vec![], + proposed_app_version: proposal.proposed_app_version, + proposer_pro_tx_hash: proposal.proposer_pro_tx_hash.clone(), + core_chain_locked_height: proposal.core_chain_locked_height, + }), + data: Some(Data { + txs: proposal.txs.clone(), + }), + evidence: Some(EvidenceList { evidence: vec![] }), + last_commit: None, + core_chain_lock: proposal.core_chain_lock_update.clone(), + }), + block_id: Some(BlockId { + hash: proposal.hash.clone(), + part_set_header: Some(PartSetHeader { + total: 0, + hash: vec![0u8; 32], + }), + state_id: vec![0u8; 32], + }), + } + } + + /// Processes `proposal`, which this node must accept, and returns its app hash + fn accept(outcome: &ChainExecutionOutcome, proposal: &RequestProcessProposal) -> Vec { + let response = outcome + .abci_app + .process_proposal(proposal.clone()) + .expect("expected to process the proposal"); + assert_eq!(response.status, ProposalStatus::Accept as i32); + response.app_hash + } + + /// The V24 record in the transaction of the block being executed + fn uncommitted_v24(outcome: &ChainExecutionOutcome) -> Option { + let transaction = outcome + .abci_app + .transaction + .read() + .expect("expected the block transaction"); + outcome + .abci_app + .platform + .drive + .fetch_core_fork_activation_height( + CoreFork::V24, + transaction.as_ref(), + PlatformVersion::latest(), + ) + .expect("expected to read the V24 record") + } + + /// A round of a height runs in the transaction left by the last commit, or by init chain + /// for height 1, and a round that is thrown away takes its record with it: the committed + /// record is the committed round's. Round 0 carries a new chain locked height, 3, where V24 + /// is active, and records it, or fails on a Core error. Round 1 is committed, at the last + /// committed height (no question to Core), at 3 again, or at 4. For height 1, a record + /// written at init chain survives every round. + #[stack_size(4 * 1024 * 1024)] + #[test] + async fn should_commit_only_the_v24_record_of_the_round_that_is_committed() { + struct Case { + blocks_before: u64, + v24_activation_height: u32, + round_0_core_down: bool, + round_1_core_height: u32, + after_round_0: Option, + committed: Option, + coinbase_reads: Vec, + } + let cases = [ + // Recorded at init chain, at the genesis core height. + Case { + blocks_before: 0, + v24_activation_height: 1, + round_0_core_down: false, + round_1_core_height: 1, + after_round_0: Some(1), + committed: Some(1), + coinbase_reads: vec![1], + }, + Case { + blocks_before: 0, + v24_activation_height: 3, + round_0_core_down: false, + round_1_core_height: 1, + after_round_0: Some(3), + committed: None, + coinbase_reads: vec![1, 3], + }, + Case { + blocks_before: 0, + v24_activation_height: 3, + round_0_core_down: false, + round_1_core_height: 4, + after_round_0: Some(3), + committed: Some(4), + coinbase_reads: vec![1, 3, 4], + }, + Case { + blocks_before: 2, + v24_activation_height: 3, + round_0_core_down: false, + round_1_core_height: 1, + after_round_0: Some(3), + committed: None, + coinbase_reads: vec![1, 3], + }, + Case { + blocks_before: 2, + v24_activation_height: 3, + round_0_core_down: false, + round_1_core_height: 4, + after_round_0: Some(3), + committed: Some(4), + coinbase_reads: vec![1, 3, 4], + }, + // Round 1 asks Core again rather than reuse round 0's answer. + Case { + blocks_before: 2, + v24_activation_height: 3, + round_0_core_down: false, + round_1_core_height: 3, + after_round_0: Some(3), + committed: Some(3), + coinbase_reads: vec![1, 3, 3], + }, + // Round 0 fails on the Core error and writes nothing. + Case { + blocks_before: 2, + v24_activation_height: 3, + round_0_core_down: true, + round_1_core_height: 3, + after_round_0: None, + committed: Some(3), + coinbase_reads: vec![1, 3, 3], + }, + ]; + for case in cases { + let core = Core::new(case.v24_activation_height); + let mut platform = + platform_with_core(PlatformVersion::latest().protocol_version, &core); + let outcome = run_chain_for_strategy( + &mut platform, + case.blocks_before, + NetworkStrategy { + // Without a block, the runner stops after init chain, whose transaction is + // not committed yet, instead of reading the committed state. + failure_testing: (case.blocks_before == 0).then(|| FailureStrategy { + dont_finalize_block: true, + ..Default::default() + }), + ..strategy(None) + }, + config(), + 15, + &mut None, + &mut None, + ) + .await; + let state = outcome.abci_app.platform.state.load(); + assert_eq!( + state.last_committed_block_height(), + case.blocks_before, + "the committed blocks" + ); + assert_eq!(state.last_committed_core_height(), 1); + drop(state); + let label = format!( + "after {} blocks, round 0 Core down {}, round 1 at core height {}", + case.blocks_before, case.round_0_core_down, case.round_1_core_height + ); + + let round_0 = proposal(&outcome, 0, 3); + core.coinbase_down + .store(case.round_0_core_down, Ordering::SeqCst); + let round_0_result = outcome.abci_app.process_proposal(round_0); + core.coinbase_down.store(false, Ordering::SeqCst); + if case.round_0_core_down { + assert!(round_0_result.is_err(), "{label}: round 0 must fail"); + } else { + let response = round_0_result.expect("expected to process round 0"); + assert_eq!(response.status, ProposalStatus::Accept as i32, "{label}"); + } + assert_eq!(uncommitted_v24(&outcome), case.after_round_0, "{label}"); + + let round_1 = proposal(&outcome, 1, case.round_1_core_height); + let app_hash = accept(&outcome, &round_1); + outcome + .abci_app + .finalize_block(finalize_request(&round_1, app_hash)) + .expect("expected to finalize round 1"); + assert_eq!( + committed_v24(outcome.abci_app.platform), + case.committed, + "{label}" + ); + assert_eq!(core.coinbase_reads(), case.coinbase_reads, "{label}"); + } + } +} diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs index 4190abd35c4..032d644d686 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs @@ -2,6 +2,7 @@ mod address_tests; mod basic_tests; mod chain_lock_update_tests; mod comprehensive_tests; +mod core_fork_activation_tests; mod core_height_increase; mod core_update_tests; mod credit_pool_backfill_tests; diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs index 85c8007e9dc..fda1c95ffdb 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs @@ -3129,6 +3129,11 @@ mod tests { .core_rpc .expect_get_credit_pool_balance() .returning(|_| Ok(dash_to_duffs!(120))); + // V24 is not active. + platform + .core_rpc + .expect_get_coinbase_payload_version() + .returning(|_| Ok(Some(3))); platform .core_rpc .expect_send_raw_transaction() diff --git a/packages/rs-drive/grovedb-structure.json b/packages/rs-drive/grovedb-structure.json index 5615ae6ea1d..f3a50384023 100644 --- a/packages/rs-drive/grovedb-structure.json +++ b/packages/rs-drive/grovedb-structure.json @@ -4425,7 +4425,7 @@ "since": 1, "presence": "always", "source": "packages/rs-drive/src/drive/mod.rs", - "description": "Chain wide values: total credits, total token supplies, the genesis core height, and the documents waiting to expire.", + "description": "Chain wide values: total credits, total token supplies, the genesis core height, the Core forks Platform has seen active, and the documents waiting to expire.", "children": [ { "id": "misc.genesis_core_height", @@ -4529,6 +4529,24 @@ } ] }, + { + "id": "misc.core_fork_v24", + "key": { + "type": "fixed", + "hex": "4600", + "label": "CoreForkV24Activation", + "constant": "CORE_FORK_V24_ACTIVATION_KEY" + }, + "kinds": [ + "Item" + ], + "value": "u32 big endian", + "since": 15, + "presence": "lazy", + "source": "packages/rs-drive/src/drive/system/core_fork_activations/mod.rs", + "description": "The chain locked Core height of the first block that saw Dash Core's V24 active: the coinbase of that Core block has payload version 4 or higher. Written once, by a block that moves the chain locked height, or at init chain; never changed. The key is `F` and the fork's id.", + "children": [] + }, { "id": "misc.total_token_supplies", "key": { diff --git a/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/mod.rs b/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/mod.rs new file mode 100644 index 00000000000..02d920e84a0 --- /dev/null +++ b/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/mod.rs @@ -0,0 +1,51 @@ +mod v0; + +use crate::drive::system::core_fork_activations::CoreFork; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::prelude::CoreBlockHeight; +use dpp::version::PlatformVersion; +use grovedb::TransactionArg; + +impl Drive { + /// Fetches the chain locked Core height of the first Platform block that saw `fork` active. + /// + /// Read in a block after its Core info is updated, `Some` means the fork is active at the + /// block's chain locked height. `None` means Platform has not seen it active, not that it + /// is inactive: a block that keeps the chain locked height does not look. + /// + /// # Parameters + /// - `fork`: the Core fork. + /// - `transaction`: the transaction to read in; in a block, the block's transaction. + /// - `platform_version`: the platform version. + /// + /// # Returns + /// The recorded height, or `None` when nothing is recorded. + pub fn fetch_core_fork_activation_height( + &self, + fork: CoreFork, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result, Error> { + match platform_version + .drive + .methods + .platform_system + .fetch_core_fork_activation_height + { + Some(0) => { + self.fetch_core_fork_activation_height_v0(fork, transaction, platform_version) + } + Some(version) => Err(Error::Drive(DriveError::UnknownVersionMismatch { + method: "fetch_core_fork_activation_height".to_string(), + known_versions: vec![0], + received: version, + })), + None => Err(Error::Drive(DriveError::VersionNotActive { + method: "fetch_core_fork_activation_height".to_string(), + known_versions: vec![0], + })), + } + } +} diff --git a/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/v0/mod.rs b/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/v0/mod.rs new file mode 100644 index 00000000000..7969dc7c97a --- /dev/null +++ b/packages/rs-drive/src/drive/system/core_fork_activations/fetch_core_fork_activation_height/v0/mod.rs @@ -0,0 +1,38 @@ +use crate::drive::system::core_fork_activations::CoreFork; +use crate::drive::system::misc_path; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use crate::util::grove_operations::DirectQueryType; +use dpp::prelude::CoreBlockHeight; +use dpp::version::PlatformVersion; +use grovedb::TransactionArg; + +impl Drive { + #[inline(always)] + pub(super) fn fetch_core_fork_activation_height_v0( + &self, + fork: CoreFork, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result, Error> { + let Some(element) = self.grove_get_raw_optional( + (&misc_path()).into(), + fork.activation_key(), + DirectQueryType::StatefulDirectQuery, + transaction, + &mut vec![], + &platform_version.drive, + )? + else { + return Ok(None); + }; + let bytes = element.into_item_bytes()?; + let core_height = u32::from_be_bytes(bytes.try_into().map_err(|_| { + Error::Drive(DriveError::CorruptedSerialization( + "a Core fork activation height must be a u32".to_string(), + )) + })?); + Ok(Some(core_height)) + } +} diff --git a/packages/rs-drive/src/drive/system/core_fork_activations/mod.rs b/packages/rs-drive/src/drive/system/core_fork_activations/mod.rs new file mode 100644 index 00000000000..62194172b95 --- /dev/null +++ b/packages/rs-drive/src/drive/system/core_fork_activations/mod.rs @@ -0,0 +1,195 @@ +//! The Dash Core forks Platform has seen active, each with the chain locked Core height of the +//! first Platform block that saw it. + +mod fetch_core_fork_activation_height; +mod store_core_fork_activation_height; + +/// The first byte of every Core fork activation key in `Misc`; the fork's id follows it. +pub const CORE_FORK_ACTIVATION_KEY_PREFIX: u8 = b'F'; + +/// The `Misc` key holding the Core height at which Platform first saw Core's V24 active +pub const CORE_FORK_V24_ACTIVATION_KEY: &[u8; 2] = + &[CORE_FORK_ACTIVATION_KEY_PREFIX, CoreFork::V24 as u8]; + +/// A Dash Core fork whose activation Platform records. +/// +/// The discriminant is the fork's id in its `Misc` key, so it never changes. +// @append_only +#[repr(u8)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CoreFork { + /// Dash Core's `DEPLOYMENT_V24` + V24 = 0, +} + +impl CoreFork { + /// The `Misc` key holding the fork's activation height + pub const fn activation_key(self) -> &'static [u8; 2] { + match self { + CoreFork::V24 => CORE_FORK_V24_ACTIVATION_KEY, + } + } +} + +#[cfg(feature = "server")] +#[cfg(test)] +mod tests { + use super::{CoreFork, CORE_FORK_V24_ACTIVATION_KEY}; + use crate::drive::system::misc_path; + use crate::error::drive::DriveError; + use crate::error::Error; + use crate::util::grove_operations::DirectQueryType; + use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; + use dpp::version::PlatformVersion; + use grovedb::Element; + + #[test] + fn should_return_none_for_a_core_fork_never_recorded() { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + let transaction = drive.grove.start_transaction(); + + assert_eq!( + drive + .fetch_core_fork_activation_height( + CoreFork::V24, + Some(&transaction), + platform_version + ) + .expect("expected to read an absent record"), + None + ); + } + + /// The record is a big endian `u32` item without flags at `[b'F', 0]` in `Misc`, which + /// clients can prove through `getPathElements`. It is visible in the transaction that + /// wrote it before that transaction commits, and nowhere else. + #[test] + fn should_read_back_the_observed_height() { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + let transaction = drive.grove.start_transaction(); + + drive + .store_core_fork_activation_height( + CoreFork::V24, + 1_234_567, + Some(&transaction), + platform_version, + ) + .expect("expected to record V24"); + + assert_eq!( + drive + .fetch_core_fork_activation_height( + CoreFork::V24, + Some(&transaction), + platform_version + ) + .expect("expected to read the record"), + Some(1_234_567) + ); + assert_eq!(CORE_FORK_V24_ACTIVATION_KEY, &[b'F', 0]); + assert_eq!( + drive + .grove_get_raw_optional( + (&misc_path()).into(), + CORE_FORK_V24_ACTIVATION_KEY, + DirectQueryType::StatefulDirectQuery, + Some(&transaction), + &mut vec![], + &platform_version.drive, + ) + .expect("expected to read the element"), + Some(Element::Item(1_234_567u32.to_be_bytes().to_vec(), None)) + ); + assert_eq!( + drive + .fetch_core_fork_activation_height(CoreFork::V24, None, platform_version) + .expect("expected to read outside the transaction"), + None, + "an uncommitted record must not be visible outside its transaction" + ); + } + + /// A recorded height is what every later block compares with, so recording the fork again + /// is a bug and leaves the first height in place. + #[test] + fn should_refuse_to_record_a_core_fork_twice() { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + let transaction = drive.grove.start_transaction(); + + drive + .store_core_fork_activation_height( + CoreFork::V24, + 1_000, + Some(&transaction), + platform_version, + ) + .expect("expected to record V24"); + + for core_height in [1_000, 2_000] { + let result = drive.store_core_fork_activation_height( + CoreFork::V24, + core_height, + Some(&transaction), + platform_version, + ); + assert!( + matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + ), + "recording V24 again at {core_height} must fail, got {result:?}" + ); + } + assert_eq!( + drive + .fetch_core_fork_activation_height( + CoreFork::V24, + Some(&transaction), + platform_version + ) + .expect("expected to read the record"), + Some(1_000) + ); + } + + #[test] + fn should_refuse_a_stored_height_that_is_not_four_bytes() { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + drive + .grove_insert( + (&misc_path()).into(), + CORE_FORK_V24_ACTIVATION_KEY, + Element::Item(vec![0, 0, 1], None), + None, + None, + &mut vec![], + &platform_version.drive, + ) + .expect("expected to write a malformed record"); + + assert!(matches!( + drive.fetch_core_fork_activation_height(CoreFork::V24, None, platform_version), + Err(Error::Drive(DriveError::CorruptedSerialization(_))) + )); + } + + #[test] + fn should_neither_read_nor_record_before_protocol_version_15() { + let platform_version = PlatformVersion::get(14).expect("expected protocol version 14"); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + + assert!(matches!( + drive.fetch_core_fork_activation_height(CoreFork::V24, None, platform_version), + Err(Error::Drive(DriveError::VersionNotActive { .. })) + )); + assert!(matches!( + drive.store_core_fork_activation_height(CoreFork::V24, 1_000, None, platform_version), + Err(Error::Drive(DriveError::VersionNotActive { .. })) + )); + } +} diff --git a/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/mod.rs b/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/mod.rs new file mode 100644 index 00000000000..3f89a92401d --- /dev/null +++ b/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/mod.rs @@ -0,0 +1,55 @@ +mod v0; + +use crate::drive::system::core_fork_activations::CoreFork; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::prelude::CoreBlockHeight; +use dpp::version::PlatformVersion; +use grovedb::TransactionArg; + +impl Drive { + /// Records `core_height` as the chain locked Core height of the first Platform block that + /// saw `fork` active. The record is written once and never changed: recording a fork again + /// is an error. The write is applied at once, so later reads in the transaction see it. + /// + /// # Parameters + /// - `fork`: the Core fork. + /// - `core_height`: the chain locked Core height of the block that saw it active. + /// - `transaction`: the transaction to write in; in a block, the block's transaction. + /// - `platform_version`: the platform version. + /// + /// # Returns + /// `Ok(())` once the height is written, `CorruptedCodeExecution` when the fork is already + /// recorded. + pub fn store_core_fork_activation_height( + &self, + fork: CoreFork, + core_height: CoreBlockHeight, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + match platform_version + .drive + .methods + .platform_system + .store_core_fork_activation_height + { + Some(0) => self.store_core_fork_activation_height_v0( + fork, + core_height, + transaction, + platform_version, + ), + Some(version) => Err(Error::Drive(DriveError::UnknownVersionMismatch { + method: "store_core_fork_activation_height".to_string(), + known_versions: vec![0], + received: version, + })), + None => Err(Error::Drive(DriveError::VersionNotActive { + method: "store_core_fork_activation_height".to_string(), + known_versions: vec![0], + })), + } + } +} diff --git a/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/v0/mod.rs b/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/v0/mod.rs new file mode 100644 index 00000000000..45393b34668 --- /dev/null +++ b/packages/rs-drive/src/drive/system/core_fork_activations/store_core_fork_activation_height/v0/mod.rs @@ -0,0 +1,34 @@ +use crate::drive::system::core_fork_activations::CoreFork; +use crate::drive::system::misc_path; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::prelude::CoreBlockHeight; +use dpp::version::PlatformVersion; +use grovedb::{Element, TransactionArg}; + +impl Drive { + #[inline(always)] + pub(super) fn store_core_fork_activation_height_v0( + &self, + fork: CoreFork, + core_height: CoreBlockHeight, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + let inserted = self.grove_insert_if_not_exists( + (&misc_path()).into(), + fork.activation_key(), + Element::Item(core_height.to_be_bytes().to_vec(), None), + transaction, + None, + &platform_version.drive, + )?; + if !inserted { + return Err(Error::Drive(DriveError::CorruptedCodeExecution( + "a Core fork activation is recorded only once", + ))); + } + Ok(()) + } +} diff --git a/packages/rs-drive/src/drive/system/mod.rs b/packages/rs-drive/src/drive/system/mod.rs index 53e62a973ef..0ed2f208897 100644 --- a/packages/rs-drive/src/drive/system/mod.rs +++ b/packages/rs-drive/src/drive/system/mod.rs @@ -1,3 +1,5 @@ +/// Core fork activations module +pub mod core_fork_activations; mod estimation_costs; mod fetch_elements; /// Genesis time module diff --git a/packages/rs-drive/src/drive/system/structure.rs b/packages/rs-drive/src/drive/system/structure.rs index 1defaccef89..825e21a9a8b 100644 --- a/packages/rs-drive/src/drive/system/structure.rs +++ b/packages/rs-drive/src/drive/system/structure.rs @@ -1,6 +1,7 @@ use crate::drive::balances::{TOTAL_SYSTEM_CREDITS_STORAGE_KEY, TOTAL_TOKEN_SUPPLIES_STORAGE_KEY}; use crate::drive::document::expiration::paths::DOCUMENTS_EXPIRATIONS_KEY; use crate::drive::initialization::genesis_core_height::GENESIS_CORE_HEIGHT_KEY; +use crate::drive::system::core_fork_activations::CORE_FORK_V24_ACTIVATION_KEY; use crate::drive::RootTree; use crate::structure::{ElementKind, KeyEncoding, KeyMatcher, StructureNode}; @@ -11,8 +12,9 @@ pub(crate) fn structure() -> StructureNode { .source("packages/rs-drive/src/drive/mod.rs") .describe( "Chain wide values: total credits, total token \ - supplies, the genesis core height, and the documents \ - waiting to expire.", + supplies, the genesis core height, the Core forks \ + Platform has seen active, and the documents waiting \ + to expire.", ) .children(vec![ StructureNode::fixed( @@ -31,6 +33,25 @@ pub(crate) fn structure() -> StructureNode { Written when the chain is initialized, after the \ structure.", ), + StructureNode::fixed( + "core_fork_v24", + CORE_FORK_V24_ACTIVATION_KEY, + "CoreForkV24Activation", + "CORE_FORK_V24_ACTIVATION_KEY", + ) + .kind(ElementKind::Item) + .lazy() + .since(15) + .source("packages/rs-drive/src/drive/system/core_fork_activations/mod.rs") + .value("u32 big endian") + .describe( + "The chain locked Core height of the first block that \ + saw Dash Core's V24 active: the coinbase of that Core \ + block has payload version 4 or higher. Written once, \ + by a block that moves the chain locked height, or at \ + init chain; never changed. The key is `F` and the \ + fork's id.", + ), StructureNode::fixed( "documents_expirations", DOCUMENTS_EXPIRATIONS_KEY, diff --git a/packages/rs-drive/src/structure/tests.rs b/packages/rs-drive/src/structure/tests.rs index 978dd03afc1..e793daf99ec 100644 --- a/packages/rs-drive/src/structure/tests.rs +++ b/packages/rs-drive/src/structure/tests.rs @@ -279,6 +279,7 @@ mod fixtures { use super::*; use crate::drive::credit_pools::epochs::operations_factory::EpochOperations; use crate::drive::credit_pools::operations::update_lifetime_storage_fee_pool_operation; + use crate::drive::system::core_fork_activations::CoreFork; use crate::drive::votes::resolved::vote_polls::contested_document_resource_vote_poll::ContestedDocumentResourceVotePollWithContractInfo; use crate::drive::Drive; use crate::fees::op::LowLevelDriveOperation; @@ -1241,6 +1242,16 @@ mod fixtures { conformance_of(&drive, "core_anchored_withdrawal_accounting", run); } + /// A Core fork Platform has seen active. + fn core_fork_activations(run: &mut FixtureRun) { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(Some(platform_version)); + drive + .store_core_fork_activation_height(CoreFork::V24, 1_000, None, platform_version) + .expect("expected to record V24"); + conformance_of(&drive, "core_fork_activations", run); + } + /// An epoch while it runs, then after it was paid out: payout deletes the /// proposers and both fee items and keeps the epoch tree. The finished /// epoch info is written at payout, so no epoch ever holds all nine keys. @@ -1877,6 +1888,7 @@ mod fixtures { tokens_and_group_actions(&mut run); address_balances(&mut run); core_anchored_withdrawal_accounting(&mut run); + core_fork_activations(&mut run); current_then_paid_epoch(&mut run); contested_documents(&mut run); yes_no_vote_polls(&mut run); diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs index 3961aec78ab..11a2e2eddb9 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs @@ -1,6 +1,7 @@ use super::v10::DRIVE_ABCI_METHOD_VERSIONS_V10; use super::{ - DkgMiningSchedule, DriveAbciCoreChainLockMethodVersionsAndConstants, DriveAbciMethodVersions, + DkgMiningSchedule, DriveAbciCoreBasedUpdatesMethodVersions, + DriveAbciCoreChainLockMethodVersionsAndConstants, DriveAbciMethodVersions, DriveAbciVotingMethodVersions, }; @@ -90,8 +91,12 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ /// Drive ABCI method versions 11, introduced in protocol v15. Differs from v10 in the yes/no /// poll kind's voting methods (`voting.tally_votes_for_yes_no_vote_poll`, /// `keep_record_of_finished_yes_no_vote_poll`, `clean_up_after_yes_no_vote_polls_end` and -/// `on_yes_no_vote_poll_finished` are `Some(0)`, `None` in every earlier table) and in the chain -/// lock verification, which only works as a whole: +/// `on_yes_no_vote_poll_finished` are `Some(0)`, `None` in every earlier table), in +/// `core_based_updates.update_core_info`, which is 1 rather than 0: init chain and a block that +/// move the chain locked Core height record, the first time they see it, that Core's V24 is +/// active there (the coinbase of that Core block has payload version 4 or higher), and Core is +/// not asked again once it is recorded; and in the chain lock verification, which only works as +/// a whole: /// - `core_chain_lock.verify_chain_lock_locally` is 1 rather than 0: a chain lock is rejected /// locally only when Platform holds exactly the quorums active at its signing height, using /// Dash Core's DKG mining schedule in `core_chain_lock.dkg_mining_schedules`. When a quorum may @@ -109,6 +114,10 @@ pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMet dkg_mining_schedules: Some(DKG_MINING_SCHEDULES_V1), ..DRIVE_ABCI_METHOD_VERSIONS_V10.core_chain_lock }, + core_based_updates: DriveAbciCoreBasedUpdatesMethodVersions { + update_core_info: 1, // changed in v15: records the chain locked Core height at which V24 is first seen active + ..DRIVE_ABCI_METHOD_VERSIONS_V10.core_based_updates + }, voting: DriveAbciVotingMethodVersions { tally_votes_for_yes_no_vote_poll: Some(0), keep_record_of_finished_yes_no_vote_poll: Some(0), diff --git a/packages/rs-platform-version/src/version/drive_versions/mod.rs b/packages/rs-platform-version/src/version/drive_versions/mod.rs index cd70ea2f37b..0bfc945eafc 100644 --- a/packages/rs-platform-version/src/version/drive_versions/mod.rs +++ b/packages/rs-platform-version/src/version/drive_versions/mod.rs @@ -15,6 +15,7 @@ use drive_token_method_versions::DriveTokenMethodVersions; use drive_verify_method_versions::DriveVerifyMethodVersions; use drive_vote_method_versions::DriveVoteMethodVersions; use grovedb_version::version::GroveVersion; +use versioned_feature_core::OptionalFeatureVersion; pub mod drive_address_funds_method_versions; pub mod drive_contract_group_method_versions; @@ -166,6 +167,12 @@ pub struct DriveFeesMethodVersions { #[derive(Clone, Debug, Default)] pub struct DrivePlatformSystemMethodVersions { pub estimation_costs: DriveSystemEstimationCostsMethodVersions, + /// Reads the chain locked Core height at which Platform first saw a Core fork active. + /// `None` (not active) before protocol version 15. + pub fetch_core_fork_activation_height: OptionalFeatureVersion, + /// Records the chain locked Core height at which Platform first saw a Core fork active. + /// `None` (not active) before protocol version 15. + pub store_core_fork_activation_height: OptionalFeatureVersion, } #[derive(Clone, Debug, Default)] diff --git a/packages/rs-platform-version/src/version/drive_versions/v1.rs b/packages/rs-platform-version/src/version/drive_versions/v1.rs index ed81188a1fb..66b1056aa23 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v1.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v1.rs @@ -73,6 +73,8 @@ pub const DRIVE_VERSION_V1: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v10.rs b/packages/rs-platform-version/src/version/drive_versions/v10.rs index ff1ce116d60..dcdc5ac3f1d 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v10.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v10.rs @@ -2,7 +2,9 @@ use crate::version::drive_versions::drive_contract_method_versions::v5::DRIVE_CO use crate::version::drive_versions::drive_verify_method_versions::v4::DRIVE_VERIFY_METHOD_VERSIONS_V4; use crate::version::drive_versions::drive_vote_method_versions::v4::DRIVE_VOTE_METHOD_VERSIONS_V4; use crate::version::drive_versions::v9::DRIVE_VERSION_V9; -use crate::version::drive_versions::{DriveMethodVersions, DriveVersion}; +use crate::version::drive_versions::{ + DriveMethodVersions, DrivePlatformSystemMethodVersions, DriveVersion, +}; /// Drive version 10. /// Introduced in protocol v15: @@ -21,6 +23,9 @@ use crate::version::drive_versions::{DriveMethodVersions, DriveVersion}; /// `cleanup.remove_vote_poll_end_date_query_operations`, which removes finished polls from /// the end date index, is `Some(0)` (`None` before). `DRIVE_VERIFY_METHOD_VERSIONS_V4` makes /// the yes/no proof verifiers `Some(0)` (`None` before). +/// * **Core fork activations**: `platform_system.fetch_core_fork_activation_height` and +/// `store_core_fork_activation_height` are `Some(0)` (`None` before): Drive keeps, in `Misc`, +/// the chain locked Core height at which Platform first saw each tracked Core fork active. /// /// Everything else matches `DRIVE_VERSION_V9`. pub const DRIVE_VERSION_V10: DriveVersion = DriveVersion { @@ -28,6 +33,11 @@ pub const DRIVE_VERSION_V10: DriveVersion = DriveVersion { contract: DRIVE_CONTRACT_METHOD_VERSIONS_V5, // changed in v10: compiled contract storage vote: DRIVE_VOTE_METHOD_VERSIONS_V4, // changed in v10: the yes/no poll kind verify: DRIVE_VERIFY_METHOD_VERSIONS_V4, // changed in v10: the yes/no proof verifiers + platform_system: DrivePlatformSystemMethodVersions { + fetch_core_fork_activation_height: Some(0), // changed in v10: the Core fork activations + store_core_fork_activation_height: Some(0), // changed in v10: the Core fork activations + ..DRIVE_VERSION_V9.methods.platform_system + }, ..DRIVE_VERSION_V9.methods }, ..DRIVE_VERSION_V9 diff --git a/packages/rs-platform-version/src/version/drive_versions/v2.rs b/packages/rs-platform-version/src/version/drive_versions/v2.rs index 3281d9e4bff..a4097a1cde3 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v2.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v2.rs @@ -73,6 +73,8 @@ pub const DRIVE_VERSION_V2: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v3.rs b/packages/rs-platform-version/src/version/drive_versions/v3.rs index 63b71e91a8d..f685007a316 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v3.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v3.rs @@ -73,6 +73,8 @@ pub const DRIVE_VERSION_V3: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v4.rs b/packages/rs-platform-version/src/version/drive_versions/v4.rs index 8a9b94fc500..d8e2731c2f5 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v4.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v4.rs @@ -73,6 +73,8 @@ pub const DRIVE_VERSION_V4: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v5.rs b/packages/rs-platform-version/src/version/drive_versions/v5.rs index c36faf06791..f494c9d4c5e 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v5.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v5.rs @@ -75,6 +75,8 @@ pub const DRIVE_VERSION_V5: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v6.rs b/packages/rs-platform-version/src/version/drive_versions/v6.rs index 2af2c251d77..d07683bf664 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v6.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v6.rs @@ -77,6 +77,8 @@ pub const DRIVE_VERSION_V6: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v7.rs b/packages/rs-platform-version/src/version/drive_versions/v7.rs index 579be7e1c49..8ffa072432b 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v7.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v7.rs @@ -75,6 +75,8 @@ pub const DRIVE_VERSION_V7: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v8.rs b/packages/rs-platform-version/src/version/drive_versions/v8.rs index 6b2344ebe1b..23a5097ef4c 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v8.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v8.rs @@ -75,6 +75,8 @@ pub const DRIVE_VERSION_V8: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/drive_versions/v9.rs b/packages/rs-platform-version/src/version/drive_versions/v9.rs index 6e6c1131852..22ee74cbeca 100644 --- a/packages/rs-platform-version/src/version/drive_versions/v9.rs +++ b/packages/rs-platform-version/src/version/drive_versions/v9.rs @@ -124,6 +124,8 @@ pub const DRIVE_VERSION_V9: DriveVersion = DriveVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/mocks/v2_test.rs b/packages/rs-platform-version/src/version/mocks/v2_test.rs index 469ff4894bb..85ce495793f 100644 --- a/packages/rs-platform-version/src/version/mocks/v2_test.rs +++ b/packages/rs-platform-version/src/version/mocks/v2_test.rs @@ -113,6 +113,8 @@ pub const TEST_PLATFORM_V2: PlatformVersion = PlatformVersion { estimation_costs: DriveSystemEstimationCostsMethodVersions { for_total_system_credits_update: 0, }, + fetch_core_fork_activation_height: None, + store_core_fork_activation_height: None, }, operations: DriveOperationsMethodVersion { rollback_transaction: 0, diff --git a/packages/rs-platform-version/src/version/v15.rs b/packages/rs-platform-version/src/version/v15.rs index 855e4a41567..34b05cf148e 100644 --- a/packages/rs-platform-version/src/version/v15.rs +++ b/packages/rs-platform-version/src/version/v15.rs @@ -223,12 +223,26 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// (`DRIVE_VERIFY_METHOD_VERSIONS_V4`). /// Nothing calls the opener yet, so at this version every yes/no vote is /// refused for want of a poll. +/// +/// 11. **Core fork activations recorded in state**: `update_core_info` 1 +/// (`DRIVE_ABCI_METHOD_VERSIONS_V11`) asks Core, in a block that moves the chain locked +/// Core height, whether Dash Core's V24 is active at that height: the coinbase of that +/// Core block has payload version 4 or higher, which Core requires exactly while V24 is +/// active. The first block that sees it active stores the height in `Misc` +/// (`CORE_FORK_V24_ACTIVATION_KEY`, a big endian `u32` written once, through +/// `DRIVE_VERSION_V10`'s `platform_system` methods), where the app hash covers it, and no +/// later block asks Core about V24. A chain born at this version asks at init chain, at +/// the genesis core height. A block that keeps the chain locked height asks nothing, so +/// a chain upgraded to this version records V24 at its first block that moves the height. +/// A Core error or a coinbase without a payload fails the block, as other Core errors in +/// `update_core_info` do. Protocol v14 and earlier keep `update_core_info` 0, which +/// neither asks nor records. pub const PLATFORM_V15: PlatformVersion = PlatformVersion { protocol_version: PROTOCOL_VERSION_15, - drive: DRIVE_VERSION_V10, // changed: apply_contract_with_serialization 1 writes nothing for a contract whose format version 1 bytes match the stored one's (note 8); vote method versions v4: register_identity_vote 1 registers a yes/no vote, remove_all_votes_given_by_identities 1 also removes the yes/no votes of a masternode that left the list, every yes_no method and remove_vote_poll_end_date_query_operations Some(0) make yes/no polls active, add_vote_poll_end_date_query_operations 1 adds only the entry when the batch already queues the end date's tree with other storage flags, and verify methods v4 make the yes/no proof verifiers active (note 10) + drive: DRIVE_VERSION_V10, // changed: apply_contract_with_serialization 1 writes nothing for a contract whose format version 1 bytes match the stored one's (note 8); vote method versions v4: register_identity_vote 1 registers a yes/no vote, remove_all_votes_given_by_identities 1 also removes the yes/no votes of a masternode that left the list, every yes_no method and remove_vote_poll_end_date_query_operations Some(0) make yes/no polls active, add_vote_poll_end_date_query_operations 1 adds only the entry when the batch already queues the end date's tree with other storage flags, and verify methods v4 make the yes/no proof verifiers active (note 10); fetch_core_fork_activation_height and store_core_fork_activation_height Some(0) keep the Core fork activations (note 11) drive_abci: DriveAbciVersion { structs: DRIVE_ABCI_STRUCTURE_VERSIONS_V2, - methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: the yes/no poll's tally, record, clean-up and finish hook are Some(0) (note 10); verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock + methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: the yes/no poll's tally, record, clean-up and finish hook are Some(0) (note 10); verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock; update_core_info 1 records the chain locked height at which Core's V24 is first seen active (note 11) validation_and_processing: DRIVE_ABCI_VALIDATION_VERSIONS_V11, // changed: identity update advanced_structure 1 refuses unpaid an identity update whose added key's proof of possession does not verify (note 2); create_domain_data_trigger 2 (note 3); contract update transform_into_action 1 holds an update to the compiled size cap in check_tx (note 8); batch advanced_structure 2 refuses a token config update on a read-only contract (note 9); masternode vote transform_into_action 1 looks up a masternode's previous answer on a yes/no poll (note 10) withdrawal_constants: DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V3, query: DRIVE_ABCI_QUERY_VERSIONS_V2, From 2ea239bfc2edd53de3d14c6e12c1b50b3403297d Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 30 Oct 2026 16:30:43 +0700 Subject: [PATCH 06/10] docs(platform): say when an upgraded chain records V24 before Core activates it Note 11 said a chain upgraded to protocol version 15 records V24 at its first block that moves the chain locked height. That holds only where V24 is already active, as on testnet; where protocol version 15 activates first, the record waits for the first block that moves the height to one where V24 is active. Comment-only change, no test. Co-Authored-By: Claude Opus 5.5 --- packages/rs-platform-version/src/version/v15.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/rs-platform-version/src/version/v15.rs b/packages/rs-platform-version/src/version/v15.rs index 34b05cf148e..52a42daedc5 100644 --- a/packages/rs-platform-version/src/version/v15.rs +++ b/packages/rs-platform-version/src/version/v15.rs @@ -233,8 +233,9 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// `DRIVE_VERSION_V10`'s `platform_system` methods), where the app hash covers it, and no /// later block asks Core about V24. A chain born at this version asks at init chain, at /// the genesis core height. A block that keeps the chain locked height asks nothing, so -/// a chain upgraded to this version records V24 at its first block that moves the height. -/// A Core error or a coinbase without a payload fails the block, as other Core errors in +/// a chain upgraded to this version records V24 at its first block that moves the height +/// to one where V24 is active, which can be a few blocks after Core activated it. A Core +/// error or a coinbase without a payload fails the block, as other Core errors in /// `update_core_info` do. Protocol v14 and earlier keep `update_core_info` 0, which /// neither asks nor records. pub const PLATFORM_V15: PlatformVersion = PlatformVersion { From 31588eb7f2454c369ef02a3978120dab7ce1612e Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 30 Oct 2026 16:45:20 +0700 Subject: [PATCH 07/10] feat(drive-abci)!: read V24 from the Core fork record instead of asking Core per pooling block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_untied_withdrawal_transactions_from_documents v1 asked Core for the coinbase payload version at the chain locked height in every block that pooled a withdrawal, even when that height had not moved, so a Core outage failed such blocks and made their proposers panic, where protocol 14 keeps producing blocks. It now reads Drive's Core fork record in the block's transaction: update_core_info v1 records V24 earlier in the first block that moves the chain locked height to one where V24 is active (or at init chain), and forks never switch off, so a recorded V24 is active at every later block's chain locked height. Until V24 is recorded, withdrawals stay version 1, which Core always accepts. Pooling no longer calls Core. The dispatcher gains the block transaction. In place, the shared pooling helper pool_withdrawals_up_to_limit_v1 (protocols 1 to 15) passes its transaction to the dispatcher; version 0 of the builder, which protocols 1 to 14 select, ignores it, so their blocks pool the same transactions. Tests would have caught this in CI: against the per-block RPC, seven unit tests and the three V24 strategy tests failed (✖: Core asked in every pooling block, [1, 1, 49, 49, 50, 50, 50] instead of [1, 49, 50]) and pass after the switch (✔). Passing None instead of the block transaction turns the pool-level and switch tests red. Co-Authored-By: Claude Opus 5.5 --- .../mod.rs | 10 +- .../v1/mod.rs | 241 +++++++++--------- .../v1/mod.rs | 4 + .../v2/mod.rs | 90 ++++++- .../test_cases/withdrawal_tests.rs | 184 +++++++++---- .../drive_abci_method_versions/v11.rs | 8 +- .../rs-platform-version/src/version/v15.rs | 27 +- 7 files changed, 363 insertions(+), 201 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs index 25af7c3340b..cffb6baf407 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/mod.rs @@ -7,6 +7,7 @@ use dpp::document::Document; use dpp::fee::Credits; use dpp::version::PlatformVersion; use dpp::withdrawal::{WithdrawalTransactionIndex, WithdrawalTransactionIndexAndBytes}; +use drive::grovedb::TransactionArg; mod v0; mod v1; @@ -24,9 +25,10 @@ where /// /// * `documents` - A mutable reference to a vector of `Document` representing the withdrawal requests. /// * `start_index` - The starting index for the transaction, of type `WithdrawalTransactionIndex`. - /// * `block_info` - A reference to the `BlockInfo`, which provides the current block's timestamp - /// and, from version 1, the chain locked height whose Core block decides the asset unlock - /// payload version. + /// * `block_info` - A reference to the `BlockInfo`, which provides the current block's timestamp. + /// * `transaction` - The block's transaction, in which version 1 reads whether Platform has + /// recorded Dash Core's V24 as active, which decides the asset unlock payload version. + /// Version 0 does not read it. /// * `platform_version` - A reference to the `PlatformVersion` that specifies the version of the platform being used. /// /// # Returns @@ -38,6 +40,7 @@ where documents: &mut [Document], start_index: WithdrawalTransactionIndex, block_info: &BlockInfo, + transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result<(Vec, Credits), Error> { match platform_version @@ -56,6 +59,7 @@ where documents, start_index, block_info, + transaction, platform_version, ), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs index c69ed0bf956..b86b325d047 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -13,9 +13,11 @@ use crate::{ error::{execution::ExecutionError, Error}, execution::platform_events::withdrawals::withdrawal_amount_is_core_dust, platform_types::platform::Platform, - rpc::core::{CoreRPCLike, V24_COINBASE_PAYLOAD_VERSION}, + rpc::core::CoreRPCLike, }; use dpp::version::PlatformVersion; +use drive::drive::system::core_fork_activations::CoreFork; +use drive::grovedb::TransactionArg; impl Platform where @@ -27,10 +29,15 @@ where /// Core's V24 is active. The txid of a version 2 asset unlock leaves out the quorum signing /// info, so every instance Platform signs of one withdrawal shares one txid, and Core may /// InstantSend-lock it before it is mined. Core refuses version 2 before V24, so the - /// version follows Core's fork state at the block's chain locked height, which every node - /// reads alike: V24 is active there exactly when the coinbase of that Core block has - /// payload version 4 or higher. A withdrawal whose whole amount is below Core's dust - /// threshold stays at version 1 (see `withdrawal_amount_is_core_dust`). + /// version follows Drive's record of the Core forks Platform has seen active, read in the + /// block's `transaction`: `update_core_info` records V24, earlier in the block, at init + /// chain or in the first block that moves the chain locked height to one where V24 is + /// active, and forks never switch off, so a recorded V24 is active at the chain locked + /// height of that block and every later one. Until V24 is recorded, which can be a few + /// blocks after Core activated it, withdrawals stay at version 1, which Core always + /// accepts. Core is not asked. A + /// withdrawal whose whole amount is below Core's dust threshold stays at version 1 (see + /// `withdrawal_amount_is_core_dust`). /// /// The version is decided here once and kept in the untied transaction bytes, which every /// later signing reuses unchanged. Nothing may rewrite those bytes, nor change how a @@ -42,6 +49,7 @@ where documents: &mut [Document], start_index: WithdrawalTransactionIndex, block_info: &BlockInfo, + transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result<(Vec, Credits), Error> { if documents.is_empty() { @@ -55,17 +63,10 @@ where "build_untied_withdrawal_transactions_from_documents v1 requires a Core dust relay fee", )))?; - // Every block at a Core height Platform follows carries a coinbase payload. - let core_height = block_info.core_height; - let coinbase_payload_version = self - .core_rpc - .get_coinbase_payload_version(core_height)? - .ok_or_else(|| { - ExecutionError::DashCoreBadResponseError(format!( - "the coinbase of the block at core height {core_height} has no payload" - )) - })?; - let v24_active = coinbase_payload_version >= V24_COINBASE_PAYLOAD_VERSION; + let v24_active = self + .drive + .fetch_core_fork_activation_height(CoreFork::V24, transaction, platform_version)? + .is_some(); documents.iter_mut().enumerate().try_fold( (Vec::new(), 0u64), // Start with an empty vector for transactions and 0 for total amount. @@ -136,16 +137,14 @@ where #[cfg(test)] mod tests { - use crate::error::execution::ExecutionError; use crate::error::Error; use crate::rpc::core::MockCoreRPCLike; - use crate::test::helpers::setup::TestPlatformBuilder; + use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; use dpp::balances::credits::CREDITS_PER_DUFF; use dpp::block::block_info::BlockInfo; use dpp::block::epoch::Epoch; use dpp::dashcore::consensus::Decodable; use dpp::dashcore::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBaseTransactionInfo; - use dpp::dashcore_rpc::Error as CoreRpcError; use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; use dpp::data_contracts::withdrawals_contract::WithdrawalStatus; @@ -164,7 +163,10 @@ mod tests { }; use drive::config::DEFAULT_QUERY_LIMIT; use drive::drive::identity::withdrawals::paths::get_withdrawal_transactions_broadcasted_path; - use drive::grovedb::Element; + use drive::drive::system::core_fork_activations::{CoreFork, CORE_FORK_V24_ACTIVATION_KEY}; + use drive::drive::system::misc_path; + use drive::error::{drive::DriveError, Error as DriveErrorKind}; + use drive::grovedb::{Element, TransactionArg}; use drive::util::test_helpers::setup::{setup_document, setup_system_data_contract}; /// The chain locked height of the Platform block that pools the withdrawals. @@ -250,10 +252,39 @@ mod tests { .collect() } - /// Pools `documents` through the dispatcher at `platform_version`, against `core_rpc`. - /// Returns the untied transactions, their total amount and the pooled documents. + /// A platform whose Core answers nothing, so a test fails if it is asked, with the + /// withdrawals contract's state structure. + fn platform_without_core() -> TempPlatform { + let mut platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_initial_state_structure(); + platform.core_rpc = MockCoreRPCLike::new(); + platform + } + + /// Records V24 at `core_height` in `transaction`, as `update_core_info` does earlier in a + /// block that sees it active. + fn record_v24( + platform: &TempPlatform, + core_height: u32, + transaction: TransactionArg, + ) { + platform + .drive + .store_core_fork_activation_height( + CoreFork::V24, + core_height, + transaction, + PlatformVersion::latest(), + ) + .expect("expected to record V24"); + } + + /// Pools `documents` through the dispatcher at `platform_version`, in a block transaction + /// holding V24's record at `v24_recorded_at`, or none. Returns the untied transactions, + /// their total amount and the pooled documents. fn build_untied( - core_rpc: MockCoreRPCLike, + v24_recorded_at: Option, mut documents: Vec, platform_version: &PlatformVersion, ) -> ( @@ -261,49 +292,39 @@ mod tests { Credits, Vec, ) { - let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); - platform.core_rpc = core_rpc; + let platform = platform_without_core(); + let transaction = platform.drive.grove.start_transaction(); + if let Some(core_height) = v24_recorded_at { + record_v24(&platform, core_height, Some(&transaction)); + } let (transactions, total_amount) = platform .build_untied_withdrawal_transactions_from_documents( &mut documents, START_INDEX, &block_info(), + Some(&transaction), platform_version, ) .expect("expected to build the untied withdrawal transactions"); (transactions, total_amount, documents) } - /// A Core whose coinbase at `CORE_HEIGHT` has `payload_version`, asked exactly once. - fn core_with_coinbase_payload_version(payload_version: u16) -> MockCoreRPCLike { - let mut core_rpc = MockCoreRPCLike::new(); - core_rpc - .expect_get_coinbase_payload_version() - .withf(|core_height| *core_height == CORE_HEIGHT) - .times(1) - .returning(move |_| Ok(Some(payload_version))); - core_rpc - } - /// Decodes the untied transaction bytes the pooling step stores. fn decode(untied_transaction_bytes: &[u8]) -> AssetUnlockBaseTransactionInfo { AssetUnlockBaseTransactionInfo::consensus_decode(&mut &untied_transaction_bytes[..]) .expect("expected an untied asset unlock transaction") } - /// Core requires coinbase payload version 4 or higher exactly at the blocks where V24 is - /// active, so that is when a withdrawal pooled at that chain locked height gets payload - /// version 2. Core is asked once per block, however many withdrawals the block pools, and - /// apart from the payload version the transactions, their total and the pooled documents - /// are those version 0 builds. + /// A withdrawal pooled once Platform has recorded Core's V24 as active, at the block's + /// chain locked height or below it, gets payload version 2, and one pooled before gets + /// version 1; Core is never asked. Apart from the payload version the transactions, their + /// total and the pooled documents are those version 0 builds. #[test] - fn should_choose_the_asset_unlock_payload_version_from_the_coinbase_at_the_blocks_chain_locked_height( - ) { + fn should_choose_the_asset_unlock_payload_version_from_the_v24_record() { let platform_version = PlatformVersion::latest(); let documents = queued_withdrawals(&[1_000_000, 2_500_000, 70_000_000], platform_version); - let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); - platform.core_rpc = MockCoreRPCLike::new(); + let platform = platform_without_core(); let mut expected_documents = documents.clone(); let (expected_transactions, expected_total_amount) = platform .build_untied_withdrawal_transactions_from_documents_v0( @@ -314,12 +335,13 @@ mod tests { ) .expect("expected to build the untied withdrawal transactions"); - for (coinbase_payload_version, payload_version) in [(3, 1), (4, 2), (5, 2)] { - let (transactions, total_amount, pooled_documents) = build_untied( - core_with_coinbase_payload_version(coinbase_payload_version), - documents.clone(), - platform_version, - ); + for (v24_recorded_at, payload_version) in [ + (None, 1), + (Some(CORE_HEIGHT), 2), + (Some(CORE_HEIGHT - 1_000), 2), + ] { + let (transactions, total_amount, pooled_documents) = + build_untied(v24_recorded_at, documents.clone(), platform_version); assert_eq!(total_amount, expected_total_amount); assert_eq!(pooled_documents, expected_documents); @@ -331,7 +353,7 @@ mod tests { let mut transaction = decode(bytes); assert_eq!( transaction.base_payload.version, payload_version, - "coinbase payload version {coinbase_payload_version}" + "V24 recorded at {v24_recorded_at:?}" ); transaction.base_payload.version = 1; assert_eq!(transaction, decode(expected_bytes)); @@ -339,97 +361,74 @@ mod tests { if payload_version == 1 { assert_eq!( transactions, expected_transactions, - "before V24 the bytes are version 0's" + "before V24 is recorded the bytes are version 0's" ); } } } - /// Protocol version 14 emits payload version 1 whatever Core's fork state, without asking - /// Core, so the blocks committed under it replay byte for byte; protocol version 15 emits - /// version 2 for the same withdrawals once V24 is active. + /// Protocol version 14 emits payload version 1 whatever the record holds, so the blocks + /// committed under it replay byte for byte; protocol version 15 emits version 2 for the + /// same withdrawals once V24 is recorded. #[test] - fn should_keep_emitting_version_1_asset_unlocks_at_protocol_version_14_even_when_v24_is_active() - { + fn should_keep_emitting_version_1_asset_unlocks_at_protocol_version_14_even_when_v24_is_recorded( + ) { let platform_version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); let documents = queued_withdrawals(&[1_000_000, 2_500_000], platform_version_14); - let mut core_rpc = MockCoreRPCLike::new(); - core_rpc - .expect_get_coinbase_payload_version() - .times(0) - .returning(|_| Ok(Some(4))); - let (transactions, _, _) = build_untied(core_rpc, documents.clone(), platform_version_14); + let (transactions, _, _) = + build_untied(Some(CORE_HEIGHT), documents.clone(), platform_version_14); assert!(transactions .iter() .all(|(_, bytes)| decode(bytes).base_payload.version == 1)); - let (transactions, _, _) = build_untied( - core_with_coinbase_payload_version(4), - documents, - PlatformVersion::latest(), - ); + let (transactions, _, _) = + build_untied(Some(CORE_HEIGHT), documents, PlatformVersion::latest()); assert!(transactions .iter() .all(|(_, bytes)| decode(bytes).base_payload.version == 2)); } - /// A block that pools nothing does not depend on Core answering. + /// A record that cannot be read fails the block instead of being read as V24 not being + /// recorded, before any document is pooled. #[test] - fn should_not_ask_core_when_no_withdrawal_is_pooled() { - let mut core_rpc = MockCoreRPCLike::new(); - core_rpc.expect_get_coinbase_payload_version().times(0); - - let (transactions, total_amount, _) = - build_untied(core_rpc, vec![], PlatformVersion::latest()); - - assert!(transactions.is_empty()); - assert_eq!(total_amount, 0); - } - - /// A Core error, or a coinbase without a payload, fails the block instead of being read as - /// V24 not being active, before any document is pooled. - #[test] - fn should_fail_rather_than_guess_when_core_cannot_tell_whether_v24_is_active() { + fn should_fail_rather_than_guess_when_the_v24_record_cannot_be_read() { let platform_version = PlatformVersion::latest(); let documents = queued_withdrawals(&[1_000_000], platform_version); + let platform = platform_without_core(); + let transaction = platform.drive.grove.start_transaction(); + platform + .drive + .grove_insert( + (&misc_path()).into(), + CORE_FORK_V24_ACTIVATION_KEY, + Element::Item(vec![0, 0, 1], None), + Some(&transaction), + None, + &mut vec![], + &platform_version.drive, + ) + .expect("expected to write a malformed record"); - for answer in [ - Ok(None), - Err(CoreRpcError::UnexpectedStructure("refused".to_string())), - ] { - let no_payload = answer.is_ok(); - let mut platform = TestPlatformBuilder::new().build_with_mock_rpc(); - let mut core_rpc = MockCoreRPCLike::new(); - core_rpc - .expect_get_coinbase_payload_version() - .times(1) - .return_once(move |_| answer); - platform.core_rpc = core_rpc; - - let mut pooled_documents = documents.clone(); - let result = platform.build_untied_withdrawal_transactions_from_documents( - &mut pooled_documents, - START_INDEX, - &block_info(), - platform_version, - ); + let mut pooled_documents = documents.clone(); + let result = platform.build_untied_withdrawal_transactions_from_documents( + &mut pooled_documents, + START_INDEX, + &block_info(), + Some(&transaction), + platform_version, + ); - if no_payload { - assert!( - matches!( - result, - Err(Error::Execution(ExecutionError::DashCoreBadResponseError( - _ - ))) - ), - "{result:?}" - ); - } else { - assert!(matches!(result, Err(Error::CoreRpc(_))), "{result:?}"); - } - assert_eq!(pooled_documents, documents, "no document may be pooled"); - } + assert!( + matches!( + result, + Err(Error::Drive(DriveErrorKind::Drive( + DriveError::CorruptedSerialization(_) + ))) + ), + "{result:?}" + ); + assert_eq!(pooled_documents, documents, "no document may be pooled"); } /// The status the expiry step leaves an expired withdrawal of `amount` credits to @@ -526,7 +525,7 @@ mod tests { let viable = threshold_duffs * CREDITS_PER_DUFF; let (transactions, _, _) = build_untied( - core_with_coinbase_payload_version(4), + Some(CORE_HEIGHT), [dust, viable] .iter() .map(|amount| { diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v1/mod.rs index 2c079903994..3ccfa4fb460 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v1/mod.rs @@ -151,6 +151,10 @@ where &mut documents_to_process, start_transaction_index, block_info, + // Read only by version 1 of the builder, which protocol version 15 selects; + // version 0, which protocol versions 1 to 14 select, ignores it, so their blocks + // pool the same transactions. + transaction, platform_version, )?; diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs index ac42b43accf..8ae41796a79 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs @@ -78,6 +78,8 @@ mod tests { use crate::test::helpers::setup::TestPlatformBuilder; use dpp::block::epoch::Epoch; use dpp::dash_to_credits; + use dpp::dashcore::consensus::Decodable; + use dpp::dashcore::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBaseTransactionInfo; use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::data_contracts::SystemDataContract; use dpp::identifier::Identifier; @@ -89,6 +91,7 @@ mod tests { use dpp::tests::fixtures::get_withdrawal_document_fixture; use dpp::withdrawal::Pooling; use drive::config::DEFAULT_QUERY_LIMIT; + use drive::drive::system::core_fork_activations::CoreFork; use drive::util::test_helpers::setup::{setup_document, setup_system_data_contract}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; @@ -102,6 +105,20 @@ mod tests { amount: u64, pool_duffs: u64, ) -> (usize, usize, usize) { + let (pooled, balance_reads, coinbase_reads, _) = + pool_with_v24(platform_version, amount, pool_duffs, false); + (pooled, balance_reads, coinbase_reads) + } + + /// `pool`, in a block transaction that holds V24's record when `v24_recorded`, as it does + /// once `update_core_info` has recorded V24 earlier in the block. Also returns the asset + /// unlock payload versions of the pooled transactions, in queue order. + fn pool_with_v24( + platform_version: &PlatformVersion, + amount: u64, + pool_duffs: u64, + v24_recorded: bool, + ) -> (usize, usize, usize, Vec) { let mut platform = TestPlatformBuilder::new() .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() @@ -127,6 +144,17 @@ mod tests { platform.core_rpc = core_rpc; let transaction = platform.drive.grove.start_transaction(); + if v24_recorded { + platform + .drive + .store_core_fork_activation_height( + CoreFork::V24, + 10_000, + Some(&transaction), + PlatformVersion::latest(), + ) + .expect("expected to record V24"); + } let block_info = BlockInfo { time_ms: 1, @@ -188,10 +216,28 @@ mod tests { ) .expect("to fetch withdrawal documents") .len(); + let payload_versions = platform + .drive + .dequeue_untied_withdrawal_transactions( + 16, + Some(&transaction), + &mut vec![], + platform_version, + ) + .expect("expected to read the queued withdrawal transactions") + .iter() + .map(|(_, bytes)| { + AssetUnlockBaseTransactionInfo::consensus_decode(&mut bytes.as_slice()) + .expect("expected an untied asset unlock transaction") + .base_payload + .version + }) + .collect(); ( pooled, balance_reads.load(Ordering::SeqCst), coinbase_reads.load(Ordering::SeqCst), + payload_versions, ) } @@ -238,27 +284,45 @@ mod tests { ); assert_eq!( pool(PlatformVersion::latest(), 1000, 1_000_000_000_000), - (2, 50, 1) + (2, 50, 0) ); } - /// From protocol version 15 the block asks Core whether V24 is active once when it pools - /// withdrawals, and not at all when it pools none, so a block that pools nothing does not - /// depend on Core answering. Protocol version 14 never asks. + /// Pooling never asks Core whether V24 is active: from protocol version 15 the payload + /// version comes from the Core fork record in Drive, so a block that pools withdrawals at + /// an unchanged chain locked height does not depend on Core answering. Protocol version 14 + /// does not ask either. #[test] - fn should_ask_core_whether_v24_is_active_only_in_blocks_that_pool_withdrawals() { + fn should_pool_without_asking_core_whether_v24_is_active() { let coinbase_reads = |(_, _, coinbase_reads): (usize, usize, usize)| coinbase_reads; let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); let latest = PlatformVersion::latest(); assert_eq!(coinbase_reads(pool(version_14, 1000, 1_000_000_000_000)), 0); - assert_eq!(coinbase_reads(pool(latest, 1000, 1_000_000_000_000)), 1); - // Nothing pools when Core's credit pool admits nothing, nor when the oldest withdrawal - // does not fit Platform's own limit. - assert_eq!(coinbase_reads(pool(latest, 1000, 0)), 0); - assert_eq!( - coinbase_reads(pool(latest, dash_to_credits!(3000), 1_000_000_000_000)), - 0 - ); + assert_eq!(coinbase_reads(pool(latest, 1000, 1_000_000_000_000)), 0); + } + + /// Pooling reads the Core fork record in the block's transaction, where `update_core_info` + /// writes it earlier in the block that records V24: from protocol version 15 withdrawals + /// pooled once V24 is recorded are version 2, and at protocol version 14 they stay version + /// 1. + #[test] + fn should_pool_version_2_asset_unlocks_once_v24_is_recorded_in_the_block_transaction() { + // 1 Dash each, above Core's dust threshold, which keeps a withdrawal at version 1. + let payload_versions = |platform_version, v24_recorded| { + pool_with_v24( + platform_version, + dash_to_credits!(1), + 1_000_000_000_000, + v24_recorded, + ) + .3 + }; + let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); + let latest = PlatformVersion::latest(); + + assert_eq!(payload_versions(latest, true), vec![2, 2]); + assert_eq!(payload_versions(latest, false), vec![1, 1]); + assert_eq!(payload_versions(version_14, true), vec![1, 1]); } } diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs index 3b5a9f03f3c..abd29305451 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs @@ -11,6 +11,7 @@ mod tests { use dpp::dashcore::transaction::TransactionPayload::AssetUnlockPayloadType; use dpp::dashcore::{BlockHash, ChainLock, Transaction, Txid}; use dpp::dashcore_rpc::dashcore_rpc_json::{AssetUnlockStatus, AssetUnlockStatusResult}; + use dpp::dashcore_rpc::Error as CoreRpcError; use dpp::data_contracts::withdrawals_contract; use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; use dpp::document::{Document, DocumentV0Getters}; @@ -31,6 +32,7 @@ mod tests { ChainLockConfig, ExecutionConfig, InstantLockConfig, PlatformConfig, PlatformTestConfig, ValidatorSetConfig, }; + use drive_abci::platform_types::platform_state::PlatformStateV0Methods; use drive_abci::rpc::core::MockCoreRPCLike; use drive_abci::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; use platform_version::version::mocks::v3_test::TEST_PLATFORM_V3; @@ -4001,6 +4003,8 @@ mod tests { broadcast: Vec, /// Every core height Platform asked the coinbase payload version of, in order. coinbase_reads: Vec, + /// Whether Core fails to read a coinbase, as it does while it is down. + coinbase_down: bool, } impl V24CoreState { @@ -4011,6 +4015,7 @@ mod tests { v24_activation_height, broadcast: vec![], coinbase_reads: vec![], + coinbase_down: false, })) } @@ -4082,6 +4087,11 @@ mod tests { .returning(move |core_height| { let mut state = state.lock().unwrap(); state.coinbase_reads.push(core_height); + if state.coinbase_down { + return Err(CoreRpcError::UnexpectedStructure( + "Core is down".to_string(), + )); + } let v24_active = core_height >= state.v24_activation_height; Ok(Some(if v24_active { 4 } else { 3 })) }); @@ -4408,13 +4418,19 @@ mod tests { withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::COMPLETE).len(), 1 ); + assert_eq!( + core_state.lock().unwrap().coinbase_reads, + vec![1], + "V24 is recorded at init chain, and no block asks Core about it again" + ); } - /// Platform emits version 2 from the first block whose chain locked height has V24 active, - /// and only for withdrawals pooled from then on. One pooled before the switch stays version - /// 1 when it is first signed after the switch, and when it expires and is re-signed after - /// it, with a new txid each time, as Core keeps accepting version 1. Core is asked about the - /// chain locked height of each block that pools a withdrawal, and of no other block. + /// Platform emits version 2 from the block that records V24 as active, the first block + /// whose chain locked height has V24 active, and only for withdrawals pooled from then on. + /// One pooled before the switch stays version 1 when it is first signed after the switch, + /// and when it expires and is re-signed after it, with a new txid each time, as Core keeps + /// accepting version 1. Core is asked at init chain and at each height the chain locked + /// height moves to until V24 is recorded, and by no block that pools a withdrawal. #[tokio::test] async fn should_switch_to_version_2_asset_unlocks_at_the_first_chain_locked_height_where_v24_is_active( ) { @@ -4433,65 +4449,139 @@ mod tests { core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT - 1; let outcome = next_block(outcome, 5, &withdraw, &config).await; - // Block 6, at the first core height with V24 active, signs it, and the first one - // expires, 49 core blocks after it was signed. + // Block 6, at the first core height with V24 active, records V24, pools one, signs the + // one pooled before, and the first one expires, 49 core blocks after it was signed. core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT; - let outcome = next_block(outcome, 6, &idle, &config).await; + let outcome = next_block(outcome, 6, &withdraw, &config).await; let pooled_just_before = asset_unlock_index(outcome.withdrawals.first().expect("signed")); assert_eq!( withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::EXPIRED).len(), 1 ); + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::POOLED).len(), + 1, + "block 6 pools the withdrawal it creates" + ); + assert!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::QUEUED) + .is_empty() + ); - // Block 7 re-signs the expired one and pools one at the activation height; block 8 - // signs that. + // Block 7, at the same core height, re-signs the expired one, signs the one pooled in + // block 6 and pools another; block 8 signs that. let outcome = next_block(outcome, 7, &withdraw, &config).await; + let pooled_when_recorded = outcome + .withdrawals + .iter() + .map(asset_unlock_index) + .find(|index| *index != pooled_long_before) + .expect("expected the withdrawal pooled in block 6 to be signed"); let outcome = next_block(outcome, 8, &idle, &config).await; - let pooled_at_activation = asset_unlock_index(outcome.withdrawals.first().expect("signed")); + let pooled_after_recorded = + asset_unlock_index(outcome.withdrawals.first().expect("signed")); - { - let state = core_state.lock().unwrap(); - let payload_versions_and_heights = |index| { - state - .broadcast_for(index) - .iter() - .map(|transaction| { - let payload = asset_unlock_payload(transaction); - (payload.base.version, payload.request_info.request_height) - }) - .collect::>() - }; - assert_eq!( - payload_versions_and_heights(pooled_long_before), - vec![(1, 1), (1, V24_ACTIVATION_HEIGHT)] - ); + let outcome = mine_all_and_complete(outcome, 9, &idle, &config, &core_state).await; + assert_eq!( + withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::COMPLETE).len(), + 4 + ); + + let state = core_state.lock().unwrap(); + let payload_versions_and_heights = |index| { + state + .broadcast_for(index) + .iter() + .map(|transaction| { + let payload = asset_unlock_payload(transaction); + (payload.base.version, payload.request_info.request_height) + }) + .collect::>() + }; + assert_eq!( + payload_versions_and_heights(pooled_long_before), + vec![(1, 1), (1, V24_ACTIVATION_HEIGHT)] + ); + assert_eq!( + payload_versions_and_heights(pooled_just_before), + vec![(1, V24_ACTIVATION_HEIGHT)] + ); + assert_eq!( + payload_versions_and_heights(pooled_when_recorded), + vec![(2, V24_ACTIVATION_HEIGHT)] + ); + assert_eq!( + payload_versions_and_heights(pooled_after_recorded), + vec![(2, V24_ACTIVATION_HEIGHT)] + ); + + let re_signed_version_1 = state.broadcast_for(pooled_long_before); + assert_ne!( + re_signed_version_1[0].txid(), + re_signed_version_1[1].txid(), + "a version 1 txid covers the signing info" + ); + + // Init chain, then blocks 5 and 6, which moved the chain locked height before V24 was + // recorded. Blocks 3, 6 and 7 pooled, and block 9 moved the height again. + assert_eq!( + state.coinbase_reads, + vec![1, V24_ACTIVATION_HEIGHT - 1, V24_ACTIVATION_HEIGHT] + ); + } + + /// Once a block has recorded V24, a block that pools a withdrawal at the same chain locked + /// height reads the record instead of asking Core, so a Core that refuses the coinbase + /// payload version no longer stops it: the withdrawal pools as version 2, and the chain is + /// the one a node whose Core answers commits. Only that question fails here; the + /// withdrawal scan, which reads Core's credit pool from the same coinbase, makes no call at + /// an unchanged chain locked height once it has caught up, as at protocol version 14. + #[tokio::test] + async fn should_keep_pooling_version_2_withdrawals_at_an_unchanged_chain_locked_height_without_asking_core_about_v24( + ) { + const V24_ACTIVATION_HEIGHT: u32 = 50; + let config = v24_test_config(); + let mut app_hashes = vec![]; + for coinbase_down in [false, true] { + let core_state = V24CoreState::new(V24_ACTIVATION_HEIGHT); + let mut platform = platform_with_v24_core(&config, &core_state); + let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + + // Block 3 moves the chain locked height to the activation height and records V24. + core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT; + let outcome = next_block(outcome, 3, &idle, &config).await; assert_eq!( - payload_versions_and_heights(pooled_just_before), - vec![(1, V24_ACTIVATION_HEIGHT)] + core_state.lock().unwrap().coinbase_reads, + vec![1, V24_ACTIVATION_HEIGHT] ); + core_state.lock().unwrap().coinbase_down = coinbase_down; + + // Blocks 4 and 5 pool a withdrawal each at the same height; blocks 5 and 6 sign them. + let outcome = next_block(outcome, 4, &withdraw, &config).await; + let outcome = next_block(outcome, 5, &withdraw, &config).await; + let outcome = next_block(outcome, 6, &idle, &config).await; + let signed = core_state.lock().unwrap().broadcast.clone(); + assert_eq!(signed.len(), 2); + assert!(signed + .iter() + .all(|transaction| asset_unlock_payload(transaction).base.version == 2)); assert_eq!( - payload_versions_and_heights(pooled_at_activation), - vec![(2, V24_ACTIVATION_HEIGHT)] - ); - - let re_signed_version_1 = state.broadcast_for(pooled_long_before); - assert_ne!( - re_signed_version_1[0].txid(), - re_signed_version_1[1].txid(), - "a version 1 txid covers the signing info" + core_state.lock().unwrap().coinbase_reads, + vec![1, V24_ACTIVATION_HEIGHT], + "no block after the one that recorded V24 asks Core about it" ); - - // Blocks 3, 5 and 7 pooled; blocks 4, 6 and 8 did not. - assert_eq!( - state.coinbase_reads, - vec![1, V24_ACTIVATION_HEIGHT - 1, V24_ACTIVATION_HEIGHT] + app_hashes.push( + outcome + .abci_app + .platform + .state + .load() + .last_committed_block_app_hash(), ); } - - let outcome = mine_all_and_complete(outcome, 9, &idle, &config, &core_state).await; assert_eq!( - withdrawal_documents(&outcome, withdrawals_contract::WithdrawalStatus::COMPLETE).len(), - 3 + app_hashes[0], app_hashes[1], + "the node whose Core refuses the coinbase payload version must commit the same chain" ); } } diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs index 81a3afccdfa..25269e81fb8 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v11.rs @@ -110,9 +110,9 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ /// only safe together with this check. /// /// The asset unlocks: `withdrawals.build_untied_withdrawal_transactions_from_documents` is 1 -/// rather than 0: a withdrawal pooled at a chain locked height where Dash Core's V24 is active -/// (the coinbase of that Core block has payload version 4 or higher) gets asset unlock payload -/// version 2, whose txid every re-signed instance shares, unless its amount is Core dust. +/// rather than 0: a withdrawal pooled once `update_core_info` 1 has recorded Dash Core's V24 as +/// active gets asset unlock payload version 2, whose txid every re-signed instance shares, +/// unless its amount is Core dust. It reads the record and does not ask Core. pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMethodVersions { core_chain_lock: DriveAbciCoreChainLockMethodVersionsAndConstants { verify_chain_lock_locally: 1, @@ -132,7 +132,7 @@ pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMet ..DRIVE_ABCI_METHOD_VERSIONS_V10.voting }, withdrawals: DriveAbciIdentityCreditWithdrawalMethodVersions { - build_untied_withdrawal_transactions_from_documents: 1, // changed in v15: asset unlock payload version 2 once Core's V24 is active at the chain locked height + build_untied_withdrawal_transactions_from_documents: 1, // changed in v15: asset unlock payload version 2 once V24 is recorded as active ..DRIVE_ABCI_METHOD_VERSIONS_V10.withdrawals }, ..DRIVE_ABCI_METHOD_VERSIONS_V10 diff --git a/packages/rs-platform-version/src/version/v15.rs b/packages/rs-platform-version/src/version/v15.rs index 51b5b144a96..80aed1a7658 100644 --- a/packages/rs-platform-version/src/version/v15.rs +++ b/packages/rs-platform-version/src/version/v15.rs @@ -241,24 +241,25 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// 12. **Asset unlocks with a stable txid once Core's V24 is active**: /// `build_untied_withdrawal_transactions_from_documents` 1 /// (`DRIVE_ABCI_METHOD_VERSIONS_V11`) gives a withdrawal it pools asset unlock payload -/// version 2 when the coinbase of the Core block at the Platform block's chain locked height -/// has payload version 4 or higher, which Core requires exactly while V24 is active, and the -/// withdrawal's whole amount is not below Core's dust threshold for its output script -/// (`core_dust_relay_fee_per_kb`); otherwise version 1. The txid of a version 2 asset -/// unlock leaves out the requested height, quorum hash and signature, so a withdrawal -/// re-signed after it expires keeps its txid and Core can InstantSend-lock it before it is -/// mined. The version is kept in the untied transaction bytes the queue stores, so -/// withdrawals pooled before the switch stay version 1 when they are re-signed, and a dust -/// withdrawal, the only kind the expiry step marks FAILED, is never version 2. Pooling -/// blocks ask Core for that coinbase, and fail when Core cannot answer. Protocol v14 and -/// earlier keep `build_untied_withdrawal_transactions_from_documents` 0, which emits version -/// 1 without asking Core. +/// version 2 once Platform has recorded Dash Core's V24 as active (note 11), read in the +/// block's transaction, and the withdrawal's whole amount is not below Core's dust +/// threshold for its output script (`core_dust_relay_fee_per_kb`); otherwise version 1. +/// The txid of a version 2 asset unlock leaves out the requested height, quorum hash and +/// signature, so a withdrawal re-signed after it expires keeps its txid and Core can +/// InstantSend-lock it before it is mined. The version is kept in the untied transaction +/// bytes the queue stores, so withdrawals pooled before the switch stay version 1 when +/// they are re-signed, and a dust withdrawal, the only kind the expiry step marks FAILED, +/// is never version 2. Pooling does not ask Core: a withdrawal pooled in the block that +/// records V24 is already version 2, and until a block records it, the first that moves +/// the chain locked height to one where V24 is active, withdrawals stay version 1, which +/// Core always accepts. Protocol v14 and earlier keep +/// `build_untied_withdrawal_transactions_from_documents` 0, which emits version 1. pub const PLATFORM_V15: PlatformVersion = PlatformVersion { protocol_version: PROTOCOL_VERSION_15, drive: DRIVE_VERSION_V10, // changed: apply_contract_with_serialization 1 writes nothing for a contract whose format version 1 bytes match the stored one's (note 8); vote method versions v4: register_identity_vote 1 registers a yes/no vote, remove_all_votes_given_by_identities 1 also removes the yes/no votes of a masternode that left the list, every yes_no method and remove_vote_poll_end_date_query_operations Some(0) make yes/no polls active, add_vote_poll_end_date_query_operations 1 adds only the entry when the batch already queues the end date's tree with other storage flags, and verify methods v4 make the yes/no proof verifiers active (note 10); fetch_core_fork_activation_height and store_core_fork_activation_height Some(0) keep the Core fork activations (note 11) drive_abci: DriveAbciVersion { structs: DRIVE_ABCI_STRUCTURE_VERSIONS_V2, - methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: the yes/no poll's tally, record, clean-up and finish hook are Some(0) (note 10); verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock; update_core_info 1 records the chain locked height at which Core's V24 is first seen active (note 11); build_untied_withdrawal_transactions_from_documents 1 emits asset unlock payload version 2 once Core's V24 is active (note 12) + methods: DRIVE_ABCI_METHOD_VERSIONS_V11, // changed: the yes/no poll's tally, record, clean-up and finish hook are Some(0) (note 10); verify_chain_lock_locally 1 rejects a chain lock locally only when Platform holds the quorums active at its signing height, and verify_chain_lock_through_core 1 has Core's verifychainlock check the others before submitchainlock; update_core_info 1 records the chain locked height at which Core's V24 is first seen active (note 11); build_untied_withdrawal_transactions_from_documents 1 emits asset unlock payload version 2 once V24 is recorded as active (note 12) validation_and_processing: DRIVE_ABCI_VALIDATION_VERSIONS_V11, // changed: identity update advanced_structure 1 refuses unpaid an identity update whose added key's proof of possession does not verify (note 2); create_domain_data_trigger 2 (note 3); contract update transform_into_action 1 holds an update to the compiled size cap in check_tx (note 8); batch advanced_structure 2 refuses a token config update on a read-only contract (note 9); masternode vote transform_into_action 1 looks up a masternode's previous answer on a yes/no poll (note 10) withdrawal_constants: DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V3, query: DRIVE_ABCI_QUERY_VERSIONS_V2, From 2096e7233ba4a9d3b91eea31beb51be7fac5c36e Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 30 Oct 2026 18:13:28 +0700 Subject: [PATCH 08/10] test(drive-abci): fail when the rejections directory cannot be read rejections_recorded_for read an unreadable rejections directory as zero rejections, so the benign-rejection assertions could pass on a fixture or file system failure. The directory is a temporary one the test creates, so reading it must succeed. Test-only change. Co-Authored-By: Claude Opus 5.5 --- .../v0/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs index ae073e375d7..191a057063b 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/append_signatures_and_broadcast_withdrawal_transactions/v0/mod.rs @@ -264,8 +264,8 @@ mod tests { .expect("a refused broadcast does not fail the block"); fs::read_dir(rejections.path()) - .map(|entries| entries.count()) - .unwrap_or(0) + .expect("expected to read the rejections directory") + .count() } /// Core refuses an instance it already holds a fresher one of, and a withdrawal it already From cfef63418d214c7a4f1b57864a4dedeebe9bd22b Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Sat, 31 Oct 2026 21:33:43 +0700 Subject: [PATCH 09/10] test(drive-abci): pin that the Core dust predicate agrees with the converted asset unlock output withdrawal_amount_is_core_dust reads a withdrawal's whole amount, while the asset unlock built from it pays the Core fee out of that amount. The conversion caps that fee at what the amount holds above the output script's dust threshold for every withdrawal document (none is stamped with a contract version), so the output is dust exactly when the amount is, and the predicate's verdict is the output's. The new test pins that at the P2PKH and P2SH thresholds and for a 600-duff withdrawal at 1 duff per byte, which was valid before protocol version 12: its 190-duff fee is capped at 54 duffs, the output is 546 duffs, and it pools as version 2. With the fee taken in full, as for a stamped document, the test fails (a 545-duff output becomes 355 duffs). Also joins a doc comment line the switch to the record split. Co-Authored-By: Claude Opus 5.5 --- .../v1/mod.rs | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs index b86b325d047..dca33e44c61 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -35,9 +35,8 @@ where /// active, and forks never switch off, so a recorded V24 is active at the chain locked /// height of that block and every later one. Until V24 is recorded, which can be a few /// blocks after Core activated it, withdrawals stay at version 1, which Core always - /// accepts. Core is not asked. A - /// withdrawal whose whole amount is below Core's dust threshold stays at version 1 (see - /// `withdrawal_amount_is_core_dust`). + /// accepts. Core is not asked. A withdrawal whose whole amount is below Core's dust + /// threshold stays at version 1 (see `withdrawal_amount_is_core_dust`). /// /// The version is decided here once and kept in the untied transaction bytes, which every /// later signing reuses unchanged. Nothing may rewrite those bytes, nor change how a @@ -138,6 +137,7 @@ where #[cfg(test)] mod tests { use crate::error::Error; + use crate::execution::platform_events::withdrawals::withdrawal_amount_is_core_dust; use crate::rpc::core::MockCoreRPCLike; use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; use dpp::balances::credits::CREDITS_PER_DUFF; @@ -431,6 +431,63 @@ mod tests { assert_eq!(pooled_documents, documents, "no document may be pooled"); } + /// The fee of every withdrawal document's asset unlock comes out of its amount without ever + /// taking the output below Core's dust threshold, as for one queued before the fee came out + /// of the amount, so the output is dust exactly when the whole amount is. The dust predicate, + /// which reads the amount, therefore gives the converted output's verdict: a withdrawal + /// stays at version 1, and can end FAILED, only when Core could never relay its output. + /// Amounts down to 190 duffs were valid before protocol version 12. + #[test] + fn should_classify_a_withdrawal_as_dust_exactly_when_its_converted_output_is_dust() { + let platform_version = PlatformVersion::latest(); + let dust_relay_fee_per_kb = platform_version + .system_limits + .core_dust_relay_fee_per_kb + .expect("expected a Core dust relay fee"); + + // At 1 duff per byte the requested fee is 190 duffs; above the threshold it is capped + // at what the amount holds above it. + for (output_script, cases) in [ + ( + p2pkh(), + [(545, 545, 0, 1), (546, 546, 0, 2), (600, 546, 54, 2)], + ), + ( + p2sh(), + [(539, 539, 0, 1), (540, 540, 0, 2), (600, 540, 60, 2)], + ), + ] { + for (amount_duffs, output_duffs, fee_duffs, payload_version) in cases { + let document = withdrawal_document( + amount_duffs * CREDITS_PER_DUFF, + &output_script, + WithdrawalStatus::QUEUED, + None, + platform_version, + ); + let (transactions, _, _) = + build_untied(Some(CORE_HEIGHT), vec![document.clone()], platform_version); + let transaction = decode(&transactions[0].1); + let output = &transaction.output[0]; + assert_eq!(output.value, output_duffs, "{amount_duffs} duffs"); + assert_eq!(u64::from(transaction.base_payload.fee), fee_duffs); + assert_eq!(transaction.base_payload.version, payload_version); + + let output_is_dust = output.value + < core_dust_threshold_duffs( + output.script_pubkey.as_script(), + dust_relay_fee_per_kb, + ); + assert_eq!( + withdrawal_amount_is_core_dust(&document, dust_relay_fee_per_kb) + .expect("expected to classify the withdrawal"), + output_is_dust, + "{amount_duffs} duffs: the predicate must agree with the converted output" + ); + } + } + } + /// The status the expiry step leaves an expired withdrawal of `amount` credits to /// `output_script` in, at `platform_version`: FAILED when it gives up on the withdrawal, /// BROADCASTED when it re-signs it. From 1c1505d1ca708625a45e33bca3943da290b65300 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 4 Nov 2026 18:32:31 +0700 Subject: [PATCH 10/10] test(drive-abci): return named structs from withdrawal test helpers Test-only refactor with no behaviour change; every assertion checks the same values as before. - pool_withdrawals_into_transactions_queue v2: merge `pool` and `pool_with_v24` into one `pool(.., v24_recorded)` that returns a `PoolingOutcome` (pooled, credit_pool_balance_reads, coinbase_payload_version_reads, payload_versions) instead of a tuple. Callers that do not record V24 pass `false`. - build_untied_withdrawal_transactions_from_documents v1: `build_untied` returns `UntiedWithdrawals` (transactions, total_amount, pooled_documents) instead of a 3-tuple. - strategy withdrawal_tests: `run_identity_blocks` returns `IdentityBlocks` (outcome, withdraw, idle), so its two `NetworkStrategy` values can no longer be swapped by position. No regression test: there is no behaviour change to pin. Co-Authored-By: Claude Opus 5.5 --- .../v1/mod.rs | 50 ++++++---- .../v2/mod.rs | 91 ++++++++++--------- .../test_cases/withdrawal_tests.rs | 39 ++++++-- 3 files changed, 114 insertions(+), 66 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs index dca33e44c61..44f15545997 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -280,18 +280,23 @@ mod tests { .expect("expected to record V24"); } + /// What `build_untied` built. + struct UntiedWithdrawals { + /// The untied transactions, with their withdrawal indices. + transactions: Vec, + /// The total amount of the transactions. + total_amount: Credits, + /// The documents, pooled. + pooled_documents: Vec, + } + /// Pools `documents` through the dispatcher at `platform_version`, in a block transaction - /// holding V24's record at `v24_recorded_at`, or none. Returns the untied transactions, - /// their total amount and the pooled documents. + /// holding V24's record at `v24_recorded_at`, or none. fn build_untied( v24_recorded_at: Option, mut documents: Vec, platform_version: &PlatformVersion, - ) -> ( - Vec, - Credits, - Vec, - ) { + ) -> UntiedWithdrawals { let platform = platform_without_core(); let transaction = platform.drive.grove.start_transaction(); if let Some(core_height) = v24_recorded_at { @@ -306,7 +311,11 @@ mod tests { platform_version, ) .expect("expected to build the untied withdrawal transactions"); - (transactions, total_amount, documents) + UntiedWithdrawals { + transactions, + total_amount, + pooled_documents: documents, + } } /// Decodes the untied transaction bytes the pooling step stores. @@ -340,8 +349,11 @@ mod tests { (Some(CORE_HEIGHT), 2), (Some(CORE_HEIGHT - 1_000), 2), ] { - let (transactions, total_amount, pooled_documents) = - build_untied(v24_recorded_at, documents.clone(), platform_version); + let UntiedWithdrawals { + transactions, + total_amount, + pooled_documents, + } = build_untied(v24_recorded_at, documents.clone(), platform_version); assert_eq!(total_amount, expected_total_amount); assert_eq!(pooled_documents, expected_documents); @@ -376,14 +388,14 @@ mod tests { let platform_version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); let documents = queued_withdrawals(&[1_000_000, 2_500_000], platform_version_14); - let (transactions, _, _) = - build_untied(Some(CORE_HEIGHT), documents.clone(), platform_version_14); + let transactions = + build_untied(Some(CORE_HEIGHT), documents.clone(), platform_version_14).transactions; assert!(transactions .iter() .all(|(_, bytes)| decode(bytes).base_payload.version == 1)); - let (transactions, _, _) = - build_untied(Some(CORE_HEIGHT), documents, PlatformVersion::latest()); + let transactions = + build_untied(Some(CORE_HEIGHT), documents, PlatformVersion::latest()).transactions; assert!(transactions .iter() .all(|(_, bytes)| decode(bytes).base_payload.version == 2)); @@ -465,8 +477,9 @@ mod tests { None, platform_version, ); - let (transactions, _, _) = - build_untied(Some(CORE_HEIGHT), vec![document.clone()], platform_version); + let transactions = + build_untied(Some(CORE_HEIGHT), vec![document.clone()], platform_version) + .transactions; let transaction = decode(&transactions[0].1); let output = &transaction.output[0]; assert_eq!(output.value, output_duffs, "{amount_duffs} duffs"); @@ -581,7 +594,7 @@ mod tests { let dust = (threshold_duffs - 1) * CREDITS_PER_DUFF; let viable = threshold_duffs * CREDITS_PER_DUFF; - let (transactions, _, _) = build_untied( + let transactions = build_untied( Some(CORE_HEIGHT), [dust, viable] .iter() @@ -596,7 +609,8 @@ mod tests { }) .collect(), platform_version, - ); + ) + .transactions; let payload_versions = transactions .iter() .map(|(_, bytes)| decode(bytes).base_payload.version) diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs index 8ae41796a79..ab273833578 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v2/mod.rs @@ -96,29 +96,28 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + /// What `pool` pooled and asked of Core. + struct PoolingOutcome { + /// How many of the two withdrawals were pooled. + pooled: usize, + /// How many credit pool balances were asked of Core. + credit_pool_balance_reads: usize, + /// How many coinbase payload versions were asked of Core. + coinbase_payload_version_reads: usize, + /// The asset unlock payload versions of the pooled transactions, in queue order. + payload_versions: Vec, + } + /// Pools two queued withdrawals of `amount` credits each through the dispatcher, at /// `platform_version`, against a Core whose credit pool holds `pool_duffs` at every height - /// and where V24 is not active. Returns how many were pooled, how many credit pool balances - /// were asked of Core and how many coinbase payload versions. + /// and where V24 is not active, in a block transaction that holds V24's record when + /// `v24_recorded`, as it does once `update_core_info` has recorded V24 earlier in the block. fn pool( platform_version: &PlatformVersion, amount: u64, pool_duffs: u64, - ) -> (usize, usize, usize) { - let (pooled, balance_reads, coinbase_reads, _) = - pool_with_v24(platform_version, amount, pool_duffs, false); - (pooled, balance_reads, coinbase_reads) - } - - /// `pool`, in a block transaction that holds V24's record when `v24_recorded`, as it does - /// once `update_core_info` has recorded V24 earlier in the block. Also returns the asset - /// unlock payload versions of the pooled transactions, in queue order. - fn pool_with_v24( - platform_version: &PlatformVersion, - amount: u64, - pool_duffs: u64, v24_recorded: bool, - ) -> (usize, usize, usize, Vec) { + ) -> PoolingOutcome { let mut platform = TestPlatformBuilder::new() .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() @@ -233,19 +232,19 @@ mod tests { .version }) .collect(); - ( + PoolingOutcome { pooled, - balance_reads.load(Ordering::SeqCst), - coinbase_reads.load(Ordering::SeqCst), + credit_pool_balance_reads: balance_reads.load(Ordering::SeqCst), + coinbase_payload_version_reads: coinbase_reads.load(Ordering::SeqCst), payload_versions, - ) + } } #[test] fn should_pool_what_fits_both_the_daily_limit_and_cores_credit_pool() { // Plenty in Core's pool: both withdrawals pool. assert_eq!( - pool(PlatformVersion::latest(), 1000, 1_000_000_000_000).0, + pool(PlatformVersion::latest(), 1000, 1_000_000_000_000, false).pooled, 2 ); } @@ -254,8 +253,8 @@ mod tests { fn should_leave_queued_what_cores_credit_pool_could_not_give_up() { // A pool of 1 duff, 1,000 credits: it fits one 1,000 credit withdrawal, not two, // although the daily limit (2,000 Dash before any history) would allow both. - assert_eq!(pool(PlatformVersion::latest(), 1000, 1).0, 1); - assert_eq!(pool(PlatformVersion::latest(), 1000, 0).0, 0); + assert_eq!(pool(PlatformVersion::latest(), 1000, 1, false).pooled, 1); + assert_eq!(pool(PlatformVersion::latest(), 1000, 0, false).pooled, 0); } /// Through the dispatcher on both sides of the gate: version 1 (protocol version 13) pools @@ -263,8 +262,11 @@ mod tests { #[test] fn should_hold_back_on_cores_credit_pool_only_from_protocol_version_14() { let version_13 = PlatformVersion::get(13).expect("expected protocol version 13"); - assert_eq!(pool(version_13, 1000, 1), (2, 0, 0)); - assert_eq!(pool(PlatformVersion::latest(), 1000, 1).0, 1); + let outcome = pool(version_13, 1000, 1, false); + assert_eq!(outcome.pooled, 2); + assert_eq!(outcome.credit_pool_balance_reads, 0); + assert_eq!(outcome.coinbase_payload_version_reads, 0); + assert_eq!(pool(PlatformVersion::latest(), 1000, 1, false).pooled, 1); } /// While the oldest queued withdrawal does not fit Platform's own limit nothing pools, @@ -274,18 +276,20 @@ mod tests { #[test] fn should_pool_nothing_while_the_oldest_does_not_fit_the_daily_limit() { // 3,000 Dash each, over the flat 2,000 Dash before any history. - assert_eq!( - pool( - PlatformVersion::latest(), - dash_to_credits!(3000), - 1_000_000_000_000 - ), - (0, 50, 0) - ); - assert_eq!( - pool(PlatformVersion::latest(), 1000, 1_000_000_000_000), - (2, 50, 0) + let outcome = pool( + PlatformVersion::latest(), + dash_to_credits!(3000), + 1_000_000_000_000, + false, ); + assert_eq!(outcome.pooled, 0); + assert_eq!(outcome.credit_pool_balance_reads, 50); + assert_eq!(outcome.coinbase_payload_version_reads, 0); + + let outcome = pool(PlatformVersion::latest(), 1000, 1_000_000_000_000, false); + assert_eq!(outcome.pooled, 2); + assert_eq!(outcome.credit_pool_balance_reads, 50); + assert_eq!(outcome.coinbase_payload_version_reads, 0); } /// Pooling never asks Core whether V24 is active: from protocol version 15 the payload @@ -294,12 +298,17 @@ mod tests { /// does not ask either. #[test] fn should_pool_without_asking_core_whether_v24_is_active() { - let coinbase_reads = |(_, _, coinbase_reads): (usize, usize, usize)| coinbase_reads; let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); let latest = PlatformVersion::latest(); - assert_eq!(coinbase_reads(pool(version_14, 1000, 1_000_000_000_000)), 0); - assert_eq!(coinbase_reads(pool(latest, 1000, 1_000_000_000_000)), 0); + assert_eq!( + pool(version_14, 1000, 1_000_000_000_000, false).coinbase_payload_version_reads, + 0 + ); + assert_eq!( + pool(latest, 1000, 1_000_000_000_000, false).coinbase_payload_version_reads, + 0 + ); } /// Pooling reads the Core fork record in the block's transaction, where `update_core_info` @@ -310,13 +319,13 @@ mod tests { fn should_pool_version_2_asset_unlocks_once_v24_is_recorded_in_the_block_transaction() { // 1 Dash each, above Core's dust threshold, which keeps a withdrawal at version 1. let payload_versions = |platform_version, v24_recorded| { - pool_with_v24( + pool( platform_version, dash_to_credits!(1), 1_000_000_000_000, v24_recorded, ) - .3 + .payload_versions }; let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); let latest = PlatformVersion::latest(); diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs index abd29305451..89c17e481e9 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs @@ -4188,12 +4188,21 @@ mod tests { } } - /// Runs blocks 1 and 2, which create the identities that later withdraw. Returns the - /// outcome and the strategies of a block with one withdrawal and of an empty block. + /// What `run_identity_blocks` ran, and the strategies of the blocks after it. + struct IdentityBlocks<'a> { + /// The outcome of blocks 1 and 2. + outcome: ChainExecutionOutcome<'a>, + /// The strategy of a block with one withdrawal. + withdraw: NetworkStrategy, + /// The strategy of an empty block. + idle: NetworkStrategy, + } + + /// Runs blocks 1 and 2, which create the identities that later withdraw. async fn run_identity_blocks<'a>( platform: &'a mut TempPlatform, config: &PlatformConfig, - ) -> (ChainExecutionOutcome<'a>, NetworkStrategy, NetworkStrategy) { + ) -> IdentityBlocks<'a> { let outcome = run_chain_for_strategy( platform, 2, @@ -4228,7 +4237,11 @@ mod tests { ); let idle = v24_test_strategy(vec![], Some(outcome.signer.clone())); - (outcome, withdraw, idle) + IdentityBlocks { + outcome, + withdraw, + idle, + } } /// Runs block `block`, the one after `outcome`, with `strategy`, and checks that every state @@ -4362,7 +4375,11 @@ mod tests { // V24 is active from the first core height. let core_state = V24CoreState::new(1); let mut platform = platform_with_v24_core(&config, &core_state); - let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + let IdentityBlocks { + outcome, + withdraw, + idle, + } = run_identity_blocks(&mut platform, &config).await; // Block 3 pools a withdrawal at core height 1; block 4 signs it there. let outcome = next_block(outcome, 3, &withdraw, &config).await; @@ -4438,7 +4455,11 @@ mod tests { let config = v24_test_config(); let core_state = V24CoreState::new(V24_ACTIVATION_HEIGHT); let mut platform = platform_with_v24_core(&config, &core_state); - let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + let IdentityBlocks { + outcome, + withdraw, + idle, + } = run_identity_blocks(&mut platform, &config).await; // Block 3 pools a withdrawal at core height 1, block 4 signs it. let outcome = next_block(outcome, 3, &withdraw, &config).await; @@ -4545,7 +4566,11 @@ mod tests { for coinbase_down in [false, true] { let core_state = V24CoreState::new(V24_ACTIVATION_HEIGHT); let mut platform = platform_with_v24_core(&config, &core_state); - let (outcome, withdraw, idle) = run_identity_blocks(&mut platform, &config).await; + let IdentityBlocks { + outcome, + withdraw, + idle, + } = run_identity_blocks(&mut platform, &config).await; // Block 3 moves the chain locked height to the activation height and records V24. core_state.lock().unwrap().chain_locked_height = V24_ACTIVATION_HEIGHT;