From 8b05c5807a9ccb4937292d34388bf14d4ad04607 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 24 Sep 2026 02:21:16 +0700 Subject: [PATCH 1/3] fix(drive-abci)!: bill the contract fetch of a fee claim deterministically The contract fee claim billed its contract read from the fee stored in the cached DataContractFetchInfo. That fee is only there when the cache entry was built with an epoch: the refresh after a contract create or update and the getDocuments handler cache the contract without one. A node holding such an entry billed nothing for the read, a cold node billed it, and their balances and app hashes diverged. Sending getDocuments to some nodes was enough to cause it. Bill the FeeResult the fetch returns, before anything else is checked, as the contract update and the contract user moderation do. A claim on a contract that does not exist becomes a paid refusal that bumps the contract nonce, like every other refusal of the claim. ContractFeeClaim only exists from protocol version 14, which is unreleased, so v0 is edited in place. Co-Authored-By: Claude Opus 5.5 --- .../contract_fee_claim/state/v0/mod.rs | 40 +++++--- .../contract_fee_claim/tests.rs | 98 +++++++++++++++++-- 2 files changed, 115 insertions(+), 23 deletions(-) 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..57cbfd446b7 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,23 @@ 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() + fee_result.clone() } StateTransitionExecutionResult::PaidConsensusError { actual_fees, .. } => { - actual_fees.total_base_fee() + actual_fees.clone() } 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,7 +663,7 @@ 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 claim = claim_of( &setup.moderator_a, @@ -644,12 +676,64 @@ 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 ); } +#[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) + }; + + // 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_eq!(cached.fee, None); + 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_eq!(cached.fee, None); + let after_a_query = 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); +} + #[tokio::test] async fn should_prove_the_pot_and_the_balances_of_everyone_it_paid() { let setup = Setup::new(Team::TwoModerators).await; From 08d54ce6524f917582ce671e8b6613c4d148b46d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 24 Sep 2026 03:17:16 +0700 Subject: [PATCH 2/3] fix(drive): bill a cached contract read from its cost, never from a stored fee A cache hit billed the fee its entry carried when it had one. That fee was calculated under the fee schedule active when the contract was cached, and nothing clears the contract cache on a protocol change, so a schedule that changes read costs would bill warm and cold nodes differently. The cache hit now always calculates the fee from the entry's read cost under the active schedule; the write-back that only memoized it is gone. This is consensus-neutral on every shipped version: every stored fee is calculate_fee over the entry's own cost, a read's fee depends on the epoch for nothing, and every shipped schedule (and FEE_VERSION3) shares the storage and processing costs. DataContractFetchInfo::fee becomes crate-private with a corrected doc, so no caller outside Drive can bill it again; tests use has_fee_for_tests. The fee claim tests gain a leg with a fee-carrying cache entry and check the unknown-contract refusal's nonce bump, and the book no longer calls that refusal unpaid. Co-Authored-By: Claude Opus 5.5 --- book/src/data-model/contract-moderation.md | 2 +- .../contract_fee_claim/tests.rs | 56 +++++++++++++++---- .../src/drive/contract/contract_fetch_info.rs | 16 ++++-- .../get_contract_with_fetch_info/mod.rs | 48 ++++++++++++++++ .../get_contract_with_fetch_info/v0/mod.rs | 43 +++++--------- 5 files changed, 121 insertions(+), 44 deletions(-) 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/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 57cbfd446b7..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 @@ -382,14 +382,10 @@ fn claim_by(claimant: &Actor, epoch_index: EpochIndex) -> Option FeeResult { +fn fees(execution: &StateTransitionExecutionResult) -> &FeeResult { match execution { - StateTransitionExecutionResult::SuccessfulExecution { fee_result, .. } => { - fee_result.clone() - } - StateTransitionExecutionResult::PaidConsensusError { actual_fees, .. } => { - actual_fees.clone() - } + StateTransitionExecutionResult::SuccessfulExecution { fee_result, .. } => fee_result, + StateTransitionExecutionResult::PaidConsensusError { actual_fees, .. } => actual_fees, other => panic!("expected a paid result, got {other:?}"), } } @@ -665,9 +661,11 @@ 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_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; @@ -688,6 +686,21 @@ async fn should_refuse_a_claim_on_an_unknown_contract_and_charge_for_the_lookup( 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] @@ -703,7 +716,7 @@ async fn should_bill_a_claim_the_same_whether_its_contract_is_cached_or_not() { let transaction = setup.platform.drive.grove.start_transaction(); let result = setup.process(&claim, 1, &transaction); assert_success(&result); - fees(&result) + fees(&result).clone() }; // A node that executed the contract create: the cache refresh after the write stored the @@ -711,7 +724,7 @@ async fn should_bill_a_claim_the_same_whether_its_contract_is_cached_or_not() { let cached = contracts .get(contract_id, true) .expect("expected the create to cache the contract"); - assert_eq!(cached.fee, None); + 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 @@ -722,9 +735,29 @@ async fn should_bill_a_claim_the_same_whether_its_contract_is_cached_or_not() { let cached = contracts .get(contract_id, true) .expect("expected the query to cache the contract"); - assert_eq!(cached.fee, None); + 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()); @@ -732,6 +765,7 @@ async fn should_bill_a_claim_the_same_whether_its_contract_is_cached_or_not() { 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..826abe31d9c 100644 --- a/packages/rs-drive/src/drive/contract/contract_fetch_info.rs +++ b/packages/rs-drive/src/drive/contract/contract_fetch_info.rs @@ -22,15 +22,23 @@ pub struct DataContractFetchInfo { /// The contract's potential storage flags pub storage_flags: Option, /// These are the operations that are used to fetch a contract - /// This is only used on epoch change + /// A read served from the cache is billed from this cost 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. It is never + /// billed once the entry is cached: entries are cached with and without one, and the cache + /// outlives a change of fee schedule. A read is billed from the fee + /// `Drive::get_contract_with_fetch_info_and_fee` returns. + 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-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs index 8e7056105aa..7b9f81747aa 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs @@ -185,6 +185,7 @@ impl Drive { #[cfg(test)] mod tests { use crate::drive::contract::tests::setup_reference_contract; + use crate::drive::contract::DataContractFetchInfo; use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; @@ -287,6 +288,53 @@ mod tests { assert!(result.1.is_none()); } + #[test] + fn should_bill_a_cached_contract_from_its_cost_not_from_the_fee_it_carries() { + let (drive, contract) = setup_reference_contract(); + let platform_version = PlatformVersion::latest(); + let epoch = Epoch::new(0).expect("should create epoch"); + let contract_id = contract.id().to_buffer(); + let contracts = &drive.cache.data_contracts; + + contracts.clear(); + let (cold_fee, fetch_info) = drive + .get_contract_with_fetch_info_and_fee( + contract_id, + Some(&epoch), + false, + None, + platform_version, + ) + .expect("should get contract"); + let cold_fee = cold_fee.expect("should have a fee"); + let fetch_info = fetch_info.expect("should be present"); + + // An entry cached under another fee schedule carries a fee the read no longer costs. + let stale_fee = FeeResult::new_from_processing_fee(1); + assert_ne!(stale_fee, cold_fee); + contracts.insert_committed( + Arc::new(DataContractFetchInfo { + fee: Some(stale_fee.clone()), + ..(*fetch_info).clone() + }), + contracts.committed_generation(), + ); + let cached = contracts.get(contract_id, false).expect("should be cached"); + assert_eq!(cached.fee, Some(stale_fee)); + + let (warm_fee, _) = drive + .get_contract_with_fetch_info_and_fee( + contract_id, + Some(&epoch), + false, + None, + platform_version, + ) + .expect("should get contract"); + + assert_eq!(warm_fee, Some(cold_fee)); + } + #[test] fn should_always_have_then_same_cost() { // Merk trees have own cache and depends on does contract node cached or not diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs index 59fb641e2b3..c8bea2bffd4 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs @@ -110,7 +110,7 @@ impl Drive { /// Returns the contract with fetch info and operations with the given ID. /// /// `add_to_cache_if_pulled` is whether this call may write to the cache at all: storing a - /// contract it pulled from state, and storing the fee it calculated for a cached contract. + /// contract it pulled from state. /// Validation modes that run off the consensus thread (`check_tx`) pass `false`, so that /// only block execution ever writes the block cache. #[inline(always)] @@ -160,33 +160,20 @@ impl Drive { Some(contract_fetch_info) => { // we only need to pay if epoch is set if let Some(epoch) = epoch { - let fee = if let Some(known_fee) = &contract_fetch_info.fee { - known_fee.clone() - } else { - // we need to calculate new fee - let op = vec![CalculatedCostOperation(contract_fetch_info.cost.clone())]; - let fee = Drive::calculate_fee( - None, - Some(op), - epoch, - self.config.epochs_per_era, - platform_version, - None, - )?; - - if add_to_cache_if_pulled { - let updated_contract_fetch_info = Arc::new(DataContractFetchInfo { - contract: contract_fetch_info.contract.clone(), - storage_flags: contract_fetch_info.storage_flags.clone(), - cost: contract_fetch_info.cost.clone(), - fee: Some(fee.clone()), - }); - // we override the cache for the contract as the fee is now calculated - cache_contract(updated_contract_fetch_info); - } - - fee - }; + // The fee is calculated from the cost of the read under the active fee + // schedule every time, never taken from the entry: a fee stored there was + // calculated under the schedule active when the contract was cached, and + // the cache outlives a protocol change, so billing it would make the fee + // depend on when this node cached the contract. + let op = vec![CalculatedCostOperation(contract_fetch_info.cost.clone())]; + let fee = Drive::calculate_fee( + None, + Some(op), + epoch, + self.config.epochs_per_era, + platform_version, + None, + )?; drive_operations.push(PreCalculatedFeeResult(fee)); } Ok(Some(contract_fetch_info)) From 10b52d238395f33c0fe6ccf8bf64894ad209aa1d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 24 Sep 2026 07:00:01 +0700 Subject: [PATCH 3/3] fix(drive-abci): empty the contract cache on the first block of a protocol change Replaces the previous commit's Drive change, which recalculated every cache hit's fee in a read path all protocol versions share. The fee a cache entry carries can only go stale when the fee schedule changes, and that only happens at a protocol change, so emptying the cache there is enough. perform_events_on_first_block_of_protocol_change v2 clears the contract cache and then runs v1, whose system contract seeding has to come after the clear. Only protocol version 14's method table selects it, so upgrades into 10 to 13 keep running v1. The rs-drive fetch code is back to what it was; DataContractFetchInfo::fee stays crate-private, with a doc that now says when the cache is cleared. Co-Authored-By: Claude Opus 5.5 --- .../mod.rs | 96 ++++++++++++++++++- .../v2/mod.rs | 40 ++++++++ .../src/drive/contract/contract_fetch_info.rs | 11 ++- .../get_contract_with_fetch_info/mod.rs | 48 ---------- .../get_contract_with_fetch_info/v0/mod.rs | 43 ++++++--- .../drive_abci_method_versions/v10.rs | 2 +- 6 files changed, 169 insertions(+), 71 deletions(-) create mode 100644 packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v2/mod.rs 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/src/drive/contract/contract_fetch_info.rs b/packages/rs-drive/src/drive/contract/contract_fetch_info.rs index 826abe31d9c..35a3186a236 100644 --- a/packages/rs-drive/src/drive/contract/contract_fetch_info.rs +++ b/packages/rs-drive/src/drive/contract/contract_fetch_info.rs @@ -22,12 +22,13 @@ pub struct DataContractFetchInfo { /// The contract's potential storage flags pub storage_flags: Option, /// These are the operations that are used to fetch a contract - /// A read served from the cache is billed from this cost + /// This is only used on epoch change pub(crate) cost: OperationCost, - /// The fee of the read that built this entry, when it was built with an epoch. It is never - /// billed once the entry is cached: entries are cached with and without one, and the cache - /// outlives a change of fee schedule. A read is billed from the fee - /// `Drive::get_contract_with_fetch_info_and_fee` returns. + /// 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, } diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs index 7b9f81747aa..8e7056105aa 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/mod.rs @@ -185,7 +185,6 @@ impl Drive { #[cfg(test)] mod tests { use crate::drive::contract::tests::setup_reference_contract; - use crate::drive::contract::DataContractFetchInfo; use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; @@ -288,53 +287,6 @@ mod tests { assert!(result.1.is_none()); } - #[test] - fn should_bill_a_cached_contract_from_its_cost_not_from_the_fee_it_carries() { - let (drive, contract) = setup_reference_contract(); - let platform_version = PlatformVersion::latest(); - let epoch = Epoch::new(0).expect("should create epoch"); - let contract_id = contract.id().to_buffer(); - let contracts = &drive.cache.data_contracts; - - contracts.clear(); - let (cold_fee, fetch_info) = drive - .get_contract_with_fetch_info_and_fee( - contract_id, - Some(&epoch), - false, - None, - platform_version, - ) - .expect("should get contract"); - let cold_fee = cold_fee.expect("should have a fee"); - let fetch_info = fetch_info.expect("should be present"); - - // An entry cached under another fee schedule carries a fee the read no longer costs. - let stale_fee = FeeResult::new_from_processing_fee(1); - assert_ne!(stale_fee, cold_fee); - contracts.insert_committed( - Arc::new(DataContractFetchInfo { - fee: Some(stale_fee.clone()), - ..(*fetch_info).clone() - }), - contracts.committed_generation(), - ); - let cached = contracts.get(contract_id, false).expect("should be cached"); - assert_eq!(cached.fee, Some(stale_fee)); - - let (warm_fee, _) = drive - .get_contract_with_fetch_info_and_fee( - contract_id, - Some(&epoch), - false, - None, - platform_version, - ) - .expect("should get contract"); - - assert_eq!(warm_fee, Some(cold_fee)); - } - #[test] fn should_always_have_then_same_cost() { // Merk trees have own cache and depends on does contract node cached or not diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs index c8bea2bffd4..59fb641e2b3 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_contract_with_fetch_info/v0/mod.rs @@ -110,7 +110,7 @@ impl Drive { /// Returns the contract with fetch info and operations with the given ID. /// /// `add_to_cache_if_pulled` is whether this call may write to the cache at all: storing a - /// contract it pulled from state. + /// contract it pulled from state, and storing the fee it calculated for a cached contract. /// Validation modes that run off the consensus thread (`check_tx`) pass `false`, so that /// only block execution ever writes the block cache. #[inline(always)] @@ -160,20 +160,33 @@ impl Drive { Some(contract_fetch_info) => { // we only need to pay if epoch is set if let Some(epoch) = epoch { - // The fee is calculated from the cost of the read under the active fee - // schedule every time, never taken from the entry: a fee stored there was - // calculated under the schedule active when the contract was cached, and - // the cache outlives a protocol change, so billing it would make the fee - // depend on when this node cached the contract. - let op = vec![CalculatedCostOperation(contract_fetch_info.cost.clone())]; - let fee = Drive::calculate_fee( - None, - Some(op), - epoch, - self.config.epochs_per_era, - platform_version, - None, - )?; + let fee = if let Some(known_fee) = &contract_fetch_info.fee { + known_fee.clone() + } else { + // we need to calculate new fee + let op = vec![CalculatedCostOperation(contract_fetch_info.cost.clone())]; + let fee = Drive::calculate_fee( + None, + Some(op), + epoch, + self.config.epochs_per_era, + platform_version, + None, + )?; + + if add_to_cache_if_pulled { + let updated_contract_fetch_info = Arc::new(DataContractFetchInfo { + contract: contract_fetch_info.contract.clone(), + storage_flags: contract_fetch_info.storage_flags.clone(), + cost: contract_fetch_info.cost.clone(), + fee: Some(fee.clone()), + }); + // we override the cache for the contract as the fee is now calculated + cache_contract(updated_contract_fetch_info); + } + + fee + }; drive_operations.push(PreCalculatedFeeResult(fee)); } Ok(Some(contract_fetch_info)) 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 {