aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2024-05-21Merge bitcoin/bitcoin#29421: net: make the list of known message types a ↵Ava Chow
compile time constant 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
2024-05-21Merge bitcoin/bitcoin#26606: wallet: Implement independent BDB parsermerge-script
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 https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca TheCharlatan: Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca furszy: reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca laanwj: re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca theStack: ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
2024-05-20Merge bitcoin/bitcoin#30133: test: remove unneeded `-maxorphantx=1000` settingsglozow
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
2024-05-20Merge bitcoin/bitcoin#30066: test: add conflicting topology test caseglozow
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](https://github.com/bitcoin/bitcoin/pull/30066/commits/9365baa489e123d9bcaf986e4311d3fa3f1e3f88) Tree-SHA512: d58661064ca099ac0447c331a5020c74c0cdfe24259aa875592805bbd63de1bf23aa7ced9ff485fef90dc0602fcb997e631aaf1aa2e9805d2cf5f0e5c9b2f0e2
2024-05-17test: remove unneeded `-maxorphantx=1000` settingsSebastian Falbesoner
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).
2024-05-17Merge bitcoin/bitcoin#30048: crypto: add `NUMS_H` constAva Chow
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](https://github.com/ElementsProject/secp256k1-zkp/blob/11af7015de624b010424273be3d91f117f172c82/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
2024-05-16wallet, test: Be able to always swap BDB endiannessAva Chow
2024-05-16test: Test bdb_ro dump of wallet without reset LSNsAva Chow
2024-05-16test: Test dumping dbs with overflow pagesAva Chow
2024-05-16test: Test dumps of other endian BDB filesAva Chow
2024-05-16wallettool: Optionally use BERKELEY_RO as format when dumping BDB walletsAva Chow
In order to ease the transition to not having BDB, make the dump tool use DatabaseFormmat::BERKELEY_RO when -withinternalbdb is set.
2024-05-16Merge bitcoin/bitcoin#27101: Support JSON-RPC 2.0 when requested by clientRyan Ofsky
cbc6c440e3811d342fa570713702900b3e3e75b9 doc: add comments and release-notes for JSON-RPC 2.0 (Matthew Zipkin) e7ee80dcf2b68684eae96070875ea13a60e3e7b0 rpc: JSON-RPC 2.0 should not respond to "notifications" (Matthew Zipkin) bf1a1f1662427fbf1a43bb951364eface469bdb7 rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests (Matthew Zipkin) 466b90562f4785de74b548f7c4a256069e2aaf43 rpc: Add "jsonrpc" field and drop null "result"/"error" fields (Matthew Zipkin) 2ca1460ae3a7217eaa8c5972515bf622bedadfce rpc: identify JSON-RPC 2.0 requests (Matthew Zipkin) a64a2b77e09bff784a2635ba19ff4aa6582bb5a5 rpc: refactor single/batch requests (Matthew Zipkin) df6e3756d6feaf1856e7886820b70874209fd90b rpc: Avoid copies in JSONRPCReplyObj() (Matthew Zipkin) 09416f9ec445e4d6bb277400758083b0b4e8b174 test: cover JSONRPC 2.0 requests, batches, and notifications (Matthew Zipkin) 4202c170da37a3203e05a9f39f303d7df19b6d81 test: refactor interface_rpc.py (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/2960 Bitcoin Core's JSONRPC server behaves with a special blend of 1.0, 1.1 and 2.0 behaviors. This introduces compliance issues with more strict clients. There are the major misbehaviors that I found: - returning non-200 HTTP codes for RPC errors like "Method not found" (this is not a server error or an HTTP error) - returning both `"error"` and `"result"` fields together in a response object. - different error-handling behavior for single and batched RPC requests (batches contain errors in the response but single requests will actually throw HTTP errors) https://github.com/bitcoin/bitcoin/pull/15495 added regression tests after a discussion in https://github.com/bitcoin/bitcoin/pull/15381 to kinda lock in our RPC behavior to preserve backwards compatibility. https://github.com/bitcoin/bitcoin/pull/12435 was an attempt to allow strict 2.0 compliance behind a flag, but was abandoned. The approach in this PR is not strict and preserves backwards compatibility in a familiar bitcoin-y way: all old behavior is preserved, but new rules are applied to clients that opt in. One of the rules in the [JSON RPC 2.0 spec](https://www.jsonrpc.org/specification#request_object) is that the kv pair `"jsonrpc": "2.0"` must be present in the request. Well, let's just use that to trigger strict 2.0 behavior! When that kv pair is included in a request object, the [response will adhere to strict JSON-RPC 2.0 rules](https://www.jsonrpc.org/specification#response_object), essentially: - always return HTTP 200 "OK" unless there really is a server error or malformed request - either return `"error"` OR `"result"` but never both - same behavior for single and batch requests If this is merged next steps can be: - Refactor bitcoin-cli to always use strict 2.0 - Refactor the python test framework to always use strict 2.0 for everything - Begin deprecation process for 1.0/1.1 behavior (?) If we can one day remove the old 1.0/1.1 behavior we can clean up the rpc code quite a bit. ACKs for top commit: cbergqvist: re ACK cbc6c440e3811d342fa570713702900b3e3e75b9 ryanofsky: Code review ACK cbc6c440e3811d342fa570713702900b3e3e75b9. Just suggested changes since the last review: changing uncaught exception error code from PARSE_ERROR to MISC_ERROR, renaming a few things, and adding comments. tdb3: re ACK for cbc6c440e3811d342fa570713702900b3e3e75b9 Tree-SHA512: 0b702ed32368b34b29ad570d090951a7aeb56e3b0f2baf745bd32fdc58ef68fee6b0b8fad901f1ca42573ed714b150303829cddad4a34ca7ad847350feeedb36
2024-05-15Merge bitcoin/bitcoin#30000: p2p: index TxOrphanage by wtxid, allow entries ↵Ryan Ofsky
with same txid 0fb17bf61a40b73a2b81a18e70b3de180c917f22 [log] updates in TxOrphanage (glozow) b16da7eda76944719713be68b61f03d4acdd3e16 [functional test] attackers sending mutated orphans (glozow) 6675f6428d653bf7a53537bd773114f4fb5ba53f [unit test] TxOrphanage handling of same-txid-different-witness txns (glozow) 8923edfc1f12ebc6a074651c084ba7d249074799 [p2p] allow entries with the same txid in TxOrphanage (glozow) c31f148166f01a9167d82501a77823785d28a841 [refactor] TxOrphanage::EraseTx by wtxid (glozow) efcc5930175f31b685adb4627a038d9f0848eb1f [refactor] TxOrphanage::HaveTx only by wtxid (glozow) 7e475b9648bbee04f5825b922ba0399373eaa5a9 [p2p] don't query orphanage by txid (glozow) Pull request description: Part of #27463 in the "make orphan handling more robust" section. Currently the main map in `TxOrphanage` is indexed by txid; we do not allow 2 transactions with the same txid into TxOrphanage. This means that if we receive a transaction and want to store it in orphanage, we'll fail to do so if a same-txid-different-witness version of the tx already exists in the orphanage. The existing orphanage entry can stay until it expires 20 minutes later, or until we find that it is invalid. This means an attacker can try to block/delay us accepting an orphan transaction by sending a mutated version of the child ahead of time. See included test. Prior to #28970, we don't rely on the orphanage for anything and it would be relatively difficult to guess what transaction will go to a node's orphanage. After the parent(s) are accepted, if anybody sends us the correct transaction, we'll end up accepting it. However, this is a bit more painful for 1p1c: it's easier for an attacker to tell when a tx is going to hit a node's orphanage, and we need to store the correct orphan + receive the parent before we'll consider the package. If we start out with a bad orphan, we can't evict it until we receive the parent + try the 1p1c, and then we'll need to download the real child, put it in orphanage, download the parent again, and then retry 1p1c. ACKs for top commit: AngusP: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 itornaza: trACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 instagibbs: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 theStack: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 sr-gi: crACK [0fb17bf](https://github.com/bitcoin/bitcoin/pull/30000/commits/0fb17bf61a40b73a2b81a18e70b3de180c917f22) stickies-v: ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22 Tree-SHA512: edcbac7287c628bc27036920c2d4e4f63ec65087fbac1de9319c4f541515d669fc4e5fdc30c8b9a248b720da42b89153d388e91c7bf5caf4bc5b3b931ded1f59
2024-05-14rpc: JSON-RPC 2.0 should not respond to "notifications"Matthew Zipkin
For JSON-RPC 2.0 requests we need to distinguish between a missing "id" field and "id":null. This is accomplished by making the JSONRPCRequest id property a std::optional<UniValue> with a default value of UniValue::VNULL. A side-effect of this change for non-2.0 requests is that request which do not specify an "id" field will no longer return "id": null in the response.
2024-05-14rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requestsMatthew Zipkin
Avoid returning HTTP status errors for non-batch JSON-RPC 2.0 requests if the RPC method failed but the HTTP request was otherwise valid. Batch requests already did not return HTTP errors previously.
2024-05-14rpc: Add "jsonrpc" field and drop null "result"/"error" fieldsMatthew Zipkin
Only for JSON-RPC 2.0 requests.
2024-05-14rpc: identify JSON-RPC 2.0 requestsMatthew Zipkin
2024-05-14test: add conflicting topology test caseGreg Sanders
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.
2024-05-14[log] updates in TxOrphanageglozow
- Add elapsed time in "remove orphan" log - Add size in "stored orphan" log - grammar edit
2024-05-14[functional test] attackers sending mutated orphansglozow
2024-05-14crypto: add NUMS_H constjosibake
2024-05-10Merge bitcoin/bitcoin#29948: test: add missing comparison of node1's mempool ↵Ava Chow
in MempoolPackagesTest e912717ff63f111d8f1cd7ed1fcf054e28f36409 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi) Pull request description: #29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed. Add missing comparison for TODO comments in `mempool_packages.py` Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits. ACKs for top commit: kevkevinpal: ACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) achow101: ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409 alfonsoromanz: Tested ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409. The code looks good to me and the test execution is successful. rkrux: tACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
2024-05-09Merge bitcoin/bitcoin#30006: test: use sleepy wait-for-log in reindex readonlyAva Chow
fd6a7d3a13d89d74e161095b0e9bd3570210a40c test: use sleepy wait-for-log in reindex readonly (Matthew Zipkin) Pull request description: Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152 ACKs for top commit: maflcko: utACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c achow101: ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c tdb3: ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c rkrux: ACK [fd6a7d3](https://github.com/bitcoin/bitcoin/pull/30006/commits/fd6a7d3a13d89d74e161095b0e9bd3570210a40c) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8
2024-05-09Merge bitcoin/bitcoin#29939: test: add MiniWallet tagging support to avoid ↵Ava Chow
UTXO mixing, use in `fill_mempool` dd8fa861939d5b8bdd844ad7cab015d08533a91a test: use tagged ephemeral MiniWallet instance in fill_mempool (Sebastian Falbesoner) b2037ad4aeb4e16c7eb1e5756d0d1ee20172344b test: add MiniWallet tagging support to avoid UTXO mixing (Sebastian Falbesoner) c8e6d08236ff225db445009bf513d6d25def8eb2 test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempool (Sebastian Falbesoner) 4f347140b1a31237597dd1821adcde8bd5761edc test: refactor: move fill_mempool to new module mempool_util (Sebastian Falbesoner) Pull request description: Different MiniWallet instances using the same mode (either ADDRESS_OP_TRUE, RAW_OP_TRUE or RAW_P2PK) currently always create and spend UTXOs with identical output scripts, which can cause unintentional tx dependencies (see e.g. the discussion in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465). In order to avoid mixing of UTXOs between instances, this PR introduces the possibility to provide a MiniWallet tag name, that is used to derive a different internal key for the taproot construction, leading to a different P2TR output script. Note that since we use script-path spending and only the key-path is changed here, no changes in the MiniWallet spending logic are needed. The new tagging option is then used in the `fill_mempool` helper to create an ephemeral wallet for the filling txs, as suggested in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565964264. To avoid circular dependencies, `fill_mempool` is moved to a new module `mempool_util.py` first. I'm still not sure if a generic word like "tag" is the right term for what this tries to achieve, happy to pick up better suggestions. Also, maybe passing a tag name is overkill and a boolean flag like "random_output_script" is sufficient? ACKs for top commit: glozow: ACK dd8fa861939 achow101: ACK dd8fa861939d5b8bdd844ad7cab015d08533a91a rkrux: tACK [dd8fa86](https://github.com/bitcoin/bitcoin/pull/29939/commits/dd8fa861939d5b8bdd844ad7cab015d08533a91a) brunoerg: utACK dd8fa861939d5b8bdd844ad7cab015d08533a91a Tree-SHA512: 5ef3558c3ef5ac32cfa79c8f751972ca6bceaa332cd7daac7e93412a88e30dec472cb041c0845b04abf8a317036d31ebddfc3234e609ed442417894c2bdeeac9
2024-05-09Merge bitcoin/bitcoin#29122: test: adds outbound eviction functional tests, ↵Ava Chow
updates comment in ConsiderEviction d53d84834747c37f4060a9ef379e0a6b50155246 test: adds outbound eviction tests for non outbound-full-relay peers (Sergi Delgado Segura) a8d9a0edc7cef2c31a557ef53eb45520976b0d65 test: adds outbound eviction functional tests, updates comment in ConsiderEviction (Sergi Delgado Segura) Pull request description: ## Motivation While checking the outbound eviction code I realized a case was not considered within the comments, which in turn made me realize we had no functional tests for the outbound eviction case (when I went to check/add the test case). This PR updates the aforementioned comment and adds functional tests to cover the outbound eviction logic, in addition to the existing unit tests found at `src/test/denialofservice_tests.cpp`. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/d53d84834747c37f4060a9ef379e0a6b50155246 tdb3: Re ACK for d53d84834747c37f4060a9ef379e0a6b50155246 achow101: ACK d53d84834747c37f4060a9ef379e0a6b50155246 cbergqvist: ACK d53d84834747c37f4060a9ef379e0a6b50155246 Tree-SHA512: 633b84bb1229fe21e2f650c1beada33ca7f190b64eafd64df2266516d21175e5d652e019ff7114f00cb8bd19f5817dc19e65adf75767a88e24dc0842ce40c63e
2024-05-09Merge bitcoin/bitcoin#29973: test: Assumeutxo: ensure failure when importing ↵Ava Chow
a snapshot twice b259b0e8d360726b062c4b0453d1cf5a68e1933f [Test] Assumeutxo: ensure failure when importing a snapshot twice (Alfonso Roman Zubeldia) Pull request description: I am getting familiar with the `assume_utxo` tests and I found that the scenario of trying to activate a snapshot twice is not covered. This test is to ensure failure when loading a snapshot if there is already a snapshot-based chainstate. ACKs for top commit: fjahr: Code review ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f kevkevinpal: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) achow101: ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f rkrux: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) Tree-SHA512: 3510861390d0e40cdad6861b728df04827a1b63e642f3d956aee66ed2770b1cb7e3aa3eb00c62eb9da0544703c943cc5296936c9ebfcac18c719741c354421bb
2024-05-08Merge bitcoin/bitcoin#29292: rpc: improve submitpackage documentation and ↵Ava Chow
other improvements 78e52f663f3e3ac86260b913dad777fd7218f210 doc: rpc: fix submitpackage examples (stickies-v) 1a875d4049574730d4a53a1b68bd29b80ad96d38 rpc: update min package size error message in submitpackage (stickies-v) f9ece258aa868d0776caa86b94e71ba05a9b287e doc: rpc: submitpackage takes sorted array (stickies-v) 17f74512f0d19cb452ed79a4bff5a222fcdb53c4 test: add bounds checking for submitpackage RPC (stickies-v) Pull request description: `submitpackage` requires the package to be topologically sorted with the child being the last element in the array, but this is not documented in the RPC method or the error messages. Also sneaking in some other minor improvements that I found while going through the code: - Informing the user that `package` needs to be an array of length between `1` and `MAX_PACKAGE_COUNT` is confusing when `IsChildWithPackage()` requires that the package size >= 2. Remove this check to avoid code duplication and sending a confusing error message. - fixups to the `submitpackage` examples ACKs for top commit: fjahr: re-ACK 78e52f663f3e3ac86260b913dad777fd7218f210 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29292/commits/78e52f663f3e3ac86260b913dad777fd7218f210 achow101: ACK 78e52f663f3e3ac86260b913dad777fd7218f210 glozow: utACK 78e52f663f3e3ac86260b913dad777fd7218f210 Tree-SHA512: a8845621bb1cbf784167fc7c82cb8ceb105868b65b26d3465f072d1c04ef3699e85a21a524ade805d423bcecbc34f7d5bff12f2c21cbd902ae1fb154193ebdc9
2024-05-08Merge bitcoin/bitcoin#29335: test: Handle functional test disk-full errorAva Chow
357ad110548d726021547d85b5b2bfcf3191d7e3 test: Handle functional test disk-full error (Brandon Odiwuor) Pull request description: Fixes: https://github.com/bitcoin/bitcoin/issues/23099 Handle disk-full more gracefully in functional tests ACKs for top commit: itornaza: re-ACK 357ad110548d726021547d85b5b2bfcf3191d7e3 achow101: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3 cbergqvist: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good! tdb3: re ACK for 357ad110548d726021547d85b5b2bfcf3191d7e3 Tree-SHA512: 9bb0d3fbe84600c88873b9f55d4b5d1443f79ec303467680c301be2b4879201387f203d9d1984169461f321037189b5e10a6a4b9d61750de638f072d2f95d77e
2024-05-08Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with ↵Ava Chow
specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
2024-05-08Merge bitcoin/bitcoin#30053: test: added test coverage to loadtxoutset could ↵merge-script
not open file ee67bba76cca2355541f99bb731f58479981b29e test: added test coverage to loadtxoutset (kevkevin) Pull request description: The functional test coverage did not cover the rpc error of "Couldn't open file..." for loadtxoutset and this test adds coverage for it This adds coverage to this line https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2777 ACKs for top commit: maflcko: ACK ee67bba76cca2355541f99bb731f58479981b29e davidgumberg: LGTM ACK https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e rkrux: ACK [ee67bba](https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e) alfonsoromanz: ACK ee67bba76cca2355541f99bb731f58479981b29e. Code looks good to me. I also ran `test/functional/feature_assumeutxo.py` to make sure all tests passes, including this one. tdb3: ACK for ee67bba76cca2355541f99bb731f58479981b29e Tree-SHA512: 210a7eb928f625d2a8d9acb63ee83cb4aaec9c267e5a0c52ad219c2935466e2cdc68667e30ad29566e6060981587e5bec42805d296f6e60f9b3b13f3330575f2
2024-05-08Merge bitcoin/bitcoin#30025: doc: fix broken relative md linksmerge-script
4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 doc: fix broken relative md links (willcl-ark) Pull request description: These relative links in our documentation are broken, fix them. ACKs for top commit: maflcko: ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 ryanofsky: Code review ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3. Thanks for the updates! ismaelsadeeq: Re ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 Tree-SHA512: df4ef5ddece6c21125ce719ed6a4f69aba4f884c353ff7a8445ecb6438ed6bf0ff8268a1ae19cdd910adaadc189c6861c445b4d469f92ee81874d810dcbd0846
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-07rpc: update min package size error message in submitpackagestickies-v
Currently, the only allowed package topology has a min size of 2. Update the error message to reflect that.
2024-05-07test: add bounds checking for submitpackage RPCstickies-v
2024-05-06test: added test coverage to loadtxoutsetkevkevin
The functional test coverage did not cover the rpc error of Couldn't open file for loadtxoutset and this test adds coverage for it
2024-05-06Merge bitcoin/bitcoin#29845: rpc: return warnings as an array instead of ↵Ava Chow
just a single one 42fb5311b19582361409d65c6fddeadbee14bb97 rpc: return warnings as an array instead of just a single one (stickies-v) Pull request description: The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning (i.e. the latest one that is set) is returned, the other ones are ignored. Fix that by returning all warnings as an array. As a side benefit, clean up the GetWarnings() logic. Since this PR changes the RPC result schema, I've added release notes. Users can temporarily revert to the old results by using `-deprecatedrpc=warnings`, until it's removed in a future version. --- Some historical context from git log: - when `GetWarnings` was introduced in 401926283a200994ecd7df8eae8ced8e0b067c46, it was used in the `getinfo` RPC, where only a [single error/warning was returned](https://github.com/bitcoin/bitcoin/commit/401926283a200994ecd7df8eae8ced8e0b067c46#diff-7442c48d42cd5455a79915a0f00cce5e13359db46437a32b812876edb0a5ccddR250) (similar to how it is now). - later on, "warnings" RPC response fields were introduced, e.g. in ef2a3de25c882396e1776b554878d2784b6b7391, with the description [stating](https://github.com/bitcoin/bitcoin/commit/ef2a3de25c882396e1776b554878d2784b6b7391#diff-1021bd3c74415ad9719bd764ad6ca35af5dfb33b1cd863c0be49bdf52518af54R411) that it returned "any network warnings" but in practice still only a single warning was returned ACKs for top commit: achow101: re-ACK 42fb5311b19582361409d65c6fddeadbee14bb97 tdb3: Re ACK for 42fb5311b19582361409d65c6fddeadbee14bb97 TheCharlatan: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 maflcko: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 🔺 Tree-SHA512: 4225ed8979cd5f030dec785a80e7452a041ad5703445da79d2906ada983ed0bbe7b15889d663d75aae4a77d92e302c93e93eca185c7bd47c9cce29e12f752bd3
2024-05-05test: use tagged ephemeral MiniWallet instance in fill_mempoolSebastian Falbesoner
2024-05-05test: add MiniWallet tagging support to avoid UTXO mixingSebastian Falbesoner
Note that this commit doesn't change behaviour yet, as tagging isn't used in any MiniWallet instance.
2024-05-05test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempoolSebastian Falbesoner
2024-05-05test: refactor: move fill_mempool to new module mempool_utilSebastian Falbesoner
This is needed to avoid circular dependencies in later commits. Can be reviewed via `--color-moved=dimmed-zebra`.
2024-05-03Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with ↵Ava Chow
MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
2024-05-03doc: fix broken relative md linkswillcl-ark
These relative links in our documentation are broken, fix them.
2024-05-03[test]: remove duplicate WITNESS_SCALE_FACTORismaelsadeeq
2024-05-02Merge bitcoin/bitcoin#29617: test: Validate UTXO snapshot with coin height > ↵Ava Chow
base height & amount > MAX_MONEY supply ec1f1abfefa281e62bb876aa1c4738d576ef9a47 test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply (jrakibi) Pull request description: ### Ensure snapshot loading fails for coins exceeding base height **Objective**: This test verifies that snapshot loading is correctly rejected for coins with a height greater than the base height. **Update**: - Added `test_invalid_snapshot_wrong_coin_code` to `feature_assumeutxo.py`. - The test artificially sets a coin's height above 299 in a snapshot and checks for load failure. - Edit: Added a test case for outputs whose amounts surpass the MAX_MONEY supply limit. This implementation addresses the request for enhancing `assumeutxo` testing as outlined in issue #28648 --- **Edit: This is an explanation on how I arrive at content values: b"\x84\x58" and b"\xCA\xD2\x8F\x5A"** You can use this tool to decode the utxo snapshot https://github.com/jrakibi/utxo-live Here’s an overview of how it’s done: The serialization format for a UTXO in the snapshot is as follows: 1. Transaction ID (txid) - 32 bytes 2. Output Index (outnum)- 4 bytes 3. VARINT (code) - A varible-length integer encoding the height and whether the transaction is a coinbase. The format of this VARINT is (height << 1) | coinbase_flag. 4. VARINT (amount_v) - A variable-length integer that represents a compressed format of the output amount (in satoshis). For the test cases mentioned: * **`b"\x84\x58"`** - This value corresponds to a VARINT representing the height and coinbase flag. Once we decode this code, we can extract the height and coinbase using `height = code_decoded >> 1` and `coinbase = code_decoded & 0x01`. In our case, with code_decoded = 728, it results in `height = 364` and `coinbase = 0`. * **`b"\xCA\xD2\x8F\x5A"`** - This byte sequence represents a compressed amount value. The decompression function takes this value and translates it into a full amount in satoshis. In our case, the decompression of this amount translates to a number larger than the maximum allowed value of coins (21 million BTC) ACKs for top commit: fjahr: re-ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 maflcko: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑 achow101: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47 Tree-SHA512: 42b36fd1d76e9bc45861028acbb776bd2710c5c8bff2f75c751ed505995fbc1d4bc698df3be24a99f20bcf6a534615d2d9678fb3394162b88133eaec88ca2120
2024-05-02Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZEJon Atack
2024-05-02Merge bitcoin/bitcoin#30010: lint: [doc] Clarify Windows line endings (CR ↵merge-script
LF) not to be used fa9be2f79520fff9cfe2ed35ace05cb322680af3 lint: [doc] Clarify Windows line endings (CR LF) not to be used (MarcoFalke) Pull request description: It has been this case since the linter was introduced years ago. Given a misunderstanding (https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2088028856), clarify the docs. ACKs for top commit: brunoerg: ACK fa9be2f79520fff9cfe2ed35ace05cb322680af3 Tree-SHA512: be714db9df533e0962ed84102ffdb72717902949b930d58cf5a79cba36297f6b2b1f75e65a2c1f46bcb8e2f4ad5d025f3d15210f468a5ec9631a580b74f923ea
2024-05-01Merge bitcoin/bitcoin#29120: test: Add test case for spending bare multisigmerge-script
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/29113 ACKs for top commit: ajtowns: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f achow101: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f willcl-ark: reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
2024-05-01rpc: return warnings as an array instead of just a single onestickies-v
The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning is returned. Fix that by returning all warnings as an array. As a side benefit, cleans up the GetWarnings() logic.
2024-05-01Add lint check for bitcoin-config.h include IWYU pragmaMarcoFalke
Also, remove the no longer needed, remaining definitions and checks of HAVE_CONFIG_H.
2024-05-01lint: [doc] Clarify Windows line endings (CR LF) not to be usedMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-17 18:42:36 +0000