LNC-mobile: add support for multiple connections #6
Conversation
kaloudis
changed the title
kaloudis
force-pushed
the
lnc-multiple-connections
branch
5 times, most recently
from
b613de7 to
9de15fe
Compare
|
I haven't been able to get the demo app to run on my machine. Even the old From skimming the code, the only thing that stood out to me was that "pnemonic" should be spelled "mnemonic". |
kaloudis
force-pushed
the
lnc-multiple-connections
branch
from
9de15fe to
f80c6dd
Compare
|
@jamaljsr: review reminder |
There was a problem hiding this comment.
tACK 🙌
Apologies on the delay, I had a bunch of trouble getting this running but finally was able to run the android app and connect to multiple nodes.
A few questions:
- Do you want me to also do a code review or were you just looking for functional review?
- Do you want me to test iOS as well or is that still WIP?
- Is it possible to use relative paths in the
metro.config.jsandpackage.jsoninstead of hard-coding a path that only work on your machine (/Users/satoshi/Repos/lightninglabs/lnc-rn)? I keep forgetting to change this after switching branches.
|
Thanks for testing
|
| @observable public lnc1: any; | ||
| @observable public lnc2: any; | ||
| @observable public info1: any; | ||
| @observable public info2: any; |
There was a problem hiding this comment.
Is there a reason why you use any here instead of the LNC and GetInfoResponse types?
demos/multi-connect-demo/Home.tsx
Outdated
| mnemonic1: | ||
| 'drop sell moment shift doll pull october gold squeeze gas', | ||
| mnemonic2: | ||
| 'prosper brother journey park peanut ten exact photo harbor picnic' |
There was a problem hiding this comment.
Would it make sense to have these be entered by the user in the UI opposed to needing to change the hard-coded the values?
| // only load if pairingPhrase is set | ||
| if (!pairingPhrase) return; | ||
| try { | ||
| const key = `${STORAGE_KEY}:${pairingPhrase}`; |
There was a problem hiding this comment.
I don't think it's a good idea to store the pairing phrase as plain text in local storage. I always worry about devs seeing this and thinking it is safe to do in a real app. Couldn't we use the namespace as the storage key.
| <TextInput | ||
| onChangeText={this.onChangeMnemonic1} | ||
| placeholder="Enter pairing mnemonic #1 here" | ||
| value={mnemonic1} | ||
| /> |
There was a problem hiding this comment.
Ahhh.. I didn't even realize these were inputs that I could modify in the UI. This makes it much clearer.
There was a problem hiding this comment.
Thanks for the feedback!
lightning-node-connectcommit: lightninglabs/lightning-node-connect@0ef6a51