-
-
Notifications
You must be signed in to change notification settings - Fork 130
Wallet schema v2 #2146
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
Wallet schema v2 #2146
Conversation
7efb350
to
a10af10
Compare
eccc4d0
to
840bb20
Compare
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
16991000 | Triggered | Generic High Entropy Secret | c52b973 | docker/db/wallet-seed.sql | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
36e027f
to
84a7dcd
Compare
a10af10
to
020e297
Compare
9a6ce22
to
c2a22f6
Compare
020e297
to
e604617
Compare
6c07696
to
e60046f
Compare
310f17c
to
e604617
Compare
For some reason, if we run the vault_refactor migration before we run the delete_duplicate_wallets migration, the wallet_v2 migration fails with this error: ``` app | 2025-05-16T20:56:53.127625000Z Applying migration `20250516042253_wallet_v2` app | 2025-05-16T20:56:53.202274000Z Error: P3018 app | 2025-05-16T20:56:53.202294000Z app | 2025-05-16T20:56:53.202309000Z A migration failed to apply. New migrations cannot be applied before the error is recovered from. Read more about how to resolve migration issues in a production database: https://pris.ly/d/migrate-resolve app | 2025-05-16T20:56:53.202322000Z app | 2025-05-16T20:56:53.202337000Z Migration name: 20250516042253_wallet_v2 app | 2025-05-16T20:56:53.202349000Z app | 2025-05-16T20:56:53.202365000Z Database error code: 23503 app | 2025-05-16T20:56:53.202379000Z app | 2025-05-16T20:56:53.202392000Z Database error: app | 2025-05-16T20:56:53.202406000Z ERROR: insert or update on table "Withdrawl" violates foreign key constraint "Withdrawl_walletId_fkey" app | 2025-05-16T20:56:53.202427000Z DETAIL: Key (walletId)=(29) is not present in table "ProtocolWallet". ``` I don't really understand how the wallet_v2 migration error is related to the order of the vault_refactor and delete_duplicate_wallets migrations.
See #2092 (comment) |
Description
based on #2092 because it's the target branch
This PR does the following:
WalletTemplate
which stores which send and receive protocols a wallet inUserWallet
supports=> allows multiple send and receive protocols per wallet
WalletSend...
,WalletRecv...
) which store configuration per user walletHow to add a new wallet:
WalletV2
table and declare which send and receive protocols it supportsMy beautiful TODO list
walletId
in wallet protocol tables so a user wallet can't have the same protocol twice for the same wallet ✅check if HTTP protocols to send and recv (LNbits, Blink, Phoenixd) can be abstracted away via one WalletSendHTTP table without resorting to JSON columns❌ won't do in this PR, maybe in the futureverify wallet migration with SQL assertions after migration❌ decided not to, too much workProtocolWallet
row ✅split JSON column into two columns for send and receive?❌ no longer needed since we switched toProtocolWallet
Withdrawal
foreign key fails ✅ fixed it, but didn't really figure out what happenedpriority
toProtocolWallet
, too? ✅ No, I don't think that's important and we can change it later.walletId
foreign keys that point toUserWallet
touserWalletId
to avoid confusion? 🤔add❌ out of scope of this PRUser(id, vaultKeyHash)
foreign key toVault
table with newuserId
,keyHash
columns?Additional Context
This is based on the latest discussion with @huumn how to plan further ahead with the wallet schema than to just migrate the vault in #2092.
Checklist
Are your changes backwards compatible? Please answer below:
tbd
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
tbd
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a
Did you introduce any new environment variables? If so, call them out explicitly here:
no