aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-03-04Merge #17809: rpc: Auto-format RPCResultMarcoFalke
fa6b061fc118995eec41766519a11bc0dd1a901d rpc: Auto-format RPCResult (MarcoFalke) fa7d0503d320900e14c4d9bc016d65c7431070bb rpc: Move OuterType enum to header (MarcoFalke) Pull request description: This enforces most syntax rules of the RPCResult at compile time (or some at run time during unit and functional tests) Apart from normalizing the syntax, by separating stylistic formatting from the structure, we could in theory directly generate the html for e.g. https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/ ACKs for top commit: Sjors: Indeed, re-ACK fa6b061fc118995eec41766519a11bc0dd1a901d ajtowns: ACK fa6b061fc118995eec41766519a11bc0dd1a901d -- skimmed code changes and differences to rpc help output Tree-SHA512: 5b510b3aa0b7c7b9189a48c77593159409069f939145b9a00c5478e894cf65f994d44d633eb7bb7dbea40ee820645a2930976c24772379d96929002b120efa28
2020-03-04Merge #18253: doc: Correct spelling errors in commentsfanquake
9b0e16226e6c1fb6a3550d635339f1bbb49a852f doc: Correct spelling errors in comments (Ben Woosley) Pull request description: And ci script output. Identified via test/lint/lint-spelling Before: ``` $ test/lint/lint-spelling.sh ci/test/05_before_script.sh:29: explicitely ==> explicitly src/compressor.h:43: Ser ==> Set src/compressor.h:78: Ser ==> Set src/logging/timer.h:88: outputing ==> outputting src/node/psbt.cpp:87: minumum ==> minimum src/qt/coincontroldialog.cpp:372: UnSelect ==> deselect src/qt/coincontroldialog.cpp:443: unselect ==> deselect src/qt/coincontroldialog.cpp:448: UnSelect ==> deselect src/qt/coincontroldialog.cpp:699: UnSelect ==> deselect src/serialize.h:211: Ser ==> Set src/serialize.h:213: Ser ==> Set src/serialize.h:228: Ser ==> Set src/serialize.h:246: Ser ==> Set src/serialize.h:484: Ser ==> Set src/serialize.h:490: Ser ==> Set src/serialize.h:510: Ser ==> Set src/serialize.h:622: Ser ==> Set src/serialize.h:740: Ser ==> Set src/test/base32_tests.cpp:14: fo ==> of, for src/test/base64_tests.cpp:14: fo ==> of, for src/txmempool.h:756: incomaptible ==> incompatible src/undo.h:26: Ser ==> Set src/wallet/coincontrol.h:74: UnSelect ==> deselect test/functional/feature_backwards_compatibility.py:116: Abondon ==> Abandon test/functional/rpc_getaddressinfo_label_deprecation.py:7: superceded ==> superseded test/lint/lint-shell.sh:44: desriptor ==> descriptor ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt ``` After: ``` $ test/lint/lint-spelling.sh src/test/base32_tests.cpp:14: fo ==> of, for src/test/base64_tests.cpp:14: fo ==> of, for test/functional/rpc_getaddressinfo_label_deprecation.py:7: superceded ==> superseded ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt ``` ACKs for top commit: practicalswift: ACK 9b0e16226e6c1fb6a3550d635339f1bbb49a852f MarcoFalke: ACK 9b0e16226e6c1fb6a3550d635339f1bbb49a852f Tree-SHA512: 9ce203700b11596e4b920b3c5b04f59bc7784fe5b495868d43423608180a9a553ec7efcc5ad70384f3ce462b036c2a682260efebce493c5e6a3d48716b268179
2020-03-02doc: Correct spelling errors in commentsBen Woosley
And ci script output. Identified via test/lint/lint-spelling
2020-03-02Merge #18168: httpserver: use own HTTP status codesWladimir J. van der Laan
aff2748f8aee72f03b5fb6f6f97f0d0f66391755 httpserver: use own HTTP status codes (Filip Gospodinov) Pull request description: Before, macros defined in `<event2/http.h>` have been used for some HTTP status codes. `<event2/http.h>` is included implicitly and the usage of its status code macros is inconsistent with the majority HTTP response implementations in this file. Now, the `HTTPStatusCode` enum from `<rpc/protocol.h>` is consistently used for all HTTP response implementations. ACKs for top commit: practicalswift: ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755 -- patch looks correct laanwj: ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755 Tree-SHA512: 6a7043488b88dcd584215d16b5f16f7bd668fe5553d31298d1beef134d2b0648aef81533014e34d1cd600baa36ee4e853f195352f4a00c866bd5ab1ff688bd50
2020-03-02Merge #18224: Make AnalyzePSBT next role calculation simple, correctSamuel Dobson
1ef28b4f7cfba410fef524def1dac24bbc4086ca Make AnalyzePSBT next role calculation simple, correct (Gregory Sanders) Pull request description: Sniped test and alternative to https://github.com/bitcoin/bitcoin/pull/18220 Sjors documenting the issue: ``` A PSBT signed by ColdCard was analyzed as follows (see #17509 (comment)) { "inputs": [ { "has_utxo": true, "is_final": false, "next": "finalizer" } ], "estimated_vsize": 141, "estimated_feerate": 1e-05, "fee": 1.41e-06, "next": "signer" } I changed AnalyzePSBT so that it returns "next": "finalizer" instead. ``` It makes it much clearer that the role has been decided before hitting the `calc_fee` block, and groups all state-deciding in one spot instead of 2. Note that this assumes that PSBT roles are a complete ordering, which for now and in the future seems to be a correct assumption. ACKs for top commit: Sjors: ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca, much nicer. Don't forget to document the bug fix. achow101: ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca Empact: ACK https://github.com/bitcoin/bitcoin/pull/18224/commits/1ef28b4f7cfba410fef524def1dac24bbc4086ca Tree-SHA512: 22ba4234985c6f9c1445b14565c71268cfaa121c4ef000ee3d5117212b09442dee8d46d9701bceddaf355263fe25dfe40def2ef614d4f2fe66c9ce876cb49934
2020-03-01Merge #17399: validation: Templatize ValidationState instead of subclassingMarcoFalke
10efc0487c442bccb0e4a9ac29452af1592a3cf2 Templatize ValidationState instead of subclassing (Jeffrey Czyz) 10e85d4adc9b7dbbda63e00195e0a962f51e4d2c Remove ValidationState's constructor (Jeffrey Czyz) 0aed17ef2892478c28cd660e53223c6dd1dc0187 Refactor FormatStateMessage into ValidationState (Jeffrey Czyz) Pull request description: This removes boilerplate code in the subclasses which otherwise only differ by the result type. The subclassing was introduced in a27a295. ACKs for top commit: MarcoFalke: ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 🐱 ajtowns: ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 -- looks good to me jonatack: ACK 10efc048 code review, build/tests green, nice cleanup Tree-SHA512: 765dd52dde7d49b9a5c6d99d97c96f4492673e2aed0b0604faa88db0308fa4500a26bf755cca0b896be283874096c215932e1110a2d01dc012cd36a5fce58a42
2020-02-29Merge #18229: random: drop unused MACH time headersfanquake
d36146009fb3fc9b9a772823b4df139a85173481 Drop unused mach time headers (Ben Woosley) Pull request description: Now that we're no longer special-casing clock usage for MacOS (see #17800), we're not referencing anything defined in these headers. Incidentally, this removes our last reference to the `__MACH__` system def. 🎉 ACKs for top commit: jonasschnelli: utACK d36146009fb3fc9b9a772823b4df139a85173481 fanquake: ACK d36146009fb3fc9b9a772823b4df139a85173481 - thanks. Tree-SHA512: 246045b0683a705ad034416e8ace2024e652026a6c0517b6797320e52fc18a6e111ec2e405ca40653bd1d6421bb7755232e8fec22651fff8e448eb7d5646a954
2020-02-29Merge #18225: util: Fail to parse empty string in ParseMoneyfanquake
8888461f6814ae8b6221b02049fb9e1f69a5ff70 util: Fail to parse empty string in ParseMoney (MarcoFalke) fab30b61eb51538a4db62e34f7133c44575b3fe9 util: Remove unused ParseMoney that takes a c_str (MarcoFalke) Pull request description: Supplying a fee rate or an amount on the command line as an empty string, which currently parses as `0` seems fragile and confusing. See for example the confusion in #18214. Fixes #18214 ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/18225/commits/8888461f6814ae8b6221b02049fb9e1f69a5ff70 achow101: ACK 8888461f6814ae8b6221b02049fb9e1f69a5ff70 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/18225/commits/8888461f6814ae8b6221b02049fb9e1f69a5ff70 Tree-SHA512: ac2d6b7fa89fe5809c34d5f49831042032591c34fb3c76908d72fed51e8bced41bf2b41dc1b3be34ee691a40463355649857a7a8f378709d38ae89503feb11c2
2020-02-28Drop unused mach time headersBen Woosley
Now that we're no longer special-casing clock usage for MacOS, we're not referencing anything defined in these headers.
2020-02-28Merge #16562: Refactor message transport packagingMarcoFalke
16d6113f4faa901e248adb693d4768a9e5019a16 Refactor message transport packaging (Jonas Schnelli) Pull request description: This PR factors out transport packaging logic from `CConnman::PushMessage()`. It's similar to #16202 (where we refactor deserialization). This allows implementing a new message transport protocol like BIP324. ACKs for top commit: dongcarl: ACK 16d6113f4faa901e248adb693d4768a9e5019a16 FWIW ariard: Code review ACK 16d6113 elichai: semiACK 16d6113f4faa901e248adb693d4768a9e5019a16 ran functional+unit tests. MarcoFalke: ACK 16d6113f4faa901e248adb693d4768a9e5019a16 🙎 Tree-SHA512: 8c2f8ab9f52e9b94327973ae15019a08109d5d9f9247492703a842827c5b5d634fc0411759e0bb316d824c586614b0220c2006410851933613bc143e58f7e6c1
2020-02-28Merge #17800: random: don't special case clock usage on macOSWladimir J. van der Laan
dc9305b6162ec615ff5fb2876e4f312051b543af random: don't special case clock usage on macOS (fanquake) Pull request description: `clock_gettime()`, `CLOCK_MONOTONIC` and `CLOCK_REALTIME` are all available for use on macOS (now that we require macOS >=10.12 and build against 10.14). Use them rather than the [deprecated](https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/Mach/Mach.html) `mach_timespec_t` time API. I mentioned the possibility for this change [in #17270](https://github.com/bitcoin/bitcoin/pull/17270#discussion_r346090606). [master](1dbf3350c683f93d7fc9b861400724f6fd2b2f1d): ```bash 2019-12-23T20:49:43Z Feeding 216 bytes of dynamic environment data into RNG 2019-12-23T20:50:43Z Feeding 216 bytes of dynamic environment data into RNG ``` This PR: ```bash 2019-12-23T20:32:41Z Feeding 232 bytes of dynamic environment data into RNG 2019-12-23T20:33:42Z Feeding 232 bytes of dynamic environment data into RNG ``` ~~Depends on #16392.~~ Merged. ACKs for top commit: laanwj: ACK dc9305b6162ec615ff5fb2876e4f312051b543af Tree-SHA512: 18c2f336ea628f9cf7339b817381d230a18893fd9c0351bf99a39ca6f45c5b0a20af9d599d48d6c09515627d5edafa91337c17f9f790264251d2cdcb3763bbd5
2020-02-29Merge #18173: refactor: test/bench: deduplicate SetupDummyInputs()MarcoFalke
7bf4ce4f644bb7dac9b63172c656b5d599eedea3 refactor: test/bench: dedup SetupDummyInputs() (Sebastian Falbesoner) Pull request description: The only difference between `SetupDummyInputs()` in `test/transaction_tests.cpp` and the one in `bench/ccoins_caching.cpp` was the nValue amounts of the outputs, so we allow to pass those in an extra (fixed-size) array parameter. ACKs for top commit: MarcoFalke: re-ACK 7bf4ce4f64, only change is schuffling includes 🚶 Empact: ACK https://github.com/bitcoin/bitcoin/pull/18173/commits/7bf4ce4f644bb7dac9b63172c656b5d599eedea3 Tree-SHA512: e13643b2470f6b6ab429da0c0a8eebd4cb41e2ff2e421ef36f85fa4847bf4ea8aab88d59a01e94cac4c4eb85edb561463f02215b174c50b573ac6bbcc2bf98a3
2020-02-28refactor: test/bench: dedup SetupDummyInputs()Sebastian Falbesoner
The only difference between SetupDummyInputs() in test/transaction_tests.cpp and the one in bench/ccoins_caching.cpp was the nValue amounts of the outputs, so we allow to pass those in an extra (fixed-size) array parameter.
2020-02-29Merge #17959: test: check specific reject reasons in feature_csv_activation.pyMarcoFalke
54be4e71d898de8f14e3269550d56097c023d1cc test: check specific reject reasons in feature_csv_activation.py (Sebastian Falbesoner) Pull request description: This is kind of a prequel to #17921: increases the general quality of the functional test `feature_csv_activation.py` by checking for the specific reject reasons whenever the sending of a block fails. To get the reason, we have to limit the script threads to 1 via the parameter `-par=1`, like it is also done in `feature_cltv.py`: https://github.com/bitcoin/bitcoin/blob/a654626f076a72416a3d354218d7107571d6caaf/test/functional/feature_cltv.py#L57-L61 The commit also fixes a bug that was uncovered with this checks: for the BIP112 version 1 tx tests, txs from `bip112txs_vary_OP_CSV_v1` have been add twice to the list `failed_txs`: https://github.com/bitcoin/bitcoin/blob/a654626f076a72416a3d354218d7107571d6caaf/test/functional/feature_csv_activation.py#L396-L397 leading also to a block rejection as expected but for the wrong reason. It seems one of those two tx lists was meant to be `bip112txs_vary_OP_CSV_v1` (without the `_9`) and it was a typo. ACKs for top commit: MarcoFalke: ACK 54be4e71d898de8f14e3269550d56097c023d1cc 📶 Tree-SHA512: 9aac11aee3f53f1ae95ddb346a2f268872038f4d118c8dcf81b8201dee869774c9f3c3f1c326e370b8fd4eaf8e0673371689a96d9b1cb91be4286c88824725c3
2020-02-28test: check specific reject reasons in feature_csv_activation.pySebastian Falbesoner
this also fixes a bug that was uncovered with this checks: for the BIP112 version 1 tx tests, certain txs (bip112txs_vary_OP_CSV_v1) have been sent twice due to a typo, leading also to a failure as expected but for the wrong reason
2020-02-28Merge #18135: build: add --enable-determinism configure flagWladimir J. van der Laan
3d9b41ecc02a69daa19df2bc850d60bf9917b724 build: add --enable-determinism configure flag (fanquake) Pull request description: This adds a `--enable-determinsm` configure flag, which if used, will enable additional compile / link time flags to make subsequent builds of bitcoind deterministic. The first flag enabled is `--no-insert-timestamp`. This prevents the linker from embedding timestamps, and makes consecutive builds of `bitcoind.exe` deterministic. This will likely also be used for [Guix Windows builds](https://github.com/bitcoin/bitcoin/pull/17595#issuecomment-582696467). [`--no-insert-timestamp`](https://manpages.debian.org/buster/binutils-mingw-w64-x86-64/x86_64-w64-mingw32-ld.bfd.1.en.html): > The option --no-insert-timestamp can be used to insert a zero value for the timestamp, this ensuring that binaries produced from identical sources will compare identically. Diff of consecutive builds of [master](https://github.com/bitcoin/bitcoin/commit/2bdc476d4d23256d8396bb9051a511f540d87392): ```diff --- bitcoind.exe.1 +++ bitcoind.exe.2 @@ -2,20 +2,20 @@ 00000060: 7420 6265 2072 756e 2069 6e20 444f 5320 t be run in DOS 00000070: 6d6f 6465 2e0d 0d0a 2400 0000 0000 0000 mode....$....... -00000080: 5045 0000 6486 1400 57e8 445e 00da 6900 PE..d...W.D^..i. +00000080: 5045 0000 6486 1400 e3e9 445e 00da 6900 PE..d.....D^..i. 00000090: e015 0100 f000 2600 0b02 021f 00de 4900 ......&.......I. 000000a0: 00b0 5b00 008a 0000 e014 0000 0010 0000 ..[............. 000000b0: 0000 4000 0000 0000 0010 0000 0002 0000 ..@............. 000000c0: 0400 0000 0000 0000 0500 0200 0000 0000 ................ -000000d0: 00f0 6a00 0006 0000 bd31 af00 0300 6001 ..j......1....`. +000000d0: 00f0 6a00 0006 0000 d434 af00 0300 6001 ..j......4....`. 000000e0: 0000 2000 0000 0000 0010 0000 0000 0000 .. ............. @@ -373594,15 +373594,15 @@ 005b35f0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ -005b3600: 0000 0000 57e8 445e 0000 0000 7ce1 5b00 ....W.D^....|.[. +005b3600: 0000 0000 e2e9 445e 0000 0000 7ce1 5b00 ......D^....|.[. 005b3610: 0100 0000 2200 0000 2200 0000 28e0 5b00 ...."..."...(.[. ``` ACKs for top commit: practicalswift: ACK 3d9b41ecc02a69daa19df2bc850d60bf9917b724 -- patch looks correct laanwj: ACK 3d9b41ecc02a69daa19df2bc850d60bf9917b724 Tree-SHA512: 1ff9dab7fa818b1fc6b0eb3b7a1e0468aac9e2578f4451aa300a648f883fa83f83722067f1adf27ebad54157790857d1501d573adbd7ebdf6962858cc669960d
2020-02-29Merge #18195: test: Add cost_of_change parameter assertions to bnb_search_testMarcoFalke
c72a11a1a030036eb1fe4472086a9733731961ce test: Add cost_of_change parameter assertions to bnb_search_test (Yancy Ribbens) Pull request description: If the `cost_of_change` variable is removed from the method body `SelectCoinsBnB`, there are currently no failing unit tests. This PR adds assertions about the behavior of the `cost_of_change`: If the cost of creating a change output is greater than what's leftover, then consume the output and create no change, otherwise, don't consume the output (no match found). ACKs for top commit: achow101: ACK c72a11a1a030036eb1fe4472086a9733731961ce Tree-SHA512: 613aa411df5e2911446e0e8bf3309336faaadf2d3c56e7d125b76454e7c6f9e4f5e8f0910dc6222282628e38cd8a4a7c56bb3d36b564a17f396b9b503ecc64c8
2020-02-29util: Fail to parse empty string in ParseMoneyMarcoFalke
2020-02-29util: Remove unused ParseMoney that takes a c_strMarcoFalke
2020-02-28Make AnalyzePSBT next role calculation simple, correctGregory Sanders
2020-02-28Merge #17921: test: test OP_CSV empty stack fail in feature_csv_activation.pyMarcoFalke
5ffaf883b93fb8d3d841c36e808b90d61d39d492 test: eliminiated magic numbers in feature_csv_activation.py (Sebastian Falbesoner) 09f706ab8e47ddfdfa41418f2e7cb6d87661147a test: check for OP_CSV empty stack fail reject reason in feature_csv_activation.py (Sebastian Falbesoner) cbd345a75c2be26e17fce4c65c0c1ca19a3eb9e0 test: test OP_CSV empty stack fail in feature_csv_activation.py (Sebastian Falbesoner) Pull request description: Adds an empty stack failure check for OP_CSV (BIP112) to the functional test `feature_csv_activation.py` by prepending a valid scriptSig with `OP_CHECKSEQUENCEVERIFY`. If BIP112 is inactive, the operator just behaves as a NOP (for both tx versions 1 and 2) and the transaction remains valid -- if it is active, the tx is invalid due to an empty stack (for both tx versions 1 and 2, as well). Top commit has no ACKs. Tree-SHA512: 81102aaead5be11e02b894867fa9a9cc17358ec0eb2f21ce2d3db845b87691d305e6ed7c525f9c7e5bcb3c5c609eb28deca0fbaa3d5e9ff928cecd3b91ff129a
2020-02-28test: eliminiated magic numbers in feature_csv_activation.pySebastian Falbesoner
2020-02-28test: check for OP_CSV empty stack fail reject reason in ↵Sebastian Falbesoner
feature_csv_activation.py
2020-02-28test: test OP_CSV empty stack fail in feature_csv_activation.pySebastian Falbesoner
With BIP112 activated, the operation OP_CHECKSEQUENCEVERIFY (former OP_NOP3) leads to script interpreter termination with an error if one of the following conditions is true: -> stack is empty -> top item on stack is negative (< 0) -> top item on stack has disable flag unset and at least one of four other conditions is true (contains the core CSV logic) This commits adds the missing empty stack failure test to the functional test by prepending a valid scriptSig with just OP_CHECKSEQUENCEVERIFY. If BIP112 is inactive, the operator just behaves as a NOP (for both tx versions 1 and 2) and the transaction remains valid -- if it is active, the tx is invalid due to an empty stack (for both tx versions 1 and 2, as well).
2020-02-28Merge #18209: test: Reduce unneeded whitelist permissions in testsfanquake
fa45d606461dbf5bf1017d6ab15e89c1bcf821a6 test: Reduce unneeded whitelist permissions in tests (MarcoFalke) Pull request description: It makes the tests confusing and fragile when overwriting default command line values that are not needed to be overwritten. ACKs for top commit: fanquake: ACK fa45d606461dbf5bf1017d6ab15e89c1bcf821a6 laanwj: ACK fa45d606461dbf5bf1017d6ab15e89c1bcf821a6 Tree-SHA512: 8ae5ad8c6be156b1a983adccbca8d868ef841e00605ea88e24227f1b7493987c50b3e62e68dd7dc785ad73d6e14279eb13d7a151cb0a976426fe2fd63ce5cbcd
2020-02-27Templatize ValidationState instead of subclassingJeffrey Czyz
This removes boilerplate code in the subclasses which otherwise only differ by the result type.
2020-02-27Remove ValidationState's constructorJeffrey Czyz
2020-02-27Refactor FormatStateMessage into ValidationStateJeffrey Czyz
2020-02-28Merge #18212: doc: add missing step in win deployment instructionsfanquake
76445677586a4c2fa72606b662269a4390c2e71f Add missing step in win deployment instructions (Dan Gershony) Pull request description: As explained in #17864 there is a missing step that was required to finish the compilation for Bitcoin Core on Windows. ACKs for top commit: sipsorcery: ACK 76445677586a4c2fa72606b662269a4390c2e71f. Tree-SHA512: 0d9ed248f511ea4f440d6c2f3e1235abbb3f9c0c576ca715df3cda91682d668991001197930e687ee48709eedbcf148d8ac9236464e9ce1d2ed15d8b3b4b252d
2020-02-28Merge #17461: test: check custom descendant limit in mempool_packages.pyMarcoFalke
b902bd66b0f35c5016dc5d7aaf501940935edd62 test: check custom descendant limit in mempool_packages.py (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #17435, testing the custom descendant limit, passed by the argument `-limitdescendantcount`. ~~It was more tricky than expected, mainly because we don't know for sure at which point node1 has got all the transactions broadcasted from node0 (for the ancestor test this wasn't a problem since the txs were immediately available through `invalidateblock`) -- a simple `sync_mempools()` doesn't work here since the mempool contents are not equal due to different ancestor/descendant limits. Hence I came up with a "hacky manual sync":~~ 1. ~~wait until the mempool has the _expected_ tx count (see conditions below)~~ 2. ~~after that, wait some time and get sure that the mempool contents haven't changed in-between~~ ~~Like for~~ Similar to the ancestor test, we overall check for ~~three~~ four conditions: - the # of txs in the node1 mempool is equal to the descendant limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) ~~(done by the hacky sync above)~~ - all txs in node1 mempool are a subset of txs in node0 mempool - part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool - the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool ACKs for top commit: JeremyRubin: Excellent. utACK b902bd6 Tree-SHA512: 7de96dd248f16ab740e178ac5b64b57ead18cdcf74adfe989709d215e4a67b6b6d20de22c48e885d5f2edc55caaddd44a4261e996c5c87687ceb6a47f1d1fdaf
2020-02-28Merge #17771: tests: Add fuzzing harness for V1TransportDeserializer (P2P ↵MarcoFalke
transport) 2f63ffd15caeb79867e56c8cedbe2c702952db9e tests: Add fuzzing harness for V1TransportDeserializer (P2P transport) (practicalswift) Pull request description: Add fuzzing harness for `V1TransportDeserializer` (P2P transport). **Testing this PR** Run: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/p2p_transport_deserializer … ``` ACKs for top commit: MarcoFalke: ACK 2f63ffd15caeb79867e56c8cedbe2c702952db9e Tree-SHA512: 8507d4a0414d16f1b8cc9649e3e638f74071dddc990d7e5d7e6faf77697f50bdaf133e49e2371edd29068a069a074469ef53148c6bfc9950510460b81d87646a
2020-02-27Add missing step in win deployment instructions Dan Gershony
As explained in #17864 there is a missing step that was required to finish the compilation for bitcoin core on windows
2020-02-27build: add --enable-determinism configure flagfanquake
If used, this will enable additional compile / link time flags that will make subsequent builds of bitcoind determinisitic.
2020-02-27Merge #18211: test: Disable mockforward scheduler unit test for nowfanquake
fab2527515e8db944ae044bea8580e2cd9414bcd test: Disable mockforward scheduler unit test for now (MarcoFalke) Pull request description: This should be a workaround to fix #18174 in the short run and buy us more time to investigate the issue while ci runs are green again :pray: ACKs for top commit: fanquake: ACK fab2527515e8db944ae044bea8580e2cd9414bcd - be good to get Travis back. laanwj: ACK fab2527515e8db944ae044bea8580e2cd9414bcd Tree-SHA512: 027e86b3dfec203a464e5bf528e9933c208c36633c2d4bfcdbc10da1799637a5d6ea0a63af33a4174fb1ad7115df631a4cb838f56e31f4cbd15498e1e9fdf9cc
2020-02-27test: check custom descendant limit in mempool_packages.pySebastian Falbesoner
To test the custom descendant limit on node1 (passed by the argument -limitdescendantcount), we check for four conditions: -> the # of txs in the node1 mempool is equal to the limit (plus 1 for the parent tx, plus the # txs from the previous ancestor test which are still in) -> all txs in node1 mempool are a subset of txs in node0 mempool -> part of the constructed descendant-chain (the first ones up to the limit) are contained in node1 mempool -> the remaining part of the constructed descendant-chain (all after the first ones up to the limit) is *not* contained in node1 mempool
2020-02-26Merge #18167: Fix a violation of C++ standard rules where unions are used ↵Wladimir J. van der Laan
for type-punning 0653939ac130eddffe40c53ac418bea305d3bf82 Add static_asserts to ser_X_to_Y() methods (Samer Afach) be94096dfb0c4862e2314cbae4120d7360b08ef2 Fix a violation of C++ standard rules that unions cannot be switched. (Samer Afach) Pull request description: Type punning in C++ is not like C. As per the C++ standard, one cannot use unions to convert the bit type. A discussion about this can be found [here](https://stackoverflow.com/questions/25664848/unions-and-type-punning). In C++, a union is supposed to only hold one type at a time. It's intended to be used only as `std::variant`. Switching types is undefined behavior. In fact, C++20 has a special casting function, called [`bit_cast`](https://en.cppreference.com/w/cpp/numeric/bit_cast) that solved this problem. Why has it been working so far? Because some compilers tolerate using unions and switching types, like gcc. More information [here](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning). One important thing to mention is that performance is generally not affected by that memcpy. Compilers are smart enough to convert that to a memory cast when possible. But we have to do it the right way, otherwise, it's jut undefined behavior that depends on the compiler. ACKs for top commit: practicalswift: ACK 0653939ac130eddffe40c53ac418bea305d3bf82 elichai: ACK 0653939ac130eddffe40c53ac418bea305d3bf82 laanwj: Code review ACK 0653939ac130eddffe40c53ac418bea305d3bf82 kristapsk: ACK 0653939ac130eddffe40c53ac418bea305d3bf82 Tree-SHA512: f6e89de39fc964750429139bab6b5a1346f7060334b7afa020e315bdad8f8c195bce2b8a9e343f06e7fff175e2dfb1cdabfcb6fe405bea0febe4962f0cc62557
2020-02-26Merge #17985: net: Remove forcerelay of rejected txsWladimir J. van der Laan
facb71576cd4d2e90fd03e09d29b42fa3d730e8c net: Remove forcerelay of rejected txs (MarcoFalke) Pull request description: This removes the code that supposedly handled the forced relay of txs from a permissioned peer that were rejected from our mempool. The removal should be fine, because it is dead code for the following reasons: * While `RelayTransaction` enqueues the inv for all peers, the inv is never processed because it can not be found in the mempool. See https://github.com/bitcoin/bitcoin/blob/4a072330763b3ff2d1b5c5b8d30a9517873ac6de/src/net_processing.cpp#L3862-L3866 * Even if the peers we intended to send the inv to can somehow reply with a getdata to the never-received inv, they won't receive the tx as a reply because it was never added to the "relay memory" (`mapRelay`) The dead code is (obviously) untested: https://marcofalke.github.io/btc_cov/total.coverage/src/net_processing.cpp.gcov.html#2574 This feature was (intentionally or accidentally) removed in 4d8993b3469915d8c9ba4cd3b918f16782edf0de, which was released in Bitcoin Core 0.13.0. So all currently supported versions of Bitcoin Core ship without this feature. I am not aware of any complaints about this feature or actual documented use-cases. So instead of reviving an unneeded feature, just remove the dead code. ACKs for top commit: hebasto: ACK facb71576cd4d2e90fd03e09d29b42fa3d730e8c, locally running the unit and functional tests. Tree-SHA512: bfceae6f2983c1510fa0649a9a63c343cbbc1c4ab3a3698039cccf454c81e58c8f5114b147ed42a1bc867da74c43a5b53764ab14f942e191b6f59079044108b5
2020-02-27test: Disable mockforward scheduler unit test for nowMarcoFalke
2020-02-26Merge #18206: tests: Add fuzzing harness for bloom filter classes ↵MarcoFalke
(CBloomFilter + CRollingBloomFilter) eabbbe409f397e97b1e6fad7385d9d1813ae2880 tests: Add fuzzing harness for rolling bloom filter class CRollingBloomFilter (practicalswift) 2a6a6ea0f5b97cba95b5678268d638c81764b9b1 tests: Add fuzzing harness for bloom filter class CBloomFilter (practicalswift) Pull request description: Add fuzzing harness for bloom filter classes (`CBloomFilter` + `CRollingBloomFilter`). Test this PR using: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/bloom_filter … $ src/test/fuzz/rolling_bloom_filter … ``` ACKs for top commit: MarcoFalke: ACK eabbbe409f397e97b1e6fad7385d9d1813ae2880 🤞 Tree-SHA512: 765d30bc52e3eb04dbd4d2b8f517387aa61312416e8fea3767250ef5c074e08641699019ee4600d42303de32f98379c20bfc0c0e60cb5154d0338088c1d29cb6
2020-02-25tests: Add fuzzing harness for rolling bloom filter class CRollingBloomFilterpracticalswift
2020-02-25tests: Add fuzzing harness for bloom filter class CBloomFilterpracticalswift
2020-02-26test: Reduce unneeded whitelist permissions in testsMarcoFalke
2020-02-25rpc: Auto-format RPCResultMarcoFalke
2020-02-25rpc: Move OuterType enum to headerMarcoFalke
This is needed so that it can be used by RPCResult Also, * rename NAMED_ARG to NONE for generalization. * change RPCArg constructors to initialize the members by moving values
2020-02-25Merge #17264: rpc: set default bip32derivs to true for psbt methodsSamuel Dobson
5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost) 29a21c90610aed88b796a7a5900e42e9048b990e [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost) Pull request description: In https://github.com/bitcoin/bitcoin/pull/13557#pullrequestreview-135905054 I recommended not including bip32 deriviation by default in PSBTs: > _Bit of a privacy issue_: let's say person A and B are about to spend from a multisig address, sending everything to person A. Person A gives their address to person B, their wallet wallet creates a PSBT, but doesn't sign it. Wallet A then calls `walletprocesspsbt` which signs it and _spontaneously adds the master_fingerprint and bip32 path_. Same issue with `walletcreatefundedpsbt`. > > Adding `bip32_derivs` should probably be opt-in. In practice I find this default quite annoying because I forget it and end up with a confused hardware wallet. More importantly, in the multisig example I provided, it's actually essential for the other side to know the derivation details (in addition to an xpub). This allows them to check that change is going to an address you can still co-sign for (because the spending policy is unchanged except for an index). ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17264/commits/5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5 jonatack: ACK 5bad7921d0 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests. meshcollider: utACK 5bad7921d0b33b62c0a59a478c2e8c869fc5e3b5 Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd
2020-02-25Merge #17577: refactor: deduplicate the message sign/verify codeSamuel Dobson
e193a84fb28068e38d5f54fbfd6208428c5bb655 Refactor message hashing into a utility function (Jeffrey Czyz) f8f0d9893d7969bdaa870fadb94ec5d0dfa8334d Deduplicate the message signing code (Vasil Dimov) 2ce3447eb1e25ec7aec4b300dabf6c1e394f1906 Deduplicate the message verifying code (Vasil Dimov) Pull request description: The message signing and verifying logic was replicated in a few places in the code. Consolidate in a newly introduced `MessageSign()` and `MessageVerify()` and add unit tests for them. ACKs for top commit: Sjors: re-ACK e193a84fb28068e38d5f54fbfd6208428c5bb655 achow101: ACK e193a84fb28068e38d5f54fbfd6208428c5bb655 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17577/commits/e193a84fb28068e38d5f54fbfd6208428c5bb655 meshcollider: utACK e193a84fb28068e38d5f54fbfd6208428c5bb655 Tree-SHA512: b0e02a7d4623a98c8f8c77627af1725e6df07700de4630c2f75da6beacdf55414c38ba147bc6d2a757491ab07c827dddf93e8632fe600478760e255714ddab88
2020-02-25Merge #18162: util: Avoid potential uninitialized read in ↵fanquake
FormatISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return value 12a2f377185a413b740460db36812de22ee2e041 util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t nTime) by checking gmtime_s/gmtime_r return value (practicalswift) Pull request description: Avoid potential uninitialized read in `FormatISO8601DateTime(int64_t)` by checking `gmtime_s`/`gmtime_r` return value. Before this patch `FormatISO8601DateTime(67768036191676800)` resulted in: ``` ==5930== Conditional jump or move depends on uninitialised value(s) ==5930== at 0x4F44C0A: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25) ==5930== by 0x4F511A4: std::ostream& std::ostream::_M_insert<long>(long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25) ==5930== by 0x4037C3: void tinyformat::formatValue<int>(std::ostream&, char const*, char const*, int, int const&) (tinyformat.h:358) ==5930== by 0x403725: void tinyformat::detail::FormatArg::formatImpl<int>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:543) ==5930== by 0x402E02: tinyformat::detail::FormatArg::format(std::ostream&, char const*, char const*, int) const (tinyformat.h:528) ==5930== by 0x401B16: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:907) ==5930== by 0x4017AE: tinyformat::vformat(std::ostream&, char const*, tinyformat::FormatList const&) (tinyformat.h:1054) ==5930== by 0x401765: void tinyformat::format<int, int, int, int, int, int>(std::ostream&, char const*, int const&, int const&, int const&, int const&, int const&, int const&) (tinyformat.h:1064) ==5930== by 0x401656: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<int, int, int, int, int, int>(char const*, int const&, int const&, int const&, int const&, int const&, int const&) (tinyformat.h:1073) ==5930== by 0x4014CC: FormatISO8601DateTime[abi:cxx11](long) (…) ``` The same goes for other very large positive and negative arguments. Fix by simply checking the `gmtime_s`/`gmtime_r` return value :) ACKs for top commit: MarcoFalke: ACK 12a2f377185a413b740460db36812de22ee2e041 theStack: re-ACK https://github.com/bitcoin/bitcoin/commit/12a2f377185a413b740460db36812de22ee2e041 elichai: re ACK 12a2f377185a413b740460db36812de22ee2e041 Tree-SHA512: 066142670d9bf0944d41fa3f3c702b1a460b5471b93e76a619b1e818ff9bb9c09fe14c4c37e9536a04c99533f7f21d1b08ac141e1b829ff87ee54c80d0e61d48
2020-02-24Merge #18193: scripted-diff: Wallet: Rename incorrectly named *UsedDestinationMarcoFalke
bca8665d0895c450e552c357a036d9e9579e3678 scripted-diff: Wallet: Rename incorrectly named *UsedDestination (Luke Dashjr) Pull request description: These functions are used to mark/check if a key of our own has been used to spend (and only for avoid-reuse wallets), which has nothing to do with the destination/address itself. Give them more accurate names to avoid confusion. -BEGIN VERIFY SCRIPT- sed -i -e 's/UsedDestination/SpentKey/g' $(git grep -l 'UsedDestination' ./src) -END VERIFY SCRIPT- ACKs for top commit: practicalswift: ACK bca8665d0895c450e552c357a036d9e9579e3678 -- patch looks correct and rationale makes sense instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/18193/commits/bca8665d0895c450e552c357a036d9e9579e3678, much more meaningful name, thanks kallewoof: ACK bca8665d0895c450e552c357a036d9e9579e3678 Tree-SHA512: ff13d9061ffa748e92eb41ba962c3ec262a43e4b6abd62408b38c6f650395d6ae5851554257d1900fb02767a88d08380d592a27210192ee9abb72d0945976686
2020-02-22test: Add cost_of_change parameter assertions to bnb_search_testYancy Ribbens
2020-02-22Merge #18181: test: Remove incorrect assumptions in validation_flush_testsMarcoFalke
faca8eff39876cc8c0ee609a89bdcebc21976d47 test: Remove incorrect assumptions in validation_flush_tests (MarcoFalke) fa31eebfe9107e14dc1d6b588f5b4c878d1010de test: Tabs to spaces in all tests (MarcoFalke) Pull request description: The tests assume standard library internals that may not hold on all supported archs or when the code is instrumented for sanitizer or debug use cases Fixes #18111 ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/18181/commits/faca8eff39876cc8c0ee609a89bdcebc21976d47 pending passing tests fjahr: ACK faca8eff39876cc8c0ee609a89bdcebc21976d47 Tree-SHA512: 60a5ae824bdffb0762f82f67957b31b185385900be5e676fcb12c23d53f5eea734601680c2e3f0bdb8052ce90e7ca1911b1342affb67e43d91a506b111406f41
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-19 21:10:56 +0000