Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: trivial 2024 06 05 #6047

Merged
17 commits merged into from
Jun 7, 2024

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Trivial batch of backports

What was done?

trivial backports

How Has This Been Tested?

Unit tests ran; waiting on CI

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
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

5202bd1 test: Bump shellcheck version to 0.8.0 (Hennadii Stepanov)

Pull request description:

  Among [added](https://github.com/koalaman/shellcheck/blob/master/CHANGELOG.md#v080---2021-11-06) rules, SC2295 could be [useful](bitcoin#23506 (comment)) for us.

ACKs for top commit:
  dongcarl:
    Code Review ACK 5202bd1
  fanquake:
    ACK 5202bd1 - would have rather this just been a part of bitcoin#23506 to avoid another PR and pointless rebasing.

Tree-SHA512: fd7ff801c71af03c5a5b2823b7daba25a430b3ead5e5e50a3663961ee2223e55d322aec91d79999814cd35bd7ed6e9415a0b797718ceb8c0b1dbdbb40c336b82
@PastaPastaPasta PastaPastaPasta added this to the 21 milestone Jun 5, 2024
@PastaPastaPasta PastaPastaPasta changed the base branch from master to develop June 5, 2024 14:19
test/functional/data/rpc_decodescript.json Outdated Show resolved Hide resolved
test/functional/README.md Outdated Show resolved Hide resolved
test/functional/feature_maxtipage.py Outdated Show resolved Hide resolved
MarcoFalke and others added 16 commits June 6, 2024 22:57
fa72dd3 fuzz: Move ISO8601 to one place (MarcoFalke)

Pull request description:

  Seems confusing to split this to two places.

  Also fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=42178

ACKs for top commit:
  fanquake:
    ACK fa72dd3

Tree-SHA512: 637b0671078848ea417fdf66b92715602040fad34d4ca5f7b843a519a1cfeebe5d992a79a399deba39926905125681d66ab0dc05f66f79a26f3bf555e12fb0ba
0a1b6fa test: fix intermittent timeouts in p2p_timeouts.py (Martin Zumsande)

Pull request description:

  Fixes  bitcoin#23800 by making sure that all peers are connected (i.e. `m_connected` is set) before the mocktime is bumped.
  We can't wait for verack here, but we can wait for a debug log entry ("Added connection peer=2") instead.

  In the failed CI runs (e.g. https://cirrus-ci.com/task/5600553806856192?logs=ci#L7469)  different peers were added at different mocktimes.

ACKs for top commit:
  naumenkogs:
    ACK 0a1b6fa
  theStack:
    Concept and approach ACK 0a1b6fa

Tree-SHA512: 1a3c8a9a79339d4adc6ecb1731eb0d0eadb2e5024ad3c6779b4696691f85d6c3304ef8689746d0332150a4cf04489ca4b2ff3eeb0bb76feec28c1e4bb9dbca19
1268532 test: add functional test for -startupnotify (Bruno Garcia)

Pull request description:

  This PR adds a functional test for -startupnotify. It basically starts the node passing a command on -startupnotify to create a file on tmp and then, we check if the file has been successfully created.

ACKs for top commit:
  theStack:
    Tested ACK 1268532
  kristapsk:
    re-ACK 1268532

Tree-SHA512: 5bf3e46124ee5c9d609c9993e6465d5a71a8d2275dcf07c8ce0549f013f7f8863d483b46b7164152f566468a689371ccb87f01cf118c3c9cac5b2be673b61a5c
…le in startupnotify test

96eb009 test: wait rather than assert presence of file in startupnotify test (fanquake)

Pull request description:

  Should fix bitcoin#23967.

ACKs for top commit:
  brunoerg:
    crACK 96eb009
  kristapsk:
    utACK 96eb009

Tree-SHA512: 9107970e45c027cfc6c6cbfcfd5a7d9f5956259bbbb11f5b180c3947126e42e62c0f8ffd69cf7b39b51c9c5b4fedbb753839d59aebe876be68c1484bb6065819
…the correct size

ac617cc wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)

Pull request description:

  After parsing the checksum, make sure that it is the size that we expect it to be.

  This issue was reported by Pedro Baptista.

ACKs for top commit:
  laanwj:
    Code review ACK ac617cc

Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
86a4a15 Highlight DNS request part (Prayank)

Pull request description:

  _What?_

  Highlight DNS requests part in Proxy section

  _Why?_

  1. DNS requests are very important while considering privacy
  2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now
  3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools

  _How?_

  Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](bitcoin#21157 (comment)). Harding suggested using [bold/italic in `<pre></pre>`](bitcoin#21157 (comment)). I have used the suggestions from previous PR and added `---`

  This is a part of alternative described in bitcoin#22316

ACKs for top commit:
  jonatack:
    ACK 86a4a15
  Rspigler:
    ACK 86a4a15
  achow101:
    ACK 86a4a15
  RiccardoMasutti:
    ACK 86a4a15
  lsilva01:
    ACK bitcoin@86a4a15
  kristapsk:
    ACK 86a4a15
  theStack:
    ACK 86a4a15

Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
…ionStr() helper

6f2593d gui, refactor: use std::chrono for formatDurationStr() helper (Jon Atack)

Pull request description:

  Updates `formatDurationStr()` to use the `chrono` standard lib. No change in behavior.

ACKs for top commit:
  RandyMcMillan:
    tACK 6f2593d
  shaavan:
    ACK 6f2593d
  w0xlt:
    tACK 6f2593d on Ubuntu 21.10 Qt 5.15.2
  promag:
    Code review ACK 6f2593d.

Tree-SHA512: 61e9afdb1db779150df338e6af08727c34f69639add465c2f7003ff775d97dce3e78e78d325bc6dea5bc13f0fce9ef1c3506d13f1661a5e083e52bba8a32ba44
f59bee3 fuzz: execute each file in dir without fuzz engine (Anthony Towns)

Pull request description:

  Phony fuzzing (phuzzing)! Run the fuzz testing code against known inputs to detect errors. Advantage is you can easily test using the existing qa-assets datasets without having to compile with fuzzing enabled; disadvantage is that it doesn't do any actual fuzzing.

  Example usage:

  ```
  $ for a in ${QA_ASSETS}/fuzz_seed_corpus/*; do echo ${a##*/}; done | xargs -P8 -I {} /bin/sh -c "FUZZ={} test/fuzz/fuzz ${QA_ASSETS}/fuzz_seed_corpus/{}"
  No fuzzer for address_deserialize.
  No fuzzer for addrdb.
  No fuzzer for banentry_deserialize.
  addition_overflow: succeeded against 848 files in 0s.
  asmap: succeeded against 981 files in 0s.
  checkqueue: succeeded against 211 files in 0s.
  ...
  ```

  (`-P8` says run 8 of the tasks in parallel)

  If there are failures, the first one will be reported and the program will abort with output like:

  ```
  fuzz: test/fuzz/versionbits.cpp:336: void (anonymous namespace)::versionbits_fuzz_target(FuzzBufferType): Assertion `exp_state != ThresholdState::FAILED' failed.
  Error processing seed "corpus/versionbits/35345ae8e722234095810b1117a29b63af7621af"
  ```

  Rebase of bitcoin#22763, which was a rebase of bitcoin#21496, but also reports the name of the fuzzer and the time taken.

  Fixes bitcoin#21461

Top commit has no ACKs.

Tree-SHA512: d8d046d4a309652eb13de42116276bf992480bc887ad3535a8ff18b354cb24826bc562b06af63802ec945c637f046563b6a5601d6321b46a5543127daafea09b
…x raises an error

d6bc232 test: -peerblockfilters without -blockfilterindex raises an error (brunoerg)

Pull request description:

  This PR adds test coverage for the following init error:
  https://github.com/bitcoin/bitcoin/blob/2a3e8fb3592e42300ec96c9f6724e15346e30ea7/src/init.cpp#L850

  Setting -peerblockfilters without -blockfilterindex should raise an error when initializing.

ACKs for top commit:
  ccdle12:
    Tested ACK bitcoin@d6bc232

Tree-SHA512: e740c2ccde6bb1bb8381bb676a6d01bd5746cf9ce0c8dadd62067a6b9b380027bfe8b8cdeae9846a0ab18385f3dc5dff607fe5274cb55107d47470db00015fb2
9d65ad3 Clear vTxHashes when mapTx is cleared (Peter Bushnell)

Pull request description:

  vTxHashes is a vector of all entries in mapTx, if you clear one you should clear the other, lest someone try to use the txiter in vTxHashes which would result in a segfault.

ACKs for top commit:
  laanwj:
    Code review ACK 9d65ad3

Tree-SHA512: 6832755e43ab7f528b46817aeadcb6ffdc965b97f59ab96bb053dedbb7e68155ba3db52286355dca33b509237f80eda249760b26db493762bc50d8e2cef16d8f
…ng returned

0cea7b1 print `(none)` if no warnings in -getinfo (/dev/fd0)

Pull request description:

  Adds `(none)` in warnings when no warnings returned by -getinfo

  Reviewers can test this by making the following change in `/src/warnings.cpp`:

  ```diff
  bilingual_str GetWarnings(bool verbose)
  {
      bilingual_str warnings_concise;
      std::vector<bilingual_str> warnings_verbose;

      LOCK(g_warnings_mutex);

      // Pre-release build warning
      if (!CLIENT_VERSION_IS_RELEASE) {
  -        warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");;
  +        warnings_concise = _("");;
  ```

  Before this pull request:

  ```
  $ bitcoin-cli -getinfo
  Chain: regtest
  Blocks: 0
  Headers: 0
  Verification progress: 100.0000%
  Difficulty: 4.656542373906925e-10

  Network: in 0, out 0, total 0
  Version: 239900
  Time offset (s): 0
  Proxies: n/a
  Min tx relay fee rate (BTC/kvB): 0.00001000

  Warnings:
  ```

  After this pull request:

  ```diff
  $ bitcoin-cli -getinfo
  Chain: regtest
  Blocks: 0
  Headers: 0
  Verification progress: 100.0000%
  Difficulty: 4.656542373906925e-10

  Network: in 0, out 0, total 0
  Version: 239900
  Time offset (s): 0
  Proxies: n/a
  Min tx relay fee rate (BTC/kvB): 0.00001000

  Warnings: (none)
  ```

ACKs for top commit:
  jonatack:
    ACK 0cea7b1
  laanwj:
    Tested ACK 0cea7b1

Tree-SHA512: a12499d11ff84bc954db354f968eb1f5ee4999d8b80581fe0bdf604732b2e2f608cb5c35c4ca8cb5a430f3991954a6207f0758302618662e6b9505044cf2dc95
3ae7791 refactor: use Span in random.* (pasta)

Pull request description:

  ~This PR does two things~
  1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes

  ~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
  This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~

  MarcoFalke this was inspired by your comment here: bitcoin#24185 (comment) about using Span, so hopefully I'll be able to get this PR done and merged 😂

  ~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~

ACKs for top commit:
  laanwj:
    Thank you! Code review re-ACK 3ae7791

Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
…rtions.py

172c233 Porting lint-assertions.sh to lint-assertions.py (hiago)

Pull request description:

  This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of bitcoin#24783.

ACKs for top commit:
  laanwj:
    Tested ACK 172c233

Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
4637bbe rpc: Explain active and internal in listdescriptors (Andrew Chow)

Pull request description:

  The current help text for active and internal in listdescriptors is not particularly helpful. They require the reader to already know what those terms mean. This help text is updated to actually explain the definitions of those words in context of a descriptor wallet.

ACKs for top commit:
  S3RK:
    ACK 4637bbe
  jarolrod:
    ACK 4637bbe
  w0xlt:
    ACK bitcoin@4637bbe

Tree-SHA512: 0af2c04f3b9920799cf616ad618bde9248eb9f74cc28f443b5b0f6646deba76e9b1415aca0865ad3bcc24aa6af0e9d07ad7b7cd80f0fe80838cf847f1b944426
88044a1 Guard `#include <config/bitcoin-config.h>` (Hennadii Stepanov)

Pull request description:

  A fix for builds when the `HAVE_CONFIG_H` macro is not defined.

ACKs for top commit:
  Empact:
    Code Review ACK bitcoin@88044a1

Tree-SHA512: f2bf1693c7671d7113dccaf66ae34a84719d86cb3271fa18b36611deab93a48d787b3ccfbd735d3b763017d709971cb1151d8d7f30390720009e6e2a6275b5b0
…n::SweepBanned()`

ab75388 refactor: Remove redundant scope in `BanMan::SweepBanned()` (Hennadii Stepanov)
52c0b3e refactor: Add thread safety annotation to `BanMan::SweepBanned()` (Hennadii Stepanov)
3919059 refactor: Move code from ctor into private `BanMan::LoadBanlist()` (Hennadii Stepanov)

Pull request description:

  This PR adds a proper thread safety annotation to `BanMan::SweepBanned()`.

  Also a simple refactoring applied.

ACKs for top commit:
  ajtowns:
    ACK ab75388
  w0xlt:
    ACK bitcoin@ab75388
  theStack:
    Code-review ACK ab75388

Tree-SHA512: 8699079c308454f3ffe72be2e77f0935214156bd509f9338b1104f8d128bfdd02ee06ee8c1c99b2eefdf317a00edd555d52990caaeb1ed4540eedc1e3c9d1faf
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@PastaPastaPasta PastaPastaPasta closed this pull request by merging all changes into dashpay:develop in d7413ff Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants