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#21718, 21759, 21872 #5989

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

vijaydasmp
Copy link

bitcoin backports

@vijaydasmp vijaydasmp changed the title backport: backport:Merge bitcoin#21718,21679 Apr 22, 2024
@vijaydasmp vijaydasmp changed the title backport:Merge bitcoin#21718,21679 backport: Merge bitcoin#21718, 21679 Apr 22, 2024
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21718, 21679 backport: Merge bitcoin#21718 Apr 23, 2024
@vijaydasmp vijaydasmp force-pushed the bp22_34 branch 2 times, most recently from 8acabd4 to f2c8fd1 Compare April 24, 2024 02:07
Copy link

This pull request has conflicts, please rebase.

…datatype.

a411494 rpc: Improve getblock error message for invalid data type. (klementtan)

Pull request description:

  Improve error messages for getblock invalid datatype.

  fixes: bitcoin#21717

ACKs for top commit:
  instagibbs:
    utACK bitcoin@a411494
  theStack:
    ACK a411494
  promag:
    Code review ACK a411494.

Tree-SHA512: 6e7d8290681e8ab375629f81669d0f8e0c21f9eb7ed9e2455cd19ea013e69b2d95fa7a9ee795315b2d5c60c96035c6cefc3d6e1039a06fd88c1dc7fe275ee6a1
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21718 backport: Merge bitcoin#21718, 21169, 21759, 21872 Apr 25, 2024
@vijaydasmp vijaydasmp force-pushed the bp22_34 branch 2 times, most recently from d62502c to be90adf Compare April 26, 2024 01:55
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#21718, 21169, 21759, 21872 backport: Merge bitcoin#21718, 21759, 21872 Apr 26, 2024
@vijaydasmp vijaydasmp marked this pull request as ready for review May 5, 2024 06:42
src/wallet/coinselection.h Outdated Show resolved Hide resolved
src/wallet/coinselection.h Show resolved Hide resolved
src/wallet/coinselection.h Show resolved Hide resolved
src/wallet/wallet.cpp Outdated Show resolved Hide resolved
src/wallet/wallet.h Show resolved Hide resolved
fanquake and others added 2 commits May 14, 2024 06:03
6ba8921 refactor + document coin selection strategy (glozow)
58ea324 [docs] add doxygen comments to wallet code (glozow)
0c74716 [docs] format existing comments as doxygen (glozow)

Pull request description:

  I think it would help code review to have more documentation + doxygen comments

ACKs for top commit:
  Xekyo:
    ReACK bitcoin@6ba8921
  achow101:
    ACK 6ba8921

Tree-SHA512: 74a78d9b0e0c1d5659bed566432a5b3511511d8b2432f440565f443da7b8257a1b90e70aa7505a7f8abf618748eeb43d166e84f278bdee3d34ce5d5c37dc573a
09205b3 net: Clarify message header validation errors (W. J. van der Laan)
955eee7 net: Sanitize message type for logging (W. J. van der Laan)

Pull request description:

  - Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. I have checked all logging in `net.cpp`.

  - For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes (as hex) should be enough.

  - Update `p2p_invalid_messages.py` test to check this.

  - Improve error messages in a second commit.

  Issue reported by gmaxwell.

ACKs for top commit:
  MarcoFalke:
    re-ACK 09205b3 only change is log message fixup 🔂
  practicalswift:
    re-ACK 09205b3

Tree-SHA512: 8fe5326af135cfcf39ea953d9074a8c966b9b85a810b06a2c45b8a745cf115de4f321e72fc769709d6bbecfc5953aab83176db6735b04c0bc6796f59272cadce
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.

light ACK

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.

None yet

3 participants