fa52a86fd3acbcfc4b5ca1304c19d81df66d85d7 fuzz: Rework rpc fuzz target (MarcoFalke)
Pull request description:
Changes (reason):
* Return `void` in `CallRPC` (the result is unused anyway)
* Reduce the `catch`-scope of `std::runtime_error` to `RPCConvertValues` (Code clarity and easier bug-finding)
* Crash when an internal bug is detected (bugs are bad)
ACKs for top commit:
shaavan:
Code Review ACK fa52a86fd3acbcfc4b5ca1304c19d81df66d85d7
Tree-SHA512: 576411a0e50bca9be3e6ffaf745001b1808fd37029251f8ec2c279e0671efe91d43dd81fd4ca26871c28b119e593ee2a0043d4b75f44da578f17541ee3afd696
9ba7c44265a47880585e39d0167d057ba935ff16 refactor: get wallet path relative to wallet_dir (Michael Dietz)
Pull request description:
Now that boost has been updated > 1.60 (see #22320), we can simplify how we get
wallet path relative to wallet_dir by using:
`boost::filesystem::lexically_relative`, removing a TODO.
Test coverage comes from `test/functional/wallet_multiwallet.py`
I first tried this in #20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this.
ACKs for top commit:
ryanofsky:
Code review ACK 9ba7c44265a47880585e39d0167d057ba935ff16. Basically this same code change is made in #20744 commit b70c84348ac7a8e427a1183f894c73e52c734529, so this PR helps simplify that one
lsilva01:
Code Review ACK 9ba7c44
Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
3e33d170cc0a8f386791777f3cc597e2bd0bf2ee Add ipc::Context and ipc::capnp::Context structs (Russell Yanofsky)
Pull request description:
These are currently empty structs but they will be used to pass some function and object pointers from bitcoin application code to IPC hooks that run, for example, when a remote object is created or destroyed, or a new process is created.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
ariard:
Code Review ACK 3e33d170
Tree-SHA512: fd949fae5f1a973d39cb97f2745821ab2f62b98e166e53bc2801f97dcde988e18faaaaa0ffc2a82c170938b3a18078b6162fa35460e6e7c635e681b3c9e5b0a6
40c0e06047 fix: use coinstatsindex instead blockfilterindex in feature_prunning (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
This is follow-up changes for backports bitcoin#15946 and bitcoin#19521
It fixes failure:
TestFramework (INFO): Test invalid pruning command line options
TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_node.py", line 511, in assert_start_raises_init_error
ret = self.process.wait(timeout=self.rpc_timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 1264, in wait
return self._wait(timeout=timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 2038, in _wait
raise TimeoutExpired(self.args, timeout)
subprocess.TimeoutExpired: Command '['dashsrc/dashd', '-datadir=/tmp/dash_func_test_m8w6q7a2/node0', '-logtimemicros', '-debug', '-debugexclude=libevent', '-debugexclude=leveldb', '-mocktime=1417713337', '-uacomment=testnode0', '-logthreadnames', '-logsourcelocations', '-createwalletbackups=0', '-prune=550', '-blockfilterindex', '-mocktime=1417713337']' timed out after 120 seconds
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "dashtest/functional/feature_pruning.py", line 495, in run_test
self.test_invalid_command_line_options()
File "dashtest/functional/feature_pruning.py", line 149, in test_invalid_command_line_options
self.nodes[0].assert_start_raises_init_error(
File "dashtest/functional/test_framework/test_node.py", line 541, in assert_start_raises_init_error
self._raise_assertion_error(assert_msg)
File "dashtest/functional/test_framework/test_node.py", line 180, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] dashd should have exited within 120s with expected error Error: Prune mode is incompatible with -blockfilterindex.
TestFramework (INFO): Stopping nodes
## What was done?
Removed `blockfilterindex` test, enabled `coinstatsindex` test.
## How Has This Been Tested?
Run manually: `test/functional/feature_pruning.py`
This test is not run by our CI
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 40c0e06047
UdjinM6:
ACK 40c0e06047
Tree-SHA512: 94005c52a39cc02be8df65898844584fb247b71abab36341c3d8b4e5018902931ed8554734f27bb5d54fae2b7867ba40f136fc089fd6f9f0e57852203a5d4011
52b9fcecb4 fix: can't mix `-` and `:` (UdjinM6)
2f12f91d39 fix: 6205 follow-up (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
I think I messed up whitespaces so these jobs can't run now, they fail with `[Error] Invalid type for on` https://github.com/dashpay/dash/actions/runs/10353281487...
Weird that they succeeded in #6205https://github.com/dashpay/dash/actions/runs/10352521500/workflowhttps://github.com/dashpay/dash/actions/runs/10352521490/workflow🤷♂️
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 52b9fcecb4
Tree-SHA512: f4dff14b727164c5a79545dc2650dc716eb08ddb6445d76fc70a3bdc5044d22212ce44fd942db65559154e989c0ff272d733558b1773440a88a691a222642746
0dd997c4e5 Merge bitcoin/bitcoin#26835: contrib: add PE Canary check to security-check (fanquake)
b6bde7322c Merge bitcoin/bitcoin#26827: doc: use "std lib clock" over "C++11 clock" (MarcoFalke)
93c4652a05 Merge bitcoin/bitcoin#26821: refactor: Make `ThreadHTTP` return void (Andrew Chow)
07f4c39c44 Merge bitcoin/bitcoin#26730: test: add coverage for `purpose` arg in `listlabels` (MarcoFalke)
6fe46fc02a Merge bitcoin/bitcoin#26864: doc: net: fix link to onion address encoding scheme [ONIONADDRESS] (MarcoFalke)
d1b93c78b7 Merge bitcoin/bitcoin#26818: test: Fix feature_startupnotify intermittent issue (MarcoFalke)
864d02e4a9 Merge bitcoin/bitcoin#26809: compat: use STDIN_FILENO over 0 (Andrew Chow)
092ddc3a3e Merge bitcoin/bitcoin#26603: doc: CalculateSequenceLocks: prevHeights entries are set to 0, not removed (glozow)
7f2b934089 Merge bitcoin/bitcoin#26772: contrib: fix sha256 check in install_db4.sh for FreeBSD (fanquake)
df2f533aaf Merge bitcoin/bitcoin#26759: test: Drop no longer needed `race:epoll_ctl` TSan suppression (MarcoFalke)
9590929900 Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and invalid input test coverage (MacroFake)
Pull request description:
## Issue being fixed or feature implemented
Trivial backports; need to see ci pass
## What was done?
## How Has This Been Tested?
built
## Breaking Changes
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0dd997c4e5
UdjinM6:
utACK 0dd997c4e5
Tree-SHA512: e23207508dfc88a961e0eed031b1c6d195469d61343f0d1c66be853a6fcd882a84d8d40670c263264fe96c9a8a98d83fc0f0a05c263eadc64056edb563c588db
That follow-up changes for backports bitcoin#15946 and bitcoin#19521
It fixes failure:
TestFramework (INFO): Test invalid pruning command line options
TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_node.py", line 511, in assert_start_raises_init_error
ret = self.process.wait(timeout=self.rpc_timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 1264, in wait
return self._wait(timeout=timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 2038, in _wait
raise TimeoutExpired(self.args, timeout)
subprocess.TimeoutExpired: Command '['dashsrc/dashd', '-datadir=/tmp/dash_func_test_m8w6q7a2/node0', '-logtimemicros', '-debug', '-debugexclude=libevent', '-debugexclude=leveldb', '-mocktime=1417713337', '-uacomment=testnode0', '-logthreadnames', '-logsourcelocations', '-createwalletbackups=0', '-prune=550', '-blockfilterindex', '-mocktime=1417713337']' timed out after 120 seconds
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "dashtest/functional/feature_pruning.py", line 495, in run_test
self.test_invalid_command_line_options()
File "dashtest/functional/feature_pruning.py", line 149, in test_invalid_command_line_options
self.nodes[0].assert_start_raises_init_error(
File "dashtest/functional/test_framework/test_node.py", line 541, in assert_start_raises_init_error
self._raise_assertion_error(assert_msg)
File "dashtest/functional/test_framework/test_node.py", line 180, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] dashd should have exited within 120s with expected error Error: Prune mode is incompatible with -blockfilterindex.
TestFramework (INFO): Stopping nodes
9ad26f1664 fix: follow-up bitcoin#20773 - coinjoin loader can be nullptr too (Konstantin Akimov)
4125485401 fix: follow-up bitcoin#20773 - for collateral lock/unlock coins (Konstantin Akimov)
06ea87040b Merge bitcoin/bitcoin#20773: refactor: split CWallet::Create (W. J. van der Laan)
d89847f43e Merge bitcoin/bitcoin#22004: fuzz: Speed up transaction fuzz target (MarcoFalke)
82a6aa5907 Merge bitcoin/bitcoin#21810: fuzz: Various RPC fuzzer follow-ups (MarcoFalke)
3bfefde53b Merge bitcoin/bitcoin#21931: ci: Bump cirrus fuzz CPUs to avoid timeout (MarcoFalke)
2398283ff6 Merge bitcoin/bitcoin#21922: fuzz: Avoid timeout in EncodeBase58 (fanquake)
813993d44a Merge bitcoin/bitcoin#21892: fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
4dd36f603a Merge bitcoin/bitcoin#21798: fuzz: Create a block template in tx_pool targets (MarcoFalke)
c2bd834f3b Merge bitcoin/bitcoin#21169: fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v22
## What was done?
see commits
## How Has This Been Tested?
Run unit/functional tests
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
Top commit has no ACKs.
Tree-SHA512: 5d2cca761bb7971b3d83f1b54a5c26d62c9495becdf6d897e22bfd975837605bb5326dc37793f838389c9145db34ef45992be95e789af8c12af9b6ade79a0038
489ebb7b34c403a3ce78ff6fb271f8e6ecb47304 wallet: make chain optional for CWallet::Create (Ivan Metlushko)
d73ae939649f3b30e52b5a2cccd7fafd1ab36766 CWallet::Create move chain init message up into calling code (Ivan Metlushko)
44c430ffac940e1d1dd7f5939a495470bc694489 refactor: Add CWallet:::AttachChain method (Russell Yanofsky)
e2a47ce08528dfb39c0340145c6977f6afd587f6 refactor: move first run detection to client code (Ivan Metlushko)
Pull request description:
This is a followup for https://github.com/bitcoin/bitcoin/pull/20365#discussion_r522265003
First part of a refactoring with overall goal to simplify `CWallet` and de-duplicate code with `wallettool`
**Rationale**: split `CWallet::Create` and create `CWallet::AttachChain`.
`CWallet::AttachChain` takes chain as first parameter on purpose. In future I suggest we can remove `chain` from `CWallet` constructor.
The second commit is based on be164f9cf89b123f03b926aa980996919924ee64 from #15719 (thanks ryanofsky)
cc ryanofsky achow101
ACKs for top commit:
ryanofsky:
Code review ACK 489ebb7b34c403a3ce78ff6fb271f8e6ecb47304. Only changes since last review were adding a const variable declaration, and implementing suggestion not to move feerate option checks to AttachChain. Thanks for updates and fast responses!
Tree-SHA512: 00235abfe1b00874c56c449adcab8a36582424abb9ba27440bf750af8f3f217b68c11ca74eb30f78a2109ad1d9009315480effc78345e16a3074a1b5d8128721
bbbb51877a96c3473aeea5914f751eec7835b5c4 fuzz: Speed up transaction fuzz target (MarcoFalke)
Pull request description:
`hashBlock` and `include_addresses` are orthogonal, so no need to do an exhaustive "search".
Might fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34491
ACKs for top commit:
practicalswift:
cr ACK bbbb51877a96c3473aeea5914f751eec7835b5c4: patch looks correct, and `TxToUniv` surprisingly wide in the `transaction_fuzz_target` flame graph! Putting it on a diet makes sense.
Tree-SHA512: 1e7c30c7fecf96364a9a1597c0a22139389fdeb67db59f3c2c6fc088196e3332877b2865991a957980d542f99a2f48cc066dd7cc16c695a5113190fe06205089
5252f86eb616a1112e427c268c8e8825f2a63d67 fuzz: Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of #ifdef forests (practicalswift)
54549dda310e2becee9cb4997d1408a90e91934f fuzz: RPC fuzzer post-merge follow-ups. Remove unused includes. Update list of fuzzed RPC commands. (practicalswift)
Pull request description:
Various RPC fuzzer follow-ups:
* Remove unused includes.
* Update list of fuzzed RPC commands.
* Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of `#ifdef` forests.
Context: https://github.com/bitcoin/bitcoin/pull/21169#pullrequestreview-646723483
ACKs for top commit:
MarcoFalke:
Concept ACK 5252f86eb616a1112e427c268c8e8825f2a63d67
Tree-SHA512: 286d70798131706ffb157758e1c73f7f00ed96ce120c7d9dc849e672b283f1362df47b206cfec9da44d5debb5869225e721761dcd5c38a7d5d1019dc6c912ab2
99993f066405863c66ccaec0a8427129f4515768 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
Pull request description:
Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078
ACKs for top commit:
practicalswift:
cr ACK 99993f066405863c66ccaec0a8427129f4515768: patch looks correct despite no `fa` prefix in commit hash
Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6
fa03d0acd6bd8bb6d3d5227512f042ff537ad993 fuzz: Create a block template in tx_pool targets (MarcoFalke)
fa61ce5cf5c1d73d352173806571bcd7799ed2ee fuzz: Limit mocktime to MTP in tx_pool targets (MarcoFalke)
fab646b8ea293bb2b03707c6ef6790982625e492 fuzz: Use correct variant of ConsumeRandomLengthString instead of hardcoding a maximum size (MarcoFalke)
fae2c8bc54e6c0fe69a82bd1b232c52edd1acd34 fuzz: Allow to pass min/max to ConsumeTime (MarcoFalke)
Pull request description:
Relatively simple check to ensure a block can always be created from the mempool
ACKs for top commit:
practicalswift:
Tested ACK fa03d0acd6bd8bb6d3d5227512f042ff537ad993
Tree-SHA512: e613376ccc88591cbe594db14ea21ebc9b2b191f6325b3aa4ee0cd379695352ad3b480e286134ef6ee30f043d486cf9792a1bc7e44445c41045ac8c3b931c7ff
545404e7e1c72985557ccffe865cea269143e5dd fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (practicalswift)
Pull request description:
Add RPC interface fuzzing.
This PR increases overall fuzzing line coverage from [~65%](https://marcofalke.github.io/btc_cov/fuzz.coverage/) to ~70% 🎉
To test this PR:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make -C src/ test/fuzz/fuzz
$ FUZZ=rpc src/test/fuzz/fuzz
```
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for more information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
Concept ACK 545404e7e1c72985557ccffe865cea269143e5dd
Tree-SHA512: 35fc1b508af42bf480ee3762326b09ff2eecdb7960a1917ad16345fadd5c0c21d666dafe736176e5a848ff6492483c782e4ea914cd9000faf50190df051950fd
2777d30caa fix(ci): only run some GH actions on PR approval, not on every comment (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 2777d30caa
Tree-SHA512: 183deae1d1b32408bd81962721141e976912b2614a046ab23565537db62fcff7d367331db8ced7cf176dc67adbc18b96e7678b37068f807aaafb1718d55dea70
540f6871d3 fix: lock `::cs_main` before accessing `ChainstateManager::m_best_header` (Kittywhiskers Van Gogh)
aafded67d9 fix: compilation error due to rebase error between bitcoin#22937 and ipc/process (Kittywhiskers Van Gogh)
Pull request description:
## Issue being fixed or feature implemented
CI failure for multiprocess
## What was done?
It resolve compilation failure on develop (apply changes from 22937 to ipc/process)
See alternate solutions:
- https://github.com/dashpay/dash/pull/6192
- https://github.com/dashpay/dash/pull/6195
## How Has This Been Tested?
Run build locally:
```
make MULTIPROCESS=1 -j10
./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings --enable-werror --enable-debug --enable-crash-hooks --enable-maintainer-mode --enable-stacktraces --enable-multi-process
```
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 540f6871d3
Tree-SHA512: 75b675e93763e8e53086a10b93516b8ec94418902f9be2de738517176cc835c0dad25c286426089a5327a9c13d1787b89debda2c6025cb598489204d7d5af2cf
5394d63e18 feat: improve merge-check action to leave a comment and label to PRs (pasta)
Pull request description:
## Issue being fixed or feature implemented
this is untested but I think it'll work, simply add comment and label to PRs which fail this CI
## What was done?
## How Has This Been Tested?
hasn't but if this action breaks that is not critical
## Breaking Changes
None
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 5394d63e18
Tree-SHA512: 78ae7c05446a0625143fb7e04b7e63ae0830ca45cde4f924eb4e04dfb12ac3cc9e697efbd914a71a9b5ba98400597e7559258b5d93419ed7eb7cc14a9b876fe7
770651aa15 set hosts in guix-check (pasta)
580bbe6d1c feat: improve guix building; run always, save artifacts (pasta)
101a31555f refactor: simplify caching setup, add a restore key to actually cache besides 1 run (pasta)
1b139e4837 feat: automatically run guix-build on all tags pushed (pasta)
Pull request description:
## Issue being fixed or feature implemented
Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts.
In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour
## What was done?
## How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600
## Breaking Changes
None
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 770651aa15
Tree-SHA512: 639b95c3b6a26f205ed00c138a9189f915cfc36a815516035e59ceda82675414b1bd31a361b33449b5e4c58a7655f3a7d616b362c23f7fa75e72b1284be06b9e
92f82f6fc6 fix: group the ENV in quotations (pasta)
2237b4266b refactor: use `=` with ENV in dockerfiles (pasta)
Pull request description:
## Issue being fixed or feature implemented
see: https://docs.docker.com/go/dockerfile/rule/legacy-key-value-format/
Basically avoid old format of ENV in docker files
## What was done?
see commit
## How Has This Been Tested?
Building the containers locally, seems all fine
## Breaking Changes
_Please describe any breaking changes your code introduces_
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 92f82f6fc6
UdjinM6:
utACK 92f82f6fc6
Tree-SHA512: 0fb5028b9fd8e8f2cac6d7488c3254f58a9ee96f2c236a1d0b90ea48f57b793be3caab6fc764b4ac1127eada9fae85468a13d2eb2a1c3e0a6940a6c8bc601681
672f7ad747ecc6e04472f96fa88332be1f39d39b doc: remove usages of C++11 (fanquake)
Pull request description:
These were new in C++11, and now they are just our standard library.
ACKs for top commit:
jarolrod:
re-ACK 672f7ad747ecc6e04472f96fa88332be1f39d39b
hebasto:
re-ACK 672f7ad747ecc6e04472f96fa88332be1f39d39b
Tree-SHA512: 7e3b8b0346ba29b19e6d8536700ca510e2b543cdeecd9e740bba71ea6d0133dd96cdaeaa00f371f8ef85913ff5aaabe12878255f393dac7d354a8b89b58d050a
45553e11c965db218733f9ad32ecde391b393443 refactor: Make `ThreadHTTP` return void (Hennadii Stepanov)
Pull request description:
The `bool` return value was introduced in 755aa05174 (https://github.com/bitcoin/bitcoin/pull/8421).
It has been not used since 8d3f46ec3938e2ba17654fecacd1d2629f9915fd (https://github.com/bitcoin/bitcoin/pull/14670).
No behavior change.
ACKs for top commit:
achow101:
ACK 45553e11c965db218733f9ad32ecde391b393443
brunoerg:
crACK 45553e11c965db218733f9ad32ecde391b393443
w0xlt:
ACK 45553e11c9
stickies-v:
ACK 45553e11c
Tree-SHA512: 1593a5740e729967fbe1363235cd5b77ecf431b29bc740a89a6c70fc838ad97a2e4a2cd7cd63aa482f7c50bc2ffabc8cd53e8f64d6032603cb3b662229bc3dc2
c467cfffcebb30f829eeb8160166a6b941d97ed6 test: add coverage for `purpose` arg in `listlabels` (brunoerg)
Pull request description:
This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive).
dcdfd72861/src/wallet/rpc/addresses.cpp (L698-L704)
ACKs for top commit:
kristapsk:
ACK c467cfffcebb30f829eeb8160166a6b941d97ed6
Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8
3076f1815d64f448aa9dff6e48e07004f42ac0fc doc: net: fix link to onion address encoding scheme [ONIONADDRESS] (Sebastian Falbesoner)
Pull request description:
Instead of referring to a fixed line number to a file in master (which is obviously always quickly outdated), use a permalink tied to the latest commit.
ACKs for top commit:
vasild:
ACK 3076f1815d64f448aa9dff6e48e07004f42ac0fc
Tree-SHA512: 7070a7e47d683b1539f33daa4c67093a87d6121a84430a3b24afee139a7f2b3cab32fcdf0bc561f8e177b5ba864a98e4491e08dac90cdd4bd2e4e6b8fa7e4b14
585c6722128537f772043ef4c87238e283669b8a compat: use STDIN_FILENO over 0 (fanquake)
Pull request description:
This is already used throughout this file, and is self-documenting.
ACKs for top commit:
john-moffett:
ACK 585c6722128537f772043ef4c87238e283669b8a
achow101:
ACK 585c6722128537f772043ef4c87238e283669b8a
hebasto:
ACK 585c6722128537f772043ef4c87238e283669b8a, I have reviewed the code and it looks OK, I agree it can be merged.
kristapsk:
utACK 585c6722128537f772043ef4c87238e283669b8a
aureleoules:
ACK 585c6722128537f772043ef4c87238e283669b8a
Tree-SHA512: c0114ae896ba5404be70b804ee9f454d213f1d789c8f5a578c422dd15a308a214e6851fee76c0ec736a212bc86fb33ec17af1b22e5d23422c375ca4458251356
22e9afe40d987f4f90bc8469f9475df138fe6261 use sha256 command instead of sha256sum on FreeBSD (Murray Nesbitt)
Pull request description:
The FreeBSD version of `sha256sum` takes different arguments than the GNU version.
The `sha256_check` function in `contrib/install_db4.sh` has code specific to FreeBSD, however it doesn't get reached because while the `sha256sum` command does exist on FreeBSD, it is incompatible and results in an error:
```
sha256sum: option requires an argument -- c
usage: sha256sum [-pqrtx] [-c file] [-s string] [files ...]
```
This change moves the FreeBSD-specific code before the check for the `sha256sum` command.
Fixes: #26774
Top commit has no ACKs.
Tree-SHA512: 2485e2e7d8fdca3b072b29fb22bbdfd69e520740537b331b33c64cc645b63da712cfa63a23bdf039bbc92a6558fc7bf03323a51784bf601ff360ff0ef59506c8
a3f5e541523a843e834df1858e16f89188fe19a2 test: Drop no longer needed `race:epoll_ctl` TSan suppression (Hennadii Stepanov)
Pull request description:
The removed suppression seems no needed.
I cannot point the exact commit/PR which makes this change possible.
Top commit has no ACKs.
Tree-SHA512: 8ee79cbdb2bc62796d72c69be4a818379132eae47be33951e8b9d224b049ff77e867004801c7cb0cc564a5374f318dafd9142b5231e9bd428f80acc75253933e
2ef5294a5bb68ceb3797d2638567a172cc21699f rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack)
734b9669ff7b2f5e2820993443a6f868f6b0b20a test: add getblockfrompeer coverage of invalid inputs (Jon Atack)
Pull request description:
The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs.
Fix all issues.
ACKs for top commit:
brunoerg:
ACK 2ef5294a5bb68ceb3797d2638567a172cc21699f
Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25
acf1315270 Merge bitcoin/bitcoin#25091: test: Remove extended lint (cppcheck) (MacroFake)
4dbdecdd1e refactor: rename builder-image -> build-image and builder as image name to dashcore-ci-runner (pasta)
ed8ffa7fb4 feat: have cppcheck linter respect CACHE_DIR env variable (pasta)
d1addb27aa fix: change fallback download path to be an s3 link which includes a few packages (pasta)
35c76705d1 feat: implement basic Github Actions based CI, which reuses underlying logic from GitLab CI (pasta)
Pull request description:
## Issue being fixed or feature implemented
We currently rely on GitLab for running CI, and while it has worked quite well, I am worried about having all of our eggs in one basket! As such, I've long wanted to explore implemeenting Github Actions based CI, but was too lazy! Well I finally spent some 60 commits trying to figure everything out, and this PR is the result of it.
As a result, we will now have two semi-redunant CI systems, the primary one on GitLab, and this one on Github Actions. Currently the GA based CI will only do one host, and does linting. Be aware this GA based CI does not actually run the tests, it does build depends, src and run linters on 1 host.
In the future, we should expand it from simply arm32 builds to having feature parity to GitLab.
While it appears the GA default runners are a bit slower than what we have on GitLab, there's a big difference, the GA runners are free :D If we decide to make the GA based CI primary, we'll probably want to setup some custom runners to have improved build speeds. Even still, a heavily cached build doing all linters took around 5 minutes if I recall correctly. Without caches I think it took maybe an hour, so defenitely not bad.
## What was done?
See the individual commits, they're pretty self explanatory
## How Has This Been Tested?
Lots of CI runs on my prior branch :) CI should run on this PR, and we should see how long it'll take w/o cache :D
## Breaking Changes
N/A - CI only
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: fef41d1b73fdeac29e5096ffa7fa26660efc44ac274376d5880b5fabf6b0e760aeda4a4ae9c24656ee0e3adb760459f0d45b955cefffc9d0eeb5384afbc9d473
BACKPORT NOTICE:
we keep and maintain cppcheck linter as lint-cppcheck-dash.sh
efae252f3072da598160670691757a0d60b9beb4 test: Remove extended lint (cppcheck) (laanwj)
Pull request description:
These are unreferenced in the CI and documentation, and have been since 2019 (see #17549).
I'm not sure the cppcheck is worthwhile. It takes a long time to run (I think this is why it isn't in the normal lints), and right
now it only appears to find implicit constructors. The list of exceptions is out of date. But if anyone wants to bring it back at any
time in the future they can do so from git history (and port it to Python).
ACKs for top commit:
fanquake:
ACK efae252f3072da598160670691757a0d60b9beb4
Tree-SHA512: 1a770b5d20ff1199d0d6bc471ae3d2c3438f0f0b169ce8d2fe73480daf8d3a7146c066b799afc90aa7898982c5fee79c1daca10e16e2bff0a7b38850aedd55b2
Reasonably enough I guess, it appears that a number of our dependencies block github runner IP addresses? the blobs stored at the s3 link were downloadable from the normal location when accessed locally, but returned a 404 from github runners. These blobs were also not present in the bitcoincore depends-sources anyhow.
