Bitcoin.org icon indicating copy to clipboard operation
Bitcoin.org copied to clipboard

Add fltrWallet for Apple devices

Open fltrWallet opened this issue 1 year ago • 5 comments

fltrWallet avatar Jan 26 '23 15:01 fltrWallet

Thanks for this. If you add a random new commit, it should trigger a Travis build to check for bugs. Sorry, but at the time you created this PR, we had some problems with our config.

Cobra-Bitcoin avatar Jan 26 '23 15:01 Cobra-Bitcoin

Thanks for the submission! I respect many of your privacy and UI design decisions. Have you carefully reviewed the requirements listed here and believe that fltrWallet meets those requirements?

I spent a few minutes looking at the app and here are some of my first impressions in no particular order:

  • There don't seem to be many updates to the repository or the version in the app store. Is this under active development?
  • The seed phrase contains upper cased words which is not consistent with other wallets and could result in issues for users trying to restore this wallet on other wallets. This seems a bit dangerous.
  • When restoring a wallet with a phrase on an iPad, getting tap focus on each of the 12 entry fields seemed a bit difficult.
  • There was inconsistent terminology used in the app: Passphrase vs Recovery Phrase
  • The website FAQ mentions using testnet (faucets). How can testnet be selected on the wallet?
  • On the send screen text entry field (iPad) taproot addresses are (visually) clipped
  • A new wallet does not seem to use BIP32 receive address index 0 and starts at index 1. Is there a particular reason for this? There is an extremely slight chance this could make recovery of funds on another wallet more difficult.
  • The wallet has displayed incorrect balances that may be related to RBF or not handling change correctly on unconfirmed transactions. Details have been sent to the support email address.
  • The wallet crashed (possibly when receiving a transaction from the network). Details have been emailed.
  • It would be nice to have the app version number displayed in settings.

crwatkins avatar Jan 31 '23 14:01 crwatkins

Thank you for the detailed and valuable feedback. We are happy that the extensive work with the UI and UX was met with your approval. As you may have noticed the app supports all orientations and sharing the screen with other apps on iPad. We have focused on ease of use through the user interface and hopefully introduce novice Bitcoin users to a modern and privacy focused wallet.

I will do my best with answering each point below. Issues will be tracked under the fltrWallet/fltrWallet repository.

There don't seem to be many updates to the repository or the version in the app store. Is this under active development?

Yes, it is continuously undergoing active development. It is a full stack implementation from the network protocol level all the way up to the SwiftUI user interface with a fully reactive user interface reflecting network changes as they happen. It uses asynchronous code throughout, leveraging Apple’s Swift NIO package as well as Apple Combine framework. Everything is fully open sourced. We only have a single major non-Apple dependency in libsecp256k1 of Bitcoin Core fame. This is to provide the best experience possible on Apple devices with minimal footprint and optimal execution speed. We have a footprint of less than 100MB and synchronize while executing compact filter matches in parallel in well above 100Mbit speeds. This is to my knowledge therefore the first general purpose fully compact filter based mobile wallet protecting the user privacy and transaction history in full.

The last six months plus have been focused towards opening up the source code under the Apache license. It involved breaking up the project in logical modules that can hopefully be useful in other projects (we now have 21 repos of variable significance on GitHub). The networking client itself is currently undergoing modularisation so that it can be used with as few fltrWallet dependencies as possible. So, in short it has been undergoing massive development to support all the ins and outs of breaking down dependencies under Swift Package Manager (SPM). The project is also extensively tested under both unit and integration tests. The Bitcoin client is self contained or bootstrapped in that it can verify command execution while itself acting both as a client and a server in integration tests.

We plan to continue evolving the project and support new features of the Bitcoin protocol. Currently we are evaluating an encryption layer as specified by BIP324 to provide the integrity and privacy necessary without resorting to the TOR network which is blocked by some ISP:s.

The seed phrase contains upper cased words which is not consistent with other wallets and could result in issues for users trying to restore this wallet on other wallets. This seems a bit dangerous.

I agree with this assessment. We are happy to change this and we will provide issue no. 2 to track this request.

When restoring a wallet with a phrase on an iPad, getting tap focus on each of the 12 entry fields seemed a bit difficult.

This has to do with the limited UI component support in SwiftUI version 1. We have tried to remain compatible with a baseline of SwiftUI 1 to support as many devices as possible. The customisation of focus has been crippled in SwiftUI 1 on top of other rudimentary limitations in controlling “TextField” SwiftUI component. We are looking at alternatives or implementing a different workflow in inputing these words, for example one by one. See issue no. 4.

There was inconsistent terminology used in the app: Passphrase vs Recovery Phrase

This definitely does not help the user experience. We would like to use Recovery Phrase throughout to avoid confusion with the custom password or passphrase (additional word) as provided by BIP39. Issue no. 3 tracks this change.

The website FAQ mentions using testnet (faucets). How can testnet be selected on the wallet?

We had a testnet release in closed beta earlier and the FAQ needs updating. Now that the source code is fully available it is possible to run a testnet wallet from Xcode. We do not want to add testnet to the App Store release since it confuses beginners and due to Apple’s reluctance to accept submissions for testnet (we have tried before).

On the send screen text entry field (iPad) taproot addresses are (visually) clipped

The clipped address can be tapped to get a magnified view of the full address. Please give this a try. Maybe this needs UX work if it is not self explanatory? Maybe an addition to the FAQ solves this issue short term?

A new wallet does not seem to use BIP32 receive address index 0 and starts at index 1. Is there a particular reason for this? There is an extremely slight chance this could make recovery of funds on another wallet more difficult.

When cross testing with other wallet implementations we noticed that some (BRD wallet for one, off the top of my head) start at index 1 instead of index 0. If we generate index 0 transactions, those would not be recovered by said other wallet brands. We will obviously monitor transactions on index 0 forever as well so those transactions would never be missed.

Are you sure that there are wallets that would not recover transactions that start at index 1? From my research all HD-wallets we have tested does have some limited lookahead that would at least cover the index 1 case. We are ready to make this change if deemed necessary but it is a shame if others that start at index 1 cannot recover fltrWallet wallets.

The wallet has displayed incorrect balances that may be related to RBF or not handling change correctly on unconfirmed transactions. Details have been sent to the support email address.

and

The wallet crashed (possibly when receiving a transaction from the network). Details have been emailed.

We will track this issue under no. 5 and find a solution to it. Let us keep the discussion going and we will find a way to reproduce it and in the end make sure to find a solution. This is a bit of a torture test. We perhaps did not envision this use case of starting a transaction off of a different wallet software but it is a good idea to support such situations.

In the short term, recovering fltrWallet from the seed phrase should provide the correct balance and recover all transactions once confirmed. If the unconfirmed transaction is performed while fltrWallet is not running, the fulfilled and confirmed transaction should synch correctly. Again, this is not a long term solution but perhaps a way to maintain the two wallets semi simultaneously.

I will email you with some follow-up questions in regards to this issue.

Have you carefully reviewed the requirements listed here and believe that fltrWallet meets those requirements?

Yes, I did carefully study those requirements and held off with this submission to make sure all requirements were met. Please let me know if there is anything more we can do to facilitate this process.

fltrWallet avatar Jan 31 '23 17:01 fltrWallet

When cross testing with other wallet implementations we noticed that some (BRD wallet for one, off the top of my head) start at index 1 instead of index 0. If we generate index 0 transactions, those would not be recovered by said other wallet brands. We will obviously monitor transactions on index 0 forever as well so those transactions would never be missed.

Are you sure that there are wallets that would not recover transactions that start at index 1? From my research all HD-wallets we have tested does have some limited lookahead that would at least cover the index 1 case. We are ready to make this change if deemed necessary but it is a shame if others that start at index 1 cannot recover fltrWallet wallets.

I don't believe this should cause any problem for any wallet that I know of. The BIP44 gap limit is 20 and if that is followed, this certainly is not an issue.

I mainly asked because I don't know of any modern wallet that starts at 1 and I was curious if you were using index 0 for some special usage.

crwatkins avatar Feb 02 '23 15:02 crwatkins

An update had been accepted to the app store which prevents the precondition failure identified above.

fltrWallet avatar Feb 24 '23 19:02 fltrWallet

@fltrWallet Is this project still under development? There are only 6 commits (over all time) in the GitHub repository. A requirement for listing is that source code is public and kept up to date under version control.

crwatkins avatar Apr 09 '24 15:04 crwatkins