# Burner Wallet "lost private keys" post-mortem

###### tags: `work` `dedication` `unfinished` `buffer`

On 6/8/2019, David Mihal discovered an issue where a private key was reset to
"0" and funds were lost. David and Tim investigated, and broke down the
incident to the following issues:

## Issue #1: burner-core typo:
User: David on localhost:3000, running both the normal Burner Wallet, as well as the beta Burner Wallet 2.0 (https://github.com/dmihal/burner-wallet/tree/burner-2-prototype).

A typo in the module LocalSigner resulted in new private keys not being saved to localStorage or cookies.

The LocalSigner class defines a variable `_savekey` in the constructor. This variable is intended to disable saving keys to localStorage in unit tests (since unit tests do not have a global localStorage). However, the method `_saveAccount()` was reading the property `savekey` instead of `_savekey`. This caused the saving of private keys to be overwritten in all cases.

It is likely that this issue in itself did not affect any user funds for the following reasons:

1. The integration of the @burner-wallet/core module into the Burner Wallet did not remove or replace any of the existing key-handling code, which is still handled within Dapparatus
2. This bug would not cause the key to be erased or overwritten, it would only cause new keys to not be saved.
3. The bug was discovered while testing the Burner Wallet 2.0, which depends entirely on @burner-wallet/core for key support. Burner Wallet 2.0 is not deployed publicly anywhere, and has only been tested by David.
4. The public burner wallet on xdai.io has not been released in nearly a month, meaning no public users are exposed to any of these code changes.

This typo [was fixed](https://github.com/austintgriffith/burner-core/commit/63465b062a7ec64ea7be44964deb08997cf2d0e9#diff-295791e4e0b980616599227d1bb31148L82) in the @burner-wallet/core package and released as version 0.0.5. The integration of @burner-wallet/core [has been reverted on the master branch on Github](https://github.com/austintgriffith/burner-wallet/commit/1e3a2b50568a7ab63221d20e9d0ca1bed3820f7b). The change can be re-introduced once it is determined that there is no risk to user keys or funds.

## Issue #2: Chromium regenerates `metaPrivateKey`

User: Tim on localhost:3000 but using xdai network
Browser: Chromium Version 70.0.3538.77 (Official Build) (64-bit)
OS: Mac OS X 10.12.6
Metamask: No (never installed or activated)
Commit: [here](https://github.com/austintgriffith/burner-wallet/commit/1e3a2b50568a7ab63221d20e9d0ca1bed3820f7b)

While testing David Mihal's [PR for burner-core integration](https://github.com/austintgriffith/burner-wallet/pull/211) Tim opened the burner-wallet on his Chromium for the first time on Thursday, 06/06/2019. During the testing session, Tim sent roughly $1 worth of XDAI to this wallet running on the XDAI live network. He also copied over the Chromium wallet's private key to his Firefox wallet.

A day later, when testing the PR a second time, Tim noticed that in Chromium the balance was "gone" (all coins showed a balance of $0). Tim initially didn't follow up on the issue as he thought it was a problem with his computer rather than the burner-wallet code base.

After David reported to Tim on Saturday 08/06/2019 and they decided to investigate the original issue collectively in a call, the noticed that `metaPrivateKey` in Tim's Chromium would always regenerate when Tim relaunched the process in Mac OS.
To reproduce:

- Update your local burner-wallet to the relevant commit mentioned at the top of this section
- Delete localStorage for Chromium and domain (e.g. http://localhost:3000)
- open http://localhost:3000 in Chromium
- open debug tool and inspect `metaPrivateKey`
- memorise couple of places of the `metaPrivateKey``
- close all Chromium related processes
- reopen Chromium and notice that `metaPrivateKey` has changed

Tim failed to reproduce this behavior on the exact same burner-wallet configuration in Chrome Version 74.0.3729.169 (Official Build) (64-bit) and Firefox Version 67.0. In fact, since Tim had copied his Chromium `metaPrivateKey` to his Firefox, his funds are still available there.

Tim and David hence concluded in the call on Saturday 08/06/2019 that the issue of Chromium regenerating `metaPrivateKey` is either limited to Tim's computer or Tim's Chromium

A follow up investigation on this issue is urgently advised. If you're able to reproduce this result as described here, please reach out.

## Issue #3: Private keys are lost when run with Metamask
Existing issue: https://github.com/austintgriffith/burner-wallet/issues/101

Suspicious code: https://github.com/austintgriffith/dapparatus/blob/5370af55966e3843bae9ac7c0e1067dea27e1168/src/dapparatus.js#L68

The initial incident that prompted this investigation seems to be the result of a combination of two issues:

1) Running the Burner Wallet with Metamask installed will cause the private keys in localStorage and cookies to be set to the string "0", causing the loss of funds in the metaaccount.
2) Burner Wallet 2.0 does not support Metamask, and is entirely dependent on metaaccounts and keys stored in localstorage. For compatability, @burner-wallet/core uses the same private keys as Dapparatus.

This caused the keys that were generated for Burner Wallet 2.0 to be erased when the Burner Wallet 1.0 was run with Metamask installed.

## Safety recommendations for other contributors

- Please make sure to update your local master branch to the latest master on GitHub austingriffith/burner-wallet

## Conclusion (interim)

- xdai.io wallets are not affected as code hasn't been updated since May 18th
- very recent forks from austingriffith/burner-wallet master that use burner-core 0.0.4 might be affected (very unlikely that somebody is running this though...)
- Chromium regeneration problem: No conclusion can be drawn before it cannot be reproduced by other users

## Potential next steps

- Deeper investigations into the above described issues + fixes
- Updates to quality assurance processes such that these type of bugs are extremely unlikely to make it to the live site
- All contributors should always be aware of what version is running in production. Pushing to production should be announced upfront and publicly
- The burner-wallet doesn't auto-update for the user. Instead the user decides when to update the wallet's software
- This will be difficult to implement for a web app. We may be able to keep the user on a cached version of the app using service workers, but eventually they will have to update. (David)
- Backward-compatability tests to secure existing wallets from bugs like these
- Forced private key backups for all users of xdai.io