6b8d86ddb8 Require a public key to be retrieved when signing a P2PKH input (Andrew Chow)
Pull request description:
If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it.
This fixes a problem where a PSBT with a P2PKH output would include invalid BIP 32 derivation paths that are missing the public key.
Tree-SHA512: 850d5e74c06833da937d5bf0348bd134180be7167b6f9b9cecbf09f75e3543fbad60d0abbc0b9afdfa51ce165aa36168849f24a7c5abf1e75f37ce8f9a13d127
e13fea975d Add regression test for PSBT signing bug #14473 (Glenn Willen)
565500508a Refactor PSBTInput signing to enforce invariant (Glenn Willen)
0f5bda2bd9 Simplify arguments to SignPSBTInput (Glenn Willen)
53e6fffb8f Add bool PSBTInputSigned (Glenn Willen)
65166d4cf8 New PartiallySignedTransaction constructor from CTransction (Glenn Willen)
4f3f5cb4b1 Remove redundant txConst parameter to FillPSBT (Glenn Willen)
fe5d22bc67 More concise conversion of CDataStream to string (Glenn Willen)
Pull request description:
As discussed in the comments on #14473, I think that bug was caused primarily by failure to adhere to the invariant that a PSBTInput always has exactly one of the two utxo fields present -- an invariant that is already enforced by PSBTInput::IsSane, but which we were temporarily suspending during signing.
This refactor repairs the invariant, also fixing the bug. It also simplifies some other code, and removes redundant parameters from some related functions.
fixes#14473
Tree-SHA512: cbad3428175e30f9b7bac3f600668dd1a8f9acde16b915d27a940a2fa6d5149d4fbe236d5808fd590fb20a032274c99e8cac34bef17f79a53fdf69a5948c0fd0
Just a preparatory commit to add the header to the includes and run
clang-format to sort the include lists.
Splitting this up into a separate commit makes future scripted-diffs
easier.
081accb875 Pass chain locked variables where needed (Russell Yanofsky)
79d579f4e1 Remove uses of cs_main in wallet code (Russell Yanofsky)
ea961c3d72 Remove direct node->wallet calls in init.cpp (Russell Yanofsky)
8db11dd0b1 Pass chain and client variables where needed (Russell Yanofsky)
7e2e62cf7c Add skeleton chain and client classes (Russell Yanofsky)
Pull request description:
This creates an incomplete [`Chain`](https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h) interface in [`src/interfaces/`](https://github.com/ryanofsky/bitcoin/tree/pr/wipc-sep/src/interfaces) and begins to update wallet code to use it.
#10973 builds on this, changing the wallet to use the new interface to access chain state, instead of using CBlockIndex pointers and global variables like `chainActive`.
Tree-SHA512: 6ef05a4d8ebf57f2ad71835e4d970c9c59e34057e39e48cee76b887492c2fee907e3f6a74a9861e5a9f97cdc6823f4865ebc41ec556ab371ebca1b664c20dbea
fa4da3c058 [doc] conf: Remove deprecated options from docs, Other cleanup (MarcoFalke)
Pull request description:
Some dumb fixes, like removing the mention that free transactions are still a thing or that rpcuser/pass should be used (as opposed to rpcauth or rpc cookie).
Combined with other fixes because I don't want to create 3 pull requests:
* conf: Remove deprecated options from docs
* Remove only mention of MIT/X11
* Link to developer notes in README.md
Tree-SHA512: 9e45dc6c63037e7618cf3c871d7d9e65b66f1a952f91a6e623d97d90171e29bc40299a06029c4dc21a0f579e68021e3663186bd3a65e3ab333aff711f7dcb2bf
7a90b1b9d8 build: Fix windows build error if `--disable-bip70` (Chun Kuan Lee)
Pull request description:
Fix#14677
The SSL library seems to be used even if bip70 disabled on Windows.
Tree-SHA512: 1c5fcf98048ce9e2eedf958326c11949eef74b3379a50d73751cb871d3d4323186caf607888c461a1fe1edc5f8515bd151ab247a843e7dda79f810c06309bd88
b191c7dfb7 doc: add comment explaining recentRejects-DoS behavior (James O'Beirne)
Pull request description:
When we receive invalid txs for the first time, we mark the sender as
misbehaving. If we receive the same tx before a new block is seen, we *don't*
punish the second sender (in the same way we do the original sender). It wasn't
initially clear to me that this is intentional, so add a clarifying comment.
Tree-SHA512: d12c674db137ed3ad83e0b941bffe6ddcd2982238048742afa574a4235881f0e58cfc0a4a576a0503e74c5c5240c270b9520fa30221e8b43a371fb3e0b37066b
c82190cdb6 tests: Add Python dead code linter (vulture) (practicalswift)
590a57fdec tests: Remove unused testing code (practicalswift)
Pull request description:
Add Python dead code linter (`vulture`) to Travis.
Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`):
* Less is more :-)
* Unused code is by definition "untested"
* Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-)
* Unused code is hard to spot for humans and is thus often missed during manual review
* [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it)
Based on #14312 to make linter job pass.
Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
535203075e Avoid using numeric_limits for sequence numbers and lock times (Russell Yanofsky)
bafb921507 Remove duplicated code (Hennadii Stepanov)
e4dc39b3bc Replace platform dependent type with proper const (Hennadii Stepanov)
Pull request description:
Switches to named constants, because numeric_limits calls can be harder to read and less portable.
Change was suggested by jamesob in https://github.com/bitcoin/bitcoin/pull/10973#discussion_r213473620
There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and `MutateTxAddInput` functions would now work correctly.
Tree-SHA512: 3f5c6393c260551f65a0edfba55ef7eb3625232eec8d85b1457f26e144aa0b90c7ef5f44b2fd2f7d9be3c3bcb301030a9f5473c21b3bac566cc59b8c8780737c
99d33a6bec appveyor: Script improvement part II (Chun Kuan Lee)
Pull request description:
- decrease clone depth to 5
- Upgrade to python 3.7 that we can use `PYTHONUTF8` from PEP540.
- Set clcache version to `v4.2.0`
- Do not fetch the latest vcpkg package (The issue does not exist anymore)
- Set test_bitcoin report sink and log sink to stdout and redirect stderr to NUL to drop confusing error messages that introduced by #14146
- discard vcpkg, bench_bitcoin output
- Set functional test `--failfast` flag
- Make the log be as clear as possible. (Only ~100 lines)
Tree-SHA512: e7e1f5c2698e8a5d15394edfb4b574508081e99ef4a353995f55657cb51e642567a128d6432a899ecae6f742494c143ac16e2e64df6c26e1e575421ee4a1df50
4773fa8207 Add llvm-symbolizer directory to PATH. Needed to get symbolized stack traces from the sanitizers. (practicalswift)
5c292dafcd Add UBSan suppressions needed to pass test suite (practicalswift)
fced6b5086 Add UBSan options: print_stacktrace + halt_on_error (practicalswift)
Pull request description:
Fail the UBSan Travis build in case of newly introduced [UBSan (UndefinedBehaviorSanitizer)](https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) errors.
Prior to this commit new UBSan errors were printed but didn't fail the UBSan Travis build.
Changes:
* Travis: Add UBSan options: `print_stacktrace` + `halt_on_error`
* Travis: Add UBSan suppressions needed to pass test suite
* Travis: Add `llvm-symbolizer` directory to PATH. Needed to get symbolized stack traces from the sanitizers.
`halt_on_error` should have been part of #14252 really :-)
Tree-SHA512: 30e960659196873d4f636f3a61267b8b4441a0e8773e3f3ae4660a9341d028c363636f0cb919ef9d6662ceb484e3d58054adfb6dc76ff8a355a1c9f927c328d1
3d305e3b89 Send fewer spam messages in p2p_invalid_messages (James O'Beirne)
Pull request description:
Builds on travis are failing because the test node isn't
able to drop all the bad messages sent within the given
timeout. Reduce the number of bad messages we're sending
and increase the timeout to avoid failures on travis.
Tree-SHA512: 11c389619d9590caf7eca74e0efe6d950469415d34220072770689024b350cc08a2d5ec90634237d87ff71ba8b638c1152b8a45ffbb2815a48bde6a88fbb8fc6
Builds on travis are failing because the test node isn't
able to drop all the bad messages sent within the given
timeout. Reduce the number of bad messages we're sending
and increase the timeout to avoid failures on travis.
This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
5a05aa2db2 Add metavar to match var name in help text + Change wording for better readability (Martin Erlandsson)
Pull request description:
The help text given by `test/functional/test_runner.py -h` refers to the value `n`, which is defined as `COMBINEDLOGSLEN` in the list of commands.
To make the help text consistent, this PR changes the display name `COMBINEDLOGSLEN` to `n` by setting the argparse [`metavar`](https://docs.python.org/3/library/argparse.html#metavar) attribute. (`metavar` only changes the _displayed_ name)
Alternatively: Do the opposite and change the help text to use `COMBINEDLOGSLEN`.
---
Before PR:
```
➜ bitcoin > test/functional/test_runner.py -h | grep -A 1 combinedlogslen
--combinedlogslen COMBINEDLOGSLEN, -c COMBINEDLOGSLEN
print a combined log (of length n lines) from all test nodes and test framework to the console on failure.
```
After PR:
```
➜ bitcoin > test/functional/test_runner.py -h | grep -A 1 combinedlogslen
--combinedlogslen n, -c n
print a combined log (of length n lines) from all test nodes and test frameworks to the console on failure.
```
---
Also, fixed pluralization typo.
Tree-SHA512: a1124a4976d29fae1e8ecd7fa2ac523b7f05d541c611166532f44692995691a96faf797fa71582d78634f328b500cbee49c6ef296c8f1a898a57c050cc4e721d
Route calls during node initialization and shutdown that would happen between a
node process and wallet processes through the serializable `Chain::Client`
interface, rather than `WalletInitInterface` which is now simpler and only
deals with early initialization and parameter interaction.
This commit mostly does not change behavior. The only change is that the
"Wallet disabled!" and "No wallet support compiled in!" messages are now logged
earlier during startup.
This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
fef5adcc33 blockfilter: Use unordered_set instead of set in blockfilter. (Jim Posen)
4fb789e9b2 Extract CSipHasher to it's own file in crypto/ directory. (Jim Posen)
Pull request description:
Use `std::unordered_set` (hash set) instead of `std::set` (tree set) in blockfilter interface, as suggested by @ryanofsky in #12254. This may result in a very minor speedup, but I haven't measured.
This moves `CSipHasher` to it's own file `crypto/siphash.h`, so that it can be used in the libbitcoin_util library without including `hash.{h,cpp}`. I'm open to other suggestions on solving this issue if people would prefer to leave CSipHasher where it is.
Tree-SHA512: 593d1abda771e45f2860d5334272980d20df0b81925a402bb9ee875e17595c2517c0d8ac9c579218b84bbf66e15b49418241c1fe9f9265719bcd2377b0cd0d88
bbbbb3f885 qa: Add test to ensure node can generate all help texts at runtime (MarcoFalke)
Pull request description:
This might increase coverage, but more importantly this checks that the node doesn't crash when generating the help. (Right now the help is a static string, but in the future it might be generated at runtime)
Tree-SHA512: 0226e7c65f8a1a6fdc96c07dcf491d90559bc2355c92e9da9b1f174b09733fc349269e71da6d792f954de563a1e57c848471813eabae1a40b849a0d989520a0d
d20a9fa13d tests: add tests for invalid P2P messages (James O'Beirne)
62f94d39f8 tests: add P2PConnection.send_raw_message (James O'Beirne)
5aa31f6ef2 tests: add utility to assert node memory usage hasn't increased (James O'Beirne)
Pull request description:
- Adds `p2p_invalid_messages.py`: tests based on behavior for dealing with invalid and malformed P2P messages. Includes a test verifying that we can't DoS a node by spamming it with large invalid messages.
- Adds `TestNode.assert_memory_usage_stable`: a context manager that allows us to ensure memory usage doesn't significantly increase on a node during some test.
- Adds `P2PConnection.send_raw_message`: which allows us to construct and send messages with tweaked headers.
Tree-SHA512: 720a4894c1e6d8f1551b2ae710e5b06c9e4f281524623957cb01599be9afea82671dc26d6152281de0acb87720f0c53b61e2b27d40434d30e525dd9e31fa671f
This is a refactoring change that doesn't affect behavior. The motivation
behind the change is give BerkeleyEnvironment objects access to
BerkeleyDatabase objects so it will be possible to simplify the duplicate
wallet check and more reliably avoid opening the same databases twice.
