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