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..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 @@ -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,82 @@ 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()) + .expect("expected to read the rejections directory") + .count() + } + + /// 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); + } + + /// 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); + assert_eq!(rejections_recorded_for("bad-assetunlocktx-version-2"), 1); + } +} 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..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,8 +7,10 @@ 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; impl Platform where @@ -24,6 +26,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. + /// * `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 @@ -35,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 @@ -49,9 +55,16 @@ where block_info, platform_version, ), + 1 => self.build_untied_withdrawal_transactions_from_documents_v1( + documents, + start_index, + block_info, + transaction, + 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..44f15545997 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/build_untied_withdrawal_transactions_from_documents/v1/mod.rs @@ -0,0 +1,634 @@ +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::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::platform_value::btreemap_extensions::BTreeValueMapHelper; +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, +}; +use dpp::version::PlatformVersion; +use drive::drive::system::core_fork_activations::CoreFork; +use drive::grovedb::TransactionArg; + +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 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 + /// 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, + transaction: TransactionArg, + 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", + )))?; + + 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. + |(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)) + }, + ) + } +} + +#[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; + 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::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::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. + const CORE_HEIGHT: u32 = 2_000; + + 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, + 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]) + } + + /// 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, + 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() + } + + /// 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"); + } + + /// 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. + fn build_untied( + v24_recorded_at: Option, + mut documents: Vec, + platform_version: &PlatformVersion, + ) -> UntiedWithdrawals { + 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"); + UntiedWithdrawals { + transactions, + total_amount, + pooled_documents: documents, + } + } + + /// 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") + } + + /// 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_v24_record() { + let platform_version = PlatformVersion::latest(); + let documents = queued_withdrawals(&[1_000_000, 2_500_000, 70_000_000], platform_version); + + 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( + &mut expected_documents, + START_INDEX, + &block_info(), + platform_version, + ) + .expect("expected to build the untied withdrawal transactions"); + + for (v24_recorded_at, payload_version) in [ + (None, 1), + (Some(CORE_HEIGHT), 2), + (Some(CORE_HEIGHT - 1_000), 2), + ] { + 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); + 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, + "V24 recorded at {v24_recorded_at:?}" + ); + transaction.base_payload.version = 1; + assert_eq!(transaction, decode(expected_bytes)); + } + if payload_version == 1 { + assert_eq!( + transactions, expected_transactions, + "before V24 is recorded the bytes are version 0's" + ); + } + } + } + + /// 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_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 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()).transactions; + assert!(transactions + .iter() + .all(|(_, bytes)| decode(bytes).base_payload.version == 2)); + } + + /// 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_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"); + + 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, + ); + + assert!( + matches!( + result, + Err(Error::Drive(DriveErrorKind::Drive( + DriveError::CorruptedSerialization(_) + ))) + ), + "{result:?}" + ); + 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) + .transactions; + 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. + 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( + Some(CORE_HEIGHT), + [dust, viable] + .iter() + .map(|amount| { + withdrawal_document( + *amount, + &output_script, + WithdrawalStatus::QUEUED, + None, + platform_version, + ) + }) + .collect(), + platform_version, + ) + .transactions; + 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/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/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 8bd5a66bad4..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 @@ -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,14 +91,33 @@ 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; + /// 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. - /// 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, 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, + v24_recorded: bool, + ) -> PoolingOutcome { let mut platform = TestPlatformBuilder::new() .with_initial_protocol_version(platform_version.protocol_version) .build_with_mock_rpc() @@ -111,9 +132,28 @@ 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(); + 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, @@ -175,14 +215,36 @@ mod tests { ) .expect("to fetch withdrawal documents") .len(); - (pooled, balance_reads.load(Ordering::SeqCst)) + 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(); + PoolingOutcome { + pooled, + 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 ); } @@ -191,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 @@ -200,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)); - 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, @@ -211,17 +276,62 @@ 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. + 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 + /// 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_pool_without_asking_core_whether_v24_is_active() { + let version_14 = PlatformVersion::get(14).expect("expected protocol version 14"); + let latest = PlatformVersion::latest(); + assert_eq!( - pool( - PlatformVersion::latest(), - dash_to_credits!(3000), - 1_000_000_000_000 - ), - (0, 50) + pool(version_14, 1000, 1_000_000_000_000, false).coinbase_payload_version_reads, + 0 ); assert_eq!( - pool(PlatformVersion::latest(), 1000, 1_000_000_000_000), - (2, 50) + 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` + /// 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( + platform_version, + dash_to_credits!(1), + 1_000_000_000_000, + v24_recorded, + ) + .payload_versions + }; + 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/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..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,34 +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. -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; @@ -506,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 + ); + } } } } 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 539d84a480b..78aef779545 100644 --- a/packages/rs-drive-abci/src/platform_types/platform/mock.rs +++ b/packages/rs-drive-abci/src/platform_types/platform/mock.rs @@ -45,8 +45,8 @@ impl Platform { .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. + // no block records V24's activation and 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))); 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 fda1c95ffdb..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 @@ -5,12 +5,18 @@ mod tests { ChainExecutionOutcome, ChainExecutionParameters, NetworkStrategy, StrategyRandomness, }; use dpp::dashcore::bls_sig_utils::BLSSignature; + use dpp::dashcore::consensus; use dpp::dashcore::hashes::Hash; + use dpp::dashcore::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; 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}; use dpp::identity::{KeyType, Purpose, SecurityLevel}; + use dpp::platform_value::btreemap_extensions::BTreeValueMapHelper; use dpp::withdrawal::WithdrawalTransactionIndex; use dpp::{dash_to_credits, dash_to_duffs}; use drive::config::DEFAULT_QUERY_LIMIT; @@ -26,9 +32,12 @@ mod tests { ChainLockConfig, ExecutionConfig, InstantLockConfig, PlatformConfig, PlatformTestConfig, ValidatorSetConfig, }; - use drive_abci::test::helpers::setup::TestPlatformBuilder; + 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; use platform_version::version::PlatformVersion; + use simple_signer::signer::SimpleSigner; use std::collections::BTreeMap; use std::sync::{Arc, Mutex}; use strategy_tests::frequency::Frequency; @@ -3981,4 +3990,623 @@ mod tests { "epoch 0 must have paid Core rewards into Platform" ); } + + /// Core as the version 2 asset unlock tests see it. + struct V24CoreState { + /// What Core says about each withdrawal index; an index it does not list is unknown. + asset_unlock_statuses: BTreeMap, + 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, + /// Whether Core fails to read a coinbase, as it does while it is down. + coinbase_down: bool, + } + + 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![], + coinbase_down: false, + })) + } + + /// 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() + } + } + + /// 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"); + }; + 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(), + 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); + 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 })) + }); + + 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 + } + + /// 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, + ) -> 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() + } + } + + /// 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, + ) -> IdentityBlocks<'a> { + 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())); + + IdentityBlocks { + 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 + } + + /// The withdrawal documents in `status` after `outcome`, oldest first. + 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 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; + 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 + ); + 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 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( + ) { + 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 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; + 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, 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, &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, 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_after_recorded = + asset_unlock_index(outcome.withdrawals.first().expect("signed")); + + 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 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; + let outcome = next_block(outcome, 3, &idle, &config).await; + assert_eq!( + 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!( + core_state.lock().unwrap().coinbase_reads, + vec![1, V24_ACTIVATION_HEIGHT], + "no block after the one that recorded V24 asks Core about it" + ); + app_hashes.push( + outcome + .abci_app + .platform + .state + .load() + .last_committed_block_app_hash(), + ); + } + assert_eq!( + 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 064fa805aeb..dc31622524d 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 @@ -2,7 +2,8 @@ use super::v10::DRIVE_ABCI_METHOD_VERSIONS_V10; use super::{ DkgMiningSchedule, DriveAbciCoreBasedUpdatesMethodVersions, DriveAbciCoreChainLockMethodVersionsAndConstants, DriveAbciEngineMethodVersions, - DriveAbciMethodVersions, DriveAbciVotingMethodVersions, + DriveAbciIdentityCreditWithdrawalMethodVersions, DriveAbciMethodVersions, + DriveAbciVotingMethodVersions, }; /// Dash Core's DKG schedule (`llmq/params.h`) of the non-rotating quorum types, the ones chain @@ -95,8 +96,9 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ /// `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; in the consensus evidence policy; and in the chain lock -/// verification. The chain lock changes only work as a whole: +/// not asked again once it is recorded; in the asset unlocks it emits for withdrawals, described +/// below; in the consensus evidence policy; and in the chain lock verification. The chain lock +/// changes only work 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 @@ -108,6 +110,11 @@ const DKG_MINING_SCHEDULES_V1: &[DkgMiningSchedule] = &[ /// above. `verify_chain_lock_locally` 1 leaves to Core chain locks that 0 rejected, so it is /// only safe together with this check. /// - `engine.consensus_params_update` 2 installs the full evidence policy when it changes. +/// +/// The asset unlocks: `withdrawals.build_untied_withdrawal_transactions_from_documents` is 1 +/// 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 { engine: DriveAbciEngineMethodVersions { consensus_params_update: 2, @@ -130,5 +137,9 @@ pub const DRIVE_ABCI_METHOD_VERSIONS_V11: DriveAbciMethodVersions = DriveAbciMet on_yes_no_vote_poll_finished: Some(0), ..DRIVE_ABCI_METHOD_VERSIONS_V10.voting }, + withdrawals: DriveAbciIdentityCreditWithdrawalMethodVersions { + 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/system_limits/mod.rs b/packages/rs-platform-version/src/version/system_limits/mod.rs index b9d7f448f83..500ae9835f1 100644 --- a/packages/rs-platform-version/src/version/system_limits/mod.rs +++ b/packages/rs-platform-version/src/version/system_limits/mod.rs @@ -242,7 +242,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 1a0874a0dcd..bbb18f00188 100644 --- a/packages/rs-platform-version/src/version/v15.rs +++ b/packages/rs-platform-version/src/version/v15.rs @@ -409,6 +409,7 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// 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. +/// /// 19. **A contest is decided by its index values**: a document joins its type's contest /// when every `fieldMatches` entry matches it, and two documents with the same index /// values share one unique index entry. A `fieldMatches` field outside the index, or a @@ -428,12 +429,29 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15; /// registered earlier with such a contest keeps it, but an update, which carries the /// whole contract, is refused like a registration. Protocol v14 and earlier keep /// `check_document_type_schema` 0 and document create state validation 2. +/// +/// 20. **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 once Platform has recorded Dash Core's V24 as active (note 18), 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) and verify_private_document_entries Some(0) a page of a private type's store (note 13); state transition method versions v5: document_create_transition 1 deletes a consumed document from what its lookup read (note 11); document method versions v5: insert_empty_private_document_store_operation Some(0) creates a private document type's store, and fetch_private_document_entries and prove_private_document_entries Some(0) read and prove a page of it (note 13); apply_drive_operations 2 resolves a batch's estimate, remove_reference_for_index_level_for_contract_operations 2, stateless_delete_of_non_tree_for_costs 1 and for_total_system_credits_update 1 describe their layers as they are (note 14); create_initial_state_structure 5 adds the token marketplace escrow trees; calculate_total_tokens_balance 2 counts escrowed tokens; the escrow primitives; the identity balance, token balance, prefunded specialized balance and contract fee pot estimations (for_balances 1, for_token_balances 1, estimated_cost_for_prefunded_specialized_balance_update 2, add_estimation_costs_for_contract_fee_pot_update 1) keep a deeper layer description (note 17); fetch_core_fork_activation_height and store_core_fork_activation_height Some(0) keep the Core fork activations (note 18) 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; consensus_params_update 2 installs the full evidence policy (note 16); update_core_info 1 records the chain locked height at which Core's V24 is first seen active (note 18) + 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; consensus_params_update 2 installs the full evidence policy (note 16); update_core_info 1 records the chain locked height at which Core's V24 is first seen active (note 18); build_untied_withdrawal_transactions_from_documents 1 emits asset unlock payload version 2 once V24 is recorded as active (note 20) 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); document_reference_validation 1 keeps what a consumed commitment's lookup read (note 11); document create structure_validation 2 holds a decision proposal to its fund, rule and end date (note 12); data trigger bindings 3 refuse a replace or a delete of a token market order (note 17); document create state_validation 3 keeps a create outside a contest off the values of a live one its index values do not decide (note 19) withdrawal_constants: DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V3, query: DRIVE_ABCI_QUERY_VERSIONS_V2,