diff --git a/book/src/data-model/contract-moderation.md b/book/src/data-model/contract-moderation.md index 7fd3f288e82..1579dc2e7fb 100644 --- a/book/src/data-model/contract-moderation.md +++ b/book/src/data-model/contract-moderation.md @@ -244,7 +244,7 @@ The pots are not under the contract. The per-block total credits check (`calcula | Stage | Check | Error | |---|---|---| -| Transform (state, paid) | the contract exists | `DataContractNotPresentError`, unpaid | +| Transform (state, paid) | the contract exists | `DataContractNotPresentError` (10400) | | | the signer is a recipient of the pot: the owner for the owner pot, a member of the team for the moderators pot | 41113 | | | the pot was not paid out in this epoch yet | 41111 | | | every recipient gets at least a credit | 41112 | diff --git a/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/mod.rs index 7bdecf92795..7eb94485d87 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/mod.rs @@ -1,5 +1,6 @@ mod v0; mod v1; +mod v2; use crate::error::execution::ExecutionError; use crate::error::Error; @@ -40,6 +41,9 @@ impl Platform { /// which contains the logic for version `0`. /// - If the version is `1`, it calls `perform_events_on_first_block_of_protocol_change_v1`, which runs /// the same transitions and then refreshes the cached definitions of the contracts they rewrote. + /// - If the version is `2`, it calls `perform_events_on_first_block_of_protocol_change_v2`, which + /// empties the contract cache, so no read is billed at a fee cached under the old fee + /// schedule, and then runs v1. /// - If no version is specified (`None`), the function does nothing and returns `Ok(())`. /// - If a different version is specified, it returns an error indicating an unknown version mismatch. /// @@ -71,10 +75,17 @@ impl Platform { previous_protocol_version, platform_version, ), + Some(2) => self.perform_events_on_first_block_of_protocol_change_v2( + platform_state, + block_info, + transaction, + previous_protocol_version, + platform_version, + ), None => Ok(()), Some(version) => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "perform_events_on_first_block_of_protocol_change".to_string(), - known_versions: vec![0, 1], + known_versions: vec![0, 1, 2], received: version, })), } @@ -88,7 +99,9 @@ mod tests { use crate::test::helpers::setup::TestPlatformBuilder; use dpp::block::block_info::BlockInfo; use dpp::block::epoch::Epoch; + use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::data_contracts::SystemDataContract; + use dpp::tests::fixtures::get_data_contract_fixture; #[test] fn test_perform_events_when_version_method_is_none() { @@ -166,13 +179,92 @@ mod tests { received, })) => { assert_eq!(method, "perform_events_on_first_block_of_protocol_change"); - assert_eq!(known_versions, vec![0, 1]); + assert_eq!(known_versions, vec![0, 1, 2]); assert_eq!(received, 255); } _ => panic!("expected UnknownVersionMismatch error"), } } + #[test] + fn should_empty_the_contract_cache_on_the_first_block_of_protocol_version_14() { + let previous_version = PlatformVersion::get(13).expect("protocol 13"); + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .with_initial_protocol_version(13) + .build_with_mock_rpc() + .set_genesis_state(); + let platform_state = platform.state.load(); + let block_info = BlockInfo { + time_ms: 1_000_000, + height: 100, + core_height: 100, + epoch: Epoch::new(1).expect("epoch"), + }; + + // A user contract a node read while protocol version 13 was active, caching the fee of + // that read under version 13's fee schedule. + let contract = get_data_contract_fixture(None, 0, previous_version.protocol_version) + .data_contract_owned(); + let contract_id = contract.id().to_buffer(); + platform + .drive + .apply_contract( + &contract, + BlockInfo::default(), + true, + None, + None, + previous_version, + ) + .expect("expected to apply the contract"); + platform + .drive + .get_contract_with_fetch_info_and_fee( + contract_id, + Some(&Epoch::new(0).expect("epoch")), + true, + None, + previous_version, + ) + .expect("expected to read the contract"); + let contracts = &platform.drive.cache.data_contracts; + let is_cached_with_its_fee = || { + contracts + .get(contract_id, false) + .is_some_and(|cached| cached.has_fee_for_tests()) + }; + assert!(is_cached_with_its_fee()); + + let run_the_events = |platform_version: &PlatformVersion| { + contracts.clear_block_cache(); + let transaction = platform.drive.grove.start_transaction(); + platform + .perform_events_on_first_block_of_protocol_change( + &platform_state, + &block_info, + &transaction, + previous_version.protocol_version, + platform_version, + ) + .expect("expected the protocol change events to run"); + }; + + // The events of v1 leave it there, so a hit would keep billing the old fee. + let mut with_the_events_of_v1 = platform_version.clone(); + with_the_events_of_v1 + .drive_abci + .methods + .protocol_upgrade + .perform_events_on_first_block_of_protocol_change = Some(1); + run_the_events(&with_the_events_of_v1); + assert!(is_cached_with_its_fee()); + + run_the_events(platform_version); + assert!(contracts.get(contract_id, false).is_none()); + assert!(contracts.get(contract_id, true).is_none()); + } + #[test] fn should_rollback_and_retry_app_connect_registration_through_the_upgrade_dispatcher() { let previous_version = PlatformVersion::get(13).expect("protocol 13"); diff --git a/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v2/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v2/mod.rs new file mode 100644 index 00000000000..c19d89cc8d5 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v2/mod.rs @@ -0,0 +1,40 @@ +use crate::error::Error; +use crate::platform_types::platform::Platform; +use crate::platform_types::platform_state::PlatformState; +use dpp::block::block_info::BlockInfo; +use dpp::version::PlatformVersion; +use dpp::version::ProtocolVersion; +use drive::grovedb::Transaction; + +impl Platform { + /// Empties the contract cache, then runs the protocol change events of v1. + /// + /// A cached contract can carry the fee of the read that cached it, calculated under the fee + /// schedule of the protocol version it was read in, and a cache hit bills that fee again. A + /// protocol change is the only time the schedule can change, and nothing else empties the + /// cache, so without this a node that stayed up across the change would keep billing reads + /// at the old schedule while a node that restarted bills them at the new one. + /// + /// The cache is emptied first because v1 then seeds the block cache with the system + /// contracts the events may have rewritten. `clear` keeps the record of what this block + /// rewrote, so a transactional read of a rewritten contract still never falls back to a copy + /// a query puts into the global cache. + pub(super) fn perform_events_on_first_block_of_protocol_change_v2( + &self, + platform_state: &PlatformState, + block_info: &BlockInfo, + transaction: &Transaction, + previous_protocol_version: ProtocolVersion, + platform_version: &PlatformVersion, + ) -> Result<(), Error> { + self.drive.cache.data_contracts.clear(); + + self.perform_events_on_first_block_of_protocol_change_v1( + platform_state, + block_info, + transaction, + previous_protocol_version, + platform_version, + ) + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/state/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/state/v0/mod.rs index 65c1b505c4f..68d75a0a242 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/state/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/state/v0/mod.rs @@ -1,3 +1,4 @@ +use crate::error::execution::ExecutionError; use crate::error::Error; use crate::execution::types::execution_operation::ValidationOperation; use crate::execution::types::state_transition_execution_context::{ @@ -39,9 +40,9 @@ pub(in crate::execution::validation::state_transition::state_transitions::contra impl ContractFeeClaimStateTransitionStateValidationV0 for ContractFeeClaimTransition { /// Reads the contract and the pot and settles the payout: the signer is a recipient of the /// pot, the pot was not claimed in this epoch yet, and it holds enough to pay every - /// recipient something. Every refusal after the contract is found is paid for by bumping - /// the signer's contract nonce, and a refused claim leaves the pot's last claim epoch - /// alone. + /// recipient something. Every refusal, a contract that does not exist included, is paid for + /// by bumping the signer's contract nonce, and a refused claim leaves the pot's last claim + /// epoch alone. /// /// The action carries what each recipient is paid, so Drive pays the pot out without /// reading it again, and the mempool, which transforms without a state validation stage, @@ -58,24 +59,25 @@ impl ContractFeeClaimStateTransitionStateValidationV0 for ContractFeeClaimTransi let claimant_id = self.owner_id(); let pot = self.pot(); - let Some(contract_fetch_info) = platform - .drive - .get_contract_with_fetch_info_and_fee( + let (contract_fetch_fee, maybe_contract_fetch_info) = + platform.drive.get_contract_with_fetch_info_and_fee( contract_id.to_buffer(), Some(&block_info.epoch), false, tx, platform_version, - )? - .1 - else { - return Ok(ConsensusValidationResult::new_with_error( - DataContractNotPresentError::new(contract_id).into(), - )); - }; - if let Some(fee) = contract_fetch_info.fee.clone() { - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); - } + )?; + // The read is billed from the fee this call returns, whether the contract was pulled + // from disk, was in the cache or does not exist. The fee a cached fetch info carries is + // only there when that entry was built with an epoch, which differs from node to node, + // so billing it would make the fee, and the app hash, depend on the cache. + let contract_fetch_fee = + contract_fetch_fee.ok_or(Error::Execution(ExecutionError::CorruptedCodeExecution( + "fee must exist for the contract fetch of a contract fee claim transition", + )))?; + execution_context.add_operation(ValidationOperation::PrecalculatedOperation( + contract_fetch_fee, + )); let bump_action = || { StateTransitionAction::BumpIdentityDataContractNonceAction( @@ -91,6 +93,12 @@ impl ContractFeeClaimStateTransitionStateValidationV0 for ContractFeeClaimTransi )) }; + // Paid like every other refusal: the signer is authenticated and the lookup happened, + // as for a contract update of a contract that does not exist. + let Some(contract_fetch_info) = maybe_contract_fetch_info else { + return refuse(DataContractNotPresentError::new(contract_id).into()); + }; + // Only who a payout of the pot goes to may claim it: the contract owner for the owner // pot, a member of the moderation team for the moderators pot. A contract that // declares no moderation has no team, so nobody claims its moderators pot. diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/tests.rs index f92d4ede714..b6ff4aad782 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_fee_claim/tests.rs @@ -10,6 +10,10 @@ use crate::platform_types::platform_state::PlatformStateV0Methods; use crate::platform_types::state_transitions_processing_result::StateTransitionExecutionResult; use crate::rpc::core::MockCoreRPCLike; use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; +use dapi_grpc::platform::v0::get_documents_request::{ + GetDocumentsRequestV0, Version as GetDocumentsRequestVersion, +}; +use dapi_grpc::platform::v0::GetDocumentsRequest; use dpp::block::block_info::BlockInfo; use dpp::block::epoch::{Epoch, EpochIndex}; use dpp::consensus::codes::ErrorWithCode; @@ -24,6 +28,7 @@ use dpp::data_contract::document_type::random_document::{ }; use dpp::data_contract::document_type::DocumentType; use dpp::data_contract::DataContract; +use dpp::fee::fee_result::FeeResult; use dpp::fee::Credits; use dpp::identity::accessors::IdentityGettersV0; use dpp::platform_value::{platform_value, Bytes32, Identifier, Value}; @@ -290,6 +295,28 @@ impl Setup { .collect() } + /// Queries the contract's documents as a client's getDocuments request does: against + /// committed state, caching the contract it pulls + fn query_documents(&self) { + let state = self.platform.state.load(); + let request = GetDocumentsRequest { + version: Some(GetDocumentsRequestVersion::V0(GetDocumentsRequestV0 { + data_contract_id: self.contract.id().to_vec(), + document_type: "niceDocument".to_string(), + r#where: vec![], + limit: 0, + order_by: vec![], + prove: false, + start: None, + })), + }; + let result = self + .platform + .query_documents(request, &state, PlatformVersion::latest()) + .expect("expected to query the documents"); + assert!(result.is_valid(), "{:?}", result.errors); + } + fn pot(&self, pot: ContractFeePot, transaction: Option<&Transaction>) -> ContractFeePotState { self.platform .drive @@ -354,18 +381,19 @@ fn claim_by(claimant: &Actor, epoch_index: EpochIndex) -> Option Credits { +/// What `execution` was billed, whether it succeeded or was a paid refusal +fn fees(execution: &StateTransitionExecutionResult) -> &FeeResult { match execution { - StateTransitionExecutionResult::SuccessfulExecution { fee_result, .. } => { - fee_result.total_base_fee() - } - StateTransitionExecutionResult::PaidConsensusError { actual_fees, .. } => { - actual_fees.total_base_fee() - } + StateTransitionExecutionResult::SuccessfulExecution { fee_result, .. } => fee_result, + StateTransitionExecutionResult::PaidConsensusError { actual_fees, .. } => actual_fees, other => panic!("expected a paid result, got {other:?}"), } } +fn gas(execution: &StateTransitionExecutionResult) -> Credits { + fees(execution).total_base_fee() +} + #[tokio::test] async fn should_split_the_moderators_pot_equally_and_leave_the_remainder() { let setup = Setup::new(Team::TwoModerators).await; @@ -631,11 +659,13 @@ async fn should_leave_the_moderators_pot_of_an_unmoderated_contract_to_nobody() } #[tokio::test] -async fn should_refuse_a_claim_on_an_unknown_contract_unpaid() { +async fn should_refuse_a_claim_on_an_unknown_contract_and_charge_for_the_lookup() { let setup = Setup::new(Team::TwoModerators).await; + let unknown_contract_id = Identifier::from([0x55; 32]); + let nonce = setup.moderator_a.next_contract_nonce.get(); let claim = claim_of( &setup.moderator_a, - Identifier::from([0x55; 32]), + unknown_contract_id, ContractFeePot::Moderators, ) .await; @@ -644,10 +674,98 @@ async fn should_refuse_a_claim_on_an_unknown_contract_unpaid() { vec![DATA_CONTRACT_NOT_PRESENT] ); let transaction = setup.platform.drive.grove.start_transaction(); - assert_unpaid_with_code( - &setup.process(&claim, 0, &transaction), - DATA_CONTRACT_NOT_PRESENT, + let credits_before = setup.credits(&setup.moderator_a, Some(&transaction)); + + let result = setup.process(&claim, 0, &transaction); + + // Paid like every other refusal: the signer is authenticated and the lookup happened. + assert_paid_with_code(&result, DATA_CONTRACT_NOT_PRESENT); + let gas = gas(&result); + assert!(gas > 0, "the contract lookup is billed"); + assert_eq!( + setup.credits(&setup.moderator_a, Some(&transaction)), + credits_before - gas ); + // The refusal used up the claimant's nonce for that contract id. + assert_eq!( + setup + .platform + .drive + .fetch_identity_contract_nonce( + setup.moderator_a.id().to_buffer(), + unknown_contract_id.to_buffer(), + true, + Some(&transaction), + PlatformVersion::latest(), + ) + .expect("expected to fetch the nonce"), + Some(nonce) + ); +} + +#[tokio::test] +async fn should_bill_a_claim_the_same_whether_its_contract_is_cached_or_not() { + let setup = Setup::new(Team::TwoModerators).await; + setup.fill(ContractFeePot::Moderators, 1_000); + let claim = setup + .claim(&setup.moderator_a, ContractFeePot::Moderators) + .await; + let contracts = &setup.platform.drive.cache.data_contracts; + let contract_id = setup.contract.id().to_buffer(); + let claim_fees = || { + let transaction = setup.platform.drive.grove.start_transaction(); + let result = setup.process(&claim, 1, &transaction); + assert_success(&result); + fees(&result).clone() + }; + + // A node that executed the contract create: the cache refresh after the write stored the + // contract without a fee. + let cached = contracts + .get(contract_id, true) + .expect("expected the create to cache the contract"); + assert!(!cached.has_fee_for_tests()); + let as_the_create_left_it = claim_fees(); + + // A node whose committed cache a getDocuments query filled, also without a fee, which + // anyone can make happen on the nodes of their choosing. + contracts.merge_and_clear_block_cache(); + contracts.clear(); + setup.query_documents(); + let cached = contracts + .get(contract_id, true) + .expect("expected the query to cache the contract"); + assert!(!cached.has_fee_for_tests()); + let after_a_query = claim_fees(); + + // A node that cached the contract with the fee of its read, in an earlier epoch, as the + // validation of a contract update does. + contracts.clear(); + setup + .platform + .drive + .get_contract_with_fetch_info_and_fee( + contract_id, + Some(&Epoch::new(0).expect("expected an epoch")), + true, + None, + PlatformVersion::latest(), + ) + .expect("expected to read the contract"); + let cached = contracts + .get(contract_id, true) + .expect("expected the read to cache the contract"); + assert!(cached.has_fee_for_tests()); + let with_a_fee = claim_fees(); + + // A node that restarted, evicted the contract or joined late. + contracts.clear(); + assert!(contracts.get(contract_id, true).is_none()); + let cold = claim_fees(); + + assert_eq!(as_the_create_left_it, cold); + assert_eq!(after_a_query, cold); + assert_eq!(with_a_fee, cold); } #[tokio::test] diff --git a/packages/rs-drive/src/drive/contract/contract_fetch_info.rs b/packages/rs-drive/src/drive/contract/contract_fetch_info.rs index cd22aea53d8..35a3186a236 100644 --- a/packages/rs-drive/src/drive/contract/contract_fetch_info.rs +++ b/packages/rs-drive/src/drive/contract/contract_fetch_info.rs @@ -24,13 +24,22 @@ pub struct DataContractFetchInfo { /// These are the operations that are used to fetch a contract /// This is only used on epoch change pub(crate) cost: OperationCost, - /// The fee is updated every epoch based on operation costs - /// Except if protocol version has changed in which case all the cache is cleared - pub fee: Option, + /// The fee of the read that built this entry, when it was built with an epoch, which a cache + /// hit bills again. A read's fee depends only on the fee schedule, and from protocol + /// version 14 the contract cache is cleared on the first block of every protocol change, + /// the only time the schedule can change. Entries are cached with and without a fee, so + /// callers bill the fee `Drive::get_contract_with_fetch_info_and_fee` returns, never this. + pub(crate) fee: Option, } #[cfg(feature = "fixtures-and-mocks")] impl DataContractFetchInfo { + /// This should ONLY be used for tests: whether this entry carries the fee of the read that + /// built it. Never bill it. + pub fn has_fee_for_tests(&self) -> bool { + self.fee.is_some() + } + /// This should ONLY be used for tests pub fn dpns_contract_fixture(protocol_version: u32) -> Self { let dpns = get_dpns_data_contract_fixture(None, 0, protocol_version); diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs index 49de2a0083f..f8018ef6be9 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs @@ -56,7 +56,7 @@ pub const DRIVE_ABCI_METHOD_VERSIONS_V10: DriveAbciMethodVersions = DriveAbciMet 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(1), + perform_events_on_first_block_of_protocol_change: Some(2), // changed: empties the contract cache first, so no read is billed at a fee cached under the old fee schedule protocol_version_upgrade_percentage_needed: 67, }, block_fee_processing: DriveAbciBlockFeeProcessingMethodVersions {