From 9257dc746c38acf06fe90dd83fd393fc41904abd Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 22 Sep 2026 03:01:22 +0700 Subject: [PATCH] fix(drive-abci): recheck re-validates the document action fee agreement Advanced structure validation judges a batch's $actionFeeAgreement on the first check tx only. A transition admitted under one fee multiplier, or under amounts a contract may later change, stayed in the mempool and failed only when a block was built. Check tx now judges the agreements again on every recheck, off the action the transformer rebuilt with the contract and the epoch's multiplier as they are then, on both recheck routes. Mempool policy only: block execution already refuses the batch. Closes #4881 Co-Authored-By: Claude Fable 5.1 --- book/src/fees/overview.md | 9 +- .../check_tx_verification/v0/mod.rs | 15 ++ .../batch/tests/document/action_fees.rs | 153 ++++++++++++++++++ .../batch/tests/document/gas_sponsorship.rs | 6 +- .../rs-platform-version/src/version/v14.rs | 4 + 5 files changed, 183 insertions(+), 4 deletions(-) diff --git a/book/src/fees/overview.md b/book/src/fees/overview.md index 15386f7159a..18cb9f962e4 100644 --- a/book/src/fees/overview.md +++ b/book/src/fees/overview.md @@ -317,9 +317,12 @@ judged in the batch's advanced structure validation (`BatchTransitionAction::validate_action_fee_agreements`), off the action alone: the base action carries the declaration beside the agreement, and the batch action the multiplier its transformer read. The mempool runs the same -check. A client builds the agreement from the contract it showed its user -with `DocumentActionFeeAgreement::for_document_type_action`, never from a -contract fetched behind their back at signing time. +check when a transition arrives and again on every recheck, so a transition +whose agreement no longer holds leaves the mempool with the same error instead +of failing in the block that would have refused it. A client builds the +agreement from the contract it showed its user with +`DocumentActionFeeAgreement::for_document_type_action`, never from a contract +fetched behind their back at signing time. **The amounts do not change yet.** A contract update may not add, change or remove the `actionFees` of an existing document type, nor switch their pricing diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/check_tx_verification/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/check_tx_verification/v0/mod.rs index cee314e883d..fa35112e4d7 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/check_tx_verification/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/check_tx_verification/v0/mod.rs @@ -584,6 +584,21 @@ pub(super) fn state_transition_to_execution_event_for_check_tx_v0<'a, C: CoreRPC action }; + // Advanced structure validation judged the action fee agreements of a batch on + // the first check only. The amounts its contracts declare and the epoch's fee + // multiplier can move while it waits, and a block would refuse it; the + // transformer read both again, so the agreements are judged again off the + // action, and a batch that no longer covers them leaves the mempool. + if let StateTransitionAction::BatchAction(batch_action) = &action { + if let Err(error) = batch_action.validate_action_fee_agreements()? { + return Ok( + ConsensusValidationResult::>::new_with_errors( + vec![error], + ), + ); + } + } + let execution_event = ExecutionEvent::create_from_state_transition_action( action, maybe_identity, diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/action_fees.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/action_fees.rs index 4f8625224b9..8bba9ddf8e8 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/action_fees.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/action_fees.rs @@ -12,6 +12,7 @@ use super::*; mod action_fee_tests { use super::*; + use crate::execution::check_tx::CheckTxLevel::Recheck; use crate::platform_types::state_transitions_processing_result::StateTransitionExecutionResult::{ PaidConsensusError, SuccessfulExecution, UnpaidConsensusError, }; @@ -688,6 +689,158 @@ mod action_fee_tests { ); } + #[tokio::test] + async fn should_drop_a_transition_from_the_mempool_once_the_fee_multiplier_outran_its_tolerance( + ) { + // Admitted knowing a multiplier of 1000 and accepting 20% more; the epoch's multiplier + // then moves while the transition waits. A block refuses the transition as a paid + // nonce bump (`should_refuse_a_fee_multiplier_that_rose_beyond_the_tolerance`), so the + // recheck judges the agreement again, off the multiplier read now, and drops it first. + let setup = game( + GasFeesPaidBy::DocumentOwner, + "feeMultiplier", + dash_to_credits!(0.1), + dash_to_credits!(0.1), + None, + ); + let transition = setup + .card_creation_agreeing( + GasFeesPaidBy::DocumentOwner, + setup.card_action_fee_agreement( + DocumentTransitionActionType::Create, + AgreedFeeMultiplier { + known_permille: 1_000, + increase_tolerance_percent: 20, + }, + ), + ) + .await; + assert_eq!(setup.check_tx(&transition), Vec::::new()); + assert_eq!( + setup.check_tx_at(&transition, Recheck), + Vec::::new(), + "nothing moved" + ); + + set_fee_multiplier(&setup, 1_200); + assert_eq!( + setup.check_tx_at(&transition, Recheck), + Vec::::new(), + "a rise within the tolerance keeps the transition" + ); + + set_fee_multiplier(&setup, 1_201); + assert_eq!( + setup.check_tx_at(&transition, Recheck), + vec![DOCUMENT_ACTION_FEE_MULTIPLIER_NOT_TOLERATED], + "a rise beyond the tolerance drops it" + ); + // What the block would have done with it, had the recheck let it through. + let tx = setup.platform.drive.grove.start_transaction(); + assert_eq!( + paid_codes(&setup.process(&transition, &tx)), + vec![DOCUMENT_ACTION_FEE_MULTIPLIER_NOT_TOLERATED] + ); + drop(tx); + + set_fee_multiplier(&setup, 900); + assert_eq!( + setup.check_tx_at(&transition, Recheck), + Vec::::new(), + "a multiplier that fell back is always accepted" + ); + } + + #[tokio::test] + async fn should_drop_a_transition_from_the_mempool_once_the_declared_amounts_changed() { + // A contract update may not change the amounts of a document type yet + // (`validate_action_fees_unchanged`), so the contract is rewritten in Drive as one + // will be once a moderation charter sets the moderators part. The recheck reads the + // contract again through the transformer and judges the agreement against what it + // declares now. + let setup = game( + GasFeesPaidBy::DocumentOwner, + "fixed", + dash_to_credits!(0.1), + dash_to_credits!(0.1), + None, + ); + let transition = setup.card_creation(GasFeesPaidBy::DocumentOwner).await; + assert_eq!(setup.check_tx(&transition), Vec::::new()); + assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::::new()); + + let mut raised = setup.contract.clone(); + declare_action_fees( + &mut raised, + platform_value!({ + "pricing": "fixed", + "create": {"owner": OWNER_PART, "moderators": MODERATORS_PART + 1}, + }), + true, + ); + raised.increment_version(); + setup + .platform + .drive + .update_contract( + &raised, + BlockInfo::default(), + true, + None, + setup.platform_version, + None, + ) + .expect("expected to rewrite the contract with the raised moderators part"); + + assert_eq!( + setup.check_tx_at(&transition, Recheck), + vec![DOCUMENT_ACTION_FEE_AGREEMENT_MISMATCH] + ); + let tx = setup.platform.drive.grove.start_transaction(); + assert_eq!( + paid_codes(&setup.process(&transition, &tx)), + vec![DOCUMENT_ACTION_FEE_AGREEMENT_MISMATCH] + ); + } + + #[tokio::test] + async fn should_drop_a_sponsored_transition_from_the_mempool_once_the_fee_multiplier_outran_its_tolerance( + ) { + // A signer under the fee minimum who relies on the contract owner's sponsorship is + // rechecked against the state in full, on another route through check tx; the + // agreement is judged again on that route too. + let setup = game( + GasFeesPaidBy::ContractOwner, + "feeMultiplier", + dash_to_credits!(0.1), + 0, + None, + ); + let transition = setup + .card_creation_agreeing( + GasFeesPaidBy::ContractOwner, + setup.card_action_fee_agreement( + DocumentTransitionActionType::Create, + AgreedFeeMultiplier { + known_permille: 1_000, + increase_tolerance_percent: 20, + }, + ), + ) + .await; + assert_eq!(setup.check_tx(&transition), Vec::::new()); + assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::::new()); + + set_fee_multiplier(&setup, 1_200); + assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::::new()); + + set_fee_multiplier(&setup, 1_201); + assert_eq!( + setup.check_tx_at(&transition, Recheck), + vec![DOCUMENT_ACTION_FEE_MULTIPLIER_NOT_TOLERATED] + ); + } + #[tokio::test] async fn should_accept_a_fee_multiplier_that_fell() { let setup = game( diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/gas_sponsorship.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/gas_sponsorship.rs index 699daa92eb5..7277c833886 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/gas_sponsorship.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/gas_sponsorship.rs @@ -517,7 +517,11 @@ pub(crate) mod gas_sponsorship_tests { self.check_tx_at(transition, FirstTimeCheck) } - fn check_tx_at(&self, transition: &StateTransition, level: CheckTxLevel) -> Vec { + pub(crate) fn check_tx_at( + &self, + transition: &StateTransition, + level: CheckTxLevel, + ) -> Vec { let platform_version = self.platform_version; let state = self.platform.state.load(); let platform_ref = PlatformRef { diff --git a/packages/rs-platform-version/src/version/v14.rs b/packages/rs-platform-version/src/version/v14.rs index 22d41cbcc1e..64f89f6b189 100644 --- a/packages/rs-platform-version/src/version/v14.rs +++ b/packages/rs-platform-version/src/version/v14.rs @@ -462,6 +462,10 @@ pub const PROTOCOL_VERSION_14: ProtocolVersion = 14; /// (40132), with one to other amounts or another pricing (40133), or /// whose epoch's multiplier rose beyond the tolerance (40134), so a /// contract whose fees change cannot make a signed transition pay them. +/// Check tx judges the agreements again on every recheck, off the action +/// the transformer rebuilt with the contract and the multiplier as they +/// are then, so a batch a block would refuse leaves the mempool instead +/// of failing there (mempool policy, not consensus). /// /// 21. **Document restore by moderators**: the removal record a moderator's /// deletion leaves (19) also holds a double SHA-256 of the document as