-
Notifications
You must be signed in to change notification settings - Fork 26
fix: persist channel locally in case remote backup fails #2575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR upgrades react-native-ldk to version 0.0.161 to fix channel persistence issues when remote backup fails, and consolidates LDK debugging functionality by cleaning up the Debug screen interface.
- Upgrades
react-native-ldkfrom 0.0.159 to 0.0.161 - Includes channel monitor files (.bin) in log exports from both account root and channels folder
- Consolidates LDK debug export functionality by removing duplicate "Export Logs" from DevSettings and enhancing LdkDebug screen
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Updates react-native-ldk dependency to version 0.0.161 |
| src/utils/lightning/logs.ts | Adds channel monitor files from channels folder to binary file exports |
| src/screens/Settings/DevSettings/index.tsx | Removes duplicate LDK log export functionality and related imports |
| src/screens/Settings/DevSettings/LdkDebug.tsx | Consolidates export functionality into single "Export Files" button with confirmation dialog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
LGTM, except the Copilot comment |
ovitrif
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK.
|
Need testing of repro scenario by @catch-21 and/or @piotr-iohk, to validate the fix. Please reach out to me for info on the repro steps if needed. |
@jvsena42 That was false positive, it might affect devs on Windows machines, but most likely not. |
QA NotesQuick summary/observations:
Building NotesBuilding the iOS and Android apps for regtest in a local simulator. Clean build for iOS and Android
git fetch && git checkout fix/backup-client
watchman watch-del-all || true
rm -rf node_modules .yarn/cache
yarn install
yarn start --reset-cache
# iOS
cd ios && pod install && cd ..
yarn ios
# Android
cd android && ./gradlew clean && cd ..
yarn androidScript# Kill Metro & Watchman
pkill -f "node.*metro" || true
watchman watch-del-all || true
# Remove JS + Yarn caches
rm -rf node_modules .yarn/cache
# Remove all Android build + Gradle state
cd android
./gradlew --stop || true
rm -rf .gradle .kotlin build app/build
# Remove global Gradle + Kotlin caches
rm -rf ~/.gradle/caches ~/.gradle/daemon ~/.gradle/native ~/.gradle/wrapper ~/.konan
# Back to project root
cd ..
# Reinstall JS dependencies
yarn install
# Clear Metro cache
yarn start --reset-cache & sleep 5 && pkill -f "node.*metro"
# (Optional but good) Clear Bitkit app data from emulator
adb shell pm clear to.bitkit || true
# Rebuild Android
yarn androidBuild fail: Steps to reproduceThese steps are tailored for macOS. Cover funding a wallet, simulating backup failure, and verifying correct app behavior after the fix.
echo "127.0.0.1 bitkit.stag0.blocktank.to" | sudo tee -a /etc/hosts
sudo killall -HUP mDNSResponder
$ curl -v https://bitkit.stag0.blocktank.to/backups-ldk --max-time 5Expected: ...
curl: (7) Failed to connect to bitkit.stag0.blocktank.to port 443 after 4 ms: Couldn't connect to server
5a. Verify Settings → Dev Settings → LDK → Export Files
sudo sed -i '' '/bitkit.stag0.blocktank.to/d' /etc/hosts
sudo killall -HUP mDNSResponderExpected:
|
89afc97 to
4a82136
Compare
|
@piotr-iohk Can you try testing the same flow that you did on iOS now on Android?! Build is working again, just clone locally, then Run yarn start
# in another terminal
yarn android |
|
@ovitrif I have tested Android and iOS. Both pass. ✔️ I see that e2e-ios has some issues, seems to be hanging on "Run regtest setup" step. The same hosted macOS is used in |
|
@piotr-iohk thank you sir, so from manual testing PoV we're ready for the next step towards releasing this bugfix? cc. @catch-21 PS. i don't think we should care about the e2e tests of the RN app anymore, it's just a nice to have, but, since we would have to invest time to fix the suite, we're better to use that time for the native app. |
Description
react-native-ldkto 0.0.161 to include fix: persist channel locally in case remote backup fails react-native-ldk#294Type of change
Tests