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: Merge bitcoin#21187, 21342,17350,19238,21210,21274,(partial)21053,20370,20437,21388 #5529

Merged
merged 10 commits into from
Aug 28, 2023

Conversation

vijaydasmp
Copy link

No description provided.

@vijaydasmp vijaydasmp changed the title backport : Merge #20385 backport: Merge bitcoin #20385 Aug 4, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin #20385 backport: Merge bitcoin#20385 Aug 4, 2023
@github-actions
Copy link

github-actions bot commented Aug 5, 2023

This pull request has conflicts, please rebase.

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20385 backport: Merge bitcoin#20385, 21187, 21342, 20692,17350,19800 Aug 6, 2023
@vijaydasmp vijaydasmp force-pushed the bp22_13 branch 8 times, most recently from 8690939 to bb1e149 Compare August 7, 2023 17:01
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20385, 21187, 21342, 20692,17350,19800 backport: Merge bitcoin#21187, 21342,17350,19238 Aug 8, 2023
@vijaydasmp vijaydasmp force-pushed the bp22_13 branch 2 times, most recently from a8b56fb to 8aaa7d1 Compare August 8, 2023 15:50
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21187, 21342,17350,19238 backport: Merge bitcoin#21187, 21342,17350,19238,21210,21274,21053 Aug 8, 2023
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

This pull request has conflicts, please rebase.

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21187, 21342,17350,19238,21210,21274,21053 backport: Merge bitcoin#21187, 21342,17350,19238,21210,21274,21053,20370,20663,20437 Aug 9, 2023
@vijaydasmp vijaydasmp force-pushed the bp22_13 branch 3 times, most recently from e6c9499 to cadbaa3 Compare August 9, 2023 14:08
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21187, 21342,17350,19238,21210,21274,21053,20370,20663,20437 backport: Merge bitcoin#21187, 21342,17350,19238,21210,21274,(partial)21053,20370,20437,21388 Aug 10, 2023
@vijaydasmp vijaydasmp marked this pull request as ready for review August 10, 2023 06:14
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.

one tiny issue, LGTM otherwise

@@ -20,8 +20,7 @@ source software which enables the use of this currency.
Pre-Built Binary
----------------

For more information, as well as an immediately usable, binary version of
the Dash Core software, see https://www.dash.org/downloads/.
For more information read the original Dash whitepaper.
Copy link

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Thanks I have corrected it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be not in chapter "Pre-Build Binary" but in chapter "What is Dash"?

Probably need to remove these lines:

Pre-Built Binary
----------------

btw, @UdjinM6 what's is an original Dash whitepaper?

Copy link

Choose a reason for hiding this comment

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

I think it should be not in chapter "Pre-Build Binary" but in chapter "What is Dash"?

Probably need to remove these lines:

Pre-Built Binary
----------------

Good catch!

btw, @UdjinM6 what's is an original Dash whitepaper?

https://github.com/dashpay/dash/wiki/Whitepaper

but it's quite outdated now... maybe we should mention/link to DIPs here instead

Copy link
Author

Choose a reason for hiding this comment

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

Updated as per comments

UdjinM6
UdjinM6 previously approved these changes Aug 10, 2023
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

@vijaydasmp
Copy link
Author

Hello @knst, @PastaPastaPasta , requesting review

@knst
Copy link
Collaborator

knst commented Aug 22, 2023

Hello @knst, @PastaPastaPasta , requesting review

@vijaydasmp see #5529 (comment)

@vijaydasmp
Copy link
Author

Hello @knst, @PastaPastaPasta , requesting review

@vijaydasmp see #5529 (comment)

will check

@vijaydasmp
Copy link
Author

Hello @knst , I have updated as per comments

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK

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

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merging via merge commit

MarcoFalke and others added 10 commits August 28, 2023 11:24
…_processing

3e68efa [net] Move checks from GetLocalAddrForPeer to caller (John Newbery)
d21d2b2 [net] Change AdvertiseLocal to GetLocalAddrForPeer (John Newbery)

Pull request description:

  This is the first part of bitcoin#21186. It slightly disentangles addr handling in net/net_processing by making it explicit that net_processing is responsible for pushing addr records into `vAddrToSend`.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3e68efa 🍅

Tree-SHA512: 9af50c41f5a977e2e277f24a589db38e2980b353401def5e74b108ac5f493d9b5d6b1b8bf15323a4d66321495f04bc271450fcef7aa7d1c095f051a4f8e9b15f
f1f63ac doc: Remove outdated comment (Hennadii Stepanov)

Pull request description:

  The removed commit has been wrong [since v0.20.0](bitcoin#18331).

ACKs for top commit:
  fanquake:
    ACK - f1f63ac

Tree-SHA512: ef6191fef389fa0ee5e6cf224e4990a1804aeefd1c3e9d9a4870cf46e1833fbb8701c379b6ce4e13caa02ae2f4f86778fa2c1e994c89392c08fcf01701482d7a
40f0564 doc: Add developer documentation to isminetype (HAOYUatHZ)

Pull request description:

  Closes: bitcoin#17217

ACKs for top commit:
  meshcollider:
    utACK 40f0564

Tree-SHA512: 156ff3bc02613d65aed5fcf50250ec3f3365b6c83c810763673ecfdd081a1310e5235be05f0c782638f191be61ad0028511392c40e4106a56eb1c6a3a8ab73b9
ae98aec refactor: Make CAddrMan::cs non-recursive (Hennadii Stepanov)
f5d1c7f Add AssertLockHeld to CAddrMan private functions (Hennadii Stepanov)
5ef1d0b Add thread safety annotations to CAddrMan public functions (Hennadii Stepanov)
b138973 refactor: Avoid recursive locking in CAddrMan::Clear (Hennadii Stepanov)
f79a664 refactor: Apply consistent pattern for CAddrMan::Check usage (Hennadii Stepanov)
187b7d2 refactor: Avoid recursive locking in CAddrMan::Check (Hennadii Stepanov)
f77d9c7 refactor: Fix CAddrMan::Check style (Hennadii Stepanov)
0670397 Make CAddrMan::Check private (Hennadii Stepanov)
efc6fac refactor: Avoid recursive locking in CAddrMan::size (Hennadii Stepanov)
2da9554 test: Drop excessive locking in CAddrManTest::SimConnFail (Hennadii Stepanov)

Pull request description:

  This PR replaces `RecursiveMutex CAddrMan::cs` with `Mutex CAddrMan::cs`.

  All of the related code branches are covered by appropriate lock assertions to insure that the mutex locking policy has not been changed by accident.

  Related to bitcoin#19303.

  Based on bitcoin#22025, and first three commits belong to it.

ACKs for top commit:
  vasild:
    ACK ae98aec

Tree-SHA512: c3a2d3d955a75befd7e497a802b8c10730e393be9111ca263ad0464d32fae6c7edf9bd173ffb6bc9bb61c4b39073a74eba12979d47f26b0b7b4a861d100942df
77772a1 doc: Rework internal and external links (MarcoFalke)

Pull request description:

  Some minor changes:
  * Move Bitcoin Core download link to the very top. *Reason:* The download link has nothing to do with the section that explains Bitcoin. Also, anyone quickly looking for the download link will find it faster.
  * Add a new link to the doc folder. *Reason*: Apart from the documentation that is shipped with the binary software, the doc folder is the primary location for Bitcoin Core related documentation.
  * Remove dead link to pdf. *Reason*: The pdf can be trivially found by asking a search engine.
  * Remove reference to "build server". *Reason*: There is no "build server". The CI system is explained in the next sentence in detail.
  * Remove dead? link to translation mailing list. *Reason*: The translation process is explained in `doc/translation_process.md`, no need to explain it in detail in the main readme.

ACKs for top commit:
  laanwj:
    ACK 77772a1
  RiccardoMasutti:
    ACK 77772a1

Tree-SHA512: 365824c6da519892ff239842b0c9525f559513eb800afd77b14febdc9d1a79294d9448df6cd25b623f1734ff859d9e20951e78fb8091d19fdadde0dd24eeddd4
5e531e6 assumptions: check C++17 assumption with MSVC (fanquake)
c7b4648 assumptions: assume a C++17 compiler (fanquake)

Pull request description:

  This has been the case since bitcoin#20413.

  This should also enable the check for MSVC. From my reading of https://docs.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-160 and https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ if we set the `/Zc:__cplusplus` switch in additional options, MSVC will report the correct value for `__cplusplus`. However I have not tested this.

ACKs for top commit:
  laanwj:
    Code review ACK 5e531e6
  hebasto:
    ACK 5e531e6, checked the MS docs, and AppVeyor build is green.
  practicalswift:
    ACK 5e531e6

Tree-SHA512: a4fb525cf5c33abc944c614edb0313a39c8a39a1637a03c09342c15ba0925f4eb037062e65e51b42ade667506b7e554c7159acf86e6b8c35d0a87dd79a6f239b
…ckhash as optional

ba7e17e rpc, test: document {previous,next}blockhash as optional (Sebastian Falbesoner)

Pull request description:

  This PR updates the result help of the following RPCs w.r.t. the `previousblockhash` and `nextblockhash` fields:
  - getblockheader
  - getblock

  Also adds trivial tests on genesis block (should not contain "previousblockhash") and best block (should not contain "nextblockhash").

Top commit has no ACKs.

Tree-SHA512: ef42c5c773fc436e1b4a67be14e2532e800e1e30e45e54a57431c6abb714d2c069c70d40ea4012d549293b823a1973b3f569484b3273679683b28ed40abf46bb
fabce45 fuzz: version handshake (MarcoFalke)

Pull request description:

  Not fuzzing the version handshake will limit fuzz coverage

ACKs for top commit:
  practicalswift:
    cr ACK fabce45: patch looks very much correct

Tree-SHA512: 4091d27d39edee781d033e471b352084bb54df250d0890e4821a325926a44dff9b26a2614d67dd0529f73bd366b075d7a0a1a570c2837de286a1b93a59a8fb91
…ing harnesses by using mocked GetTime()

8c09c0c fuzz: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked GetTime() (practicalswift)

Pull request description:

  Avoid time-based "non-determinism" in fuzzing harnesses by using mocked `GetTime()`.

  Prior to this commit the fuzzing harnesses `banman`, `connman`, `net` and `rbf` had time-based "non-determinism". `addrman` is fixed in bitcoin#20425. `process_message` and `process_messages` are left to fix: simply using mock time is not enough for them due to interaction with `IsInitialBlockDownload()`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for 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:
    review ACK 8c09c0c
  practicalswift:
    > review ACK [8c09c0c](bitcoin@8c09c0c)

Tree-SHA512: 32dfbead3dfd18cf4ff56dc2ea341aa977441b4e19a54879cf54fa5820c7e2b14b92c7e238d32fd785654f3b28cc82826ae66c03e94c292633c63c41196ba9a8
fad0ae6 doc: Rename fuzz seed_dir to corpus_dir (MarcoFalke)

Pull request description:

  The fuzz corpus directory might contain hand-crafted seeds, but generally it is a set of test inputs. See also https://github.com/google/fuzzing/blob/master/docs/glossary.md#corpus

ACKs for top commit:
  practicalswift:
    cr ACK fad0ae6: patch looks correct and "why not?" :)
  fanquake:
    ACK fad0ae6 - did not test

Tree-SHA512: 38c952feb07aeeeb038b3261a12c824fab9ce5153d75f0ecf6d3f43db4f50998eeb2b14b11b7155f529189c93783fa2c11c81059021a04398c43f3505b31a2d4
@PastaPastaPasta PastaPastaPasta merged commit 4ce7ed8 into dashpay:develop Aug 28, 2023
5 checks passed
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.

7 participants