Locking/safe sync of encrypted LDK channel state in iCloud/GDrive #38
Conversation
| KeyServer.init({ baseURI: keyServerURI }); | ||
| CloudStore.init({ onOtherDeviceLogin }); | ||
| } |
There was a problem hiding this comment.
The init api of the KeyBackup module also has a new onOtherDeviceLogin argument which is a function to handle stopping of the local LDK node and logout of the wallet UI.
| { | ||
| "account": { | ||
| "name": "wallet3", | ||
| "seed": "f8478a83f41fdfca3fa802669893061b87d737e5d4699e1778a4f127a0b81591" |
There was a problem hiding this comment.
What are your thoughts on the use of a single seed for both a default single sig photon wallet and lightning wallet vs having two separate secrets?
Probably the primary reason for photon/ldk to share the same seed would be to minimise complexity. I suppose this would introduce a single point of failure but it seems to me to be preferable compared to adding backup/recovery complexity. Also, the photon cloud backup architecture obviously adds redundancy which mitigates SPOF risk wrt e.g. user error.
Would it be beneficial to store the seed encoded as a BIP39 mnemonic instead of hex, particularly if the plan is to implement with a shared seed by default?
There was a problem hiding this comment.
Unified wallet FTW! https://lightningdevkit.org/key_management/#creating-a-unified-wallet
There was a problem hiding this comment.
The test data you are pointing here is just for reference and actually isn't used in the tests. So I can remove it.
The new KeyBackup.createChannelBackup({ data, pin }) api makes no assumption about the contents of data except that it's a valid JavaScript Object that can be serialized, encrypted, and stored as a data blob (under 1MB in size). It's basically the same as the existing KeyBackup.createBackup({ data, pin }) api with some additional guarantees to prevent multiple devices/threads storing data in parallel in order to prevent race conditions.
That means you can use the old createBackup api as usual to encrypt and sync a BIP39 seed for an on-chain wallet, reuse that same seed for the LDK wallet, and then store just the channelstate with the new api.
There was a problem hiding this comment.
Yep understand it's just test data but reminded me to bring it up as I wasn't sure if this had been discussed elsewhere. Sounds good
| const deviceId = await DeviceInfo.getUniqueId(); | ||
| const storedId = await Store.getItem(DEVICE_ID); | ||
| if (storedId && storedId !== deviceId) { | ||
| _unlockChannels(); |
There was a problem hiding this comment.
Should this be _lockChannels instead or am I missing something?
There was a problem hiding this comment.
No it's unlocking, since the api itself does not need to lock anymore when the error is thrown. The consumer cannot store state and must handle the error. Otherwise the thread would sleep the next time the api is called.
There was a problem hiding this comment.
Understood. Thanks for clarifying
| @@ -68,10 +70,64 @@ export async function createBackup({ data, pin }) { | |||
| * @return {Promise<Object>} The decrypted backup payload | |||
| */ | |||
| export async function restoreBackup({ pin }) { | |||
| const { keyId, encryptionKey } = await _fetchKey({ pin }); | |||
There was a problem hiding this comment.
nit: should we rename to decryptionKey to be super clear as that's what is happening in this context?
There was a problem hiding this comment.
Mmmh. I wonder though if that might be more confusing and cause the reader to think it's a different key than used during encryption.
Co-authored-by: Conor Okus <conor.okus@hotmail.co.uk>
|
Thanks for the review @xsats and @ConorOkus 🙏 |
This is a draft PR to get feedback on syncing LDK channel state via iCloud/GDrive in a safe manner. Here's an overview of my thought process. Feedback welcome, as I am not entirely sure what I'm doing here is completely safe for all scenarios. I do believe however it is not possible to provide 100% safe/locking sync of channel state between two LDK nodes as each has its own state machine, which may make changes to local channel state before it can be detected that another device has initiated restore of channel state in its local LDK node from cloud storage. But here goes...
Basically a mutex is set in iCloud key/value store using a unique device ID so that only one device is allowed to read/write channel state at a time. In addition a mutex is set within the app to ensure thread safe access to the cloudstore from that device. When I open the app on a new device, I am asked if I want to “login” and the device ID of the new device is set as mutex to iCloud. Now only the new device can read/write to the cloudstore.
The only window of risk is really the latency time until the device id syncs to iCloud. But I’m assuming users aren't changing phones back and forth within seconds.
This way we can map channel state sync onto photon’s existing iCloud/GDrive storage backend. We also compare the backup timestamps before persisting to ensure an old channel state never overwrites a newer state. If I understand correctly, this would be in line with what Matt suggested with sequence numbers.
What’s still left is to stop LDK on
Device Aand prevent that device from starting LDK with old state whenDevice Btakes control. That can be detected using theiCloudStoreDidChangeRemotelyevent, which is exposed in the api to wallets. In case the wallet is open on both devices at once,Device Acan stop LDK and "logout" of the UI.Granted it’s not a seemless multidevice UX, but it’s good enough for my user archetype that basically only uses a wallet on one device at a time. And I would personally prefer control of backups in my iCloud account rather than fully rely on a wallet vendor.
I also want to to raise the question if it is actually possible to fully mitigate the “risk window” even with an ACID compliant db backend. Because the LDK node on
Device Acould always make a change to channels locally until it is notified thatDevice Bhas taken control. If this is true, I actually see no advantage of using a custom ACID compliant backend over iCloud. There would only be the downside of having to trust the wallet vendor's server for channel storage. But please let me know where I am wrong in my thought process. Thanks :)