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#15367, 17458, 18309, 18965, 16525 #5728

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

vijaydasmp
Copy link

@vijaydasmp vijaydasmp commented Nov 22, 2023

Issue being fixed or feature implemented

bitcoin backports

What was done?

Backported bitcoin changes

How Has This Been Tested?

CI Run

@vijaydasmp vijaydasmp changed the title Bp21 20 backport: Merge bitcoin#15367, 17458, 18309 Nov 22, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#15367, 17458, 18309 backport: Merge bitcoin#15367, 17458, 18309, 18814,18850 Nov 25, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#15367, 17458, 18309, 18814,18850 backport: Merge bitcoin#15367, 17458, 18309, 18814 Nov 25, 2023
@vijaydasmp vijaydasmp force-pushed the bp21_20 branch 2 times, most recently from f4ced33 to 4ee6a03 Compare November 26, 2023 02:36
@vijaydasmp
Copy link
Author

Low Diskspace error

�[0;36m test 2023-11-26T03:14:11.472000Z TestFramework (ERROR): �[0m

�[0;34m node0 stderr Error: Disk space is too low! �[0mEarly exiting after test failure

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#15367, 17458, 18309, 18814 backport: Merge bitcoin#15367, 17458, 18309, 18814, 18965 Nov 27, 2023
@vijaydasmp vijaydasmp marked this pull request as ready for review November 27, 2023 15:01
@vijaydasmp
Copy link
Author

Hello @UdjinM6 @knst requesting review

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#15367, 17458, 18309, 18814, 18965 backport: Merge bitcoin#15367, 17458, 18309, 18965 Nov 28, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#15367, 17458, 18309, 18965 backport: Merge bitcoin#15367, 17458, 18309, 18965, 16525 Nov 28, 2023
@vijaydasmp vijaydasmp marked this pull request as draft November 28, 2023 10:52
@vijaydasmp vijaydasmp force-pushed the bp21_20 branch 2 times, most recently from 6982d6c to 92366a9 Compare November 29, 2023 04:59
@vijaydasmp vijaydasmp marked this pull request as ready for review November 29, 2023 08:37
test/functional/test_runner.py Outdated Show resolved Hide resolved
doc/release-notes-16525.md Outdated Show resolved Hide resolved
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
src/core_write.cpp Outdated Show resolved Hide resolved
test/functional/rpc_rawtransaction.py Outdated Show resolved Hide resolved
doc/release-notes-15367.md 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

@UdjinM6 UdjinM6 added this to the 20.1 milestone Dec 2, 2023
@vijaydasmp
Copy link
Author

Hello @PastaPastaPasta , Requesting review

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
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 4 commits December 6, 2023 12:33
…p command

090530c feature: Added ability for users to add a startup command (Ben Carman)

Pull request description:

  Thoughts for adding the feature is for users to be able to add things like electrum-personal-server or lnd to run whenever Bitcoin Core is running.  Open to feedback about the feature.

ACKs for top commit:
  MarcoFalke:
    re-ACK 090530c
  dongcarl:
    tACK 090530c

Tree-SHA512: ba514d2fc8b4fb12b781c1a9c89845a25fce0b80ba7c907761cde4abb81edd03fa643682edc895986dc20b273ac3b95769508806db7fbd99ec28623f85c41e67
e66870c zmq: Append address to notify log output (nthumann)
241803d test: Add zmq test to support multiple interfaces (nthumann)
a0b2e5c doc: Add release notes to support multiple interfaces (nthumann)
b1c3f18 doc: Adjust ZMQ usage to support multiple interfaces (nthumann)
347c94f zmq: Add support to listen on multiple interfaces (Nicolas Thumann)

Pull request description:

  This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server.
  Currently, if you specify more than one e.g. `zmqpubhashblock` paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. `zmqpubhashblock=0.0.0.0:28332`), which can result in an increased attack surface.
  With this PR a user can specify multiple interfaces to listen on, e.g.
  `-zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332`.

ACKs for top commit:
  laanwj:
    Code review ACK e66870c
  instagibbs:
    reACK bitcoin@e66870c

Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
60ed339 tests: implement base58_decode (10xcryptodev)

Pull request description:

  implements TODO: def base58_decode

ACKs for top commit:
  ryanofsky:
    Code review ACK 60ed339. Just suggested changes since last review. Thank you for taking suggestions!

Tree-SHA512: b3c06b4df041a6d88033cd077a093813a688e42d0b9aa777c715e5fd69cfba7b1bf984428bd98417d3c15232d3d48bc9c163317564f9e1d562db6611c21e2c10
…in RPC/TxToUniv

e80259f Additionally treat Tx.nVersion as unsigned in joinpsbts (Matt Corallo)
970de70 Dump transaction version as an unsigned integer in RPC/TxToUniv (Matt Corallo)

Pull request description:

  Consensus-wise we already treat it as an unsigned integer (the
  only rules around it are in CSV/locktime handling), but changing
  the underlying data type means touching consensus code for a
  simple cleanup change, which isn't really worth it.

  See-also, rust-bitcoin/rust-bitcoin#299

ACKs for top commit:
  sipa:
    ACK e80259f
  practicalswift:
    ACK e80259f
  ajtowns:
    ACK e80259f code review -- checked all other uses of tx.nVersion treat it as unsigned (except for policy.cpp:IsStandard anyway), so looks good.
  naumenkogs:
    ACK e80259f

Tree-SHA512: 6760a2c77e24e9e1f79a336ca925f9bbca3a827ce02003c71d7f214b82ed3dea13fa7d9f87df9b9445cd58dff8b44a15571d821c876f22f8e5a372a014c9976b
@PastaPastaPasta PastaPastaPasta merged commit 5211a00 into dashpay:develop Dec 6, 2023
5 checks passed
PastaPastaPasta added a commit that referenced this pull request Aug 31, 2024
, bitcoin#21008, bitcoin#21310, bitcoin#22079, bitcoin#23471, bitcoin#24218 (zmq backports)

b75e83b merge bitcoin#24218: Fix implicit-integer-sign-change (Kittywhiskers Van Gogh)
8ecc22f merge bitcoin#23471: Improve ZMQ documentation (Kittywhiskers Van Gogh)
2965093 merge bitcoin#22079: Add support to listen on IPv6 addresses (Kittywhiskers Van Gogh)
3ac3714 merge bitcoin#21310: fix sync-up by matching notification to generated block (Kittywhiskers Van Gogh)
7b0c725 merge bitcoin#21008: fix zmq test flakiness, improve speed (Kittywhiskers Van Gogh)
5e87efd merge bitcoin#20523: deduplicate 'sequence' publisher message creation/sending (Kittywhiskers Van Gogh)
99c730f merge bitcoin#20953: dedup zmq test setup code (node restart, topics subscription) (Kittywhiskers Van Gogh)
982c1f0 merge bitcoin#19572: Create "sequence" notifier, enabling client-side mempool tracking (Kittywhiskers Van Gogh)
b0b4e0f zmq: Make `g_zmq_notification_interface` a smart pointer (Kittywhiskers Van Gogh)
0a1ffd3 zmq: extend appending address to log msg for Dash-specific notifications (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * [bitcoin#19572](bitcoin#19572) introduces tests in `interface_zmq.py` that validate newly introduced "sequence" reporting of all message types (`C`, `D`, `R` and `A`). The `R` message type (removed from mempool) is tested by leveraging the RBF mechanism, which isn't present in Dash.

    In order to allow the tests to successfully pass, all fee bumping and RBF-specific code had to be removed from the tests. This test also involves creating a new block to test the `C` message (connected block) that would return a `time-too-new` error and fail unless mocktime has been disabled (which has been done in this test).

  * When backporting [bitcoin#18309](bitcoin#18309) ([dash#5728](#5728)), Dash-specific ZMQ notifications did not have those changes applied to them and that particular backport was merged upstream *after* [bitcoin#19572](bitcoin#19572), meaning, for completion, the changes from [bitcoin#18309](bitcoin#18309) have been integrated into [bitcoin#19572](bitcoin#19572 backport.

    As for the Dash-specific notifications, they have been covered to ensure uniformity in a separate commit.

  * The ZMQ notification interface has been converted to a smart pointer in the interest of safety (this change is eventually done upstream in 8ed4ff8 ([bitcoin#27125](bitcoin#27125)))

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] 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 b75e83b
  PastaPastaPasta:
    utACK b75e83b
  knst:
    utACK b75e83b

Tree-SHA512: 9f860d1203bebe0914a5102f101f646873d14754830d651fb91ed0d1285a6c1a58ffc492b07d4768324d94f53171c9a4da974cf4a0b1e5c665979eace289f6f0
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.

5 participants