Skip to content

Move p2p encodings from encode and into p2p - #4571

Merged
apoelstra merged 2 commits into
rust-bitcoin:masterfrom
rustaceanrob:p2p-encode-5-27
May 31, 2025
Merged

apoelstra merged 2 commits into
rust-bitcoin:masterfrom
rustaceanrob:p2p-encode-5-27

Conversation

@rustaceanrob

Copy link
Copy Markdown
Member

To move p2p into a crate:

  1. All Encodable implementations for p2p must be defined in p2p. To accomplish this, and anything else in the future related to encoding, I added a deser module with the necessary macro to implement Encodable for Vec<T> where T: Encodable.
  2. Because the orphan rule would apply for implementing Encodablefor Vec within p2p, three types defined within p2p are used to wrap the Vec and snake around the orphan rule.

In preparation to move `p2p` to its own crate, any `Vec<T>` that satisfies
`T: Encodable` will not trivially implement `Encodable` because of the
orphan rule. A type defined within p2p may implement `Encodable`,
however, and the simplest possible type is one that simply wraps the
vector. Three types that will implement `Encodable` within `p2p` are
added here.
All of the `Encodable` implementations are defined within `p2p` with the
exception of `p2p` types that are a `Vec`. To fully decouple `p2p` from
`encode` we can define these in `p2p` like the others. This is the final
step in removing anything `p2p` related from the rest of `bitcoin`.
@github-actions github-actions Bot added the C-bitcoin PRs modifying the bitcoin crate label May 27, 2025
@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15277079593

Details

  • 31 of 31 (100.0%) changed or added relevant lines in 3 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 84.65%

Files with Coverage Reduction New Missed Lines %
bitcoin/src/psbt/mod.rs 7 86.0%
Totals Coverage Status
Change from base Build 15260698759: -0.02%
Covered Lines: 23592
Relevant Lines: 27870

💛 - Coveralls

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 20779bd

@rustaceanrob rustaceanrob changed the title Remove p2p encodings from encode and into p2p Move p2p encodings from encode and into p2p May 31, 2025
@apoelstra

Copy link
Copy Markdown
Member

Nice. Will ACK and merge. Hopefully we will be able to move encoding stuff into its own crate where we can expose the impl_vec wrapper. The wrapper types I could go either way on ... on the one hand they feel like noise, but on the other hand we are saying "these are distinct objects with distinct encodings ... that happen to all be identical to what the encoding for Vec would be if we had it". So I dunno.

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 20779bd; successfully ran local tests

@apoelstra
apoelstra merged commit 2113a32 into rust-bitcoin:master May 31, 2025
@rustaceanrob
rustaceanrob deleted the p2p-encode-5-27 branch June 1, 2025 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bitcoin PRs modifying the bitcoin crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants