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

Update rust-bitcoin in ord #3962

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

raphjaph
Copy link
Collaborator

@raphjaph raphjaph commented Sep 21, 2024

This will take forever.

@cryptoquick
Copy link
Contributor

Thank you for starting this work. Let me know if you would like some help. I keep running into this:

mismatched types
`bdk_wallet::bitcoin::Transaction` and `Transaction` have similar names, but are actually distinct types
perhaps two different versions of crate `bitcoin` are being used?rustc[Click for full compiler diagnostic](rust-analyzer-diagnostics-view:/diagnostic%20message%20%5B5%5D?5#file%3A%2F%2F%2Fhome%2Fhunter%2FProjects%2Fducat%2Fguardian%2Fcrates%2Fvault_actions%2Fsrc%2Frepay.rs)
repay.rs(299, 24): arguments to this function are incorrect
transaction.rs(671, 1): `bdk_wallet::bitcoin::Transaction` is defined in crate `bitcoin`
transaction.rs(706, 1): `Transaction` is defined in crate `bitcoin`
runestone.rs(25, 10): associated function defined here

@raphjaph
Copy link
Collaborator Author

Thank you for starting this work. Let me know if you would like some help. I keep running into this:

mismatched types
`bdk_wallet::bitcoin::Transaction` and `Transaction` have similar names, but are actually distinct types
perhaps two different versions of crate `bitcoin` are being used?rustc[Click for full compiler diagnostic](rust-analyzer-diagnostics-view:/diagnostic%20message%20%5B5%5D?5#file%3A%2F%2F%2Fhome%2Fhunter%2FProjects%2Fducat%2Fguardian%2Fcrates%2Fvault_actions%2Fsrc%2Frepay.rs)
repay.rs(299, 24): arguments to this function are incorrect
transaction.rs(671, 1): `bdk_wallet::bitcoin::Transaction` is defined in crate `bitcoin`
transaction.rs(706, 1): `Transaction` is defined in crate `bitcoin`
runestone.rs(25, 10): associated function defined here

Yeah, the rust-bitcoin library constantly moves types around between modules so the Rust type system thinks they are different. This also means dependant crates we use like miniscript and bitcoincore-rpc have to be upgraded in tandem. It's an extremely manual job. Currently I've found around 700 places in our code base where I have to change something.

@cryptoquick
Copy link
Contributor

I know. And that's just to 0.31.2. But thank you for taking this on. It'll help with bits of code like this:
script_pubkey: ScriptBuf::from_bytes(runestone.encipher().to_bytes())
That were originally just:
script_pubkey: runestone.encipher()
This might have a slight performance impact if used often enough, such as for verification, and we also get some loss of type safety.
In the grand scheme of things this is low priority, but it's still technical debt.

This was referenced Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants