This was originally added in #9366 to fix the gui build, as
Protobuf would also define these macros. Now that we're no-longer
using Protobuf, remove the additional check.
fa710066b9 ci: Disable functional tests on mac host (MarcoFalke)
Pull request description:
Judging from the lack of responses to https://github.com/bitcoin/bitcoin/issues/15400#issuecomment-543745053, no one can reproduce the failures locally. Thus, disable the tests on the ci mac host. Otherwise they cause ci failures to be ignored or overwritten by a blind re-run.
ACKs for top commit:
laanwj:
ok, ACK fa710066b9 in that case
ryanofsky:
Code review ACK fa710066b9. I've been blindly restarting these but can confirm it seemed like random test failures.
Tree-SHA512: 805f552124409f6e79b16b419826d334307202b8e3636edc8a91dadffe9949554d924c00cb16bb09473ec1aaaf2d8eaf7a97c7c2b8c11bf6372135d39bba802b
CheckInputsFromMempoolAndCache() is only called in one place, and
cacheSigStore is set to true in that call site. Remove the argument
entirely.
Also improve commenting.
6f6852335f doc: Changed miniupnp links to https (Marius Kjærstad)
Pull request description:
doc: Changed miniupnp links to https
ACKs for top commit:
fjahr:
ACK 6f68523
fanquake:
ACK 6f6852335f - as mentioned in this [comment](https://github.com/bitcoin/bitcoin/pull/17166#issuecomment-545906845), in future, there's no need to open a new PR instead than squashing. One of the requirements of contributing to Bitcoin Core is some basic `git` usage, and we have some documentation on that in our [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) (which also mentions not to open multiple PRs).
Tree-SHA512: 1e806326552b94dc1c2f105bed822a1dd56127d42bbb0c15a16623ff084b2047331f2f1ad5303c99b0d68c70ad2caf8e8028eb354ba4379973b9e1198676ce72
c2f964a674 tests: Remove Cygwin WinMain workaround (practicalswift)
db4bd32cc3 tests: Skip unnecessary fuzzer initialisation. Hold ECCVerifyHandle only when needed. (practicalswift)
Pull request description:
Skip unnecessary fuzzer initialisation. Hold `ECCVerifyHandle` only when needed.
As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/17018#discussion_r336645391.
Top commit has no ACKs.
Tree-SHA512: 598da44859d736e3fdc143b93e07f444d8ad19dfdab0cfe7c6ccff8644e862664d869337dfe6b49416ed09a0024e4a5f2220ca6246de568f9e9227d721baa28e
9075d13153 [docs] Add release notes for removal of REJECT reasons (John Newbery)
04a2f326ec [validation] Fix REJECT message comments (John Newbery)
e9d5a59e34 [validation] Remove REJECT code from CValidationState (John Newbery)
0053e16714 [logging] Don't log REJECT code when transaction is rejected (John Newbery)
a1a07cfe99 [validation] Fix peer punishment for bad blocks (John Newbery)
Pull request description:
We no longer send BIP 61 REJECT messages, so there's no need to set
a REJECT code in the CValidationState object.
Note that there is a minor bug fix in p2p behaviour here. Because the
call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only
previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`,
then there are cases were `MaybePunishNode()` can get called where it
wasn't previously:
- when `AcceptBlockHeader()` fails with `CACHED_INVALID`.
- when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`.
Note that `BlockChecked()` cannot fail with an 'internal' reject code. The
only internal reject code was `REJECT_HIGHFEE`, which was only set in
ATMP.
This reverts a minor bug introduced in 5d08c9c579.
ACKs for top commit:
ariard:
ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits
fjahr:
ACK 9075d13153, confirmed diff to last review was fixing nits in docs/comments.
ryanofsky:
Code review ACK 9075d13153. Only changes since last review are splitting the main commit and updating comments
Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
9e95931865 [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9a3b [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a02b [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b22d [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)
Pull request description:
`CommitTransaction()` returns a bool to indicate success, but since commit
b3a7410 (#9302) it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function.
ACKs for top commit:
laanwj:
ACK 9e95931865
Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
4896bacc00 Add testcase to simulate bitcoin schema in leveldb (MapleLaker)
Pull request description:
Resurrecting #14125 with updates based on comments of closed PR
ACKs for top commit:
laanwj:
ACK 4896bacc00
dongcarl:
ACK 4896bacc00
Tree-SHA512: 3290ea7e1e998901d5ee8921d1d76cec399cae30ac1911a45b86826afed47cee1acf92bd6438f1fa11ed785a3b17abdcb1c169bc0419945eda9fe4c089d0b6eb
The ancient "darwin-4.9.1" profile has long been used to match against
clang, which prior to version 9, reported 4.9.1 as its version when
invoking "clang++ -dumpversion". Presumably this was a historical
compatibility quirk related to Apple's switch from gcc to clang.
This was "fixed" in clang 9.0, so that -dumpversion reports the real
version. Unfortunately that had the side-effect of breaking the
(brittle) boost compiler detection.
Move to the seemingly more-correct "clang-darwin" profile, which passes
the checks and builds correctly.
Also switch to using ar rather than libtool for archiving, as it's what
the clang-darwin profile expects to be using.
Note that because this is using a different profile, some of the final
command-line arguments end up changing. The changes look sane at a
glance.
a0fc076476 refactor: test/bench: dedup Build{Crediting,Spending}Transaction() (Sebastian Falbesoner)
Pull request description:
prototypes used in `src/test/script_tests.cpp`:
- `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);`
- `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);`
prototypes used in `bench/verify_script.cpp`:
- `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);`
- `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);`
The more generic versions from the script tests are moved into `setup_common.cpp` and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for `BuildCreditingTransaction()`, passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in `BuildSpendingTransaction()`).
Top commit has no ACKs.
Tree-SHA512: 8444f8a18f15070eeec1e5dfd255b55a851dfc2e6647c12b1995a6f7abd7196e830db2181d0e860bcd4cf4c815967584a3756dd450346bca70649dd1d4493e04
cd68594dcd Only check the hash of transactions loaded from disk (Andrew Chow)
Pull request description:
It feels unnecessary to do a full `CheckTransaction` for every transaction saved in the wallet. It should not be possible for an invalid transaction to get into the wallet in the first place, and if there is any disk corruption, the hash check will catch it.
ACKs for top commit:
MarcoFalke:
ACK cd68594dcd
laanwj:
ACK cd68594dcd
promag:
ACK cd68594dcd, AFAICT the check is not needed, hash comparison gives data integrity.
Tree-SHA512: 5b2e719f76097cfbf125392db6cc6c764355c81f0b7a5b60aee4b06af1afcca80cfd38a3cf5307fd9e2c1afc405f8321929a4552943099a8161e6762965451fb
088a730fe6 static tooltip (JeremyCrookshank)
Pull request description:
I noticed that on Bitcoin sends the tooltip wasn't very clear for new users and I hope my PR is more concise. If it needs changing more will happily change too 👍
ACKs for top commit:
laanwj:
ACK 088a730fe6
Tree-SHA512: 2b1103ac934d8f68d22333af3c0f5d4228b665b1e507378d4ae5b83cc2b6d6aeb46a3d68298cca93feb839db5caa560322c8df5261dc2f7db5abeed9f0dd9c69
89339d1460 tests: Add test for loadblock option (Fabian Jahr)
Pull request description:
Fixes#17019
Was initially part of #17044 but as the test got larger it made sense to split it into its own commit as suggested in #17019 .
This is testing the `-loadblock` option by using the scripts in `contrib/linearize` to generate a `bootstrap.dat` file and starting a disconnected node with it. So it is also testing the linearize scripts which were untested before and needed to be made available for the CI environment, hence they are added to `DIST_CONTRIB` in `Makefile.am`.
ACKs for top commit:
laanwj:
ACK 89339d1460
Tree-SHA512: aede0cd6e8b21194973f3633bc07fa2672d66a6f85dfe6a57cee2bb269a65d19ea49d5f9ed7914a173b3847c76e70257aa865f44bde170c1999d9655b4862d1c
prototypes used in src/test/script_tests.cpp:
- CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);
- CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);
prototypes used in bench/verify_script.cpp:
- CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);
- CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);
The more generic versions from the script tests are moved into a new file pair
transaction_utils.cpp/h and the calls are adapted accordingly in the
verify_script benchmark (passing the nValue of 1 explicitely for
BuildCreditingTransaction(), passing empty scriptWitness explicitely and
converting txCredit parameter to CTransaction in BuildSpendingTransaction()).
Calling LogPrint with a category that is not enabled results in
evaluating the remaining function arguments, which may be arbitrarily
complex (and possibly expensive) expressions. Defining LogPrint as a
macro prevents this unnecessary expression evaluation.
This is a partial revert of #14209. The decision to revert is discussed
in #16688, which adds verbose logging for validation event notification.
0616138a07 tests: Remove no longer needed UBSan suppressions (issues fixed). Add documentation. (practicalswift)
Pull request description:
Remove no longer needed UBSan suppressions (issues fixed). Add documentation.
This PR is the CI-only subset of #17208 (which touches code).
From a fuzzing perspective it would be really nice to be able to run UBSan with as few suppressions as possible :)
Top commit has no ACKs.
Tree-SHA512: a926ab3e80e12a805af110fbff470cdc61ef4db536919a5b8896ea8b70f761114a52d9b1c0f48b11c1d48338351bf2e003e01ce60c613612f26ba298dcc29cd9
The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads
to smaller transaction sizes, needing adaption of some constants in the
following test cases:
- test_dust_to_fee(): adaption of dust calculation
(p2wpkh spend estimate of 67 is taken from src/policy/policy.cpp:GetDustThreshold())
- test_maxtxfee_fails(): lowering -maxtxfee setting to trigger fail
The serialization/deserialization methods for the classes CExtKey and
CExtPubKey were only used in the BIP32 unit tests, where the relevant parts are
removed as well.
facec1c643 wallet: Avoid showing GUI popups on RPC errors (MarcoFalke)
Pull request description:
RPC errors and warnings are shown as popups in the GUI instead of being returned to the RPC caller. For example,
```
$ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
error code: -4
error message:
Wallet loading failed.
```
gives me a GUI popup and no reason why loading the wallet failed.
After this pull request:
```
$ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
error code: -4
error message:
Wallet loading failed: Error loading /home/marco/workspace/btc_bitcoin_core/./test/functional/data/wallets/high_minversion/wallet.dat: Wallet requires newer version of Bitcoin Core
ACKs for top commit:
laanwj:
Code review ACK facec1c643
Tree-SHA512: c8274bbb02cfcf71676eeec1e773e51fb3538cf93f82e7cb8536f4716d44ed819cdc162dfc039ac7386a4db381a734cdb27fd32567043a1180c02519fbcba194
57e2edea0b Send amount shows minimum amount placeholder (JeremyCrookshank)
Pull request description:
Noticed that there wasn't a default value for the send amount. However if you put a value in or click the up and down arrows you're unable to get it blank again, so it makes sense that it has a default value. I hope this also makes it more clear that users can send less than 1 BTC if it shows the 8 decimal places
PR:
ACKs for top commit:
promag:
ACK 57e2edea0b.
GChuf:
ACK 57e2edea0b
laanwj:
ACK 57e2edea0b, this is a surprisingly compact solution too
Tree-SHA512: 354590d2a88231b8649f7ae985c8a7864d74ca0e1f8603cb1730ba46747084de90ee6285ce4d39ee04b054fb9cd2d78ebc71146f3af694c37a8a3aff7f051800
