Multibit-Legacy / multibit-hd

Deprecated Bitcoin Wallet
https://multibit.org/blog/2017/07/26/multibit-shutdown.html
Other
171 stars 113 forks source link

Blank datestamp prevents restore #709

Closed gary-rowe closed 8 years ago

gary-rowe commented 8 years ago

User feedback has indicated that leaving the datestamp blank prevents progressing through the restore process.

Since this is on the critical path and I'm AFK can you take a look at this @jim618 and verify that a restore is possible? If not then it needs to be fixed in 0.1.3.

jim618 commented 8 years ago

I just restored a MBHD wallet from the latest develop code with no datestamp successfully. Here it is synchronising:

screen shot 2015-08-24 at 11 56 56

This was done in the normal way e.g.:

There were no errors or anything unusual in the log file.

As MBHD has to to sync from the HD wallet creation 'birthday' it does take a while to sync (say an hour), but then that is the point of the datestamp in the first place.

Closing as 'Restore with no datestamp' is working correctly.

woodydeck commented 8 years ago

I was able to both replicate the issue, and also get it to work. Something is odd.

me4mdsp

woodydeck commented 8 years ago

Try doing that, but clicking in the field of the datestamp, or entering something then deleting it, and then adding a password. You will get it greyed out. That might be the issue. Going to previous steps does not reset anything, and the problem persists. This is the restore of a Trezor wallet, I am not sure if it differs from the other restore methods.

Edit: If you don't click the datestamp field, then it works. If you click it, but add nothing, then you cannot restore the wallet without it.

jim618 commented 8 years ago

I'll have a look at the datestamp field validation

jim618 commented 8 years ago

I have put in a fix so that a blank datestamp also enables the 'Next>' button on the 'Enter datestamp' screen.

It's not exactly perfect as the 'Verified' on the datestamp is a little consistent. If the user enters a password with no datestamp the datestamp shows as 'Verified' but then if they enter some datestamp text and then remove it the 'Verified' initially disappears (correctly) they don't see the 'Verified' come back when the field is empty. This is due to a magic value - the HD birthday - being stored in the model (but not being shown on the view). I've left this as it is minor.

Awaiting review and closing.

woodydeck commented 8 years ago

Simplest workaround to avoid confusion then is not to change code, but add a checkbox to enable the field. Guess you would need to change the dialogue too for that.

Cheers for confirming I did not lose my mind. Good luck with project.

gary-rowe commented 8 years ago

Done a code review and it looks good to me. If you choose to make the idiomatic change then it's good to close.

jim618 commented 8 years ago

I've reworked things slightly to remove the magic number in the model. This gets rid of the dissonance on the 'Verified' label appearing (or not).

Now when the wallet is created the datestamp is checked and the earliest HD wallet birthday used where appropriate. I've retested this with a missing datestamp and with a valid one ok.

Awaiting review and closing.

gary-rowe commented 8 years ago

Code review looks fine. Will perform final verification later tonight.

gary-rowe commented 8 years ago

I've added a FEST test to ensure we don't regress on this issue since it's on the critical path. The test correctly identified that the new code passed, while earlier code failed.

Verified working so closing.