Skip to content

consensus_encoding: Remove prefix_read field#5079

Closed
tcharding wants to merge 1 commit into
rust-bitcoin:masterfrom
tcharding:push-olloplxptwnk
Closed

consensus_encoding: Remove prefix_read field#5079
tcharding wants to merge 1 commit into
rust-bitcoin:masterfrom
tcharding:push-olloplxptwnk

Conversation

@tcharding

@tcharding tcharding commented Oct 3, 2025

Copy link
Copy Markdown
Member

We can just use the existence of the optional compact size decoder to abstract over whether the prefix has been read or not.

We did this already in VecDecoder.

Internal change only, no logic change.

@github-actions github-actions Bot added C-internals PRs modifying the internals crate test C-consensus_encoding labels Oct 3, 2025
@tcharding tcharding force-pushed the push-olloplxptwnk branch 2 times, most recently from f1d61b7 to 7afe2ed Compare October 7, 2025 23:19
@github-actions github-actions Bot removed C-internals PRs modifying the internals crate test labels Oct 7, 2025
@tcharding tcharding changed the title consensus_encoding: Remove prefix_read field consensus_encoding: Remove prefix_read field Oct 7, 2025
@tcharding tcharding marked this pull request as ready for review October 7, 2025 23:20
We can just use the existence of the optional compact size decoder to
abstract over whether the prefix has been read or not.

We did this already for the `VecDecoder`.

Internal change only, no logic change.
@github-actions github-actions Bot added the C-consensus_encoding PRs modifying the consensus-encoding crate label Oct 7, 2025

@nyonson nyonson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK 305062d

@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 305062d; successfully ran local tests

@tcharding tcharding closed this Oct 9, 2025
apoelstra added a commit that referenced this pull request Oct 9, 2025
305062d consensus_encoding: Remove prefix_read field (Tobin C. Harding)

Pull request description:

  We can just use the existence of the optional compact size decoder to abstract over whether the prefix has been read or not.
      
  We did this already in `VecDecoder`.
      
  Internal change only, no logic change.


ACKs for top commit:
  nyonson:
    ACK 305062d
  apoelstra:
    ACK 305062d; successfully ran local tests


Tree-SHA512: c78fbba2391519cac6d282d8bbd4c3448bd103e32d8c93fce17c7d1e7a76c39fc65a999e3c1c53c102fd5c7069eb38451a2f0da7f80bd446c8a16d78e5dc4da7
@apoelstra

Copy link
Copy Markdown
Member

This was merged, not closed, and by me, not @tcharding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-consensus_encoding PRs modifying the consensus-encoding crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants