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

Airbitz updates #652

Merged
merged 10 commits into from Feb 10, 2015
Merged

Airbitz updates #652

merged 10 commits into from Feb 10, 2015

Conversation

i3inary
Copy link
Contributor

@i3inary i3inary commented Nov 19, 2014

Added Airbitz to wallet resources under mobile/ios/android. Added coinmaps.org and Airbitz to directory resources page.

@gurnec
Copy link
Contributor

gurnec commented Nov 21, 2014

@i3inary @paullinator First I should say that all of the below is just one person's opinion, and doesn't have a lot of bearing on much else, but I thought I'd offer some food for thought anyways.

My initial impressions are quite positive-- but there are a few things which concern me a bit. In no particular order:

  1. You've listed the wallet as both checkFailPrivacyDisclosureAccount and checkPassPrivacyDisclosureAccount. Was this intentional? If not, which do you think is more accurate?
  2. You've listed the wallet as checkPassValidationServers-- does this wallet only use servers operated by Airbitz for transaction validation? If so, I think checkFailValidationCentralized would be more appropriate.
  3. There doesn't seem to be a way to export your private keys, they are essentially "locked" in the app (unless your phone is rooted). Even though they are backed up on Airbitz servers, I still think this is a big concern...
  4. On the surface (I did not examine the code carefully), the password complexity requirements, KDF (scrypt), and required wallet encryption all look very good. You may however want to remove the time-to-crack estimate, it seems to be a significant overestimate (password strength estimation is very hard)...
  5. Although the Airbitz client is not open source (specifically your 3rd clause is not by most definitions), I personally think a score of checkPassTransparencyOpenSource is still appropriate since the source is available for inspection. I think we're in somewhat uncharted waters in this respect, so others may disagree...
  6. There seems to be some currently-disabled (via a #define) code for adding fees (Airbitz-imposed, not just miner) to some future version of Airbitz. I don't have a problem with fees per se, but if in some future version of Airbitz the fees code were suddenly enabled, I wonder if that might reflect badly on bitcoin.org. Again, this is new territory...

This is already getting long enough, so I won't go into the things I liked about Airbitz except to say that there was a lot to like. Thanks for reading, and possibly addressing some of these concerns.

@saivann
Copy link
Contributor

saivann commented Nov 21, 2014

@i3inary @paullinator I didn't review the wallet yet but two things I've noticed; Your description is longer than 6 lines, this needs to be shortened. The icon also needs to be resized from 96px to 85px in the 144px png.

@saivann
Copy link
Contributor

saivann commented Nov 21, 2014

On @gurnec's comment number 5, the app should be submitted with a checkfailtransparencynew score actually, like all newly added wallet. Later the "open-source" score should apply given that the score only says that the source is available (mycelium also has a restrictive license, althought the code is public).

@paullinator
Copy link
Contributor

Hi @gurnec and thanks for the feedback.

1.Thanks for catching the checkFailPrivacyDisclosureAccount issue. We're a bit new to the terminology so let me look into the meanings and update appropriately.

  1. We use opensource Libbitcoin/Obelisk servers which are currently run by Airbitz, Darkwallet, and Open Bazaar. Due to version differences we don't yet share access to servers but we've agreed to in the very near future. We'll change this to checkFailValidationCentralized for now until we get on the same page with them.
  2. It's not user friendly, but you can export the private "seed" of each wallet by hitting the export button of each wallet in the account (upper left button) and choosing "Wallet Private Seed". This exports the raw entropy which can be swept using Libbitcoin/SX tools.
  3. People have found the time to crack estimate somewhat useful. Agreed though that we do need to downgrade the time estimate. Note that blockchain.info does this as well. Does this at all impact our scores here?
  4. @saivann, We'll go ahead and downgrade to checkfailtransparencynew for now. How long do we have to be out to get checkPassTransparencyOpenSource?
  5. I believe Multibit is already implementing a fee which has been positively received by the community. We're following in those footsteps although, as you noticed, the fee is disabled.

Thanks @saivann and @gurnec for your quick responses to our submission. We're look at making those changes ASAP.

@saivann
Copy link
Contributor

saivann commented Nov 23, 2014

@paullinator Seems like the app was publicly released one month ago, usually it's 6 months starting with the public release date, sometimes it can be longer if there's too few public feedback or if the git repository is reset. BTW, can you tag releases? This is one requirement to have a passing transparency score https://github.com/bitcoin/bitcoin.org#score .

Regarding libbitcoin, I don't know what's the status of that library, can it be considered stable enough at this point? AFAIK DarkWallet was still in alpha and I supposed that the underlying libraries were also not fully ready for prime time yet.

@gurnec Thanks for your insight and comments, very helpful and appreciated!

- Resize icon in the 144px file to 85px
- Update checkPassTransparencyOpenSource to checkfailtransparencynew
@paullinator
Copy link
Contributor

@saivann, Yes, we did publicly release about 6 weeks ago and were in beta for about 2 months prior. We will start tagging our future releases to comply with the transparency requirement. Thanks for letting us know. We contributed a LOT of work into libbitcoin since Jan 2014 to harden it for production use. Amir decided to rewrite the database of libbitcoin/obelisk while Dark Wallet is in alpha thereby putting it back into pre-release mode. We are not using the re-write at this time, but rather an older version that we ran on for months. We feel confident in libbitcoin/obelisk as a great bitcoin platform to build our wallet on.

@gurnec
Copy link
Contributor

gurnec commented Nov 24, 2014

@paullinator Thanks for taking the time to respond.

It's not user friendly, but you can export the private "seed" of each wallet by ...

Thanks for pointing that out. Exporting the seed doesn't require a connection to the server, is that correct (it's cached locally)?

(FYI I've verified that I can take the seed and generate the same addresses & keys as the Android app.)

People have found the time to crack estimate somewhat useful. Agreed though that we do need to downgrade the time estimate. Note that blockchain.info does this as well. Does this at all impact our scores here?

Not that I'm aware of, however I'm not really the right person to answer this....

I believe Multibit is already implementing a fee

MultiBit has publicly stated (via their blog) that they intend to attach fees to transactions when they release MultiBit HD, however last I looked it was still in private beta. They are renaming the old MultiBit to MultiBit Classic to differentiate it from their HD / fee-based wallet (and largely discontinuing development of MultiBit Classic).

Up to now, no bitcoin wallet has chosen to add fees (including Airbitz in its current form). If a wallet were added to the bitcoin.org website which countered this trend (including MultiBit HD), I'd be concerned over potential backlash similar what's been seen with in-app purchases. I think that wallets imposing their own fees should be marked as such on the choose-your-wallet page to avoid any potential issues.

Currently Airbitz doesn't have any fees, so this is largely irrelevant for the time being, I'm just thinking out loud here....

@paullinator
Copy link
Contributor

Exporting the seed does NOT require a server connection. It is kept locally on your phone and encrypted with users' credentials. If a user has a new device that's never been logged into, it will of course be downloaded to their device upon first login.

Should we ever implement a transaction fee we would be very clear and transparent about it. We honestly hope to never have to but rather rely on our business directory and licensing of the ABC API/Library for revenue.

@i3inary
Copy link
Contributor Author

i3inary commented Nov 26, 2014

@saivann is there anything else I need to do to get you the updates on this pull request (never really used pull requests)?

I updated my branch and it seems to have updated here in this thread. I just want to make sure you weren't waiting on me and could merge when you evaluated and saw fit.

@saivann
Copy link
Contributor

saivann commented Nov 27, 2014

@i3inary I should review / test your wallet, do my own research and comment here soon. I will let you know if anything else needs to be changed in the pull request.

@paullinator
Copy link
Contributor

@saivann vann. PLEASE try our wallet. We would love to get your feedback. We've a ton into it above and beyond most wallets out there. Check out the intro video on our homepage to give you an idea of some of the functionality in there. We also have some architectural docs we'll be sharing soon and an architectural audit done by Kristov Atlas.

@paullinator
Copy link
Contributor

Hi @saivann. Have you had a chance to review our wallet? We'd love to get listed. Note that we do RFC6979 deterministic transaction signing and we've had it in for several weeks. We're not prone to the Blockchain.info bug

@saivann
Copy link
Contributor

saivann commented Dec 8, 2014

@paullinator Hey, actually I've had issues testing the wallet in the Android Virtual Device Manager, seems like the app crashes on start with Android 4.4.2 Google APIs 19, and the app won't install on Android 5 Google APIs 21, any idea of a workaround for me? I unfortunately have an old Android device unsupported by your app.

@paullinator
Copy link
Contributor

Ack. We don't usually run in the Android Virtual Device. I'd sooner send
you a phone! :) We have some spare iphone's and galaxy s3's. Can we ship
you one and you just send it back when you're done? It will give a better
understanding of the experience users will really get.

If so, send us your address

[image: logo]
Paul Puey CEO / Co-Founder, Airbitz Inc
619.850.8624 | http://airbitz.co | San Diego
http://facebook.com/airbitz http://twitter.com/airbitz
https://plus.google.com/118173667510609425617
https://go.airbitz.co/comments/feed/ http://linkedin.com/in/paulpuey
https://angel.co/paul-puey
DOWNLOAD THE AIRBITZ WALLET:
https://play.google.com/store/apps/details?id=com.airbitz
https://itunes.apple.com/us/app/airbitz/id843536046

On Tue, Dec 9, 2014 at 5:31 AM, saivann notifications@github.com wrote:

@paullinator https://github.com/paullinator Hey, actually I've had
issues testing the wallet in the Android Virtual Device manager, seems like
the app crashes on start with Android 4.4.2 Google APIs 19, and the app
won't install on Android 5 Google APIs 21, any idea of a workaround for me?
I unfortunately have a old Android device unsupported by your wallet.


Reply to this email directly or view it on GitHub
#652 (comment).

@saivann
Copy link
Contributor

saivann commented Dec 10, 2014

@paullinator I got around the issue, please see my feedback, suggestions and questions below. Your comments are welcome if you see better ways of dealing with some of these issues.

- Some issues that may require to be fixed:

Payment request issue: For some reason, the app wouldn't let me send a payment request by email or SMS so I couldn't completely test the app. Instead, I am left with some searchbox that provides no result when typing an email address, and the app doesn't seem to have any addressbook, is this perhaps due to an incomplete feature or did I miss something?

Encrypted seed: I'd like to confirm with you that the private seed sent by email is encrypted with the password? It is identical to the one displayed on screen. Just want to be sure it isn't sent plaintext since I haven't tested restoring the wallet from it with sx/libbitcoin.

Restore feature: I couldn't find a "restore" feature, I think this would be important before the wallet is included. I've suggested this being a requirement in #670. It is nice to know the wallet can be independently restored with sx/libbitcoin though.

Release date: The public release of the wallet isn't 3 month old yet, so It's probably worth to wait at least for another month.

- Some issues that may need be fixed or require writing a different score:

Offline backup or backup on setup:

Exporting the seed does NOT require a server connection

Unfortunately, this is not exactly what I found during my testing. It works as expected if the app is in the background. However If the app is (really) closed and re-opened, it requires you to login in order to access the wallet, so as a result, the user doesn't always have full control over their private keys by default. Like what I suggested for blockchain.info, I think a new failing score would be appropriate for this case unless the wallet is updated to either always let the user backup their wallet offline or to make them backup or write down their HD wallet backup during setup.

- Some issues that may not be formal requirements but still good to discuss:

Password strength indicator: I am also unsure about the "x years to crack" language also used by blockchain.info, why not a simple "bad / medium / good" metric like most websites? Evaluating time for a bruteforce or dictionnary attack is not easy. For example, Airbitz says that it would take 7 years to crack "password" - although it's worth noting that this password isn't allowed by the app in practise.

Compatible backup: Ideally the wallet could also be restored in one stable SPV client as well in case Airbitz servers disappear one day, a similar issue exists with Mycelium. This could be an idea for a future requirement.

In general, like @gurnec, the wallet leaves me with a good impression and except for the points mentioned above, I personally see no reason to not list the wallet. More reviews and comments are welcome.

@paullinator
Copy link
Contributor

Thanks for the update @saivann. Some comments on your concerns:

Payment request issue: This will autocomplete from your address book if you give it access but otherwise, you can just type a complete email address and it will launch your default email app.

Encrypted seed: This is not encrypted at this point. We've considered only allowing it to be viewed for security and we can make that change if you feel it is important. It is identical to the one shown on screen. We really feel this is only needed in catastrophic failure as the app will let you local login & send funds even if our servers are down.

Restore: The only restore right now is via SX/libbitcoin. Once again we feel this is only for catastrophic failure when BOTH Airbitz goes down AND the user's device is lost or destroyed. If either survives, the user can get their funds off by just doing a Send.

Offline backup or backup on setup: The user can login even without access to Airbitz servers. The PIN login requires server access, but tapping [Switch User] will get the full login screen which doesn't need server access. There is a small bug right now that prevents you from getting to the full login screen if in airplane mode, but otherwise you should be fine. Just tap [Switch User] while networked. Then go to Airplane mode. Then do a full login/password login to prove that you have full access while offline. Then export the seed.

Password strength indicator: We've had positive feedback from users that the strength indicator brings a tangible feeling of importance to choosing good passwords. We've decreased the time-to-crack by 1000x to more accurately reflect most consumer hardware. We'd hate to remove this since it's been a positive aspect of our wallet.

Compatible backup: Once again, this shouldn't be critical as the wallet will be able to spend even if Airbitz goes away. You can login w/o our servers, and send directly to the network.

@saivann
Copy link
Contributor

saivann commented Dec 11, 2014

Offline backup or backup on setup: I was probably not patient enough, I was just able to access the wallet and seed in airplane mode. All good.

Restore feature: Another more common scenario in which restoring the backup is useful is if the user loses the password. Thus far I think other wallets all allow (although not always in a user-friendly way) to restore wallet from backup.

Encrypted seed: As far as I am concerned, sending the seed unencrypted over non-secure protocols like email, and letting the user store an unencrypted backup online right from the app is something I'd consider a blocking issue. The unencrypted backup seed on screen is OK with me (whoever has access to the wallet can spend funds anyway), but maybe it would be nice to warn the user here that whoever knows this seed controls the wallet? I think other wallets tend to use careful disclaimers in such situations.

Backup / Password reminder: Another suggestion that comes to my mind is to remind the user to write their username and password and/or backup the wallet. This could help some users to have proper recovery options in case they forget their passwords, or lose their device and forget their username.

Payment request issue: Ah..right! Unfortunately, the app refuses to generate a QR code for some reason, see screenshot below. the "Copy" button also seems to not copy anything in the clipboard. Any idea?

capture du 2014-12-11 12 41 27

@paullinator
Copy link
Contributor

@saivann

Restore feature: agreed, but if they are going to do that, might as well write down their password. We also have a password recovery feature. A first for a bitcoin wallet. Not easy to use right now but our 2.0 version next year will finally crack the nut and solve the problem of easy password recovery for encrypted data.

Encrypted seed: Ok, we will remove the email feature. I was antsy about this as well anyway.

Backup / Password reminder: We do that once they receive over 20mBTC in the wallet.

Payment request issue: Likely an issue with the simulator. This should work fine on all mobile devices.

@paullinator
Copy link
Contributor

Hi @saivann. We are coming up onto 3 months of public availability as of Jan 6, 2015. I believe we've addressed all your concerns from previous post so I just wanted to touch base as ask if we are clear to have our pull request accepted or if there was anything else you needed from us.

Thanks
Paul

@paullinator
Copy link
Contributor

Hi @saivann. Any updates from your end? I believe we've fulfilled all your requirements. Please let us know.

@paullinator
Copy link
Contributor

Hi @saivann. Please advise. We'd really love to get included into bitcoin.org.

Thanks

title: "Airbitz Bitcoin Wallet"
titleshort: "Airbitz"
compat: "mobile android ios"
level: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@paullinator This can be replaced with level: 3

@saivann
Copy link
Contributor

saivann commented Jan 26, 2015

@i3inary @paullinator I have reviewed the app again and this looks good to me! I've mentioned a few things that need to be fixed in this pull request, when you have a few minutes to make the changes.

Given that there is no way of re-gaining access to the wallet if the users loses his username or password, what would you think of warning the user when creating an account that they must NEVER lose their username and password, perhaps inviting them to write these information on paper and put them in a safe place?

@paullinator
Copy link
Contributor

Thanks for the feedback @saivann. We do warn the user regarding the username/password but we do that once they receive >20mBTC of funds. We've been told this is a good balance of frictionless to setup, but fair enough warning to users. Most wallets to do not warn users that if they don't backup their keys, they WILL lose access to their funds if they lose their device.

We'll look at making those changes to the pull request now.

@saivann
Copy link
Contributor

saivann commented Jan 26, 2015

@paullinator Great to know you already had it covered, thanks! :) Yes indeed this suggestion would be worth being repeated to other wallet developers as well eventually.

@saivann
Copy link
Contributor

saivann commented Jan 27, 2015

@i3inary @paullinator Thanks. A few issues with the new description:

Airbitz is a mobile Bitcoin Wallet making high levels of privacy,
security, & decentralization extremely familiar and usable to the
masses. Airbitz wallets are ALWAYS auto encrypted,
backed up, and even function when Airbitz servers go down.

, & decentralization <--- Can you perhaps use "and" instead (this will be translated)

extremely familiar <--- We try to not include superlatives, maybe you can use "very familiar" instead?

ALWAYS auto encrypted <--- Another superlative, maybe you can use lowercase letters?

@saivann
Copy link
Contributor

saivann commented Jan 27, 2015

In the absence of critical feedback, this pull request will be merged on February 10th.

@saivann saivann merged commit 8e609be into bitcoin-dot-org:master Feb 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants