The function ExtractPubKey() checks if a given script matches the P2PK pattern
(<PubKey> OP_CHECKSIG), extracts the PubKey and additionally checks if it is
cryptographically valid (full validation with ECC library via .IsFullyValid()).
Currently this is done manually in the following order:
1) check if first script OP is data push with valid PubKey length
(first part of pattern match), extract PubKey
2) create CPubKey object with extracted PubKey
3) fully validate public key
4) check if last script OP is OP_CHECKSIG
(second part of pattern match)
Using Solver, the pattern matching and PubKey extraction can be done via a
single step, leading to the following simplified order with shorter code:
1) check if given script matches P2PK pattern with Solver
(also contains valid PubKey length check), extracts Pubkey
2) create CPubKey object with extracted Pubkey
3) fully validate public key
d53d591230 Added the bench_bitcoin project to the list automatically produced by the msvc-autogen python script. (Aaron Clauson)
Pull request description:
As discussed in #16747.
The msbuild bench_bitcoin project is currently missing a number of classes. Rather than add them manually this PR adds bench_bitcoin to the list of project files that are auto generated from the main make files.
ACKs for top commit:
practicalswift:
ACK d53d591230
fanquake:
ACK d53d591230 - Thanks for following up quickly.
Tree-SHA512: dd767f3d8b1641a736a868b0ab5173369ab68dc42b5ac60f26e1d435a1813caad494aee175fc54b457a82e2faf36b9676ea35414c7d93c240d2e069434714f03
41d484d5c8 doc: Delete stale URL in test README (Michael Folkson)
Pull request description:
The resource on the Boost unit test framework previously linked to in src/test/README.md was a stale URL.
Instead of deleting it, I've replaced it with an alternative resource on the framework on [boost.org](https://www.boost.org/doc/libs/1_45_0/libs/test/doc/html/utf/tutorials.html).
ACKs for top commit:
promag:
ACK 41d484d5c8.
hebasto:
ACK 41d484d5c8, the removed link is really obsolete.
fanquake:
ACK 41d484d5c8 - Thanks.
Tree-SHA512: 764f12548441bde615f77b7a2ca7c5188b4ab936972d16b84960fbd8604d4cbd224415bc59ce839e7e63293aa84fd97f31a69e38734e531231cdb0e148d2e1bd
fa27372e65 ci: Move CCACHE_DIR and test_runner tmp dir into ./ci/scratch/ (MarcoFalke)
fa60583d23 ci: Pass down $MAKEJOBS to test_runner.py (MarcoFalke)
Pull request description:
Some changes to the ci system:
* Pass down MAKEJOBS to the test_runner, instead of falling back to the default of 4. Passing it down avoids OOM on weak machines and allows better use of resources on beefy machines.
* Move CCACHE_DIR to ./ci/scratch/ subfolder: `ccache` is executed with root permissions inside the docker, so the cache files are created with root as owner. So it might be wise to not put them in the $HOME of the host
* Use the scratch dir as prefix for the test runner, as opposed to `/tmp/`, which is often a ramdisk and thus leads to OOM on the host when either a lot of tests are run in parallel or when a lot of tests fail and the datadirs are not cleaned.
ACKs for top commit:
fanquake:
ACK fa27372e65
laanwj:
ACK fa27372e65
Tree-SHA512: 67834fbab282051ec81c319d460528b32870507e53df2b8a1ce9a1f3f6a685aaf8eb8ba03f5406918ca4a33adf736e6a4adad7134c54cf3a9e47a26c64a13442
f3b57f4a1c Unrecommend making config file owned by bitcoin (setpill)
870d4152df Set ProtectHome in systemd service file (setpill)
639a416e37 Chgrp config dir to bitcoin in systemd service (setpill)
aded0528f0 Improve clarity of systemd service file comments (setpill)
Pull request description:
Rationale: ran into a bug with the systemd service file, fixed it locally and figured I might as well contribute my fix.
Also fixed some unrelated confusing phrasing in the comments of the same file, after discussion in IRC.
ACKs for top commit:
sipsorcery:
tACK f3b57f4a1c (nothing changed since previous tACK).
ryanofsky:
utACK f3b57f4a1c. Only change since last review is removing ConfigurationDirectoryMode churn in early commits
Tree-SHA512: 2188345878925b9e8a5c2c3df8dfba443720e2252a164db54a8e1d8007846721497b2d98c56f1d9b60a9a9ed4fdb1156c7b02c699616b220a9b614671617d32a
1373fa7e3d doc: add default bitcoin.conf locations (Chuf)
Pull request description:
Added default bitcoin.conf data directories and paths
ACKs for top commit:
practicalswift:
ACK 1373fa7e3d
ryanofsky:
ACK 1373fa7e3d
fanquake:
ACK 1373fa7e3d - Already three ACKs and lots of discussion here, so I'm going to merge, and the other comment
Tree-SHA512: 8bb1ed9868c5d171b6791bd6dc9598eddfdf64977d327ff4f333323cef8e3e76b1a67da21e4199f008a12f5610ac6dc6f34f4a13235e8846754eb6d6e5075da4
9452802480 doc: Tidy up shadowing section (João Barbosa)
Pull request description:
Removes the example because it violates the code format.
ACKs for top commit:
MarcoFalke:
unsigned ACK 9452802480
ryanofsky:
ACK 9452802480
fanquake:
ACK 9452802480 - Thanks for following up.
Tree-SHA512: 1fc31355d368225713298da7803e39e99014fbfcd229f2d3b56c082de95ab2965e51c80b172a5abce4646c53f845fa62a6d94d5df714e7835cac07a8ec7d5da7
aff1c9c884 Adds an option to msbuild common configuration to ignore linker warning 4221. This warning is for object files that do not include any symbols. The warning is harmless and occurs due to some classes that are *nix only having no source to compile for an msvc build. (Aaron Clauson)
Pull request description:
Adds an option to msbuild common configuration to ignore linker warning 4221. This warning is for object files that do not include any symbols. The warning is harmless and occurs due to some classes that are *nix only having no source to compile for an msvc build.
ACKs for top commit:
MarcoFalke:
ACK aff1c9c884
hebasto:
ACK aff1c9c884, I have reviewed the code, it looks OK and I agree it can be merged.
fanquake:
ACK aff1c9c884
Tree-SHA512: fda3721462d927d8734e16fc0250c7c76b197a085219a9e5267cdd14e19f23f50807d6b5516dee8402b60f3fdf188677e3d618fe6acbdbb5ac3bb635aeb7b297
Translated strings should not end up in the debug log, stderr, or
returned by an RPC.
Changing the util methods in util/error to return a bilingual_str paves
the way to achieve this goal in the long term.
e4f4ea47eb lint: Catch use of [] or {} as default parameter values in Python functions (practicalswift)
25dd867150 Avoid using mutable default parameter values (practicalswift)
Pull request description:
Avoid common Python default parameter gotcha when mutable `dict`/`list`:s are used as default parameter values.
Examples of this gotcha caught during review:
* https://github.com/bitcoin/bitcoin/pull/16673#discussion_r317415261
* https://github.com/bitcoin/bitcoin/pull/14565#discussion_r241942304
Perhaps surprisingly this is how mutable list and dictionary default parameter values behave in Python:
```
>>> def f(i, j=[], k={}):
... j.append(i)
... k[i] = True
... return j, k
...
>>> f(1)
([1], {1: True})
>>> f(1)
([1, 1], {1: True})
>>> f(2)
([1, 1, 2], {1: True, 2: True})
```
In contrast to:
```
>>> def f(i, j=None, k=None):
... if j is None:
... j = []
... if k is None:
... k = {}
... j.append(i)
... k[i] = True
... return j, k
...
>>> f(1)
([1], {1: True})
>>> f(1)
([1], {1: True})
>>> f(2)
([2], {2: True})
```
The latter is typically the intended behaviour.
This PR fixes two instances of this and adds a check guarding against this gotcha going forward :-)
ACKs for top commit:
Sjors:
Oh Python... ACK e4f4ea47eb. Testing tip: swap the two commits.
Tree-SHA512: 56e14d24fc866211a20185c9fdb274ed046c3aed2dc0e07699e58b6f9fa3b79f6d0c880fb02d72b7fe5cc5eb7c0ff6da0ead33123344e1a872209370c2e49e3f
403e372407 qa: Relax so that the subscriber is ready before publishing zmq messages (João Barbosa)
Pull request description:
Prevents the syndrome "slow joiner" - see http://zguide.zeromq.org/py:all#sockets-and-patterns - by relaxing before publishing messages.
ACKs for top commit:
MarcoFalke:
unsigned ACK 403e372407
Tree-SHA512: 0e856accbc450a9b09160bdce5112b2103dc9436cc317d31fb1c9634ebd76823a300a2e727818057fb4d0a615271772ff23e80553a13e9aa1935500de5eeec5f
8a3b2eb175 move-only: move coins statistics utils out of RPC (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
In the short-term, this move-only commit will help with fuzzing (https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-524482297). Later, these procedures will be used to compute statistics (particularly a content hash) for UTXO sets coming in from snapshots.
Most easily reviewed with `git ... --color-moved=dimmed_zebra`. A nice follow-up would be adding unittests, which I'll do if nobody else gets around to it.
ACKs for top commit:
MarcoFalke:
ACK 8a3b2eb175, checked --color-moved=dimmed-zebra
Tree-SHA512: a187d2f7590ad2450b8e8fa3d038c80a04fc3d903618c24222d7e3172250ce51badea35860c86101f2ba266eb4354e6efb8d7d508b353f29276e4665a1efdf74
These procedures will later be used in the ChainstateManager to compute
statistics (particularly a content hash) for UTXO sets coming in from
snapshots.
eb2d64b9e0 doc: Add spacing to command in translation_process.md (Chuf)
Pull request description:
Add spacing to the command used for updating `bitcoin_locale.qrc` entries. This makes it slightly easier to copy output into the relevant Makefile.
ACKs for top commit:
fanquake:
Tested ACK - eb2d64b9e0 - thanks.
Tree-SHA512: 079180962062994a3127ba05f7bb27a4145cb6a50354076864bf546dbbd80802ba9f10c943c43b6b3544d7225b646b7a8f1e38e0f148902c1813426b6f5036c3
a64dbeffe4 doc: Update labels in CONTRIBUTING.md (Hennadii Stepanov)
Pull request description:
This PR:
- adds `build` label
- makes labels lowercase (in accordance to current customs in this repo); also a lowercase label improves readability of PR title itself, e.g.,
```
doc: Update labels in CONTRIBUTING.md
```
reads better than
```
Doc: Update labels in CONTRIBUTING.md
```
- improves label names readability
- splits long labels (as suggested by **jonatack**)
ACKs for top commit:
practicalswift:
ACK a64dbeffe4
fanquake:
ACK a64dbeffe4
Tree-SHA512: f82e5e357490978f41035763b86cc005cf4a94099012cd0b246b664e00567ac7fa7a604bbb466b8318d94568084e3c67eecff5df152e0f39997d0a2fd9cfee77
92528c260e Support serialization of std::vector<bool> (Pieter Wuille)
Pull request description:
This adds support for serialization of `std::vector<bool>`, as a prerequisite of #16702.
ACKs for top commit:
MarcoFalke:
ACK 92528c260e (only looked at the diff on GitHub)
practicalswift:
ACK 92528c260e -- diff looks correct
jamesob:
ACK 92528c260e
Tree-SHA512: 068246a55a889137098f817bf72d99fc3218a560d62a97c59cccc0392b110f6778843cee4e89d56f271ac64e03a0f64dc5e2cc22daa833fbbbe9234c5f42d7b9
66ad75472f [Doc] Add documentation for the new whitelist permissions (nicolas.dorier)
Pull request description:
Documenting the new feature https://github.com/bitcoin/bitcoin/pull/16248 . Ping Sjors .
ACKs for top commit:
Sjors:
Indeed, re-ACK 66ad75472
Tree-SHA512: e6860bb6fae921287da7920a8db534e6a1a23871dd78dd6da030f00adf23e204cd23b194d67361bf34d4ef5a7815fc3fd7c81a3f2f35e4cfbe6ee2f2e6daec25
