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

Add Clone instance to RawNetworkMessage #463

Conversation

cloudhead
Copy link
Contributor

Very small change, but I've run into this too many times now. NetworkMessage has Clone, but not RawNetworkMessage. It's useful to have when there are multiple "consumers" of incoming network messages.

Copy link
Member

@elichai elichai left a comment

Choose a reason for hiding this comment

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

LGTM

@TheBlueMatt TheBlueMatt merged commit c8c187e into rust-bitcoin:master Sep 1, 2020
@cloudhead cloudhead deleted the cloudhead/clone-instance-raw-net-message branch September 1, 2020 19:51
Davidson-Souza pushed a commit to Davidson-Souza/rust-bitcoin that referenced this pull request Jul 12, 2023
9850550 Move AsRef impl block next to Index (Tobin C. Harding)
4d42e8e Derive Copy and Clone (Tobin C. Harding)
b38ae97 Implement stable comparison functionality (Tobin C. Harding)
630fc1f Remove len and is_empty from impl_array_newtype macros (Tobin C. Harding)
9788b6d Remove leading colons from impl_array_newtype methods (Tobin C. Harding)
2bb08c2 Remove as_[mut_]ptr from impl_array_newtype macros (Tobin C. Harding)
3e28070 Duplicate impl_array_newtype (Tobin C. Harding)
6358903 Add newline to end of file (Tobin C. Harding)

Pull request description:

  Supersedes rust-bitcoin#515

  The primary aim of this PR is to fix the fact that we currently implement various comparison traits (`Ord`, `PartialEq`) by comparing the inner byte arrays. These bytes are meant to be opaque and are not guaranteed across versions of `libsecp256k1`.

  This PR is a bit involved because I had to detangle all the various types (across both `secp256k1` and `secp256k1-sys`) that use the `impl_array_newtype` macro.

  - Patch 1: is trivial cleanup
  - Patch 2: Step one of the PR is duplicating the macro into both crates so we can tease them apart.
  - Patch 3-5: Are cleanup of the now duplicated `impl_array_newtype` macros
  - Patch 6: Is the meat and potatoes
  - Patch 7,8: Further minor clean ups to the macro

  I had a lot of fun with this PR, I hope you enjoy it too.

  Fix: rust-bitcoin#463

ACKs for top commit:
  apoelstra:
    ACK 9850550

Tree-SHA512: 160381e53972ff882ceb1d2d47bac56a7301a2d13bfe75d3f6ff658ab2c6fbe516ad856587c4d23f98524205918ca1a5f9b737e35c23c7a01b476c92d8d1792f
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

5 participants