backport: merge bitcoin#20039, #22363, #23079, #23120, #23118, #23866, #23558, #24035, #23873, #23978, #23836, #24223, #24533, #24605 (test backports: part 2)

src/rest.cpp

@@ -194,7 +194,7 @@ static bool rest_headers(const CoreContext& context,
 
     const auto parsed_count{ToIntegral<size_t>(path[0])};
     if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, path[0]));
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, path[0]));
     }
 
     std::string hashStr = path[1];
@@ -340,11 +340,10 @@ static bool rest_block_notxdetails(const CoreContext& context, HTTPRequest* req,
     return rest_block(context, req, strURIPart, false);
 }
 
-
 static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart)
 {
-    if (!CheckWarmup(req))
-        return false;
+    if (!CheckWarmup(req)) return false;
+
     std::string param;
     const RetFormat rf = ParseDataFormat(param, strURIPart);
 
@@ -370,10 +369,10 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con
 
     const auto parsed_count{ToIntegral<size_t>(uri_parts[1])};
     if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, uri_parts[1]));
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, uri_parts[1]));
     }
 
-    std::vector<const CBlockIndex *> headers;
+    std::vector<const CBlockIndex*> headers;
     headers.reserve(*parsed_count);
     {
         ChainstateManager* maybe_chainman = GetChainman(context, req);
@@ -394,7 +393,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con
 
     std::vector<uint256> filter_headers;
     filter_headers.reserve(*parsed_count);
-    for (const CBlockIndex *pindex : headers) {
+    for (const CBlockIndex* pindex : headers) {
         uint256 filter_header;
         if (!index->LookupFilterHeader(pindex, filter_header)) {
             std::string errmsg = "Filter not found.";
@@ -412,7 +411,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con
 
     switch (rf) {
     case RetFormat::BINARY: {
-        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
+        CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
         for (const uint256& header : filter_headers) {
             ssHeader << header;
         }
@@ -423,7 +422,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con
         return true;
     }
     case RetFormat::HEX: {
-        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
+        CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
         for (const uint256& header : filter_headers) {
             ssHeader << header;
         }
@@ -452,8 +451,8 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con
 
 static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart)
 {
-    if (!CheckWarmup(req))
-        return false;
+    if (!CheckWarmup(req)) return false;
+
     std::string param;
     const RetFormat rf = ParseDataFormat(param, strURIPart);
 
@@ -511,7 +510,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons
 
     switch (rf) {
     case RetFormat::BINARY: {
-        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION);
+        CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
         ssResp << filter;
 
         std::string binaryResp = ssResp.str();
@@ -520,7 +519,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons
         return true;
     }
     case RetFormat::HEX: {
-        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION);
+        CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
         ssResp << filter;
 
         std::string strHex = HexStr(ssResp) + "\n";

test/functional/data/invalid_txs.py

@@ -28,11 +28,19 @@
     CTxIn,
     CTxOut,
     MAX_MONEY,
+    SEQUENCE_FINAL,
 )
-from test_framework import script as sc
 from test_framework.blocktools import create_tx_with_script, MAX_BLOCK_SIGOPS
-
-basic_p2sh = sc.CScript([sc.OP_HASH160, sc.hash160(sc.CScript([sc.OP_0])), sc.OP_EQUAL])
+from test_framework.script import (
+    CScript,
+    OP_0,
+    OP_CHECKSIG,
+    OP_TRUE,
+)
+from test_framework.script_util import (
+    script_to_p2sh_script,
+)
+basic_p2sh = script_to_p2sh_script(CScript([OP_0]))
 
 
 class BadTxTemplate:
@@ -55,7 +63,7 @@ class BadTxTemplate:
     def __init__(self, *, spend_tx=None, spend_block=None):
         self.spend_tx = spend_block.vtx[0] if spend_block else spend_tx
         self.spend_avail = sum(o.nValue for o in self.spend_tx.vout)
-        self.valid_txin = CTxIn(COutPoint(self.spend_tx.sha256, 0), b"", 0xffffffff)
+        self.valid_txin = CTxIn(COutPoint(self.spend_tx.sha256, 0), b"", SEQUENCE_FINAL)
 
     @abc.abstractmethod
     def get_tx(self, *args, **kwargs):
@@ -95,7 +103,7 @@ class SizeTooSmall(BadTxTemplate):
     def get_tx(self):
         tx = CTransaction()
         tx.vin.append(self.valid_txin)
-        tx.vout.append(CTxOut(0, sc.CScript([sc.OP_TRUE])))
+        tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
         tx.calc_sha256()
         return tx
 
@@ -111,7 +119,7 @@ def get_tx(self):
         bad_idx = num_indices + 100
 
         tx = CTransaction()
-        tx.vin.append(CTxIn(COutPoint(self.spend_tx.sha256, bad_idx), b"", 0xffffffff))
+        tx.vin.append(CTxIn(COutPoint(self.spend_tx.sha256, bad_idx), b"", SEQUENCE_FINAL))
         tx.vout.append(CTxOut(0, basic_p2sh))
         tx.calc_sha256()
         return tx
@@ -149,7 +157,7 @@ class NonexistentInput(BadTxTemplate):
 
     def get_tx(self):
         tx = CTransaction()
-        tx.vin.append(CTxIn(COutPoint(self.spend_tx.sha256 + 1, 0), b"", 0xffffffff))
+        tx.vin.append(CTxIn(COutPoint(self.spend_tx.sha256 + 1, 0), b"", SEQUENCE_FINAL))
         tx.vin.append(self.valid_txin)
         tx.vout.append(CTxOut(1, basic_p2sh))
         tx.calc_sha256()
@@ -209,7 +217,7 @@ class TooManySigops(BadTxTemplate):
     expect_disconnect = False
 
     def get_tx(self):
-        lotsa_checksigs = sc.CScript([sc.OP_CHECKSIG] * (MAX_BLOCK_SIGOPS))
+        lotsa_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS))
         return create_tx_with_script(
             self.spend_tx, 0,
             script_pub_key=lotsa_checksigs,

test/functional/feature_addressindex.py

@@ -7,10 +7,19 @@
 # Test addressindex generation and fetching
 #
 
-from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut
+from test_framework.messages import (
+    COutPoint,
+    CTransaction,
+    CTxIn,
+    CTxOut,
+    COIN,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
-from test_framework.script import CScript, OP_CHECKSIG, OP_DUP, OP_EQUAL, OP_EQUALVERIFY, OP_HASH160
+from test_framework.script_util import (
+    keyhash_to_p2pkh_script,
+    scripthash_to_p2sh_script,
+)
 from test_framework.util import assert_equal
 
 class AddressIndexTest(BitcoinTestFramework):
@@ -127,7 +136,7 @@ def run_test(self):
         # Check that outputs with the same address will only return one txid
         self.log.info("Testing for txid uniqueness...")
         addressHash = bytes.fromhex("FE30B718DCF0BF8A2A686BF1820C073F8B2C3B37")
-        scriptPubKey = CScript([OP_HASH160, addressHash, OP_EQUAL])
+        scriptPubKey = scripthash_to_p2sh_script(addressHash)
         unspent = self.nodes[0].listunspent()
         tx = CTransaction()
         tx.vin = [CTxIn(COutPoint(int(unspent[0]["txid"], 16), unspent[0]["vout"]))]
@@ -153,7 +162,7 @@ def run_test(self):
         privkey2 = "cU4zhap7nPJAWeMFu4j6jLrfPmqakDAzy8zn8Fhb3oEevdm4e5Lc"
         address2 = "yeMpGzMj3rhtnz48XsfpB8itPHhHtgxLc3"
         addressHash2 = bytes.fromhex("C5E4FB9171C22409809A3E8047A29C83886E325D")
-        scriptPubKey2 = CScript([OP_DUP, OP_HASH160, addressHash2, OP_EQUALVERIFY, OP_CHECKSIG])
+        scriptPubKey2 = keyhash_to_p2pkh_script(addressHash2)
         self.nodes[0].importprivkey(privkey2)
 
         unspent = self.nodes[0].listunspent()
@@ -245,9 +254,9 @@ def run_test(self):
         privKey3 = "cRyrMvvqi1dmpiCmjmmATqjAwo6Wu7QTjKu1ABMYW5aFG4VXW99K"
         address3 = "yWB15aAdpeKuSaQHFVJpBDPbNSLZJSnDLA"
         addressHash3 = bytes.fromhex("6C186B3A308A77C779A9BB71C3B5A7EC28232A13")
-        scriptPubKey3 = CScript([OP_DUP, OP_HASH160, addressHash3, OP_EQUALVERIFY, OP_CHECKSIG])
+        scriptPubKey3 = keyhash_to_p2pkh_script(addressHash3)
         # address4 = "2N8oFVB2vThAKury4vnLquW2zVjsYjjAkYQ"
-        scriptPubKey4 = CScript([OP_HASH160, addressHash3, OP_EQUAL])
+        scriptPubKey4 = scripthash_to_p2sh_script(addressHash3)
         unspent = self.nodes[2].listunspent()
 
         tx = CTransaction()
@@ -310,7 +319,7 @@ def run_test(self):
         privkey1 = "cMvZn1pVWntTEcsK36ZteGQXRAcZ8CoTbMXF1QasxBLdnTwyVQCc"
         address1 = "yM9Eed1bxjy7tYxD3yZDHxjcVT48WdRoB1"
         address1hash = bytes.fromhex("0909C84A817651502E020AAD0FBCAE5F656E7D8A")
-        address1script = CScript([OP_DUP, OP_HASH160, address1hash, OP_EQUALVERIFY, OP_CHECKSIG])
+        address1script = keyhash_to_p2pkh_script(address1hash)
 
         self.nodes[0].sendtoaddress(address1, 10)
         self.generate(self.nodes[0], 1)

test/functional/feature_asset_locks.py

@@ -29,10 +29,12 @@
 )
 from test_framework.script import (
     CScript,
-    OP_CHECKSIG,
     OP_RETURN,
 )
-from test_framework.script_util import key_to_p2pkh_script
+from test_framework.script_util import (
+    key_to_p2pk_script,
+    key_to_p2pkh_script,
+)
 from test_framework.test_framework import DashTestFramework
 from test_framework.util import (
     assert_equal,
@@ -76,7 +78,7 @@ def create_assetlock(self, coin, amount, pubkey):
         remaining = int(COIN * coin['amount']) - tiny_amount - amount
 
         tx_output_ret = CTxOut(amount, CScript([OP_RETURN, b""]))
-        tx_output = CTxOut(remaining, CScript([pubkey, OP_CHECKSIG]))
+        tx_output = CTxOut(remaining, key_to_p2pk_script(pubkey))
 
         lock_tx = CTransaction()
         lock_tx.vin = inputs
@@ -93,7 +95,7 @@ def create_assetunlock(self, index, withdrawal, pubkey=None, fee=tiny_amount):
         node_wallet = self.nodes[0]
         mninfo = self.mninfo
         assert_greater_than(int(withdrawal), fee)
-        tx_output = CTxOut(int(withdrawal) - fee, CScript([pubkey, OP_CHECKSIG]))
+        tx_output = CTxOut(int(withdrawal) - fee, key_to_p2pk_script(pubkey))
 
         # request ID = sha256("plwdtx", index)
         request_id_buf = ser_string(b"plwdtx") + struct.pack("<Q", index)

test/functional/feature_block.py

@@ -21,6 +21,7 @@
     CTransaction,
     CTxIn,
     CTxOut,
+    SEQUENCE_FINAL,
     uint256_from_compact,
     uint256_from_str,
 )
@@ -35,20 +36,23 @@
     OP_CHECKSIGVERIFY,
     OP_ELSE,
     OP_ENDIF,
-    OP_EQUAL,
     OP_DROP,
     OP_FALSE,
-    OP_HASH160,
     OP_IF,
     OP_INVALIDOPCODE,
     OP_RETURN,
     OP_TRUE,
     SIGHASH_ALL,
     SignatureHash,
-    hash160,
+)
+from test_framework.script_util import (
+    script_to_p2sh_script,
 )
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+)
 from data import invalid_txs
 
 # This functional test assumes DIP0001 is disabled
@@ -484,8 +488,7 @@ def run_test(self):
 
         # Build the redeem script, hash it, use hash to create the p2sh script
         redeem_script = CScript([self.coinbase_pubkey] + [OP_2DUP, OP_CHECKSIGVERIFY] * 5 + [OP_CHECKSIG])
-        redeem_script_hash = hash160(redeem_script)
-        p2sh_script = CScript([OP_HASH160, redeem_script_hash, OP_EQUAL])
+        p2sh_script = script_to_p2sh_script(redeem_script)
 
         # Create a transaction that spends one satoshi to the p2sh_script, the rest to OP_TRUE
         # This must be signed because it is spending a coinbase
@@ -813,7 +816,7 @@ def run_test(self):
         b58 = self.next_block(58, spend=out[17])
         tx = CTransaction()
         assert len(out[17].vout) < 42
-        tx.vin.append(CTxIn(COutPoint(out[17].sha256, 42), CScript([OP_TRUE]), 0xffffffff))
+        tx.vin.append(CTxIn(COutPoint(out[17].sha256, 42), CScript([OP_TRUE]), SEQUENCE_FINAL))
         tx.vout.append(CTxOut(0, b""))
         tx.calc_sha256()
         b58 = self.update_block(58, [tx])
@@ -888,7 +891,7 @@ def run_test(self):
         tx.nLockTime = 0xffffffff  # this locktime is non-final
         tx.vin.append(CTxIn(COutPoint(out[18].sha256, 0)))  # don't set nSequence
         tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
-        assert tx.vin[0].nSequence < 0xffffffff
+        assert_greater_than(SEQUENCE_FINAL, tx.vin[0].nSequence)
         tx.calc_sha256()
         b62 = self.update_block(62, [tx])
         self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True)
@@ -1036,7 +1039,7 @@ def run_test(self):
         bogus_tx = CTransaction()
         bogus_tx.sha256 = uint256_from_str(b"23c70ed7c0506e9178fc1a987f40a33946d4ad4c962b5ae3a52546da53af0c5c")
         tx = CTransaction()
-        tx.vin.append(CTxIn(COutPoint(bogus_tx.sha256, 0), b"", 0xffffffff))
+        tx.vin.append(CTxIn(COutPoint(bogus_tx.sha256, 0), b"", SEQUENCE_FINAL))
         tx.vout.append(CTxOut(1, b""))
         b70 = self.update_block(70, [tx])
         self.send_blocks([b70], success=False, reject_reason='bad-txns-inputs-missingorspent', reconnect=True)

test/functional/feature_cltv.py

@@ -13,6 +13,7 @@
 )
 from test_framework.messages import (
     CTransaction,
+    SEQUENCE_FINAL,
     msg_block,
 )
 from test_framework.p2p import P2PInterface
@@ -56,7 +57,7 @@ def cltv_invalidate(tx, failure_reason):
     # 3) the lock-time type (height vs. timestamp) of the top stack item and the
     #    nLockTime field are not the same
     # 4) the top stack item is greater than the transaction's nLockTime field
-    # 5) the nSequence field of the txin is 0xffffffff
+    # 5) the nSequence field of the txin is 0xffffffff (SEQUENCE_FINAL)
     assert failure_reason in range(5)
     scheme = [
         # | Script to prepend to scriptSig                  | nSequence  | nLockTime    |
@@ -65,7 +66,7 @@ def cltv_invalidate(tx, failure_reason):
         [[OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP],       None,       None],
         [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP],  0,          1296688602],  # timestamp of genesis block
         [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP],  0,          50],
-        [[CScriptNum(50),  OP_CHECKLOCKTIMEVERIFY, OP_DROP],  0xffffffff, 50],
+        [[CScriptNum(50),  OP_CHECKLOCKTIMEVERIFY, OP_DROP],  SEQUENCE_FINAL, 50],
     ][failure_reason]
 
     cltv_modify_tx(tx, prepend_scriptsig=scheme[0], nsequence=scheme[1], nlocktime=scheme[2])
@@ -119,7 +120,7 @@ def run_test(self):
         # create one invalid tx per CLTV failure reason (5 in total) and collect them
         invalid_cltv_txs = []
         for i in range(5):
-            spendtx = wallet.create_self_transfer(from_node=self.nodes[0])['tx']
+            spendtx = wallet.create_self_transfer()['tx']
             cltv_invalidate(spendtx, i)
             invalid_cltv_txs.append(spendtx)
 
@@ -154,7 +155,7 @@ def run_test(self):
 
         # create and test one invalid tx per CLTV failure reason (5 in total)
         for i in range(5):
-            spendtx = wallet.create_self_transfer(from_node=self.nodes[0])['tx']
+            spendtx = wallet.create_self_transfer()['tx']
             cltv_invalidate(spendtx, i)
 
             expected_cltv_reject_reason = [