aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-03-04config: use default value in -asmap configJon Atack
and move to sorted position
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-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-02wallet: use constant CWallets in rpcwallet.cppKarl-Johan Alm
* GetAvoidReuseFlag: simply gets the flag, without modifying the wallet * ListReceived: helper function to produce lists * ListTransactions: produces a list of transactions, without modifications; two cases of map [] -> .at() for verified-existing keys * DescribeWalletAddress: generates a description of a given wallet address without changing the wallet * The following functions produce a list without making any modifications to the wallet: * listaddressgroupings * listreceivedbyaddress * listreceivedbylabel * listtransactions * listsinceblock * listlockunspent * listunspent * listlabels * getreceivedbyaddress * getreceivedbylabel * getaddressesbylabel * signmessage: uses the wallet to procure a private key for signing, but does no modifications * getbalance, getunconfirmedbalance: calculates the wallet balance, without any modifications * gettransaction: procures transaction without any modifications * backupwallet: makes a backup of the wallet to disk, without changing said wallet * getwalletinfo: produces info about wallet without any modifications * signrawtransactionwithwallet: modifies incoming transaction on the fly by signing with private key procured from within wallet; no modifications to wallet * getaddressinfo: gets information about the given address, with no modifications done to the wallet; one case of [] -> .at() and one ::iterator -> ::const_iterator * walletprocesspsbt: processes the given PSBT on the fly, without modifying the wallet
2020-03-02wallet: make ReserveDestination pwallet ivar constKarl-Johan Alm
2020-03-02wallet: make BackupWallet() constKarl-Johan Alm
2020-03-02wallet: make getters constKarl-Johan Alm
2020-03-02wallet/spkm: make GetOldestKeyPoolTime() constKarl-Johan Alm
The method checks the oldest key time for key pools and returns the oldest. It does no modifications.
2020-03-02wallet: use constant CWallets in rpcdump.cppKarl-Johan Alm
* GetWalletAddressesForKey is, as the name implies, immutable; the one change besides the parameter constness is a [] -> .at() change, to a verified-existing key. * dumpprivkey and dumpwallet are both similarly immutable, for obvious reasons.
2020-03-02wallet/db: make IsDummy() constKarl-Johan Alm
This method does a simple check and no modifications.
2020-03-02wallet/db: make Backup() constKarl-Johan Alm
This method is the to-disk equivalent of serialize methods which are also const.
2020-03-02wallet: make CanGetAddresses() constKarl-Johan Alm
CWallet::CanGetAddresses() is used to check whether the wallet has available or is able to produce keys for addresses. It uses the ScriptPubKeyMan::CanGetAddresses(), which in turn uses the const KeypoolCountExternalKeys() method, all which do counting and no modifications.
2020-03-02wallet: make KeypoolCountExternalKeys() constKarl-Johan Alm
This method returns the sum of the key pool sizes. It does no modification.
2020-03-02wallet: make CanGenerateKeys() constKarl-Johan Alm
This method simply checks if HD is or can be enabled and does not require mutability.
2020-03-02make BlockUntilSyncedToCurrentChain() constKarl-Johan Alm
The method checks the chain tip for the best block, and calls SyncWithValidationInterfaceQueue() (a standalone function) if necessary.
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-03-01Change example addresses to bech32Yusuf Sahin HAMZA
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-28test: add unit test for non-standard "scriptsig-not-pushonly" txsSebastian Falbesoner
The function IsStandardTx() returns rejection reason "scriptsig-not-pushonly" if the transaction has at least one input for which the scriptSig consists of any other ops than just PUSHs.
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 #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-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 #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-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-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-25Convert blockencodings.h to new serialization frameworkPieter Wuille
2020-02-25Add CustomUintFormatterPieter Wuille
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-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