Remove high-S signature handling from miners and signers

changelog.d/7484-remove-high-s-special-casing.removed

@@ -0,0 +1 @@
+Removed the special-casing of transactions with high-S signatures from mempool admittance and block validation. Now that we've transitioned into Epoch 4.0, those signatures are forbidden by consensus and no longer need to be special-cased.

stacks-node/src/tests/mempool.rs

@@ -317,30 +317,6 @@ fn mempool_setup_chainstate() {
                 );
                 let tx =
                     StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap();
-
-                // First, submit this transaction with a high-S signature. Even though that is technically
-                // a valid signature, the mempool should reject it.
-                let high_s_tx = tx.with_negated_s_in_signature();
-                let mut high_s_tx_bytes = vec![];
-                high_s_tx.consensus_serialize(&mut high_s_tx_bytes).unwrap();
-                let e = chain_state
-                    .will_admit_mempool_tx(
-                        &NULL_BURN_STATE_DB,
-                        consensus_hash,
-                        block_hash,
-                        &high_s_tx,
-                        high_s_tx_bytes.len() as u64,
-                    )
-                    .unwrap_err();
-                eprintln!("Err: {e:?}");
-                match e {
-                    MemPoolRejection::FailedToValidate(ChainstateError::NetError(
-                        NetError::VerifyingError(msg)
-                    )) => assert_eq!(msg, "Invalid signature: high-S"),
-                    _ => panic!("unexpected error {e:?} from mempool admittance check of high-s signature transaction")
-                }
-
-                // Now, submit it with the original, low-S signature. This should be successful.
                 chain_state
                     .will_admit_mempool_tx(
                         &NULL_BURN_STATE_DB,

stacks-node/src/tests/nakamoto_integrations.rs

@@ -3590,31 +3590,6 @@ fn block_proposal_api_endpoint() {
             HTTP_UNPROCESSABLE,
             None,
         ),
-        (
-            "High-S signature",
-            {
-                let mut p = proposal.clone();
-                p.block.executed_and_skipped_txs_mut()[0] =
-                    p.block.executed_and_skipped_txs()[0].with_negated_s_in_signature();
-                // tweaking the signature changes the transaction id (which is
-                // the main problem with high-S signatures), so we need to update
-                // the transaction merkle root
-                let txid_vecs: Vec<_> = p
-                    .block
-                    .txs()
-                    .map(|tx| tx.txid().as_bytes().to_vec())
-                    .collect();
-
-                let merkle_tree = MerkleTree::<Sha512Trunc256Sum>::new(&txid_vecs);
-                let tx_merkle_root = merkle_tree.root();
-
-                p.block.header.tx_merkle_root = tx_merkle_root;
-
-                sign(&p)
-            },
-            HTTP_ACCEPTED,
-            Some(Err(ValidateRejectCode::BadTransaction)),
-        ),
     ];
 
     // Build HTTP client

stackslib/src/chainstate/stacks/db/blocks.rs

@@ -6615,18 +6615,8 @@ impl StacksChainState {
         // 2: it must be validly signed.
         let epoch = clarity_connection.get_epoch();
 
-        // Enforce low-S on the transaction signatures. While consensus allows high-S
-        // signatures at the time of writing, they are a concern because the ambiguity
-        // makes transaction ids malleable. That's why we don't admit them to the mempol,
-        // and signers reject blocks with them. Once Epoch 4.0 begins, they will also
-        // not be allowed by consensus anymore.
-        StacksChainState::process_transaction_precheck(
-            chainstate_config,
-            tx,
-            epoch,
-            Some(TransactionAuthVerificationMode::EnforceLowS),
-        )
-        .map_err(MemPoolRejection::FailedToValidate)?;
+        StacksChainState::process_transaction_precheck(chainstate_config, tx, epoch)
+            .map_err(MemPoolRejection::FailedToValidate)?;
 
         // 3: it must pay a tx fee
         let fee = tx.get_tx_fee();

stackslib/src/chainstate/stacks/db/transactions.rs

@@ -665,7 +665,6 @@ impl StacksChainState {
         config: &DBConfig,
         tx: &StacksTransaction,
         epoch_id: StacksEpochId,
-        auth_verification_mode_override: Option<TransactionAuthVerificationMode>,
     ) -> Result<(), Error> {
         // valid auth?
         if !tx.auth.is_supported_in_epoch(epoch_id) {
@@ -677,13 +676,11 @@ impl StacksChainState {
 
             return Err(Error::InvalidStacksTransaction(msg, false));
         }
-        let verification_mode = auth_verification_mode_override.unwrap_or_else(|| {
-            if epoch_id.allows_tx_signatures_with_high_s() {
-                TransactionAuthVerificationMode::AllowHighS
-            } else {
-                TransactionAuthVerificationMode::EnforceLowS
-            }
-        });
+        let verification_mode = if epoch_id.allows_tx_signatures_with_high_s() {
+            TransactionAuthVerificationMode::AllowHighS
+        } else {
+            TransactionAuthVerificationMode::EnforceLowS
+        };
 
         tx.verify(verification_mode)?;
 
@@ -1910,7 +1907,7 @@ impl StacksChainState {
         // Static precheck (size, version, anchor mode, multisig encoding,
         // Clarity version...). A problematic marker only skips payload
         // execution; the transaction must still be otherwise valid.
-        StacksChainState::process_transaction_precheck(&clarity_block.config, tx, epoch, None)?;
+        StacksChainState::process_transaction_precheck(&clarity_block.config, tx, epoch)?;
 
         let mut transaction = clarity_block.connection().start_transaction_processing();
 
@@ -1956,7 +1953,7 @@ impl StacksChainState {
         debug!("Process transaction {} ({})", tx.txid(), tx.payload.name());
         let epoch = clarity_block.get_epoch();
 
-        StacksChainState::process_transaction_precheck(&clarity_block.config, tx, epoch, None)?;
+        StacksChainState::process_transaction_precheck(&clarity_block.config, tx, epoch)?;
 
         let mut transaction = clarity_block.connection().start_transaction_processing();
 

stackslib/src/net/api/postblock_proposal.rs

@@ -728,12 +728,6 @@ impl NakamotoBlockProposal {
         let burn_chain_height = miner_tenure_info.burn_tip_height;
         let mut tenure_tx = builder.tenure_begin(&burn_dbconn, &mut miner_tenure_info)?;
 
-        // Even if consensus allows high-S signatures, we want to refuse to sign blocks that
-        // contain them. If they're allowed in the current epoch, we therefore have to check
-        // for them ourselves.
-        let consensus_allow_high_s_tx_sig =
-            tenure_tx.get_epoch().allows_tx_signatures_with_high_s();
-
         let block_deadline = Instant::now() + Duration::from_secs(timeout_secs);
         let per_tx_max_execution_time = Duration::from_secs(max_tx_execution_time_secs);
         // Bound the analysis phase during proposal validation by the
@@ -778,34 +772,14 @@ impl NakamotoBlockProposal {
 
             let tx_len = tx.tx_len();
 
-            let tx_result = {
-                // If consensus allows high-S transaction signatures, then `try_mine_tx_with_len`
-                // would allow such a transaction, so we perform the stricter verification first,
-                // and only call `try_mine_tx_with_len` if successful. If consensus forbids it,
-                // we don't have to do this, because `try_mine_tx_with_len` will do it itself.
-                let high_s_check_result = if consensus_allow_high_s_tx_sig {
-                    match tx.verify(
-                        stacks_codec::transaction::TransactionAuthVerificationMode::EnforceLowS,
-                    ) {
-                        Ok(()) => Ok(()),
-                        Err(error) => Err(TransactionResult::error(tx, error.into())),
-                    }
-                } else {
-                    Ok(())
-                };
-
-                match high_s_check_result {
-                    Ok(()) => builder.try_mine_tx_with_len(
-                        &mut tenure_tx,
-                        tx,
-                        tx_len,
-                        &BlockLimitFunction::NO_LIMIT_HIT,
-                        &resource_budgets,
-                        &mut receipts_total,
-                    ),
-                    Err(e) => e,
-                }
-            };
+            let tx_result = builder.try_mine_tx_with_len(
+                &mut tenure_tx,
+                tx,
+                tx_len,
+                &BlockLimitFunction::NO_LIMIT_HIT,
+                &resource_budgets,
+                &mut receipts_total,
+            );
 
             let reason = match tx_result {
                 TransactionResult::Success(success_result) => {