fix(drive-abci): recheck re-validates the document action fee agreement

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

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::<Option<ExecutionEvent>>::new_with_errors(
+                                vec![error],
+                            ),
+                        );
+                    }
+                }
+
                 let execution_event = ExecutionEvent::create_from_state_transition_action(
                     action,
                     maybe_identity,

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::<u32>::new());
+        assert_eq!(
+            setup.check_tx_at(&transition, Recheck),
+            Vec::<u32>::new(),
+            "nothing moved"
+        );
+
+        set_fee_multiplier(&setup, 1_200);
+        assert_eq!(
+            setup.check_tx_at(&transition, Recheck),
+            Vec::<u32>::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::<u32>::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::<u32>::new());
+        assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::<u32>::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::<u32>::new());
+        assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::<u32>::new());
+
+        set_fee_multiplier(&setup, 1_200);
+        assert_eq!(setup.check_tx_at(&transition, Recheck), Vec::<u32>::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(

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<u32> {
+        pub(crate) fn check_tx_at(
+            &self,
+            transition: &StateTransition,
+            level: CheckTxLevel,
+        ) -> Vec<u32> {
             let platform_version = self.platform_version;
             let state = self.platform.state.load();
             let platform_ref = PlatformRef {

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