-
Notifications
You must be signed in to change notification settings - Fork 2
Overall Enhanced PSBT #40
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
Conversation
- Created PSBTModal.foss.tsx that removes react-native-vision-camera dependency - Uses BarcodeZxingScan for both iOS and Android platforms - Maintains full feature parity with standard version including animated QR support - Android keeps continuous scanning, iOS uses recursive scanning for animated QR - Updated CHANGELOG.md with F-Droid compatibility entry
- Added push trigger to ensure tests run on direct pushes to branches - Maintains same path filters as pull_request trigger - Prevents bugs from entering main branch without test validation - Fixes continuous integration for changes pushed directly to branches
…bility - Add support for generating output descriptors for Legacy, SegWit Native, and SegWit Compatible address types - Implement timestamp-based legacy wallet detection (created_at <= 1765894825732) - Old wallets: All descriptors use BIP44 path with different formats (pkh/wpkh/sh(wpkh)) - New wallets: Use optimized BIP84/BIP49 paths for SegWit address types - Add new PSBT Screen with collapsible Bold Connect section - Display all three descriptor types in PSBTScreen and KeyshareModal - Add wallet creation timestamp display in KeyshareModal - Centralize descriptor generation logic in utils.js (generateAllOutputDescriptors) - Update Go native module to accept addressType parameter - Update iOS and Android native bindings - Improve UI/UX consistency across PSBT-related screens Breaking changes: None (fully backward compatible)
- Keep v2.1.1 version numbers (2.1.1, versionCode 34) - Merge CHANGELOG.md entries - Keep v2.1.1 MobileNostrPairing.foss.tsx (with PSBT support) - Resolve iOS project file version conflicts - Resolve workflow file conflicts
HalFinneyIsMyHomeBoy
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.
Still doing manual testing, but inspected the new code in psbt.go.
All logging of the pubkey+chaincode should be removed (or at least truncated) because it's sensitive data and could lead to a vulnerability.
Please see lines 64, 98-99, 202, 759, 793-794, 897 in psbt.go
BBMTLib/tss/psbt.go
Outdated
| if keyshareData.ChainCodeHex == "" { | ||
| return "", fmt.Errorf("keyshare missing chain_code_hex") | ||
| } | ||
| Logf("Keyshare parsed: pub_key (hex, full)=%s, chaincode (hex, full)=%s", keyshareData.PubKey, keyshareData.ChainCodeHex) |
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.
Remove or truncate logging of pubkey+chaincode
BBMTLib/tss/psbt.go
Outdated
| if xpubPath != "" { | ||
| Logf("COMPARISON: PSBT xpub is at path %s (account level)", xpubPath) | ||
| Logf("COMPARISON: Deriving from keyshare master key to PSBT xpub path: %s", xpubPath) | ||
| Logf("COMPARISON: Keyshare master pub_key (hex)=%s", keyshareData.PubKey) |
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.
Remove or truncate logging of pubkey+chaincode
|
Ah btw if u check in release mode logs are suppressed thats propagated |
BBMTLib/tss/psbt.go
Outdated
|
|
||
| // Derive the public key from keyshare using GetDerivedPubKey with the path from PSBT | ||
| // Use the keyshare's pub_key and chaincode to derive the corresponding public key | ||
| Logf("Input %d: Deriving public key from keyshare - xpub: %s, chaincode: %s, path: %s", i, keyshareData.PubKey, keyshareData.ChainCodeHex, inputDerivePath) |
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.
Remove or truncate logging of pubkey+chaincode
BBMTLib/tss/psbt.go
Outdated
| if keyshareData.ChainCodeHex == "" { | ||
| return "", fmt.Errorf("keyshare missing chain_code_hex") | ||
| } | ||
| Logf("Keyshare parsed: pub_key (hex, full)=%s, chaincode (hex, full)=%s", keyshareData.PubKey, keyshareData.ChainCodeHex) |
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.
Remove or truncate logging of pubkey+chaincode
BBMTLib/tss/psbt.go
Outdated
| if xpubPath != "" { | ||
| Logf("COMPARISON: PSBT xpub is at path %s (account level)", xpubPath) | ||
| Logf("COMPARISON: Deriving from keyshare master key to PSBT xpub path: %s", xpubPath) | ||
| Logf("COMPARISON: Keyshare master pub_key (hex)=%s", keyshareData.PubKey) |
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.
Remove or truncate logging of pubkey+chaincode
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.
https://github.com/BoldBitcoinWallet/BoldWallet/blob/main/BBMTLib/tss/logs.go#L22
Logs are suppressed in release mode
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.
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.
Ah, I see now. I missed that.
Still working on manual testing now.
I'll let you know if I find any issues.
BBMTLib/tss/psbt.go
Outdated
|
|
||
| // Derive the public key from keyshare using GetDerivedPubKey with the path from PSBT | ||
| // Use the keyshare's pub_key and chaincode to derive the corresponding public key | ||
| Logf("Input %d: Deriving public key from keyshare - xpub: %s, chaincode: %s, path: %s", i, keyshareData.PubKey, keyshareData.ChainCodeHex, inputDerivePath) |
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.
Remove or truncate logging of pubkey+chaincode
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.
Feel free to remove them.
As pointed out for help debugging purposes in dev mode they are logged.
In release mode almost no logs are enabled - all suppressed, and propagated.
https://github.com/BoldBitcoinWallet/BoldWallet/blob/main/App.tsx#L160
I truncated the pubkey + chaincode logging in psbt.go as an extra precaution.
HalFinneyIsMyHomeBoy
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.
Even though logging is disabled in release mode, I truncated the pubkey+chaincode logging as a precaution.
Tested manually and approved!! works like a charm!
🚀 Overall Enhanced PSBT
📋 Summary
This PR introduces a dedicated wallet mode: PSBT only - the main feature that transforms the wallet into a specialized PSBT-focused experience. Alongside this, it adds comprehensive support for generating output descriptors for all three Bitcoin address types (Legacy, SegWit Native, SegWit Compatible) while maintaining full backward compatibility with existing wallets.
✨ Features
🎯 Main Feature: Dedicated Wallet Mode - PSBT Only
🎯 Multi-Address Type Output Descriptors
pkh([fingerprint/44'/coinType'/0']xpub/0/*)wpkh([fingerprint/84'/coinType'/0']xpub/0/*)sh(wpkh([fingerprint/49'/coinType'/0']xpub/0/*))🔄 Backward Compatibility Strategy
pkh,wpkh,sh(wpkh)🎨 UI/UX Enhancements
PSBT Screen (Dedicated Wallet Mode)
Keyshare Modal (Wallet Home)
created_at)outputDescriptorfield in favor ofoutputDescriptorsobject🛠️ Technical Improvements
Code Organization
generateAllOutputDescriptors()utility function inutils.jsNative Module Updates
addressTypeparameter toGetOutputDescriptor()functionDerivation Path Logic
getDerivePathForNetwork()to support address types🔧 Technical Details
Migration Strategy
1765894825732(December 2025)isLegacyWallet(created_at)utilityFile Changes
BBMTLib/tss/common.go- Enhanced descriptor generationPSBTScreen.tsx,KeyshareModal.tsx,WalletHome.tsxutils.js- New utility functions✅ Testing Checklist
📸 Screenshots
Add screenshots of the new PSBT Screen and Keyshare Modal showing all three descriptors
🔗 Related Issues
Link any related issues here
🚦 Breaking Changes
None - This PR is fully backward compatible. Existing wallets continue to function exactly as before.
📝 Notes
outputDescriptorfield, now using onlyoutputDescriptorsobjectReady for Review ✅