Skip to content

Conversation

@thunderbiscuit
Copy link
Member

The bindings do not build when attempting this upgrade because get_balance() now returns a Balance struct (this was merged in bitcoindevkit/bdk#640)

error[E0308]: mismatched types
   --> src/lib.rs:433:9
    |
432 |     fn get_balance(&self) -> Result<u64, Error> {
    |                              ------------------ expected `Result<u64, bdk::Error>` because of return type
433 |         self.get_wallet().get_balance()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found struct `Balance`
    |
    = note: expected enum `Result<u64, _>`
               found enum `Result<Balance, _>`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `bdk-ffi` due to previous error

When we upgrade to 0.22.0 we could decide to add the Balance struct to the bindings, or simply return the total by calling get_total(), which returns a u64 (same as we have now).

@thunderbiscuit
Copy link
Member Author

thunderbiscuit commented Sep 1, 2022

Because Balance is very simple (4 u64 fields) I decided to take a stab at bringing it in the FFI layer and test it in my test suite.

As is, the Balance struct works well, but the methods on it are not ported to the FFI layer. I'm not sure how to enable them without rewriting the struct in the FFI... @notmandatory any ideas? Would this be a case of using the typedef defined here?

I tried just for fun to add

[External="bdk"]
typedef extern Balance;

to the UDL file but unfortunatly it throws this error:

error[E0432]: unresolved import `bdk::FfiConverterTypeBalance`
    --> /Users/user/repos/bdk-ffi/target/release/build/bdk-ffi-28f1e274c858a20d/out/bdk.uniffi.rs:2621:5
     |
2621 | use bdk::FfiConverterTypeBalance;
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `FfiConverterTypeBalance` in the root

For more information about this error, try `rustc --explain E0432`.
error: could not compile `bdk-ffi` due to previous error

@notmandatory
Copy link
Member

notmandatory commented Sep 5, 2022

@thunderbiscuit I think we should make a bdk-ffi version of the Balance struct that also has fields for spendable and total. I can push a patch to this PR branch with the needed changes. This way we can leave Balance as a simple dictionary. Ok?

@notmandatory notmandatory force-pushed the test/release-candidate-0.22 branch from f71c4c9 to 5d1d7ba Compare September 5, 2022 16:28
@thunderbiscuit
Copy link
Member Author

thunderbiscuit commented Sep 5, 2022

ACK a6af35c. Ready for merging when the new BDK version comes out.

@notmandatory notmandatory force-pushed the test/release-candidate-0.22 branch from 5d1d7ba to a6af35c Compare September 5, 2022 16:30
@notmandatory notmandatory marked this pull request as draft September 5, 2022 21:30
@notmandatory notmandatory changed the title Test new BDK release candidate 0.22.0-rc.1 Update bdk dependency to 0.22 Sep 5, 2022
@notmandatory notmandatory force-pushed the test/release-candidate-0.22 branch from a6af35c to ca63591 Compare September 5, 2022 21:35
@notmandatory
Copy link
Member

notmandatory commented Sep 5, 2022

I rebased and updated the CHANGELOG. I also updated the PR title and changed this to a draft, once bdk 0.22 is published this PR can then be updated to replace the bdk 0.22.0-rc.1 version with the final 0.22 version.

@notmandatory notmandatory force-pushed the test/release-candidate-0.22 branch from ca63591 to df8ddd1 Compare September 5, 2022 21:39
@notmandatory notmandatory added this to the Release 0.9.0 milestone Sep 5, 2022
@notmandatory notmandatory mentioned this pull request Sep 5, 2022
21 tasks
@notmandatory notmandatory force-pushed the test/release-candidate-0.22 branch from df8ddd1 to 3c6075a Compare September 8, 2022 13:35
@notmandatory
Copy link
Member

Updated PR branch with final bdk release version 0.22. This should be ready to merge.

@notmandatory notmandatory marked this pull request as ready for review September 8, 2022 13:38
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 3c6075a

@notmandatory notmandatory merged commit dfb350e into bitcoindevkit:master Sep 8, 2022
@thunderbiscuit thunderbiscuit deleted the test/release-candidate-0.22 branch November 14, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants