bitcoin-dot-org
Add AirGap Vault HW wallet
Some discussions took place here previously: https://github.com/bitcoin-dot-org/Bitcoin.org/issues/3894
Thank you for your submission!
A brief glance at the submission shows that AirGap Vault has been submitted in the hardware category, as opposed to a software category (such as mobile or desktop). As I commented before I don’t believe that I would be able to make a case for listing AirGap Vault as a hardware wallet for the reasons I mentioned. Currently the distinction of the hardware category is to be running on dedicated hardware. There is currently another air-gapped app (Bither) listed in the mobile category. I still recommend listing as we previously discussed.
I have briefly looked at the iOS version of AirGap Vault and signed one transaction created by Sparrow Wallet. I’ve noticed a few issues:
-
The amounts for each individual output are not displayed. The actual amount being spent cannot be confirmed (without opening debug information).
-
Change addresses (and receive addresses) owned by the account are not verified. The actual amount being spent cannot be confirmed.
-
There is no way to securely validate that a receive address is owned by the wallet (other than visually searching a long address list which impractical).
These missing features are expected in any modern Bitcoin device. The lack of these features would preclude me from recommending AirGap Vault for listing.
I reported the failure of being able to sign a Sparrow wallet generated transaction over four months ago and I again reported it last month. Just now I figured out a simple fix on my own. The default QR code V3 was not working, but when I switched to the BC UR (Beta) QR code, it worked. Note:
-
Beta features are not reviewed, so BC UR codes will need to come out of beta for Sparrow support to be reviewed.
-
With BlueWallet, the V3 QR code crashes BlueWallet (I reported this to BlueWallet). Clearly that should not be default for BlueWallet as the BC UR QR works with BlueWallet.
-
This situation causes significant concern about the level of support that the app is receiving, or perhaps just the level of Bitcoin support that the app is receiving.
The vault seems to be focused on a single Bitcoin address. There is an option to “Show Address QR” (one single address). The account seems to be identified by a single address while most modern wallets (such as Sparrow) refer to accounts using a master fingerprint and a derivation path. Further research suggests that this might be because AirGap Wallet only supports a single receive address. While the AirGap Wallet app is not being submitted for listing, it is troubling that it promotes address reuse. Listed Bitcoin wallets have not reused addresses for over eight years.
-
The vault should identify accounts with master fingerprints as do all of its supported companion wallets.
-
As AirGap Vault offers to use AirGap Wallet, even though AirGap Wallet is not being reviewed, it is very concerning that users might be directed to use a wallet that reuses addresses.
The documentation is extensive and very complete. It demonstrates a keen understanding of cryptocurrencies, wallets and security, however there is a lot of it that is out of date or incorrect. For example, the setup guides for Sparrow and BlueWallet do not match the current version of the Vault and could be quite confusing to new users. It seems that the Bitcoin documentation in particular may be being neglected.
Several UI/UX issues were also observed and were reported.
This is not a review. This is simply observations from a first use. It would be good to have the bulleted points above commented on or addressed.
Hi. Thank you for your observations. We will look into them and get back to you with a detailed answer.
Hi Craig.
I updated the PR to use the mobile category instead of the hardware category.
For the privacycheck part, I added "passes" because you have the option to use a companion app that supports those, and AirGap Vault itself doesn't have network access, so these options don't really apply.
We would like to officially submit the app for review now. We addressed a lot of your feedback over the last few months. Even if we do not pass on the first try, a thorough and official review will help us to plan ahead and see where we stand.
Thanks a lot for your feedback so far and looking forward to your response!
Thanks for the update! I hope to be able to review your changes shortly. In the meantime, could you address the bulleted points above and let us know if/how they have been addressed or if they are simply mistaken?
- The amounts for each individual output are not displayed. The actual amount being spent cannot be confirmed (without opening debug information).
It now shows all the outputs and the amount they receive.
- Change addresses (and receive addresses) owned by the account are not verified. The actual amount being spent cannot be confirmed.
The change address is now verified. In the UI, there is a note that the change address has been verified, and the derivation path is visible.
- There is no way to securely validate that a receive address is owned by the wallet (other than visually searching a long address list which impractical).
This is still the case. How would you like this to be resolved? As far as I know, if the recipient is an address owned by the wallet, the derivation path is not in the PSBT (unless it's a change address). Because the Vault doesn't know anything about the wallet, the index of the recipient address could be in the thousands, so we would have to manually check all addresses until a certain index.
To verify addresses manually, we could add a search function where you can search for an address and we can derive chunks from the xpub until it's found? Or we could let you jump to a specific index to see the address.
- Beta features are not reviewed, so BC UR codes will need to come out of beta for Sparrow support to be reviewed.
The BC UR feature is no longer in Beta and the tag has been removed.
- With BlueWallet, the V3 QR code crashes BlueWallet (I reported this to BlueWallet). Clearly that should not be default for BlueWallet as the BC UR QR works with BlueWallet.
When selecting a BC UR based wallet (BlueWallet, Sparrow, Specter), then the QR code will default to BC-UR. Also when you scan a BC UR code, then the response will be in the BC UR format as well.
- This situation causes significant concern about the level of support that the app is receiving, or perhaps just the level of Bitcoin support that the app is receiving.
While the feature was in beta, it lacked a lot of the UX features like automatic selection of the QR code format, and in the beginning also the wallet names / logos that it can be used with. Those issues have been resolved now that it's out of beta.
- The vault should identify accounts with master fingerprints as do all of its supported companion wallets.
This was not done yet. We're still discussing how to best represent accounts / fingerprints because there were some misunderstandings regarding how life-hashes were used. We're planning to address in one of the next updates.
- As AirGap Vault offers to use AirGap Wallet, even though AirGap Wallet is not being reviewed, it is very concerning that users might be directed to use a wallet that reuses addresses.
Not sure how we can solve this. On the page where you have a selection of wallets, we could add a "preferred" tag or something like that to the BC UR wallets to point users to the advanced wallets?
Hi @crwatkins, do you have everything you need from our side to do the review. I know that the review takes time and there is no hurry, just checking that this is still on your radar.
I need help
On Wed, Feb 14, 2024, 1:27 AM AndreasGassmann @.***> wrote:
Hi @crwatkins https://github.com/crwatkins, do you have everything you need from our side to do the review. I know that the review takes time and there is no hurry, just checking that this is still on your radar.
— Reply to this email directly, view it on GitHub https://github.com/bitcoin-dot-org/Bitcoin.org/pull/4019#issuecomment-1943373116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARI72LYFE4WVGRDQMQNRMITYTR7OLAVCNFSM6AAAAAAWTWRVROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBTGM3TGMJRGY . You are receiving this because you commented.Message ID: @.***>
@AndreasGassmann wrote:
It now shows all the outputs and the amount they receive.
It would be very useful to list the most important components (the ones which a user should verify) first, such as the payment amount, the payment address, and the fee. It might be useful to de-emphasize the other amounts that are less important (or even distracting) to the user.
The change address is now verified. In the UI, there is a note that the change address has been verified, and the derivation path is visible.
Great! If the address is verified, the actual display of the amount of the change could potentially be de-emphasized in the UI as the user usually doesn't have a trusted source on what that amount should be.
There is no way to securely validate that a receive address is owned by the wallet (other than visually searching a long address list which impractical).
This is still the case. How would you like this to be resolved? As far as I know, if the recipient is an address owned by the wallet, the derivation path is not in the PSBT (unless it's a change address). Because the Vault doesn't know anything about the wallet, the index of the recipient address could be in the thousands, so we would have to manually check all addresses until a certain index.
To verify addresses manually, we could add a search function where you can search for an address and we can derive chunks from the xpub until it's found? Or we could let you jump to a specific index to see the address.
Just to be clear, this is not a listed criterion. However, it is an extremely common feature for signing devices (hardware wallets) as it can mitigate the risk of malware tampering with received addresses. Wallets usually start searching from index 0 and after many seconds with no matches, the wallets report that to the user and ask the user if they want to keep searching. Most users that have a lot of transactions in their wallet will be aware of this situation.
As AirGap Vault offers to use AirGap Wallet, even though AirGap Wallet is not being reviewed, it is very concerning that users might be directed to use a wallet that reuses addresses.
Not sure how we can solve this. On the page where you have a selection of wallets, we could add a "preferred" tag or something like that to the BC UR wallets to point users to the advanced wallets?
Is this something that you might want to address in the AirGap Wallet? It's considered a very bad practice mostly for privacy reasons, but also for security reasons. bitcoin.org has not listed any wallets that reuse addresses for about a decade.
An additional note on the transaction display: The TOTAL AMOUNT seems very confusing. It seems to be the sum of all the outputs, excluding the fee, but including the change. It doesn't seem like a useful number to display and is likely to confuse.
Also, when there are multiple outputs, AMOUNT is displayed about halfway between the addresses and it is not clear which amount is associated with which address, especially on larger devices.
After a bit more experience with AirGap Vault, I remain concerned that listing only AirGap Vault and not AirGap Wallet would implicitly imply that AirGap Wallet was reviewed and recommended. For example the screenshot in this submission shows both AirGap Vault and AirGap Wallet. That one issue could of course be easily corrected, but that demonstrates how tight the linkage is. Another example is that when AirGap Vault is first installed and started, in some flows the welcome screen invites the user to Install AirGap Wallet. It would not be unreasonable for some new users to even assume that this is required.
There are definitely issues that would currently preclude AirGap Wallet from being recommended. Is this something you might want to address?
I said above:
... it is an extremely common feature for signing devices (hardware wallets) as it can mitigate the risk of malware tampering with received addresses. Wallets usually start searching from index 0 and after many seconds with no matches, the wallets report that to the user and ask the user if they want to keep searching. Most users that have a lot of transactions in their wallet will be aware of this situation.
I should also point out that keeping a cache of these addresses can be very easy and extremely efficient.