The helper assumes that the n and k values have to be provided as a
single byte push operation, which is only possible for values up to 16.
Fix that by passing the numbers directly to the CScript list, where it's
automatically converted to minimally-encoded pushes (see class
method `CScript.__coerce_instance`, branch `isinstance(other, int)`).
In case of 17..20, this means that the data-pushes are done with two
bytes using OP_PUSH1 (0x01), e.g. for n=20: 0x01,0x14
4444de152f01368e603f2b089679a86eae02e34a test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure (MarcoFalke)
fa6aa4027cecd819c1210d6959af364d5bf9f608 test: Fix typos and use names args (MarcoFalke)
Pull request description:
Otherwise, the test may fail on slow hardware when running in valgrind.
Also, use named args for the absolute timepoint, while touching this file.
ACKs for top commit:
tdb3:
ACK for 4444de152f01368e603f2b089679a86eae02e34a
AngusP:
re-ACK 4444de152f01368e603f2b089679a86eae02e34a
Tree-SHA512: 660269c8dd18887d69b284f38656899caf028159ce83ddf921f3e9c080a5d0e663989f0e42b4baf4c4939f20f34da0e7e844dff9b7c91d0cab570c60958bd0e1
07aba8dd215b23b06853b1a9fe04ac8b08f62932 functional test: ensure confirmed utxo being sourced for 2nd chain (Greg Sanders)
Pull request description:
The test could fail/stop testing what we want if non-confirmed utxos become sourced through some internal change to `MiniWallet`; better to just fetch confirmed explicitly.
ACKs for top commit:
achow101:
ACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932
ismaelsadeeq:
utACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932
theStack:
ACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932
Tree-SHA512: 66795fdf881139ed91bde0f8239a46bd9bc70bb311fa97c0e2b5537e1fd2a1fd36bf3a225fc77b9695deb835a9d6d29879aa1e05ea5054b9a33a400e199da014
2451a217dd2c21b6d2f2b2699ceddd0bf9073019 test: addmultisigaddress, coverage for script size limits (furszy)
53302a09817e5b799d345dfea432546a55a9d727 bugfix: addmultisigaddress, add unsupported operation for redeem scripts over 520 bytes (furszy)
9be6065cc03f2408f290a332b203eef9c9cebf24 test: coverage for 16-20 segwit multisig scripts (furszy)
9d9a91c4ea6b3bb32ef4131bca86f1d6683fc901 rpc: bugfix, incorrect segwit redeem script size used in signrawtransactionwithkey (furszy)
0c9fedfc45fa7cbd6801ca5fd756863ec9a6911c fix incorrect multisig redeem script size limit for segwit (furszy)
f7a173b5785cda460470df9a74a0e0f94d7f9a18 test: rpc_createmultisig, decouple 'test_sortedmulti_descriptors_bip67' (furszy)
4f33dbd8f8c0e29f37b04e6af6d2c7905ecceaf6 test: rpc_createmultisig, decouple 'test_mixing_uncompressed_and_compressed_keys' (furszy)
25a81705d376e8c96dad45436ae3fca975b3daf5 test: rpc_createmultisig, remove unnecessary checkbalances() (furszy)
b5a328943362cfac6e90fd4e1b167c357d53b7d4 test: refactor, multiple cleanups in rpc_createmultisig.py (furszy)
3635d432681847313c098f9827483372a840e70f test: rpc_createmultisig, remove manual wallet initialization (furszy)
Pull request description:
Fixing https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1674830104 and more.
Currently, redeem scripts longer than 520 bytes, which are technically valid under segwit rules, have flaws in the following processes:
1) The multisig creation process fails to deduce the output descriptor, resulting in the generation of an incorrect descriptor. Additionally, the accompanying user warning is also inaccurate.
2) The `signrawtransactionwithkey` RPC command fail to sign them.
3) The legacy wallet `addmultisigaddress` wrongly discards them.
The issue arises because most of these flows are utilizing the legacy spkm keystore, which imposes
the [p2sh max redeem script size rule](ded6873340/src/script/signingprovider.cpp (L160)) on all scripts. Which blocks segwit redeem scripts longer than
the max element size in all the previously mentioned processes (`createmultisig`, `addmultisigaddress`, and
`signrawtransactionwithkey`).
This PR fixes the problem, enabling the creation of multisig output descriptors involving more than 15 keys and
allowing the signing of these scripts, along with other post-segwit redeem scripts that surpass the 520-byte
p2sh limit.
Important note:
Instead of adding support for these longer redeem scripts in the legacy wallet, an "unsupported operation"
error has been added. The reasons behind this decision are:
1) The introduction of this feature brings about a compatibility-breaking change that requires downgrade
protection; older wallets would be unable to interact with these "new" legacy wallets.
2) Considering the ongoing deprecation of the legacy spkm, this issue provides another compelling
reason to transition towards descriptors.
Testing notes:
To easily verify each of the fixes, I decoupled the tests into standalone commits. So they can be
cherry-picked on top of master. Where `rpc_createmultisig.py` (with and without the `--legacy-wallet`
arg) will fail without the bugs fixes commits.
Extra note:
The initial commits improves the `rpc_createmultisig.py` test in many ways. I found this test very
antiquated, screaming for an update and cleanup.
ACKs for top commit:
pinheadmz:
ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019
theStack:
Code-review ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019
achow101:
ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019
Tree-SHA512: 71794533cbd46b3a1079fb4e9d190d3ea3b615de0cbfa443466e14f05e4616ca90e12ce2bf07113515ea8113e64a560ad572bb9ea9d4835b6fb67b6ae596167f
df6dc2aaaeffc664006b86ee8c8797dc484ec40e test: Assumeutxo: snapshots with less work should not be loaded (Hernan Marino)
Pull request description:
This PR adds a test which checks that snapshots with less accumulated work than the node's active chain, should not be loaded and return with an error. Although in a different context of discussion the missing test was detect in a thread in https://github.com/bitcoin/bitcoin/pull/29394 (see https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1484122214)
ACKs for top commit:
maflcko:
utACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e
kevkevinpal:
utACK [df6dc2a](df6dc2aaae)
achow101:
ACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e
alfonsoromanz:
Re ACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e. Make is successful and the test passes.
Tree-SHA512: 07a394b4b288cc8ad3f66ed4e70dcda468db18113e9442eb7215cf491768432d55efaaa5b79d633094917e05475a30f0c5e4f64f8f2da293ba306891b4485560
71aae72e1fc998b2629d68a7301d85dc1b65641e test: test sendall does ancestor aware funding (ishaanam)
36757941a05b65c2b61a83820afdf5effd8fc9a2 wallet, rpc: implement ancestor aware funding for sendall (ishaanam)
544131f3fba9ea07fee29f9d3ee0116cd5d8a5b2 rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified (ishaanam)
Pull request description:
This PR:
- Adds a functional test that `sendall` spends unconfirmed change
- Adds a functional test that `sendall` spends regular unconfirmed inputs when specified by user
- Adds ancestor aware funding to `sendall` by using `calculateCombinedBumpFee` and adjusting the effective value accordingly
- Adds a functional test for ancestor aware funding in `sendall`
ACKs for top commit:
S3RK:
ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e
achow101:
ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e
furszy:
ACK 71aae72e1f
Tree-SHA512: acaeb7c65166ce53123a1d6cb5012197202246acc02ef9f37a28154cc93afdbd77c25e840ab79bdc7e0b88904014a43ab1ddea79d5337dc310ea210634ab61f0
This removes the need to actually track misbehavior score (see further commit), because any
Misbehaving node will immediately hit the discouragement threshold.
This misbehavior was originally intended to prevent bandwidth wastage due to
actually observed very broken (but likely non-malicious) nodes that respond
to GETHEADERS with a response unrelated to the request, triggering a request
cycle.
This has however largely been addressed by the previous commit, which causes
non-connecting HEADERS that are received while a GETHEADERS has not been
responded to, to be ignored, as long as they do not time out (2 minutes).
With that, the specific misbehavior is largely irrelevant (for inbound peers,
it is now harmless; for outbound peers, the eviction logic will eventually
kick them out if they're not keeping up with headers at all).
Since https://github.com/bitcoin/bitcoin/pull/25454 we keep track of the last
GETHEADERS request that was sent and wasn't responded to. So far, every incoming
HEADERS message is treated as a response to whatever GETHEADERS was last sent,
regardless of its contents.
This commit makes this tracking more accurate, by only treating HEADERS messages
which (1) are empty, (2) connect to our existing block header tree, or (3) are a
continuation of a low-work headers sync as responses that clear the "outstanding
GETHEADERS" state (m_last_getheaders_timestamp).
That means that HEADERS messages which do not satisfy any of the above criteria
will be ignored, not triggering a GETHEADERS, and potentially (for now, but see
later commit) increase misbehavior score.
4b7d9842691046b01f0c08d69f924ddb62ccc4c6 lint: add markdown hyperlink checker (willcl-ark)
Pull request description:
Potential followup to: #30025
This should prevent us reintroducing broken markdown links.
It does not test "online" (external) links, only those within this repo. Both relative and absolute links are parsed successfully if they resolve.
ACKs for top commit:
maflcko:
re-utACK 4b7d9842691046b01f0c08d69f924ddb62ccc4c6
davidgumberg:
reACK 4b7d984269
Tree-SHA512: 9bc40d700b73499c046bb76157bc139f32ec3850f64ef813bbf7f18f9c01a253abe6a857d6f559890165f2bd26e7742c05d86232cd9b8efb33ff85d735f4f095
Currently, transport version is a global variable declared as
TRANSPORT_VERSION in v2_p2p.py. Making it an instance variable
would help in sending non empty transport version packets for
testing purposes. It might also help EncryptedP2PState be more
extensible in far future protocol upgrades.
This adds a markdown hyperlink check task to the lint test_runner. It
relies on having the [`mlc`](https://crates.io/crates/mlc) binary found
on $PATH, but will fail with `success` if the binary is not found.
`mlc` is also added to the ci/04_install.sh script run by the
containerfile.
Note that broken markdown hyperlinks will be detected in untracked
markdown files found in a dirty working directory (including e.g.
.venv).
2fd34ba504957331f5a08614b6e1f8317260f04d Add sanity checks for various ATMPArgs booleans (Greg Sanders)
20d8936d8bb6137a5a3722d34e0d7ae756031009 [refactor] make some members MemPoolAccept-wide (glozow)
cbbfe719b223b9e05398227cef68c99eb97670bd cpfp carveout is excluded in packages (glozow)
69f7ab05bafec1cf06fd7a58351f78e32bbfa2cf Add m_allow_sibling_eviction as separate ATMPArgs flag (Greg Sanders)
57ee3029ddadaee5cb48224e0a87f704b7971bd8 Add description for m_test_accept (Greg Sanders)
Pull request description:
First few commits of https://github.com/bitcoin/bitcoin/pull/28984 to set the stage for the package RBF logic.
These refactors are preparation for evaluating an RBF in a multi-proposed-transaction context instead of only a single proposed transaction. Also, carveouts and sibling evictions only should work in single RBF cases so add logic to preclude multi-tx cases in the future.
No behavior changes aside from bailing earlier from failed carve-outs.
ACKs for top commit:
glozow:
reACK 2fd34ba504957331f5a08614b6e1f8317260f04d via range-diff
sr-gi:
utACK [2fd34ba](2fd34ba504)
theStack:
re-ACK 2fd34ba504957331f5a08614b6e1f8317260f04d
Tree-SHA512: 5071c5b8d9b8d2c9faa278c8c4df31de288cb407a68e4d55544c588caff6c86160cce7825453549c6ed69e29d9ccb5ee2d4a518b18f563bfb12f2ced073fe42a
542e13b2937356810bda2c41be83c3b1675e2f2f rpc: Enhance metadata of the dumptxoutset output (Fabian Jahr)
4d8e5edbaa94805be41ae4c8aa2f4bf7aaa276fe assumeutxo: Add documentation on dumptxoutset serialization format (Fabian Jahr)
c14ed7f384075330361df636f40121cf25a066d6 assumeutxo: Add test for changed coin size value (Fabian Jahr)
de95953d870c41436de67d56c93259bc66fe1434 rpc: Optimize serialization disk space of dumptxoutset (Fabian Jahr)
Pull request description:
The second attempt at implementing the `dumptxoutset` space optimization as suggested in #25675. Closes#25675.
This builds on the work done in #26045, addresses open feedback, adds some further improvements (most importantly usage of compact size), documentation, and an additional test.
The [original snapshot at height 830,000](https://github.com/bitcoin/bitcoin/pull/29551) came in at 10.82 GB. With this change, the same snapshot is 8.94 GB, a reduction of 17.4%.
This also enhances the metadata of the output file and adds the following data to allow for better error handling and make future upgrades easier:
- A newly introduced utxo set magic
- A version number
- The network magic
- The block height
ACKs for top commit:
achow101:
ACK 542e13b2937356810bda2c41be83c3b1675e2f2f
TheCharlatan:
Re-ACK 542e13b2937356810bda2c41be83c3b1675e2f2f
theStack:
ACK 542e13b2937356810bda2c41be83c3b1675e2f2f
Tree-SHA512: 0825d30e5c3c364062db3c6cbca4e3c680e6e6d3e259fa70c0c2b2a7020f24a47406a623582040988d5c7745b08649c31110df4c10656aa25f3f27eb35843d99
The behavior is not new, but this rule exits earlier than before.
Previously, a carve out could have been granted in PreChecks() but then
nullified in PackageMempoolChecks() when CheckPackageLimits() is called
with the default limits.
154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow)
a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow)
d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow)
Pull request description:
Opening for discussion / conceptual review.
We like the idea of a smaller maximum transaction size because:
- It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction)
- They are easier to bin-pack in block template production
- They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space.
History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510):
- 2010-09-13 In 3df62878c3 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction()
- 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well
Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning).
This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult.
~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number.
ACKs for top commit:
sdaftuar:
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
achow101:
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
instagibbs:
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
t-bast:
ACK 154b2b2296
murchandamus:
crACK 154b2b2296edccb5ed24e829798dacb6195edc11
Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
6629d1d0f8285d1bf2d87341a856abe903f26c13 test: improve robustness of connect_nodes() (furszy)
Pull request description:
Decoupled from #27837 because this can help other too, found it investigating a CI failure https://cirrus-ci.com/task/5805115213348864?logs=ci#L3200.
The `connect_nodes` function in the test framework relies on a stable number of peer
connections to verify that the new connection between the nodes is successfully established.
This approach is fragile, as any of the peers involved in the process can drop, lose, or
create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
even when the peers in question were connected successfully.
This commit improves the situation by using the nodes' subversion and the connection
direction (inbound/outbound) to identify the exact peer connection and perform the
checks exclusively on it.
ACKs for top commit:
stratospher:
reACK 6629d1d.
achow101:
ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
maflcko:
utACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
AngusP:
re-ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
Tree-SHA512: 5f345c0ce49ea81b643e97c5cffd133e182838752c27592fcdeac14ad10919fb4b7ff38e289e42a7c3c638a170bd0d0b7a9cd493898997a2082a7b7ceef4aeeb
1e54d61c4698debf3329d1960e06078ccbf8063c test: add coverage for `mapped_as` from `getrawaddrman` (brunoerg)
8c2714907d1e1ffc58487b3b43e018c1ec10065b net: rpc: return peer's mapped AS in getrawaddrman (brunoerg)
Pull request description:
This PR adds two new fields in `getrawaddrman` RPC: "mapped_as" and "source_mapped_as". These fields are used to return the ASN (Autonomous System Number) mapped to the peer and its source. With these informations we can have a better view of the bucketing logic with ASMap specially in projects like [addrman-observer](https://github.com/0xb10c/addrman-observer).
ACKs for top commit:
fjahr:
Code review ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
virtu:
ACK [1e54d61](1e54d61c46)
0xB10C:
ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
glozow:
ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
Tree-SHA512: af86bcc7a2e69bebd3fa9eaa2e527e0758c44c0a958de7292514d5f99f8f01f5df3bae11400451268e0255f738ff3acccc77f48fe129937512f1e9d9963c4c5e
Test addresses are being mapped according to the ASMap
file provided properly. Compare the result of the `getrawaddrman`
RPC with the result from the ASMap Health Check.
b3efb486732f3caf8b8a8e9d744e6d20ae4255ef protocol: make message types constexpr (Vasil Dimov)
2fa9de06c2c8583ee8e2434dc97014b26e218ab5 net: make the list of known message types a compile time constant (Vasil Dimov)
Pull request description:
Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size.
---
This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR.
ACKs for top commit:
achow101:
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
jonatack:
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
maflcko:
utACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef 🎊
willcl-ark:
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
The 'connect_nodes' function in the test framework relies
on a stable number of peer connections to verify the new
connection between the nodes is successfully established.
This approach is fragile, as any of the peers involved in
the process can drop, lose, or create a connection at any
step, causing subsequent 'wait_until' checks to stall
indefinitely even when the peers in question are connected
successfully.
This commit improves the situation by using the nodes' subversion
and the connection direction (inbound/outbound) to identify the
exact peer connection and perform the checks exclusively on it.
d51fbab4b32d56765e8faab6ad01245fb259b0ca wallet, test: Be able to always swap BDB endianness (Ava Chow)
0b753156ce60c29efb2386954ba7555ad8f642f5 test: Test bdb_ro dump of wallet without reset LSNs (Ava Chow)
c1984f128284589423b7e0cc06c9a3b23a242d95 test: Test dumping dbs with overflow pages (Ava Chow)
fd7b16e391ed320e35255157a28be14c947ef30a test: Test dumps of other endian BDB files (Ava Chow)
6ace3e953f0864bd7818f040c59a1bc70aa47512 bdb: Be able to make byteswapped databases (Ava Chow)
d9878903fb34939dee8e1462f079acc68110253d Error if LSNs are not reset (Ava Chow)
4d7a3ae78e55f25868979f1bd920857a4aecb825 Berkeley RO Database fuzz test (TheCharlatan)
3568dce9e93295674cdf5458c5bdf93ff01fd0a2 tests: Add BerkeleyRO to db prefix tests (Ava Chow)
70cfbfdadf16d3b115309c6938f07ef5b96c7cc1 wallettool: Optionally use BERKELEY_RO as format when dumping BDB wallets (Ava Chow)
dd57713f6ede3d46e97ee7df87c10001b0bf4c3d Add MakeBerkeleyRODatabase (Ava Chow)
6e50bee67d1d58aecd8a0ce8b7c3f5a7979365f5 Implement handling of other endianness in BerkeleyRODatabase (Ava Chow)
cdd61c9cc108df8e13f4e3891ff2c96355b3ee38 wallet: implement independent BDB deserializer in BerkeleyRODatabase (Ava Chow)
ecba23097955dad7208baa687fc405c846aee794 wallet: implement BerkeleyRODatabase::Backup (Ava Chow)
0c8e72847603540bb29b8b8aeb80fa3f2e3a2c9a wallet: implement BerkeleyROBatch (Ava Chow)
756ff9b478484b17c4a6e65c171c2e4fecb21ad4 wallet: add dummy BerkeleyRODatabase and BerkeleyROBatch classes (Ava Chow)
ca18aea5c4975ace4e307be96c74641d203fa389 Add AutoFile::seek and tell (Ava Chow)
Pull request description:
Split from #26596
This PR adds `BerkeleyRODatabase` which is an independent implementation of a BDB file parser. It provides read only access to a BDB file, and can therefore be used as a read only database backend for wallets. This will be used for dumping legacy wallet records and migrating legacy wallets without the need for BDB itself.
Wallettool's `dump` command is changed to use `BerkeleyRODatabase` instead of `BerkeleyDatabase` (and `CWallet` itself) to demonstrate that this parser works and to test it against the existing wallettool functional tests.
ACKs for top commit:
josibake:
reACK d51fbab4b3
TheCharlatan:
Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
furszy:
reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
laanwj:
re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
theStack:
ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
8950053636cb38ed85fe2d58b53e5d0acb35c390 test: remove unneeded `-maxorphantx=1000` settings (Sebastian Falbesoner)
Pull request description:
It's unclear what the motivation for increasing the orphan pool is here, and it seems that this not needed at all. None of these tests involve orphan transactions explicitly, and if they would occur occasionally, there is no good reason to prefer a value of 1000 over the default of 100 (see DEFAULT_MAX_ORPHAN_TRANSACTIONS).
ACKs for top commit:
maflcko:
utACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
edilmedeiros:
Tested ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
AngusP:
tACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
glozow:
ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 From skimming the tests, it appears that none of these need a larger `-maxorphantx`.
Tree-SHA512: 81d4a4fb2ea92b97119f21cbc6c4b1240d863269932e6adf4982aead9726f20652523a4707add3ad38eb332d4452de41de6735265f22e62298f3b4b45de75a57
9365baa489e123d9bcaf986e4311d3fa3f1e3f88 test: add conflicting topology test case (Greg Sanders)
Pull request description:
We want to ensure that even if topologies
that are acceptable are relaxed, like
removing package-not-child-with-unconfirmed-parents, that we don't end up accepting packages we shouldn't.
ACKs for top commit:
glozow:
reACK 9365baa489
rkrux:
reACK [9365baa](9365baa489)
Tree-SHA512: d58661064ca099ac0447c331a5020c74c0cdfe24259aa875592805bbd63de1bf23aa7ced9ff485fef90dc0602fcb997e631aaf1aa2e9805d2cf5f0e5c9b2f0e2
It is best to store all key origin information
(master key fingerprint and all derivation steps)
in the multisig descriptor. Being explicit with
this information should be beneficial if this approach
is used with other wallets/signers (whether hardware
or software). There is no harm including all of this
with xpubs (if anything it simplifies the test code)
and makes this example/docs more complete and safer
incase it is referenced by others.
It's unclear what the motivation for increasing the orphan pool is, and
it seems that this not needed at all. None of these tests involve orphan
transactions explicitly, and if they would occur occasionally, there is
no good reason to prefer a value of 1000 over the default of 100 (see
DEFAULT_MAX_ORPHAN_TRANSACTIONS).
9408a04e424cee0d226bde79171bd4954f9caeb0 tests, fuzz: use new NUMS_H const (josibake)
b946f8a4c51be42e52d63a6d578158c0b2a6b7ed crypto: add NUMS_H const (josibake)
Pull request description:
Broken out from #28122
---
[BIP341](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs) defines a NUMS point `H` as *H = lift_x(0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0)* which is [constructed](11af7015de/src/modules/rangeproof/main_impl.h (L16)) by taking the hash of the standard uncompressed encoding of the [secp256k1](https://www.secg.org/sec2-v2.pdf) base point G as X coordinate."
Add this as a constant so it can be used in our codebase. My primary motivation is BIP352 specifies a special case for when taproot spends use `H` as the internal key, but outside of BIP352 it seems generally useful to have `H` in the codebase, for testing or other use cases.
ACKs for top commit:
paplorinc:
re-ACK 9408a04e424cee0d226bde79171bd4954f9caeb0
achow101:
ACK 9408a04e424cee0d226bde79171bd4954f9caeb0
theStack:
Code-review ACK 9408a04e424cee0d226bde79171bd4954f9caeb0
Tree-SHA512: ad84492f5d635c0cb05bd82546079ded7e5138e95361f20d8285a9ad6e69c10ee2cc3fe46e16b46ef03c4253c8bee1051911c6b91264c90c3b1ad33a824bff4b
Currently, we log the number of bytes of garbage when it is
generated. The log is a better fit for when the garbage
actually gets sent to the transport layer.
