Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Backup wallet seed #690

Merged
merged 5 commits into from
Aug 23, 2017
Merged

Backup wallet seed #690

merged 5 commits into from
Aug 23, 2017

Conversation

rmisio
Copy link
Contributor

@rmisio rmisio commented Aug 18, 2017

This PR implements the functionality to see your wallet recovery seeds in Settings > Advanced > Server. It also will show a warning the first time you open your wallet and it has transactions and you've never seen the warning before.

I got the messaging of the section from the Electrum-VTC wallet:

image

I know we discussed offering a Copy To Clipboard function, but since we're encouraging people to write down the seed and not store it electronically, I thought it best not to offer the copy function. If they're determined to store it electronically, they could copy the text the old fashioned way (fwiw, I considered putting in some JS to not allow that, but I'm assuming all the users will be wearing their big-boy big-girl pants).

Copy link
Contributor

@jjeffryes jjeffryes left a comment

Choose a reason for hiding this comment

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

This looks good, I just had some suggestions on wording.

"walletSeed": {
"btnShowSeed": "Show Seed",
"introLine": "Your wallet generation seed is:",
"directionLine": "Please save these words on paper (order is important).",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should stress the order more here. Maybe "Write the following words on paper, and keep them somewhere safe. They must be in this exact order."

"warningHeading": "Warning:",
"warning1": "Never disclose your seed.",
"warning2": "Never type it on a website.",
"warning3": "Do not store it electronically."
Copy link
Contributor

Choose a reason for hiding this comment

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

Add: "Do not lose your seed, or your funds will be permanently lost if your data is erased."

"introLine": "Your wallet generation seed is:",
"directionLine": "Please save these words on paper (order is important).",
"warningHeading": "Warning:",
"warning1": "Never disclose your seed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Never share your seed. Anyone with this seed can steal your funds."

@jjeffryes
Copy link
Contributor

@SamPatt what do you think about the wording on the warnings? Should we make them stronger? Add anything?

@SamPatt
Copy link
Member

SamPatt commented Aug 22, 2017

I like all of Josh's suggestions on the wording changes. With those changes I think we're all set.

I agree with not giving them a copy button. I would agree with not allowing them to copy, but I know some people store wallet seeds in their password managers (still a bad idea, but better than not storing the seed at all).

@morebrownies
Copy link

I also agree with everything said above

@jjeffryes jjeffryes merged commit e893824 into master Aug 23, 2017
@jjeffryes jjeffryes deleted the backup-wallet branch August 23, 2017 15:36
@TheButterZone
Copy link

TheButterZone commented Sep 8, 2017

Is it a BIP39 seed with m/44'/0'/0' derivation, can be imported into Electrum desktop?

@jjeffryes
Copy link
Contributor

Is it a BIP39 seed with m/44'/0'/0' derivation, can be imported into Electrum desktop?

@cpacia or @tyler-smith ?

@cpacia
Copy link
Member

cpacia commented Sep 8, 2017

It should be. Though I don't think that's what electrum uses.

@TheButterZone
Copy link

TheButterZone commented Sep 8, 2017

In the new wallet flow, Electrum lets you fill in seed words, will not let you proceed with the set I got from OB, without first clicking Options & checking 'BIP39 seed'. Then it shows m/44'/0'/0' as the default derivation, but it can be overwritten.

At some point, I would hope, we'd be able to easily put our existing Electrum wallet seed words into OB and/or the xpub (so OB can receive payments with only privacy instead of security risk & generate unsigned TXs for us to sign with Electrum).

@cpacia
Copy link
Member

cpacia commented Sep 8, 2017

m/44'/0'/0' is the correct derivation. you're saying it lets you enter that and it doesn't work?

@TheButterZone
Copy link

TheButterZone commented Sep 8, 2017

I won't know until I Accept my first mainnet payment to that OB & now also Electrum wallet.

@cpacia
Copy link
Member

cpacia commented Sep 8, 2017

I'm using other test vectors other wallets are using for bip44 so it should work. https://github.com/OpenBazaar/spvwallet/blob/master/keys_test.go#L37

@TheButterZone
Copy link

First payment went to Electrum's first Change address rather than a Receiving one (with a massive, UNeconomic fee taken out, which I submitted a feedback+screenshot on).

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

What type of payment was it? Direct, moderated, and was the vendor online.

Also, the fee is relying on the 21 fee API. So economic is whatever that returns. You paid $1.22. I wouldn't call that unacceptable. Rather somewhat lucky it confirmed.

If you want to manually set the fee you can do so in the config file:

"FeeAPI": "",
"HighFeeDefault": 160,
"LowFeeDefault": 120,
"MaxFee": 2000,
"MediumFeeDefault": 140,

Make sure the FeeAPI is set to "" so that it doesn't use the API and uses your defaults.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

Direct. I was the vendor & the buyer paid when my client+server was in sleep mode.

Lucky? The fee was 126.154 sat/B, "$1.22" on a $2 item chosen by The Boss!

8b3c7c8d8665f838b16e038f441f593712102146efb0e16b0076ac208008d530 got in the next block with only 5 sat/B.

As it's paying to myself, I'm willing to wait at a minimum the 1-2 blocks I've seen 1 sat/B get in, or hours, or days, or RBF. Fuck the 21 fee API. Might as well be written by miners.

So what do I fix the MaxFee to for non-bullshit? 2000=2000 satoshis? Put it at 234 for my 1 sat/B starting point, since that's the bytes of the final TX?

Then if I want to speed it up, I do RBF with Electrum? Or will Electrum not allow that because while it shares OB's seed words, Electrum didn't create a Replaceable TX itself?

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

1 sat/B confirming in 1-2 blocks is highly unlikely. It's possible lower fees will confirm, but it's also that there will be an influx in demand and the never will. For that reason I don't recommend paying ultra low fees.

The maxfee is the just there so 21 doesn't drain the wallet if it returns an absurd value. If you're not using the fee api then the max fee doesn't matter.

If you do an rbf it should detect the new transaction and mark the old one as dead, but I've never actually tried it.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

I only care about what I've seen actually happening, the fact that RBF exists in case 1 sat/b gets rejected for weeks, and whether RBF can actually be used. It's my BTC, not miners' & uneconomic APIs'.

Which line of your code block do I need to change to get OB to 1 sat/b & never broadcast a higher fee?

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

It's in the config file. I posted it above.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

The only difference between your code block and my same config file lines was the API URL was removed. What do I change the other lines to, for 1 sat/b?

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

you set the lowfeedefault to 1

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

The word "economic" does not appear in the config file in ~/Library/Application Support/OpenBazaar2.0

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

I edited it. Use LowFeeDefault

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

Alright, done. Also set my medium & high to 1 so even if the GUI bubble gets moved from Economic to Normal or Priority by accident, there won't be another epic fail.

Maybe my offtopic subthread needs its own issue so people can search for it easier; can it be split off with Github or would it need a new issue started from scratch?

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

btw I don't know if you plan on selling stuff but it also uses those fees for payments out of a multisig. You're not going to be able to RBF that payment without getting the other party to manually resign.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

Only ever wanted OB to sell stuff. A manual resign is unlikely, if not impossible?

Just saved the raw TX. It ends with

"complete": true,
"final": true
}

So RBF not possible even if it wasn't for multisig resigning required.

I guess there's rebroadcasting & any TX accelerators that accept 1 sat/b... (I was able to time ViaBTC at the first second of each hour successfully many times, though I'm not sure what their fee minimum is now).

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

All transactions leaving the wallet use the opt-in rbf flag. Here's an example of one https://api.blockcypher.com/v1/btc/main/txs/9d2490b779bc855f6fdc1718b5fdf842dbba500e7bac9d3d20db468a547d39b4?limit=50&includeHex=true

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

I do see opt_in_rbf: true on your example, but on mine, https://api.blockcypher.com/v1/btc/main/txs/8b3c7c8d8665f838b16e038f441f593712102146efb0e16b0076ac208008d530?limit=50&includeHex=true it doesn't exist.

Unless you mean leaving the OB seed that I made an Electrum wallet out of, but the damage was already done before "leaving", by the multisig effectively precluding RBF to allow the vendor to bump up from the absolute minimum fee he's seen confirm fee & so set in config.

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

When a vendor is offline the funds get sent into a 1 of 2 multisig address. The purpose is to prevent the buyer from sending money into a black hole and gives him the opportunity to cancel and get his money back.

Your transaction was sent out of a 1 of 2 multisig. I could probably enable the opt-in rbf flag on those type of transactions, but there are others where it isn't possible, like vendor releasing the funds after the escrow timeout.

This is due to Core abusing the sequence field for multiple purposes. You can't use it for two things at once.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

Well, the barest amount of his money back.

You could, but wouldn't it also need something added to the GUI for the buyer to re-sign the RBF? Not only would that be a time sink to code/design, the buyer would likely not do that, either because they exited the app, not to return for a long time if ever, or they'd consider their purchase done & not take the time to help the vendor RBF.

So if I run my server 24/7 on my rpi (assuming I can manage to get its uptime better than dedicated OB 1.0), the RBF won't be a problem on direct to online, since the buyer won't pay to 1 of 2 multisig?

@cpacia
Copy link
Member

cpacia commented Sep 9, 2017

correct. in a perfect world fees would be a few cents and it wouldn't be a problem.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

Damn, I was hoping to retask my rpi, hook up a 160gb HD & run a pruned Core node, once OB2 comes out of beta, thinking IPFS would let me switch from dedi rpi to my desktop that I sleep & power off.

How much trouble would it be to have a store set to only allow payments when it's online, so an order could be held in "Pending Vendor Address" status as seen by the buyer that picked up the store's listings via IPFS while vendor offline? Then the Vendor upon coming back online would be notified of the pending order in their notifications list, and that when their address is sent to the buyer, they need to stay online to watch payment come in, then fulfill?

@tyler-smith
Copy link
Member

1 satoshi/byte will likely be below the relay dust threshold so I would suspect there would be issues using that, but if not please let us know. I agree with you that fees could probably be much lower.

I'm not sure what about this means you couldn't use your rpi? It might be a bit much to have the pi running both Bitcoin and OB. In that situation I would personally just run OB on the rpi and use an external wallet.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

https://blockchain.info/block/0000000000000000006ed335c2d9ff7cdb9af0b37c1019b0778dc3023a00a927 has a couple 1 sat/b TXs at the bottom that got nextblock confirms.

Right now my rpi is only running OB 1. Once OB 1 is replaced by OB 2 final, I thought I could wipe the rpi & dedicate it to just running a pruned Core node & spin my ancient 160gb HD into the ground. I'd do that even if OB1 & OB2 didn't exist. The gift timing of my rpi was such that I could get OB1 running on it & OB2 wasn't in alpha yet. 2.0 might not even have had IPFS added yet, only planned.

@tyler-smith
Copy link
Member

I'm not sure how those 1 sat/B txs were relayed, or if something else happened, regardless we both agree the current fees from the build in wallet/estimator are very inflated. If we can show that 1-5 sat/B txs can be reliable I'd love to work on that.

2.0 might not even have had IPFS added yet, only planned.

2.0 is entirely built on IPFS right this second. It's not even a matter of planning, it's our foundation.

Right now my rpi is only running OB 1. Once OB 1 is replaced by OB 2 final, I thought I could wipe the rpi & dedicate it to just running a pruned Core node & spin my ancient 160gb HD into the ground.

Good luck. I hope OB 2.0 works for you on the rpi but if not feel free to give us some data on what the issues are. I would love to minimize resource usage as much as possible. I'd love to be able for casual vendors to use cheap and accessible hardware like an rpi.

@TheButterZone
Copy link

TheButterZone commented Sep 9, 2017

IIRC I saw a stage in the 2.0 development where the IPFS repo itself was said to be unusable for coding into 2.0, and so it was only in the roadmap and IPFS had to be left for coding in as soon as it was ready, and I saw a commit later on adding IPFS once its repo was ready.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants