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

Deprecate get_ prefix for Block and Transaction methods #128

Merged
merged 2 commits into from
Jun 8, 2022

Conversation

shesek
Copy link
Contributor

@shesek shesek commented Jun 7, 2022

To match the changes introduced in rust-bitcoin/rust-bitcoin#861.

This allows using unified code with the same methods for both elements::{Transaction,Block} and bitcoin::{Transaction,Block} (behind different features), without triggering rust-bitcoin's deprecation warnings for the get_-prefixed methods.

Note that there are other instances of get_-prefixed methods in the rust-elements codebase. I only updated the ones needed for compatibility with the changes made in rust-bitcoin.

@shesek
Copy link
Contributor Author

shesek commented Jun 7, 2022

I ran into this in electrs, where I ended up creating an EBCompact trait to make the get-less methods available on the rust-elements data structures.

Another incompatibility that I ran into is that rust-bitcoin (as of v0.28) iterates over witness elements as a &[u8], while rust-elements iterates them as a &Vec<u8>.

(Initially I had to use Vec::from instead of Clone for compatibility with rust-bitcoin v0.28, then made it conditional to support rust-elements which still requires using Clone)

@@ -372,17 +372,29 @@ impl Block {
}

/// Get the size of the block
#[deprecated(since = "0.19.1", note = "Please use `Block::size` instead.")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This assumes that the next release is going to be 0.19.1, which might not be the case.

Copy link
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 2a35bad

@apoelstra
Copy link
Member

I'm happy to do a quick point release if you want. There's nothing else in the queue.

@apoelstra apoelstra merged commit b9aa021 into ElementsProject:master Jun 8, 2022
@shesek
Copy link
Contributor Author

shesek commented Jun 8, 2022

No urgency for me, I can keep using the compatibility trait until a new release is out. Thanks for merging!

shesek added a commit to shesek/electrs that referenced this pull request Jun 11, 2022
The EBCompact compatibility trait is no longer necessary, the `get`
prefix was removed in ElementsProject/rust-elements#128
shesek added a commit to Blockstream/electrs that referenced this pull request Jun 22, 2022
The EBCompact compatibility trait is no longer necessary, the `get`
prefix was removed in ElementsProject/rust-elements#128
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

2 participants