LtbLightning / bdk-flutter

Bitcoin Development Kit - Flutter Package
MIT License
60 stars 27 forks source link

Add support for Linux x86_64 #93

Closed i5hi closed 4 months ago

i5hi commented 11 months ago

Opening this PR a bit early as I need some help with it.

Changes required:

A few other issues I have been facing which is preventing me from working on the dart codebase:


  Future<bool> isExplicitlyRbf() async {
    try {
      final res = await loaderApi.isExplicitlyRbfStaticMethodApi(tx: _tx!);
      return res; // this returns a Future<int> while the definition is bool
    } on FfiException catch (e) {
      throw configException(e.message);
    }
  }
i5hi commented 11 months ago

Update also required in lib/src/utils/loader.dart

Unable to test for this due to the Type Errors mentioned above.

DynamicLibrary _open() {
  if (Platform.isIOS) {
    return DynamicLibrary.process();
  } else if (Platform.isAndroid) {
    return DynamicLibrary.open("librust_bdk_ffi.so");
  } else if (Platform.isLinux) {
    return DynamicLibrary.open("librust_bdk_ffi.so"); // linux/x86_64/librust_bdk_ffi.so ? or is the linux subpath implied?
  } else {
    return DynamicLibrary.executable();
  }
}
i5hi commented 11 months ago

Also noticed that the binaries are being checked into git.

Better to .gitignore them?

i5hi commented 11 months ago

Also noticed some changes in root.dart that I did not recognize.

Replicated the changes I made through another repo and also finding the same result. I only change 2 files yet there are changes reflecting in the files in lib/src/generated/*

i5hi commented 11 months ago

Also noticed that the binaries are being checked into git.

Better to .gitignore them?

If pub.dev requires binaries in the repo; then having release branches might be a good solution where the binaries are checked in.

sneurlax commented 11 months ago

Also noticed that the binaries are being checked into git.

Better to .gitignore them?

at least remove them :+1: edit: nevermind!

BitcoinZavior commented 11 months ago
  • [ ] Type errors in dart code. lib/src/root.dart has type errors for the return values of several functions called from loaderApi. Most of them return an Int Pointer but their definition returns different types.
  Future<bool> isExplicitlyRbf() async {
    try {
      final res = await loaderApi.isExplicitlyRbfStaticMethodApi(tx: _tx!);
      return res; // this returns a Future<int> while the definition is bool
    } on FfiException catch (e) {
      throw configException(e.message);
    }
  }

The definition for the functions that can be called using the DynamicLibrary loaderApi is available in lib/src/generated/bindings.dart. For the function isExplicitlyRbfStaticMethodApi, the return type is Future<bool>. https://github.com/i5hi/bdk-flutter/blob/56f9feff3e9d22f5256965926e03fb8bc370ac1f/lib/src/generated/bindings.dart#L260. You can cross-check, with bindings. dart file to confirm that the return types are the correct ones.

BitcoinZavior commented 11 months ago

Unable to test for this due to the Type Errors mentioned above.

DynamicLibrary _open() {
  if (Platform.isIOS) {
    return DynamicLibrary.process();
  } else if (Platform.isAndroid) {
    return DynamicLibrary.open("librust_bdk_ffi.so");
  } else if (Platform.isLinux) {
    return DynamicLibrary.open("librust_bdk_ffi.so"); // linux/x86_64/librust_bdk_ffi.so ? or is the linux subpath implied?
  } else {
    return DynamicLibrary.executable();
  }
}

Adding a target and adding the condition for Linux is not the only thing required to add Linux support. Please refer to the Linux integration part on this page: https://cjycode.com/flutter_rust_bridge/library/platform_setup/windows_and_linux.html to get a better idea on how to go about adding Linux support.

BitcoinZavior commented 11 months ago

Also noticed that the binaries are being checked into git. Better to .gitignore them?

at least remove them 👍

The binaries are in Github so that anyone who wants to use the Github repository can readily use them. Without the binaries developers will have to build their own binaries, which can be a non trivial task for some developers. This would add additional friction hence binaries are readily available.

BitcoinZavior commented 11 months ago

Also noticed that the binaries are being checked into git. Better to .gitignore them?

If pub.dev requires binaries in the repo; then having release branches might be a good solution where the binaries are checked in.

Thanks, yes this is one possible option. When this turns out to be a problem we can consider a suitable solution.

BitcoinZavior commented 11 months ago

Also noticed some changes in root.dart that I did not recognize.

Replicated the changes I made through another repo and also finding the same result. I only change 2 files yet there are changes reflecting in the files in lib/src/generated/*

Yes, Flutter Rust Bridge auto generates these files.