Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(drive): reduce withdrawal limits [WIP] #2286

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ use crate::ProtocolError;
use platform_version::version::PlatformVersion;

mod v0;
mod v1;

pub fn daily_withdrawal_limit(
total_credits_in_platform: Credits,
platform_version: &PlatformVersion,
) -> Result<Credits, ProtocolError> {
match platform_version.dpp.methods.daily_withdrawal_limit {
0 => Ok(daily_withdrawal_limit_v0(total_credits_in_platform)),
1 => Ok(v1::daily_withdrawal_limit_v1()),
v => Err(ProtocolError::UnknownVersionError(format!(
"Unknown daily_withdrawal_limit version {v}"
))),
Expand Down
12 changes: 12 additions & 0 deletions packages/rs-dpp/src/withdrawal/daily_withdrawal_limit/v1/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use crate::fee::Credits;

/// Set constant withdrawal daily limit to 2000 Dash
///
/// # Returns
///
/// * `Credits`: The calculated daily withdrawal limit based on the available credits.
///
pub fn daily_withdrawal_limit_v1() -> Credits {
// 2000 Dash
200_000_000_000_000
}
Comment on lines +9 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using named constants for better maintainability.

The magic number could be harder to maintain and verify. Consider breaking it down into named constants that make the calculation clear.

Here's a suggested improvement:

 pub fn daily_withdrawal_limit_v1() -> Credits {
-    // 2000 Dash
-    200_000_000_000_000
+    const DASH_TO_CREDITS: Credits = 100_000_000_000;
+    const DASH_LIMIT: Credits = 2000;
+    DASH_LIMIT * DASH_TO_CREDITS
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn daily_withdrawal_limit_v1() -> Credits {
// 2000 Dash
200_000_000_000_000
}
pub fn daily_withdrawal_limit_v1() -> Credits {
const DASH_TO_CREDITS: Credits = 100_000_000_000;
const DASH_LIMIT: Credits = 2000;
DASH_LIMIT * DASH_TO_CREDITS
}

Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,13 @@ where
})?; // This is a system error

// Rebroadcast expired withdrawals if they exist
// We do that before we mark withdrawals as expired
// to rebroadcast them on the next block but not the same
// one
// TODO: It must be also only on core height change
self.rebroadcast_expired_withdrawal_documents(
&block_info,
&last_committed_platform_state,
last_committed_platform_state,
transaction,
platform_version,
)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use crate::error::execution::ExecutionError;
use crate::error::Error;
use crate::platform_types::platform::Platform;
use crate::rpc::core::{
CoreRPCLike, CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED, CORE_RPC_ERROR_ASSET_UNLOCK_NO_ACTIVE_QUORUM,
CORE_RPC_TX_ALREADY_IN_CHAIN,
};
use crate::rpc::core::{CoreRPCLike, CORE_RPC_TX_ALREADY_IN_CHAIN};
use dashcore_rpc::jsonrpc;
use dashcore_rpc::Error as CoreRPCError;
use dpp::dashcore::bls_sig_utils::BLSSignature;
Expand All @@ -18,6 +15,14 @@ use std::path::Path;
use std::time::{SystemTime, UNIX_EPOCH};
use tenderdash_abci::proto::types::VoteExtension;

// This error is returned when Core can't find a quorum for the asset unlock transaction in Core 21
const CORE_RPC_ERROR_ASSET_UNLOCK_NO_ACTIVE_QUORUM: &str = "bad-assetunlock-not-active-quorum";

// This error replaced the previous since Core 22 to make it more verbose
const CORE_RPC_ERROR_ASSET_UNLOCK_TOO_OLD_QUORUM: &str = "bad-assetunlock-too-old-quorum";

const CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED: &str = "bad-assetunlock-too-late";

impl<C> Platform<C>
where
C: CoreRPCLike,
Expand Down Expand Up @@ -90,7 +95,8 @@ where
}
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 =>
|| e.message == CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED
|| e.message == CORE_RPC_ERROR_ASSET_UNLOCK_TOO_OLD_QUORUM =>
{
tracing::debug!(
tx_id = transaction.txid().to_string(),
Expand Down
5 changes: 0 additions & 5 deletions packages/rs-drive-abci/src/rpc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,6 @@ pub const CORE_RPC_INVALID_ADDRESS_OR_KEY: i32 = -5;
/// Invalid, missing or duplicate parameter
pub const CORE_RPC_INVALID_PARAMETER: i32 = -8;

/// Asset Unlock consensus error "bad-assetunlock-not-active-quorum"
pub const CORE_RPC_ERROR_ASSET_UNLOCK_NO_ACTIVE_QUORUM: &str = "bad-assetunlock-not-active-quorum";
/// Asset Unlock consensus error "bad-assetunlock-not-active-quorum"
pub const CORE_RPC_ERROR_ASSET_UNLOCK_EXPIRED: &str = "bad-assetunlock-too-late";

macro_rules! retry {
($action:expr) => {{
/// Maximum number of retry attempts
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use versioned_feature_core::FeatureVersion;

pub mod v1;
pub mod v2;

#[derive(Clone, Debug, Default)]
pub struct DPPMethodVersions {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
use crate::version::dpp_versions::dpp_method_versions::DPPMethodVersions;
pub const DPP_METHOD_VERSIONS_V2: DPPMethodVersions = DPPMethodVersions {
epoch_core_reward_credits_for_distribution: 0,
daily_withdrawal_limit: 1,
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub mod v1;
pub mod v2;
pub mod v3;
pub mod v4;
pub mod v5;

#[derive(Clone, Debug, Default)]
pub struct DriveAbciMethodVersions {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
use crate::version::drive_abci_versions::drive_abci_method_versions::{
DriveAbciBlockEndMethodVersions, DriveAbciBlockFeeProcessingMethodVersions,
DriveAbciBlockStartMethodVersions, DriveAbciCoreBasedUpdatesMethodVersions,
DriveAbciCoreChainLockMethodVersionsAndConstants, DriveAbciCoreInstantSendLockMethodVersions,
DriveAbciEngineMethodVersions, DriveAbciEpochMethodVersions,
DriveAbciFeePoolInwardsDistributionMethodVersions,
DriveAbciFeePoolOutwardsDistributionMethodVersions,
DriveAbciIdentityCreditWithdrawalMethodVersions, DriveAbciInitializationMethodVersions,
DriveAbciMasternodeIdentitiesUpdatesMethodVersions, DriveAbciMethodVersions,
DriveAbciPlatformStateStorageMethodVersions, DriveAbciProtocolUpgradeMethodVersions,
DriveAbciStateTransitionProcessingMethodVersions, DriveAbciVotingMethodVersions,
};

pub const DRIVE_ABCI_METHOD_VERSIONS_V5: DriveAbciMethodVersions = DriveAbciMethodVersions {
engine: DriveAbciEngineMethodVersions {
init_chain: 0,
check_tx: 0,
run_block_proposal: 0,
finalize_block_proposal: 0,
consensus_params_update: 1,
},
initialization: DriveAbciInitializationMethodVersions {
initial_core_height_and_time: 0,
create_genesis_state: 0,
},
core_based_updates: DriveAbciCoreBasedUpdatesMethodVersions {
update_core_info: 0,
update_masternode_list: 0,
update_quorum_info: 0,
masternode_updates: DriveAbciMasternodeIdentitiesUpdatesMethodVersions {
get_voter_identity_key: 0,
get_operator_identity_keys: 0,
get_owner_identity_withdrawal_key: 0,
get_owner_identity_owner_key: 0,
get_voter_identifier_from_masternode_list_item: 0,
get_operator_identifier_from_masternode_list_item: 0,
create_operator_identity: 0,
create_owner_identity: 1,
create_voter_identity: 0,
disable_identity_keys: 0,
update_masternode_identities: 0,
update_operator_identity: 0,
update_owner_withdrawal_address: 1,
update_voter_identity: 0,
},
},
protocol_upgrade: DriveAbciProtocolUpgradeMethodVersions {
check_for_desired_protocol_upgrade: 1,
upgrade_protocol_version_on_epoch_change: 0,
perform_events_on_first_block_of_protocol_change: Some(0),
protocol_version_upgrade_percentage_needed: 67,
shumkov marked this conversation as resolved.
Show resolved Hide resolved
},
block_fee_processing: DriveAbciBlockFeeProcessingMethodVersions {
add_process_epoch_change_operations: 0,
process_block_fees: 0,
},
core_chain_lock: DriveAbciCoreChainLockMethodVersionsAndConstants {
choose_quorum: 0,
verify_chain_lock: 0,
verify_chain_lock_locally: 0,
verify_chain_lock_through_core: 0,
make_sure_core_is_synced_to_chain_lock: 0,
recent_block_count_amount: 2,
},
core_instant_send_lock: DriveAbciCoreInstantSendLockMethodVersions {
verify_recent_signature_locally: 0,
},
fee_pool_inwards_distribution: DriveAbciFeePoolInwardsDistributionMethodVersions {
add_distribute_block_fees_into_pools_operations: 0,
add_distribute_storage_fee_to_epochs_operations: 0,
},
fee_pool_outwards_distribution: DriveAbciFeePoolOutwardsDistributionMethodVersions {
add_distribute_fees_from_oldest_unpaid_epoch_pool_to_proposers_operations: 0,
add_epoch_pool_to_proposers_payout_operations: 0,
find_oldest_epoch_needing_payment: 0,
fetch_reward_shares_list_for_masternode: 0,
},
withdrawals: DriveAbciIdentityCreditWithdrawalMethodVersions {
build_untied_withdrawal_transactions_from_documents: 0,
dequeue_and_build_unsigned_withdrawal_transactions: 0,
fetch_transactions_block_inclusion_status: 0,
// We changed `pool_withdrawals_into_transactions_queue` to v1 in order to add pool
// withdrawals on any validator quorums. v0 allowed us to pool only on the first two
// quorums as workaround for Core v21 bug.
pool_withdrawals_into_transactions_queue: 1,
update_broadcasted_withdrawal_statuses: 0,
rebroadcast_expired_withdrawal_documents: 0,
append_signatures_and_broadcast_withdrawal_transactions: 0,
cleanup_expired_locks_of_withdrawal_amounts: 0,
},
voting: DriveAbciVotingMethodVersions {
keep_record_of_finished_contested_resource_vote_poll: 0,
clean_up_after_vote_poll_end: 0,
clean_up_after_contested_resources_vote_poll_end: 0,
check_for_ended_vote_polls: 0,
tally_votes_for_contested_document_resource_vote_poll: 0,
award_document_to_winner: 0,
delay_vote_poll: 0,
run_dao_platform_events: 0,
remove_votes_for_removed_masternodes: 0,
},
state_transition_processing: DriveAbciStateTransitionProcessingMethodVersions {
execute_event: 0,
process_raw_state_transitions: 0,
decode_raw_state_transitions: 0,
validate_fees_of_event: 0,
},
epoch: DriveAbciEpochMethodVersions {
gather_epoch_info: 0,
get_genesis_time: 0,
},
block_start: DriveAbciBlockStartMethodVersions {
clear_drive_block_cache: 0,
},
block_end: DriveAbciBlockEndMethodVersions {
update_state_cache: 0,
update_drive_cache: 0,
validator_set_update: 2,
},
platform_state_storage: DriveAbciPlatformStateStorageMethodVersions {
fetch_platform_state: 0,
store_platform_state: 0,
},
};
4 changes: 3 additions & 1 deletion packages/rs-platform-version/src/version/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod protocol_version;
use crate::version::v5::PROTOCOL_VERSION_5;
use crate::version::v6::PROTOCOL_VERSION_6;
pub use protocol_version::*;

mod consensus_versions;
Expand All @@ -17,8 +18,9 @@ pub mod v2;
pub mod v3;
pub mod v4;
pub mod v5;
pub mod v6;

pub type ProtocolVersion = u32;

pub const LATEST_VERSION: ProtocolVersion = PROTOCOL_VERSION_5;
pub const LATEST_VERSION: ProtocolVersion = PROTOCOL_VERSION_6;
pub const INITIAL_PROTOCOL_VERSION: ProtocolVersion = 1;
68 changes: 68 additions & 0 deletions packages/rs-platform-version/src/version/v6.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use crate::version::consensus_versions::ConsensusVersions;
use crate::version::dpp_versions::dpp_asset_lock_versions::v1::DPP_ASSET_LOCK_VERSIONS_V1;
use crate::version::dpp_versions::dpp_contract_versions::v1::CONTRACT_VERSIONS_V1;
use crate::version::dpp_versions::dpp_costs_versions::v1::DPP_COSTS_VERSIONS_V1;
use crate::version::dpp_versions::dpp_document_versions::v1::DOCUMENT_VERSIONS_V1;
use crate::version::dpp_versions::dpp_factory_versions::v1::DPP_FACTORY_VERSIONS_V1;
use crate::version::dpp_versions::dpp_identity_versions::v1::IDENTITY_VERSIONS_V1;
use crate::version::dpp_versions::dpp_method_versions::v2::DPP_METHOD_VERSIONS_V2;
use crate::version::dpp_versions::dpp_state_transition_conversion_versions::v2::STATE_TRANSITION_CONVERSION_VERSIONS_V2;
use crate::version::dpp_versions::dpp_state_transition_method_versions::v1::STATE_TRANSITION_METHOD_VERSIONS_V1;
use crate::version::dpp_versions::dpp_state_transition_serialization_versions::v1::STATE_TRANSITION_SERIALIZATION_VERSIONS_V1;
use crate::version::dpp_versions::dpp_state_transition_versions::v2::STATE_TRANSITION_VERSIONS_V2;
use crate::version::dpp_versions::dpp_validation_versions::v2::DPP_VALIDATION_VERSIONS_V2;
use crate::version::dpp_versions::dpp_voting_versions::v2::VOTING_VERSION_V2;
use crate::version::dpp_versions::DPPVersion;
use crate::version::drive_abci_versions::drive_abci_method_versions::v5::DRIVE_ABCI_METHOD_VERSIONS_V5;
use crate::version::drive_abci_versions::drive_abci_query_versions::v1::DRIVE_ABCI_QUERY_VERSIONS_V1;
use crate::version::drive_abci_versions::drive_abci_structure_versions::v1::DRIVE_ABCI_STRUCTURE_VERSIONS_V1;
use crate::version::drive_abci_versions::drive_abci_validation_versions::v3::DRIVE_ABCI_VALIDATION_VERSIONS_V3;
use crate::version::drive_abci_versions::drive_abci_withdrawal_constants::v2::DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V2;
use crate::version::drive_abci_versions::DriveAbciVersion;
use crate::version::drive_versions::v2::DRIVE_VERSION_V2;
use crate::version::fee::v1::FEE_VERSION1;
use crate::version::protocol_version::PlatformVersion;
use crate::version::system_data_contract_versions::v1::SYSTEM_DATA_CONTRACT_VERSIONS_V1;
use crate::version::system_limits::v1::SYSTEM_LIMITS_V1;
use crate::version::ProtocolVersion;

pub const PROTOCOL_VERSION_6: ProtocolVersion = 6;

/// This version contains some fixes for withdrawals.
pub const PLATFORM_V6: PlatformVersion = PlatformVersion {
protocol_version: PROTOCOL_VERSION_6,
drive: DRIVE_VERSION_V2,
drive_abci: DriveAbciVersion {
structs: DRIVE_ABCI_STRUCTURE_VERSIONS_V1,
// We changed `pool_withdrawals_into_transactions_queue` to v1 in order to add pool
// withdrawals on any validator quorums. v0 allowed us to pool only on the first two
// quorums as workaround for Core v21 bug.
methods: DRIVE_ABCI_METHOD_VERSIONS_V5,
validation_and_processing: DRIVE_ABCI_VALIDATION_VERSIONS_V3,
withdrawal_constants: DRIVE_ABCI_WITHDRAWAL_CONSTANTS_V2,
query: DRIVE_ABCI_QUERY_VERSIONS_V1,
},
dpp: DPPVersion {
costs: DPP_COSTS_VERSIONS_V1,
validation: DPP_VALIDATION_VERSIONS_V2,
state_transition_serialization_versions: STATE_TRANSITION_SERIALIZATION_VERSIONS_V1,
state_transition_conversion_versions: STATE_TRANSITION_CONVERSION_VERSIONS_V2,
state_transition_method_versions: STATE_TRANSITION_METHOD_VERSIONS_V1,
state_transitions: STATE_TRANSITION_VERSIONS_V2,
contract_versions: CONTRACT_VERSIONS_V1,
document_versions: DOCUMENT_VERSIONS_V1,
identity_versions: IDENTITY_VERSIONS_V1,
voting_versions: VOTING_VERSION_V2,
asset_lock_versions: DPP_ASSET_LOCK_VERSIONS_V1,
// We changed `daily_withdrawal_limit` to v1 to increase daily withdrawal limit
// to 2000 Dash.
methods: DPP_METHOD_VERSIONS_V2,
factory_versions: DPP_FACTORY_VERSIONS_V1,
},
system_data_contracts: SYSTEM_DATA_CONTRACT_VERSIONS_V1,
fee_version: FEE_VERSION1,
system_limits: SYSTEM_LIMITS_V1,
consensus: ConsensusVersions {
tenderdash_consensus_version: 1,
},
};
Loading