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

Upgrade to bdk ffi v0.31.0 #72

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Upgrade to bdk ffi v0.31.0 #72

wants to merge 16 commits into from

Conversation

BitcoinZavior
Copy link
Contributor

@BitcoinZavior BitcoinZavior commented Jan 12, 2024

[0.31.0]

APIs Changed:

  • BumpFeeTxBuilder.allowShrinking() now takes a Script as its argument.
  • The Address constructor now takes a Network argument.
  • The Payload::PubkeyHash and Payload::ScriptHash now have string arguments instead of byte arrays.

APIs Added:

  • The Address type now has the isValidForNetwork() method.

[0.30.0]

APIs added

  • Added BIP-86 descriptor templates

Copy link
Contributor

@Czino Czino left a comment

Choose a reason for hiding this comment

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

I was concerned at first that moving every function in a separate thread would break functionality (I vaguely remember something breaking, when I tried that)
However, after compiling the update and checking through every bit of functionality we use at Peach, I can confirm that all keeps working well on Android and iOS 👌

@@ -472,16 +522,12 @@ class BdkRnModule: NSObject {
resolve: @escaping RCTPromiseResolveBlock,
reject: @escaping RCTPromiseRejectBlock
) {
DispatchQueue.global().async { [self] in
DispatchQueue.main.async { [self] in
Copy link
Contributor

Choose a reason for hiding this comment

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

On second glance, I found that DispatchQueue.main is render blocking and very noticeable here as a user, because the UI freezes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interestingly, the change in the PR was meant to avoid UI thread blocking.
Will check your PR and this once again

@Czino
Copy link
Contributor

Czino commented Jan 25, 2024

Unfortunately after the last commit, my app keeps crashing right at startup.

@Czino
Copy link
Contributor

Czino commented Jan 29, 2024

This is another issue caused by the upgrade: #75

@BitcoinZavior
Copy link
Contributor Author

BitcoinZavior commented Jan 30, 2024

Unfortunately after the last commit, my app keeps crashing right at startup.

Hi @Czino
Not able to replicate.
The demo app is updated to use the latest commit: https://github.com/LtbLightning/bdk-rn-demo
(Be sure to point bdk-rn to local path in package json)
Are you able to run the app? or does that crash too?

@Czino
Copy link
Contributor

Czino commented Jan 30, 2024

Unfortunately the demo does not run at all for me
image

Looks like some extra steps are needed to make it run on my machine.

@BitcoinZavior BitcoinZavior changed the title Upgrade to bdk ffi v0.30.0 Upgrade to bdk ffi v0.31.0 Feb 5, 2024
else -> {
resolvedIndex = setAddressIndex("new")
}
}
val addressInfo = getWalletById(id).getInternalAddress(setAddressIndex(resolvedIndex))

val addressInfo = getWalletById(id).getAddress(setAddressIndex(resolvedIndex))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change on purpose? The ios/BdkRnModule.swift remains unchanged for this method.

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.

2 participants