Repository: MystenLabs/sui
File: crates/sui-core/src/consensus_handler.rs (Rust)

Code before:
        ))
    }

    fn handle_deferral_and_cancellation(
        &self,
        state: &mut CommitHandlerState,
        cancelled_txns: &mut BTreeMap<TransactionDigest, CancelConsensusCertificateReason>,
        deferred_txns: &mut BTreeMap<DeferralKey, Vec<VerifiedExecutableTransactionWithAliases>>,
        scheduled_txns: &mut Vec<VerifiedExecutableTransactionWithAliases>,
        protocol_config: &ProtocolConfig,
        commit_info: &ConsensusCommitInfo,
        transaction: VerifiedExecutableTransactionWithAliases,
        shared_object_congestion_tracker: &mut SharedObjectCongestionTracker,
        previously_deferred_tx_digests: &HashMap<TransactionDigest, DeferralKey>,
        execution_time_estimator: &ExecutionTimeEstimator,
    ) {
        // Check for unpaid amplification before other deferral checks.
        // SIP-45: Paid amplification allows (gas_price / RGP + 1) submissions.
        // Transactions with more duplicates than paid for are deferred.
        if protocol_config.defer_unpaid_amplification() {
            let occurrence_count = state
                .occurrence_counts
                .get(transaction.tx().digest())
                .copied()
                .unwrap_or(1);

            let rgp = self.epoch_store.reference_gas_price();
            let gas_price = transaction.tx().transaction_data().gas_price();
            let allowed_count = (gas_price / rgp.max(1)) + 1;

            if occurrence_count as u64 > allowed_count {
                self.metrics
                    .consensus_handler_unpaid_amplification_deferrals
                    .inc();

                let deferred_from_round = previously_deferred_tx_digests
                    .get(transaction.tx().digest())
                    .map(|k| k.deferred_from_round())
                    .unwrap_or(commit_info.round);

                let deferral_key = DeferralKey::new_for_consensus_round(
                    commit_info.round + 1,
                    deferred_from_round,
                );

                if transaction_deferral_within_limit(
                    &deferral_key,
                    protocol_config.max_deferral_rounds_for_congestion_control(),
                ) {
                    assert_reachable!("unpaid amplification deferral");
                    debug!(

Review comment (bug):
I believe [at transaction driver level](https://github.com/MystenLabs/sui/blob/c19e916de408767804b4cf60f2f0a12abdd9bfd9/crates/sui-core/src/transaction_driver/mod.rs#L170C17-L170C37) we are a bit stricter on the amplification factor (we just do `gas_price/rgp.max(1)`) , whereas it seems that here we default to `allowed_count >= 2` . 

I am now thinking that it might be ok to not be totally aligned so we allow some slack for edge cases on the submission side and avoid end up deferring every single transaction. But equally we have to be very careful now on the submission side as any change on that side will impact this side as well.


Code after:
    fn handle_deferral_and_cancellation(
        &self,
        state: &mut CommitHandlerState,
        cancelled_txns: &mut BTreeMap<TransactionDigest, CancelConsensusCertificateReason>,
        deferred_txns: &mut BTreeMap<DeferralKey, Vec<VerifiedExecutableTransactionWithAliases>>,
        scheduled_txns: &mut Vec<VerifiedExecutableTransactionWithAliases>,
        protocol_config: &ProtocolConfig,
        commit_info: &ConsensusCommitInfo,
        transaction: VerifiedExecutableTransactionWithAliases,
        shared_object_congestion_tracker: &mut SharedObjectCongestionTracker,
        previously_deferred_tx_digests: &HashMap<TransactionDigest, DeferralKey>,
        execution_time_estimator: &ExecutionTimeEstimator,
    ) {
        // Check for unpaid amplification before other deferral checks.
        // SIP-45: Paid amplification allows (gas_price / RGP + 1) submissions.
        // Transactions with more duplicates than paid for are deferred.
        if protocol_config.defer_unpaid_amplification() {
            let occurrence_count = state
                .occurrence_counts
                .get(transaction.tx().digest())
                .copied()
                .unwrap_or(0);

            let rgp = self.epoch_store.reference_gas_price();
            let gas_price = transaction.tx().transaction_data().gas_price();
            let allowed_count = (gas_price / rgp.max(1)) + 1;

            if occurrence_count as u64 > allowed_count {
                self.metrics
                    .consensus_handler_unpaid_amplification_deferrals
                    .inc();

                let deferred_from_round = previously_deferred_tx_digests
                    .get(transaction.tx().digest())
                    .map(|k| k.deferred_from_round())
                    .unwrap_or(commit_info.round);

                let deferral_key = DeferralKey::new_for_consensus_round(
                    commit_info.round + 1,
                    deferred_from_round,
                );

                if transaction_deferral_within_limit(
                    &deferral_key,
                    protocol_config.max_deferral_rounds_for_congestion_control(),
                ) {
                    assert_reachable!("unpaid amplification deferral");
                    debug!(
                        "Deferring transaction {:?} due to unpaid amplification (count={}, allowed={})",
                        transaction.tx().digest(),
                        occurrence_count,