From e4c9826447962c2e7417eb8368b231a49386720e Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 1 Oct 2024 16:43:27 +0300 Subject: [PATCH 1/4] chore(drive): logs and metrics for withdrawal daily limit --- .../engine/run_block_proposal/v0/mod.rs | 7 ++++-- .../v0/mod.rs | 9 +++++--- .../v1/mod.rs | 22 +++++++++++++++++-- packages/rs-drive-abci/src/metrics.rs | 18 +++++++++++++-- .../calculate_current_withdrawal_limit/mod.rs | 17 +++++++++++++- .../v0/mod.rs | 9 +++++--- .../mod.rs | 3 ++- 7 files changed, 71 insertions(+), 14 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs b/packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs index 1ae4a1e639..9c45694f6c 100644 --- a/packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs @@ -293,6 +293,8 @@ where // required for signature verification (core height and quorum hash) // Then we save unsigned transaction bytes to block execution context // to be signed (on extend_vote), verified (on verify_vote) and broadcasted (on finalize_block) + // Also, the dequeued untiled transaction added to the broadcasted transaction queue to for further + // resigning in case of failures. let unsigned_withdrawal_transaction_bytes = self .dequeue_and_build_unsigned_withdrawal_transactions( validator_set_quorum_hash, @@ -329,12 +331,13 @@ where // Corresponding withdrawal documents are changed from queued to pooled self.pool_withdrawals_into_transactions_queue( &block_info, - &last_committed_platform_state, + last_committed_platform_state, Some(transaction), platform_version, )?; // Cleans up the expired locks for withdrawal amounts + // to update daily withdrawal limit // This is for example when we make a withdrawal for 30 Dash // But we can only withdraw 1000 Dash a day // after the withdrawal we should only be able to withdraw 970 Dash @@ -350,7 +353,7 @@ where let mut block_execution_context: BlockExecutionContext = block_execution_context::v0::BlockExecutionContextV0 { block_state_info: block_state_info.into(), - epoch_info: epoch_info.clone(), + epoch_info, unsigned_withdrawal_transactions: unsigned_withdrawal_transaction_bytes, block_platform_state, proposer_results: None, diff --git a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v0/mod.rs index 01c84035f6..610d8e3fcc 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/withdrawals/pool_withdrawals_into_transactions_queue/v0/mod.rs @@ -19,18 +19,21 @@ where platform_version: &PlatformVersion, ) -> Result<(), Error> { // Currently Core only supports using the first 2 quorums (out of 24 for mainnet). - // For us, we just use the latest quorum to be extra safe. + // For extra safety, we use only the first quorum because our quorum info based + // on core chain locked height which is always late comparing with Core. let Some(position_of_current_quorum) = last_committed_platform_state.current_validator_set_position_in_list_by_most_recent() else { - tracing::warn!("Current quorum not in current validator set, not making withdrawals"); + tracing::warn!("Current quorum not in current validator set, do not pool withdrawals"); + return Ok(()); }; if position_of_current_quorum != 0 { tracing::debug!( - "Current quorum is not most recent, it is in position {}, not making withdrawals", + "Current quorum is not most recent, it is in position {}, do not pool withdrawals", position_of_current_quorum ); + return Ok(()); } // Just use the v1 as to not duplicate code 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 cf7c3cec29..67e9c2b0ec 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 @@ -1,4 +1,5 @@ use dpp::block::block_info::BlockInfo; +use metrics::gauge; use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::document::DocumentV0Getters; @@ -9,6 +10,9 @@ use drive::grovedb::TransactionArg; use dpp::system_data_contracts::withdrawals_contract; use dpp::system_data_contracts::withdrawals_contract::v1::document_types::withdrawal; +use crate::metrics::{ + GAUGE_CREDIT_WITHDRAWAL_LIMIT_AVAILABLE, GAUGE_CREDIT_WITHDRAWAL_LIMIT_TOTAL, +}; use crate::{ error::{execution::ExecutionError, Error}, platform_types::platform::Platform, @@ -28,6 +32,10 @@ where transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result<(), Error> { + // TODO: It will ne more efficient to query documents less than the current limit + // in this case we won't fetch documents if we aren't going to process and + // documents with big amounts created earlier won't block processing of withdrawals with smaller + // amounts (more precise limiting) let documents = self.drive.fetch_oldest_withdrawal_documents_by_status( withdrawals_contract::WithdrawalStatus::QUEUED.into(), platform_version @@ -42,10 +50,16 @@ where } // Only take documents up to the withdrawal amount - let current_withdrawal_limit = self + let withdrawals_info = self .drive .calculate_current_withdrawal_limit(transaction, platform_version)?; + let current_withdrawal_limit = withdrawals_info.available(); + + // Store prometheus metrics + gauge!(GAUGE_CREDIT_WITHDRAWAL_LIMIT_AVAILABLE).set(current_withdrawal_limit as f64); + gauge!(GAUGE_CREDIT_WITHDRAWAL_LIMIT_TOTAL).set(withdrawals_info.daily_maximum as f64); + // Only process documents up to the current withdrawal limit. let mut total_withdrawal_amount = 0u64; @@ -65,8 +79,12 @@ where )) })?; + // If adding this withdrawal would exceed the limit, stop processing further. if potential_total_withdrawal_amount > current_withdrawal_limit { - // If adding this withdrawal would exceed the limit, stop processing further. + tracing::warn!( + "Pooling is limited due to daily withdrawals limit. {} credits left", + current_withdrawal_limit + ); break; } diff --git a/packages/rs-drive-abci/src/metrics.rs b/packages/rs-drive-abci/src/metrics.rs index 5c90ec2501..943f0d5aae 100644 --- a/packages/rs-drive-abci/src/metrics.rs +++ b/packages/rs-drive-abci/src/metrics.rs @@ -6,7 +6,7 @@ use std::time::Duration; use std::{sync::Once, time::Instant}; use dapi_grpc::tonic::Code; -use metrics::{counter, describe_counter, describe_histogram, histogram, Label}; +use metrics::{counter, describe_counter, describe_gauge, describe_histogram, histogram, Label}; use metrics_exporter_prometheus::PrometheusBuilder; /// Default Prometheus port (29090) @@ -29,6 +29,10 @@ pub const LABEL_STATE_TRANSITION_NAME: &str = "st_name"; const LABEL_STATE_TRANSITION_EXECUTION_CODE: &str = "st_exec_code"; /// Metrics label to specify check tx mode: 0 - first time check, 1 - recheck pub const LABEL_CHECK_TX_MODE: &str = "check_tx_mode"; +/// Withdrawal daily limit available credits +pub const GAUGE_CREDIT_WITHDRAWAL_LIMIT_AVAILABLE: &str = "credit_withdrawal_limit_available"; +/// Total withdrawal daily limit in credits +pub const GAUGE_CREDIT_WITHDRAWAL_LIMIT_TOTAL: &str = "credit_withdrawal_limit_total"; /// Error returned by metrics subsystem #[derive(thiserror::Error, Debug)] @@ -210,7 +214,17 @@ impl Prometheus { HISTOGRAM_QUERY_DURATION, metrics::Unit::Seconds, "Duration of query request execution inside Drive per endpoint, in seconds" - ) + ); + + describe_gauge!( + GAUGE_CREDIT_WITHDRAWAL_LIMIT_AVAILABLE, + "Available withdrawal limit for last 24 hours in credits" + ); + + describe_gauge!( + GAUGE_CREDIT_WITHDRAWAL_LIMIT_TOTAL, + "Total withdrawal limit for last 24 hours in credits" + ); }); } } diff --git a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs index 172eda82c9..1ce87282e3 100644 --- a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs +++ b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs @@ -7,6 +7,21 @@ use platform_version::version::PlatformVersion; mod v0; +/// Daily withdrawal limit information +pub struct WithdrawalLimitInfo { + /// The total maximum withdrawal amount allowed in a 24-hour period. + pub daily_maximum: Credits, + /// The amount already withdrawn in the last 24 hours. + pub withdrawals_amount: Credits, +} + +impl WithdrawalLimitInfo { + /// Calculates the available credits to withdraw + pub fn available(&self) -> Credits { + self.daily_maximum.saturating_sub(self.withdrawals_amount) + } +} + impl Drive { /// Calculates the current withdrawal limit based on the total credits available in the platform /// and the amount already withdrawn in the last 24 hours, using the appropriate version-specific logic. @@ -34,7 +49,7 @@ impl Drive { &self, transaction: TransactionArg, platform_version: &PlatformVersion, - ) -> Result { + ) -> Result { match platform_version .drive .methods diff --git a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/v0/mod.rs b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/v0/mod.rs index f7610ec941..c4f2316d3f 100644 --- a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/v0/mod.rs +++ b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/v0/mod.rs @@ -1,4 +1,5 @@ use crate::drive::balances::TOTAL_SYSTEM_CREDITS_STORAGE_KEY; +use crate::drive::identity::withdrawals::calculate_current_withdrawal_limit::WithdrawalLimitInfo; use crate::drive::identity::withdrawals::paths::{ get_withdrawal_root_path, WITHDRAWAL_TRANSACTIONS_SUM_AMOUNT_TREE_KEY, }; @@ -7,7 +8,6 @@ use crate::drive::Drive; use crate::error::drive::DriveError; use crate::error::Error; use crate::util::grove_operations::DirectQueryType; -use dpp::fee::Credits; use dpp::withdrawal::daily_withdrawal_limit::daily_withdrawal_limit; use grovedb::TransactionArg; use platform_version::version::PlatformVersion; @@ -42,7 +42,7 @@ impl Drive { &self, transaction: TransactionArg, platform_version: &PlatformVersion, - ) -> Result { + ) -> Result { // The current withdrawal limit has two components // 1. The total maximum that we are allowed to do in 24 hours // 2. The amount that we have already withdrawn in the last 24 hours @@ -81,6 +81,9 @@ impl Drive { )) })?; - Ok(daily_maximum.saturating_sub(withdrawal_amount_in_last_day)) + Ok(WithdrawalLimitInfo { + daily_maximum, + withdrawals_amount: withdrawal_amount_in_last_day, + }) } } diff --git a/packages/rs-drive/src/drive/identity/withdrawals/transaction/queue/dequeue_untied_withdrawal_transactions/mod.rs b/packages/rs-drive/src/drive/identity/withdrawals/transaction/queue/dequeue_untied_withdrawal_transactions/mod.rs index 3423114aaf..cdc4672759 100644 --- a/packages/rs-drive/src/drive/identity/withdrawals/transaction/queue/dequeue_untied_withdrawal_transactions/mod.rs +++ b/packages/rs-drive/src/drive/identity/withdrawals/transaction/queue/dequeue_untied_withdrawal_transactions/mod.rs @@ -9,7 +9,8 @@ use grovedb::TransactionArg; use platform_version::version::PlatformVersion; impl Drive { - /// Get specified amount of withdrawal transactions from the DB + /// Deque specified amount of untiled withdrawal transactions + /// and move them to broadcasted queue pub fn dequeue_untied_withdrawal_transactions( &self, limit: u16, From 0419c4b0e06ab6e355d91a8481fbbce85eaa998b Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 1 Oct 2024 19:22:56 +0300 Subject: [PATCH 2/4] chore: switch limit warning to debug --- .../pool_withdrawals_into_transactions_queue/v1/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 67e9c2b0ec..5755d070e9 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 @@ -32,7 +32,7 @@ where transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result<(), Error> { - // TODO: It will ne more efficient to query documents less than the current limit + // TODO: It will be more efficient to query documents less than the current limit // in this case we won't fetch documents if we aren't going to process and // documents with big amounts created earlier won't block processing of withdrawals with smaller // amounts (more precise limiting) @@ -81,7 +81,7 @@ where // If adding this withdrawal would exceed the limit, stop processing further. if potential_total_withdrawal_amount > current_withdrawal_limit { - tracing::warn!( + tracing::debug!( "Pooling is limited due to daily withdrawals limit. {} credits left", current_withdrawal_limit ); From 920dc2b143ab965483cc20e5f0acad76ce18008f Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 1 Oct 2024 19:24:42 +0300 Subject: [PATCH 3/4] refactor: derive common traits to WithdrawalLimitInfo --- .../withdrawals/calculate_current_withdrawal_limit/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs index 1ce87282e3..1d31584f69 100644 --- a/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs +++ b/packages/rs-drive/src/drive/identity/withdrawals/calculate_current_withdrawal_limit/mod.rs @@ -8,6 +8,7 @@ use platform_version::version::PlatformVersion; mod v0; /// Daily withdrawal limit information +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct WithdrawalLimitInfo { /// The total maximum withdrawal amount allowed in a 24-hour period. pub daily_maximum: Credits, From 604578ea25992863716c3bc850dcc1b2a45a0a43 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 1 Oct 2024 19:37:44 +0300 Subject: [PATCH 4/4] docs: remove wrong todo --- .../pool_withdrawals_into_transactions_queue/v1/mod.rs | 4 ---- 1 file changed, 4 deletions(-) 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 5755d070e9..b0b0f99e65 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 @@ -32,10 +32,6 @@ where transaction: TransactionArg, platform_version: &PlatformVersion, ) -> Result<(), Error> { - // TODO: It will be more efficient to query documents less than the current limit - // in this case we won't fetch documents if we aren't going to process and - // documents with big amounts created earlier won't block processing of withdrawals with smaller - // amounts (more precise limiting) let documents = self.drive.fetch_oldest_withdrawal_documents_by_status( withdrawals_contract::WithdrawalStatus::QUEUED.into(), platform_version