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

Remove remnants of bitcoin v0.29 #5202

Open
tvolk131 opened this issue May 3, 2024 · 0 comments
Open

Remove remnants of bitcoin v0.29 #5202

tvolk131 opened this issue May 3, 2024 · 0 comments

Comments

@tvolk131
Copy link
Contributor

tvolk131 commented May 3, 2024

Follow-up to #2356

We've migrated almost all of the codebase to bitcoin v0.30, but there are a few places remaining where we're using v0.29 which require some more care to remove:

  1. PartiallySignedTransaction no longer implements Encodable or Decodable, which is why we're currently converting to/from the v0.29 version when encoding/decoding:

impl crate::encoding::Encodable for bitcoin::psbt::PartiallySignedTransaction {
fn consensus_encode<W: std::io::Write>(&self, writer: &mut W) -> Result<usize, std::io::Error> {
bitcoin29::consensus::Encodable::consensus_encode(
&bitcoin30_to_bitcoin29_psbt(self),
writer,
)
}
}
impl crate::encoding::Decodable for bitcoin::psbt::PartiallySignedTransaction {
fn consensus_decode_from_finite_reader<D: std::io::Read>(
d: &mut D,
_modules: &ModuleDecoderRegistry,
) -> Result<Self, crate::encoding::DecodeError> {
Ok(bitcoin29_to_bitcoin30_psbt(
&bitcoin29::consensus::Decodable::consensus_decode_from_finite_reader(d)
.map_err(crate::encoding::DecodeError::from_err)?,
))
}
}

Ideally we would be able to use impl_encode_decode_bridge!(bitcoin::psbt::PartiallySignedTransaction); like we did prior to the migration to v0.30 but I'm not sure how feasible that is.

  1. bitcoin::network::constants::Magic is encoded/decoded in reverse byte order to v29 (which doesn't have a struct, but instead just uses a u32). See here for details:

pub fn bitcoin29_to_bitcoin30_network_magic(magic: u32) -> bitcoin::network::Magic {
// Invert the byte order when converting from v0.29 to v0.30.
// See the following bitcoin v0.29 and v0.30 code:
// https://docs.rs/bitcoin/0.29.2/src/bitcoin/network/constants.rs.html#81-84
// https://docs.rs/bitcoin/0.30.2/src/bitcoin/network/constants.rs.html#251-258
let bytes = [
(magic & 0xFF) as u8,
((magic >> 8) & 0xFF) as u8,
((magic >> 16) & 0xFF) as u8,
((magic >> 24) & 0xFF) as u8,
];
bitcoin::network::Magic::from_bytes(bytes)
}
pub fn bitcoin30_to_bitcoin29_network_magic(magic: &bitcoin::network::Magic) -> u32 {
let bytes = magic.to_bytes();
// Invert the byte order when converting from v0.30 to v0.29.
// See the following bitcoin v0.29 and v0.30 code:
// https://docs.rs/bitcoin/0.29.2/src/bitcoin/network/constants.rs.html#81-84
// https://docs.rs/bitcoin/0.30.2/src/bitcoin/network/constants.rs.html#251-258
((bytes[3] as u32) << 24)
| ((bytes[2] as u32) << 16)
| ((bytes[1] as u32) << 8)
| (bytes[0] as u32)
}

  1. bitcoin::hashes::hex::format_hex was removed in v30. For simplicity, we've simply left v29 around to rely on this single function. Let's find a way to retain this behavior without needing v29.
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

No branches or pull requests

1 participant