Skip to content
This repository was archived by the owner on Nov 30, 2022. It is now read-only.

Implements Read on HexIterator#135

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
RCasatta:read_on_iter
Dec 23, 2021
Merged

Implements Read on HexIterator#135
apoelstra merged 1 commit intorust-bitcoin:masterfrom
RCasatta:read_on_iter

Conversation

@RCasatta
Copy link
Copy Markdown
Contributor

This would allow deserializing objects without allocating the intermediate byte vector like:

let tx = Transaction::consensus_decode(HexIterator::new("....").unwrap()).unwrap()

Especially useful for libraries like rust_bitcoincore_rpc in places like this https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/54a427f2e45b0b8712d44a6995f6a03ad5f961f8/client/src/client.rs#L322-L326

CI is failing on my repo I think for an unrelated reason I am not fully grasping at the moment, @JeremyRubin could you have a look?

@apoelstra
Copy link
Copy Markdown
Member

concept ACK. It's been a month, maybe worth just kicking the PR and see if CI is fixed?

@RCasatta
Copy link
Copy Markdown
Contributor Author

I re-kicked (by re-pushing on a different branch, I havent't permission to re-kick here) https://github.com/RCasatta/bitcoin_hashes/actions/runs/1602919828 but it didn't work.

Now I noticed we have the same issue also in master https://github.com/rust-bitcoin/bitcoin_hashes/actions/runs/1602516688

@JeremyRubin
Copy link
Copy Markdown
Contributor

weird, not sure what would have broken them.

I spent some time last month looking at this, but couldn't figure it out either.

@apoelstra
Copy link
Copy Markdown
Member

I think it was fixed in #140

@RCasatta
Copy link
Copy Markdown
Contributor Author

Rebased, thanks for the fix @JeremyRubin

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 60f51d9

Nice PR!

@stevenroose
Copy link
Copy Markdown
Collaborator

This looks super cool! :D

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants