chore: bitcoin crate upgrade #3080
Conversation
…om/dfinity/ic into mihailjianu1/bitcoin-crate-upgrade
|
@randombit wrote:
Just FYI, I tried to upgrade ckbtc minter to bitcion from bitcoin_0_28 but failed. Explanation is here #3275 (comment). Likely it will also fail for crypto. |
| @@ -64,6 +64,7 @@ pub(crate) fn address_limits(network: Network) -> (usize, usize) { | |||
| Network::Testnet => (100, 1000), | |||
| Network::Signet => (1, 1), | |||
| Network::Regtest => (1, 1), | |||
| other => unreachable!("Unsupported network: {:?}", other), | |||
There was a problem hiding this comment.
We should avoid panics. I would also avoid the catch-all pattern as it would make us blind to changes to this enum in a future upgrade. Edit: nvm this. I see that the type is non exhaustive.
I would either go with returning an Error and let the caller handle the error, or fall back to the safest return value.
CC: @rumenov
There was a problem hiding this comment.
Another options is to create our own type SupportedNetworks with the variants we support in the adapter. That way you can avoid having this unreachable! panic everywhere we match on Network
Upgrade the bitcoin crate almost everywhere.
Some test targets in the
cryptocrate have not been upgraded yet. The main issue there is that we can't have thesecp256k1dependency (transitive) on different versions from different dependencies with bazel. See more on why here: #1150.The latest
bitcoindependency depends onsecp256k1 0.29, however the latestbip32dependency has not upgraded to 0.29 yet. The best course of action would be to separate these crates, however to unblock the testnet4 support, that can be done at a later date.The ckBTC minter canister has also not been upgraded