test: `testmempoolaccept` accepts transactions from `fail_txs` in `feature_csv_activation.py` #25567

It is more a question than an issue. In feature_csv_activation.py, the same transactions that fail to be inserted in a block due to Locktime requirement not satisfied are allowed in mempool. This test does not change default mempool policy.

Shouldn't it be the same behavior in both cases?

        # All txs with nSequence 9 should fail either due to earlier mismatch or failing the CSV check
        fail_txs = all_rlt_txs(bip112txs_vary_nSequence_9_v2)
        fail_txs += [tx['tx'] for tx in bip112txs_vary_OP_CSV_9_v2 if not tx['sdf']]
        for tx in fail_txs:
            self.send_blocks([self.create_test_block([tx])], success=False,
                             reject_reason='non-mandatory-script-verify-flag (Locktime requirement not satisfied)')

+           self.miniwallet.sign_tx(tx)
+           # testmempoolaccept will return 'allowed': True for these transactions
+           print(self.nodes[0].testmempoolaccept(rawtxs=[tx.serialize().hex()]))

The transactions used in this test are consensus-valid, but don't follow the standard policy due to their scriptSig containing more than just push-only opcodes. If you want testmempoolaccept to return the same reject reason, you have to allow non-standard transactions via bitcoind parameter -acceptnonstdtxn=1 and remove the sign_tx(tx) call (it modified the tx, which doesn't make any sense?):

diff --git a/test/functional/feature_csv_activation.py b/test/functional/feature_csv_activation.py
index bff95c3b94..3cbb1df87d 100755
--- a/test/functional/feature_csv_activation.py
+++ b/test/functional/feature_csv_activation.py
@@ -99,6 +99,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
             '-whitelist=noban@127.0.0.1',
             f'-testactivationheight=csv@{CSV_ACTIVATION_HEIGHT}',
             '-par=1',  # Use only one script thread to get the exact reject reason for testing
+            '-acceptnonstdtxn=1',
         ]]
         self.supports_cli = False

@@ -449,6 +450,7 @@ class BIP68_112_113Test(BitcoinTestFramework):
         for tx in fail_txs:
             self.send_blocks([self.create_test_block([tx])], success=False,
                              reject_reason='non-mandatory-script-verify-flag (Locktime requirement not satisfied)')
+            print(self.nodes[0].testmempoolaccept(rawtxs=[tx.serialize().hex()]))

         # If SEQUENCE_LOCKTIME_DISABLE_FLAG is set in nSequence, tx should fail
         fail_txs = [tx['tx'] for tx in bip112txs_vary_nSequence_v2 if tx['sdf']]

Running this leads to:

$ ./test/functional/feature_csv_activation.py
.....
2022-07-08T01:21:38.906000Z TestFramework (INFO): Test version 2 txs
[{'txid': 'c160925d142ea9d1fde2f53ea755ea6e2788e35ee8d1710bd266c8df0f3affd5', 'wtxid': 'c160925d142ea9d1fde2f53ea755ea6e2788e35ee8d1710bd266c8df0f3affd5', 'allo
wed': False, 'reject-reason': 'non-mandatory-script-verify-flag (Locktime requirement not satisfied)'}]
[{'txid': 'fff76e4287e73f3d4f6a711c75baf2d18a207677d2cc79a0f56857cc0a1b74fc', 'wtxid': 'fff76e4287e73f3d4f6a711c75baf2d18a207677d2cc79a0f56857cc0a1b74fc', 'allo
wed': False, 'reject-reason': 'non-mandatory-script-verify-flag (Locktime requirement not satisfied)'}]
[{'txid': 'cba3a92d10bf0357bcb7e090cb3f00a8aeaf6eed80b002b0ab7562c00cf87aa3', 'wtxid': 'cba3a92d10bf0357bcb7e090cb3f00a8aeaf6eed80b002b0ab7562c00cf87aa3', 'allo
wed': False, 'reject-reason': 'non-mandatory-script-verify-flag (Locktime requirement not satisfied)'}]
.....

@theStack thanks for clarifying. I didn't realize that this test originally doesn't support non-standard transactions. Changing it to -acceptnonstdtxn=1, I was able to catch the correct error message in #25570.