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#19237,19378,18422 #5143

Merged
merged 3 commits into from
Apr 15, 2023

Conversation

vijaydasmp
Copy link

@vijaydasmp vijaydasmp commented Jan 7, 2023

No description provided.

@vijaydasmp vijaydasmp changed the title scripted-diff:Merge #19114: TxoutType C++11 scoped enum class backport : Merge bitcoin#19114 Jan 7, 2023
@vijaydasmp vijaydasmp changed the title backport : Merge bitcoin#19114 backport : Merge bitcoin#19237,19090,19378,19366 Jan 9, 2023
@vijaydasmp vijaydasmp changed the title backport : Merge bitcoin#19237,19090,19378,19366 backport: Merge bitcoin#19237,19090,19378,19366 Jan 9, 2023
@vijaydasmp vijaydasmp force-pushed the bp21_10 branch 2 times, most recently from 0a2b7b7 to 50e3698 Compare January 18, 2023 11:34
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#19237,19090,19378,19366 backport: Merge bitcoin#19237,19090,19378,19366,19272 Jan 19, 2023
@vijaydasmp vijaydasmp force-pushed the bp21_10 branch 4 times, most recently from a13864b to 8ac20b3 Compare January 19, 2023 12:17
@github-actions
Copy link

This pull request has conflicts, please rebase.

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#19237,19090,19378,19366,19272 backport: Merge bitcoin#19237,19378,19366,19272,19090,18422,18628,18653 Feb 19, 2023
@vijaydasmp vijaydasmp marked this pull request as ready for review February 24, 2023 07:19
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#19237,19378,19366,19272,19090,18422,18628,18653 backport: Merge bitcoin#19237,19378,19366,19272,18422 Feb 24, 2023
src/test/fuzz/fuzz.cpp Outdated Show resolved Hide resolved
test/functional/p2p_invalid_messages.py Outdated Show resolved Hide resolved
src/script/interpreter.cpp Outdated Show resolved Hide resolved
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 vijaydasmp changed the title backport: Merge bitcoin#19237,19378,19366,19272,18422 backport: Merge bitcoin#19237,19378,18422 Mar 26, 2023
@vijaydasmp
Copy link
Author

Hello @PastaPastaPasta , please review

@UdjinM6 UdjinM6 added this to the 20 milestone Mar 29, 2023
knst
knst previously approved these changes Apr 12, 2023
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

@UdjinM6
Copy link

UdjinM6 commented Apr 15, 2023

@PastaPastaPasta

MarcoFalke added 3 commits April 15, 2023 12:14
37ae687 Add tests for CPubKey serialization/unserialization (Elichai Turkel)
9b8907f Check size after Unserializing CPubKey (Elichai Turkel)

Pull request description:

  Found by practicalswift, closes bitcoin#19235
  Currently all the public API(except the pointer-like API) in CPubKey that sets/constructs a pubkey goes through `CPubKey::Set` which checks if that the length and size match and if not invalidates the key.

  This adds the same check to `CPubKey::Unserialize`, sadly I don't see an easy way to just push this to the existing checks in `CPubKey::Set` but it's only a simple condition.

  The problem with not invalidating is that if you write a pubkey like: `{0x02,0x00}` it will think the actual length is 33(because of `size()`) and will access uninitialized memory if you call any of the functions on CPubKey.

ACKs for top commit:
  practicalswift:
    re-ACK 37ae687
  jonatack:
    Code review re-ACK 37ae687 per `git diff eab8ee3 37ae687` only change since last review at eab8ee3 is passing the `pubkey` param by reference to const instead of by value in `src/test/key_tests.cpp::CmpSerializationPubkey`
  MarcoFalke:
    ACK 37ae687

Tree-SHA512: 30173755555dfc76d6263fb6a59f41be36049ffae7b4e1b92b922d668f5e5e2331f7374d5fa10d5d59fc53020d2966156905ffcfa8b8129c1f6d0ca062174ff1
…ed_transactions

1307686 refactor: Use Mutex type for g_cs_recent_confirmed_transactions (Hennadii Stepanov)

Pull request description:

  No need the `RecursiveMutex` type for the `g_cs_recent_confirmed_transactions`.

  Related to bitcoin#19303.

ACKs for top commit:
  MarcoFalke:
    ACK 1307686
  vasild:
    ACK 1307686

Tree-SHA512: 67f1be10c80ec18d0f80b9f5036e5a20986314da9b9364ef4e193ad1d9f3f4c8e4c2e16253ca79d649ff602d5b8c2aff58d7dd1085841afb760479a4875cffbe
…valScript code to EvalChecksig

14e8cf9 [consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig (Pieter Wuille)

Pull request description:

  This is another small refactor pulled out of the Schnorr/Taproot PR bitcoin#17977.

  This is in preparation for adding different signature verification rules,
  specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad
  as Schnorr signature verifications.

ACKs for top commit:
  sipa:
    ACK 14e8cf9, verified move-only.
  MarcoFalke:
    ACK 14e8cf9, reviewed with "git show 14e8cf9 --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space -W" 👆
  fjahr:
    Code-review ACK 14e8cf9, verified that it's move-only.
  instagibbs:
    code review ACK bitcoin@14e8cf9, verified move-only
  theStack:
    Code-Review ACK bitcoin@14e8cf9
  jonatack:
    ACK 14e8cf9

Tree-SHA512: af2efce9ae39d5ec01db5b9ef0ff383fe252ef5f33b3483927308ae17d91a619266cb45951f32ea1ce54807a4c0f052bcdefb47e244465d3a726393221c227b1
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

@PastaPastaPasta PastaPastaPasta merged commit 4b4d609 into dashpay:develop Apr 15, 2023
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.

4 participants