2b4cfb44bd gitian-keys: Add signer aliases, some historical keys (Wladimir J. van der Laan)
Pull request description:
Update `keys.txt` for `gitian-verify.py` (bitcoin-core/bitcoin-maintainer-tools#71)
- Extend line format with signer aliases in parentheses
- Add a few historical signers
- Sort the file by name
Co-authored-by: fanquake <fanquake@gmail.com>
Top commit has no ACKs.
Tree-SHA512: 0bd22161400358fb2507be3f49b0ad79f323fdc280ecb647e1728d11f9d8ddea4a71286e5808036399929ede79c541432191f93f314ccfd12e40aadad56b8faa
Currently it was not possible to run just the BlockToJsonVerboes benchmarsk because it did not set up everything it needed, running `bench_bitcoin -filter=BlockToJsonVerbose` caused this assert to fail:
```
bench_bitcoin: chainparams.cpp:506: const CChainParams& Params(): Assertion `globalChainParams' failed.
```
Initializing TestingSetup fixes this.
This commit addresses #20809.
We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
This commit adds brief documentation for this feature. Included in the
justification is the purpose of this feature as well as usage and
functionality tips.
Add a functional test for CaptureMessage. This connects and then
disconnects a peer so that the handshake can be used to check if capture
is being done correctly.
Included in a docstring in the test is the following:
From the data file we'll only check the structure.
We won't care about things like:
- Deserializing the payload of the message
- This is managed by the deserialize methods in
test_framework.messages
- The order of the messages
- There's no reason why we can't, say, change the order of the
messages in the handshake
- Message Type
- We can add new message types
We're ignoring these because they're simply too brittle to test here.
This commit adds contrib/message-capture/message-capture-parser.py, a python
script to be used alongside -capturemessages to parse the captured
messages.
It is complete with arguments and will parse any file given, sorting the
messages in the files when creating the output. If an output file is
specified with -o or --output, it will dump the messages in json format
to that file, otherwise it will print to stdout.
The small change to the unused msg_generic is to bring it in line with
the other message classes, purely to avoid a bug in the future.
This commit adds the CaptureMessage function. This will later be called
when any message is sent or received. The capture directory is fixed,
in a new folder "message_capture" in the datadir. Peers will then have
their own subfolders, named with their IP address and port, replacing
colons with underscores to keep compatibility with Windows. Inside,
received and sent messages will be captured into two binary files,
msgs_recv.dat and msgs_sent.dat.
e.g.
message_capture/203.0.113.7_56072/msgs_recv.dat
message_capture/203.0.113.7_56072/msgs_sent.dat
The format has been designed as to result in a minimal performance
impact. A parsing script is added in a later commit.
bf100f8170 [net] Cleanup InactivityChecks() and add commenting about time (John Newbery)
06fa85cd50 [net] InactivityCheck() takes a CNode reference (John Newbery)
Pull request description:
This is a pure refactor and should not change any behavior. It clarifies and documents the InactivityCheck() function
This makes #20721 easier to review. In particular, this function uses a mixture of (unmockable) system time and mockable time. It's important to understand where those are being used when reviewing #20721.
#20721 doesn't require this change, so if others don't agree that it's useful and makes review easier, then I'm happy to close this and just do #20721 directly.
ACKs for top commit:
fanquake:
ACK bf100f8170
MarcoFalke:
review ACK bf100f8170💫
Tree-SHA512: 7b001de2a5fbe8a6dc37baeae930db5775290afb2e8a6aecdf13161f1e5b06ef813bc6291d8ee5cefcf1e430c955ea702833a8db84192eebe6e6acf0b9304cb2
5b41d84b34 doc: add xorriso to macOS depends packages (fanquake)
Pull request description:
This was missed in #20470.
ACKs for top commit:
hebasto:
ACK 5b41d84b34, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: bcfd8468a099c69175f8a9d295c1466764ab25d6a61121b28675a09c3e96f45b6309e1523d341f4cb21d0ddee4945f00ba060ba02da835f2f0db7e694fd6c44b
962444295d zmq: deduplicate 'sequence' publisher message creation/sending (Sebastian Falbesoner)
Pull request description:
This small PR deduplicates common low-level creation and sending code for the 'sequence' zmq publisher message (methods `NotifyBlock{Connect,Disconnect}()`, `NotifyTransaction{Acceptance,Removal}()` in the class `CZMQPublishSequenceNotifier`) by introducing a helper function.
ACKs for top commit:
jonatack:
Code review re-ACK 962444295d per `git diff f231ffd 9624442`
instagibbs:
utACK 962444295d
Tree-SHA512: de0750d923f36d1a5751331e88eec8a1605cb88c93318830913210485e2bff712310484f18a0fb626df6ef32ce0b0cf57f4421ce62656e05fce7011a0c3c2d0e
f1694757dd guix: Fix typo (Carl Dong)
771c4b98a8 guix: README: Add darwin HOSTS entry (Carl Dong)
8dbf18cb1d guix: Check for macOS SDK before building anything (Carl Dong)
34b23f597e guix: Set ZERO_AR_DATE for darwin build determinism (Carl Dong)
f3835dc6a3 build: Make xorrisofs reproducible with -volume_date (Carl Dong)
c9eb4cf3a0 guix: Add support for darwin builds (Carl Dong)
37fe73a092 build: Add var printing target to src/Makefile.am (Carl Dong)
Pull request description:
This PR brings our Guix builds on par with Gitian in terms of supported architectures.
Reviewers: if you run a build, please submit:
```
find output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
```
So that we can compare hashes and ensure reproducibility!
ACKs for top commit:
fanquake:
ACK f1694757dd - I think we can make some small usability improvements, but this is ok to merge now.
Tree-SHA512: 4af2b71654a9736467dcc681d10601c6eee37800d7847011a50585455b67b55d61742ca5604585f310a2fd75335b674e5e27dfb5169cb2f26e112aa4c411d8be
da9caa1ced Replace fs::absolute calls with AbsPathJoin calls (Kiminuo)
66576c4fd5 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo)
Pull request description:
This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.
This PR doesn't change behavior aside from adding an assert and fixing a test bug.
ACKs for top commit:
jonatack:
Code review ACK da9caa1ced only doxygen improvements since my last review per `git diff d867d7a da9caa1`
MarcoFalke:
review ACK da9caa1ced📯
ryanofsky:
Code review ACK da9caa1ced. Just comment and test tweaks since previous review.
Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
We need this to be after a '--' as '-volume_date' is a xorriso
flag, not a xorrisofs flag. See the respective man pages.
For more details: https://issues.guix.info/issue/35283#2
ff44cae279 test: Change feature_config_args.py not to rely on strange regtest=0 behavior (Russell Yanofsky)
Pull request description:
Update test to simply generate a normal mainnet configuration file instead of using a crazy setup where a regtest=1 config file using an includeconf in the [regtest] section includes another config file that specifies regtest=0, retroactively switching the network to mainnet.
This setup was fragile and only worked because the triggered InitError happened early enough that none of the ignored [regtest] options mattered (only affecting log output).
This change was originally made as part of #17493
Top commit has no ACKs.
Tree-SHA512: 3f77305454f04438493dfc2abd78a00434b30869454d1c3f54587b9c1f63239c49c90fb3b4d3a777ad130f2184e0f2dac87cee4cd23c50f1b3496a375943da01
b396467053 locks: Annotate CTxMemPool::check to require cs_main (Carl Dong)
Pull request description:
```
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
calls GetSpendHeight which locks cs_main. This can potentially cause an
undesirable lock invesion since CTxMemPool's cs is supposed to be locked
after cs_main.
This does not cause us any problems right now because all callers of
CTxMemPool already lock cs_main before calling CTxMemPool::check, which
means that the LOCK(cs_main) in GetSpendHeight becomes benign.
However, it is currently possible for new code to be added which calls
CTxMemPool::check without locking cs_main (which would be dangerous).
Therefore we should make it explicit that cs_main needs to be held
before calling CTxMemPool::check.
NOTE: After all review-only assertions are removed in "#20158 |
tree-wide: De-globalize ChainstateManager", and assuming that we
keep the changes in "validation: Pass in spendheight to
CTxMemPool::check", we can re-evaluate to see if this annotation
is still necessary.
```
-----
Previous discussions:
1. https://github.com/bitcoin/bitcoin/pull/20158#discussion_r520639845
2. https://github.com/bitcoin/bitcoin/pull/20158#pullrequestreview-557117202
3. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559425521
ACKs for top commit:
jnewbery:
Code review ACK b396467053
MarcoFalke:
ACK b396467053
jonatack:
ACK b396467053 review and debug built, verified that `cs_main` is held by callers of `CTxMemPool::check()` in `PeerManagerImpl::ProcessOrphanTx()`, `PeerManagerImpl::ProcessMessage()`, and `CChainState::ActivateBestChainStep()`
Tree-SHA512: 4635cddb4aa1af9532bb657b2f9c4deec4568d16ba28c574eae91bb77368cd40e23c3c720a9de11cec78e7ad678a44a5e25af67f13214b86b56e777e0c35a026
abb6fa7285 fuzz: Initialize a full TestingSetup where appropriate (Carl Dong)
713314abfa fuzz: Consolidate fuzzing TestingSetup initialization (Carl Dong)
Pull request description:
```
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways:
1. Calling InitializeFuzzingContext, which implicitly constructs a static
const BasicTestingSetup
2. Directly constructing a static const BasicTestingSetup in the initialize_*
function
3. Directly constructing a static TestingSetup and reproducing the
initialization arguments (I'm assuming because
InitializeFuzzingContext only initializes a BasicTestingSetup)
The new, relatively-simple MakeFuzzingContext function allows us to
consolidate these methods of initialization by being flexible enough to
be used in all situations. It:
1. Is templated so that we can choose to initialize any of
the *TestingSetup classes
2. Has sane defaults which are often used in fuzzers but are also
easily overridable
3. Returns a unique_ptr, explicitly transferring ownership to the caller
to deal with according to its situation
```
~~Question for fuzzing people: was it intentional that `src/test/fuzz/net.cpp` would directly instantiate the `BasicTestingSetup` and thus omit the `"-nodebuglogfile"` flag?~~ [Answered](https://github.com/bitcoin/bitcoin/pull/20946#issuecomment-761537108)
ACKs for top commit:
MarcoFalke:
ACK abb6fa7285
Tree-SHA512: 96a5ca6f4cd5ea0e9483b60165b31ae3e9003918c700a7f6ade48010f419f2a6312e10b816b3187f1d263798827571866e4c4ac0bbfb2e0c79dfad254cda68e7
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways:
1. Calling InitializeFuzzingContext, which implicitly constructs a static
const BasicTestingSetup
2. Directly constructing a static const BasicTestingSetup in the initialize_*
function
3. Directly constructing a static TestingSetup and reproducing the
initialization arguments (I'm assuming because
InitializeFuzzingContext only initializes a BasicTestingSetup)
The new, relatively-simple MakeFuzzingContext function allows us to
consolidate these methods of initialization by being flexible enough to
be used in all situations. It:
1. Is templated so that we can choose to initialize any of
the *TestingSetup classes
2. Has sane defaults which are often used in fuzzers but are also
easily overridable
3. Returns a unique_ptr, explicitly transferring ownership to the caller
to deal with according to its situation
4efb6c2d3b zmq test: deduplicate test setup code (node restart, topics subscription) (Sebastian Falbesoner)
Pull request description:
This PR deduplicates common setup code for the ZMQ functional test. The following steps, previously duplicated in each sub-test, are put into a new method `setup_zmq_test(...)`:
- create subscriber sockets (`zmq.SUB`) for each topic with the specified timeout (default 60s)
- restart node0 with specified zmq notifications enabled (`-zmqpub...=tcp://127.0.0.1:...`...)
- if desired, connect node0 with node1 (note done by default)
- connect all susbcriber sockets to publisher (running on node0)
- wait a bit (currently 200ms), to _"Relax so that the subscribers are ready before publishing zmq messages"_
Note that the last point should be repaced by a more robust method, as this test is still flaky, see #20934 (also #20590 and #20538).
ACKs for top commit:
instagibbs:
ACK 4efb6c2d3b
laanwj:
Code review ACK 4efb6c2d3b
Tree-SHA512: d49626756a9c669f1133f1b73ce273994b58c760ce0d6a4bdaa384f043a74149dc2b9fa66fe990413d9105f9c3b6ea973e099669e8e02f2902a5b84fa995028c
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
calls GetSpendHeight which locks cs_main. This can potentially cause an
undesirable lock invesion since CTxMemPool's cs is supposed to be locked
after cs_main.
This does not cause us any problems right now because all callers of
CTxMemPool already lock cs_main before calling CTxMemPool::check, which
means that the LOCK(cs_main) in GetSpendHeight becomes benign.
However, it is currently possible for new code to be added which calls
CTxMemPool::check without locking cs_main (which would be dangerous).
Therefore we should make it explicit that cs_main needs to be held
before calling CTxMemPool::check.
NOTE: After all review-only assertions are removed in "#20158 |
tree-wide: De-globalize ChainstateManager", and assuming that we
keep the changes in "validation: Pass in spendheight to
CTxMemPool::check", we can re-evaluate to see if this annotation
is still necessary.
ad57fb756b wallet: Add BerkeleyDB version sanity check at init time (Wladimir J. van der Laan)
Pull request description:
Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation.
This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected.
ACKs for top commit:
decryp2kanon:
utACK ad57fb7
achow101:
ACK ad57fb756b
theStack:
utACK ad57fb756b
meshcollider:
Code review ACK ad57fb756b
Tree-SHA512: 99cd7d836bffbdeb3d4e14053f7139cc85a6d42e631a3f9a3058a848042446b364faee127500f5acb374616e6a61ab2bedebfac1ba9bc993b4d6227114c2a6c2
1fca9811e1 lint: Skip whitespace lint for guix patches (Carl Dong)
a91c46c57d guix: Make nsis reproducible by respecting SOURCE-DATE-EPOCH (Carl Dong)
Pull request description:
```
When building nsis, if VERSION is not specified, it defaults to
cvs_version which is non-deterministic as it includes the current date.
This patches nsis to default to SOURCE_DATE_EPOCH if it exists so that
nsis is reproducible.
Upstream change: https://github.com/kichik/nsis/pull/13
```
Sidenote: also a good demonstration of how Guix allows us to flexibly patch our tools!
Note to reviewers: if you want to compare hashes, please build after Jan 16th 2021 without my substitute server enabled!
ACKs for top commit:
fanquake:
ACK 1fca9811e1
Tree-SHA512: b800e0ce5f73827ad353739effb9167ec3a6bdb362c725ae20dd3f025ce78660f85c70ce1d75cd0896facf1e8fe38a9e058459ed13dec71ab3a2fe41e20eaa5d
ea0a7ec949 Remove deprecated bumpfee behavior (Andrew Chow)
Pull request description:
Removes the deprecation message, behavior, and test.
This was marked for removal in 22.0.
ACKs for top commit:
promag:
ACK ea0a7ec949, maybe add need release notes tag.
Tree-SHA512: d1626906849f6ee37213c32e5f8c1433ad8fb7beabcd88f5801b1964b322171a2341bdfbd9a3a5ab39b2fd9d9c6a05f73298583423a73cab1275653105c03e8e
92370033a2 contrib: embed C++11 patch in install_db4.sh (jackielove4u)
Pull request description:
This is a continuation of https://github.com/bitcoin/bitcoin/pull/20665.
Closes#20722.
ACKs for top commit:
laanwj:
ACK 92370033a2
fanquake:
ACK 92370033a2.
Tree-SHA512: ebfd16f5301158de1acc1b8eeca43b3d94f0a6d438832133a30648e5e8a88268b4af983be0bb57f3018e3af8459f32f0de676c1b4e8942e199a4497c776631c5
