Skip to content

Commit

Permalink
Merge #6520: backport: merge bitcoin#20039, bitcoin#22363, bitcoin#23079
Browse files Browse the repository at this point in the history 
, bitcoin#23120, bitcoin#23118, bitcoin#23866, bitcoin#23558, bitcoin#24035, bitcoin#23873, bitcoin#23978, bitcoin#23836, bitcoin#24223, bitcoin#24533, bitcoin#24605 (test backports: part 2)

14adbf5 merge bitcoin#24605: Use MiniWallet in feature_coinstatsindex (Kittywhiskers Van Gogh)
eba742d merge bitcoin#24533: use MiniWallet for feature_maxuploadtarget.py (Kittywhiskers Van Gogh)
9fec1be merge bitcoin#24223: use MiniWallet for interface_rest.py (Kittywhiskers Van Gogh)
1c87d1c merge bitcoin#23836: Expose block filters follow-ups (Kittywhiskers Van Gogh)
0e3dadf merge bitcoin#23978: use MiniWallet for mining_basic.py (Kittywhiskers Van Gogh)
16ffee4 merge bitcoin#23873: use MiniWallet for p2p_compactblocks.py (Kittywhiskers Van Gogh)
4da044b merge bitcoin#24035: use MiniWallet for mempool_accept.py (Kittywhiskers Van Gogh)
7c313d0 merge bitcoin#23558: run rpc-generateblock.py even with wallet disabled (Kittywhiskers Van Gogh)
75e0be5 merge bitcoin#23866: use MiniWallet for rpc_scantxoutset.py (Kittywhiskers Van Gogh)
d908de9 merge bitcoin#23118: introduce `script_util` helper for creating P2PK scripts (Kittywhiskers Van Gogh)
f01b2aa merge bitcoin#23120: Remove unused and confusing main parameter from script_util (Kittywhiskers Van Gogh)
adcc7a1 merge bitcoin#23079: use MiniWallet for p2p_filter.py (Kittywhiskers Van Gogh)
473620f test: make sure `MiniWallet().sendrawtransaction()` returns `txid` (Kittywhiskers Van Gogh)
24eb44c merge bitcoin#22363: use `script_util` helpers for creating P2{PKH,SH} scripts (Kittywhiskers Van Gogh)
bbb6599 merge bitcoin#20039: Convert amounts from float to decimal (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * While [bitcoin#22363](bitcoin#22363) implements helpers for P2{WPKH,WSH}, this has been omitted on account of Dash not supporting SegWit. Likewise, logic related to RBF and Taproot have been likewise omitted in other backports (e.g.  the changes extracted from [bitcoin#22998](bitcoin#22998) as 473620f).

  * In [bitcoin#23866](bitcoin#23866), `getnewdestination` will only generate `legacy` addresses (also the default type of address) on account of not supporting `bech32` addresses or SegWit.
    * In `address_to_scriptpubkey()`, the version bits were taken from testnet chain parameters ([source](https://github.com/dashpay/dash/blob/a5787c9a30896288c3e68ac443d46e54ce61c68f/src/chainparams.cpp#L448-L451))

  * The backport of [bitcoin#23079](bitcoin#23079) implements `random_p2pkh()` (as opposed to `random_p2wpkh()` upstream) on account of Dash not supporting SegWit.

  * The trio of transactions generated in `interface_rest.py` are 225 bytes each, which helps clear the "mempool consumes at least 300 bytes" assertion ([source](https://github.com/dashpay/dash/blob/a5787c9a30896288c3e68ac443d46e54ce61c68f/test/functional/interface_rest.py#L308-L309)).

    But [bitcoin#24223](bitcoin#24223) trims the transaction size down to 85 bytes each. To ensure the test passes, the assertion threshold has been reduced to 240 (assuming three transactions consume at least 80 bytes each).

  * In [bitcoin#24605](bitcoin#24605), `tx2` will spend 20.999 tDASH instead of 2.99 tBTC expected upstream. This is due to Dash having a lower feerate and the test unmodified will trigger a "Fee exceeds maximum configured by user" error. Known-good values affected by this change have been updated accordingly.

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 14adbf5
  PastaPastaPasta:
    utACK 14adbf5

Tree-SHA512: cb67402a05c4b4c91574c1846644737536677c6b16fc2548254f508fc1e43b0b04b45547fb4121df07c4c694a270752901318f9b86e181b9ab7e18663959c80f
  • Loading branch information
@PastaPastaPasta
PastaPastaPasta committed Jan 11, 2025
2 parents 5fd6a3f + 14adbf5 commit f722c7e
Show file tree
Hide file tree
Showing 31 changed files with 458 additions and 330 deletions.
25 changes: 12 additions & 13 deletions src/rest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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];
Expand Down Expand Up @@ -367,11 +367,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);

Expand All @@ -397,10 +396,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);
Expand All @@ -421,7 +420,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.";
Expand All @@ -439,7 +438,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;
}
Expand All @@ -450,7 +449,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;
}
Expand Down Expand Up @@ -479,8 +478,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);

Expand Down Expand Up @@ -538,7 +537,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();
Expand All @@ -547,7 +546,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";
Expand Down
24 changes: 16 additions & 8 deletions test/functional/data/invalid_txs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 16 additions & 7 deletions test/functional/feature_addressindex.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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"]))]
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions test/functional/feature_asset_locks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
21 changes: 12 additions & 9 deletions test/functional/feature_block.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
CTransaction,
CTxIn,
CTxOut,
SEQUENCE_FINAL,
uint256_from_compact,
uint256_from_str,
)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions test/functional/feature_cltv.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
)
from test_framework.messages import (
CTransaction,
SEQUENCE_FINAL,
msg_block,
)
from test_framework.p2p import P2PInterface
Expand Down Expand Up @@ -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 |
Expand All @@ -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])
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 = [
Expand Down
Loading

0 comments on commit f722c7e

Please sign in to comment.