-
-
Notifications
You must be signed in to change notification settings - Fork 130
Wallet v2 #2169
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
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
16991000 | Triggered | Generic High Entropy Secret | e476705 | 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.
8ab0a08
to
afcc412
Compare
d9af095
to
e94cfe7
Compare
412f2c6
to
690b10b
Compare
The old schema can contain '' instead of NULL in the columns of wallets for receiving.
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.
Bug: Promise Handling and Feedback Timing Issues
The onClick
handler in DeleteWalletLogsObstacle
attempts to both resolve (onDelete()
) and reject (onClose()
) the same promise in its success path, which is invalid. Furthermore, the success toast is displayed after the modal is closed by onClose()
, potentially hiding the feedback. The onClose()
call should be removed from the success path, and the toast should be displayed before the modal closes.
wallets/client/hooks/logger.js#L194-L205
stacker.news/wallets/client/hooks/logger.js
Lines 194 to 205 in 595729c
const onClick = useCallback(async () => { | |
try { | |
await deleteLogs() | |
onDelete() | |
onClose() | |
toaster.success('deleted wallet logs') | |
} catch (err) { | |
console.error('failed to delete wallet logs:', err) | |
toaster.danger('failed to delete wallet logs') | |
} | |
}, [onClose, deleteLogs, toaster]) |
Bug: Template Log ID Precision Issue
Template logs use Date.now()
as their ID, which can lead to duplicate IDs if multiple logs are created within the same millisecond. This causes React rendering issues due to duplicate keys and potential log management problems. A TODO
comment in the code acknowledges this and suggests using window.performance.now()
for better precision.
wallets/client/hooks/logger.js#L15-L18
stacker.news/wallets/client/hooks/logger.js
Lines 15 to 18 in 595729c
const addTemplateLog = useCallback(({ level, message }) => { | |
// TODO(wallet-v2): Date.now() might return the same value for two logs | |
// use window.performance.now() instead? | |
setTemplateLogs(prev => [{ id: Date.now(), level, message, createdAt: new Date() }, ...prev]) |
Bug: `useEffectOnce` Hook Ignores Dependency Changes
The useEffectOnce
hook accepts a deps
parameter, but the user-provided effect function (func
) only executes once. This is because didMount.current
prevents func()
from being called on subsequent renders, even when deps
change and trigger the underlying useEffect
to re-run. Consequently, the deps
parameter is effectively ignored after the initial execution, leading to inconsistent behavior.
components/use-effect-once.js#L6-L14
stacker.news/components/use-effect-once.js
Lines 6 to 14 in 595729c
export default function useEffectOnce (func, deps) { | |
const didMount = useRef(false) | |
useEffect(() => { | |
if (!didMount.current) { | |
didMount.current = true | |
return func() | |
} | |
}, deps) |
Was this report helpful? Give feedback by reacting with 👍 or 👎
Description
close #1495 fix #2234
TODOs
Vault
table no longer contains foreign keys to wallets or usersWalletTemplate
UserWallet
ProtocolWallet
WalletSendNWC
,WalletRecvNWC
, ...)WalletTemplate
tableadd placeholder wallets like Phoenix?master
)(Use test payments with HODL invoices for wallet validation #1287)include wizard / multi-step formsprobably not in this PRCryptoKey
show status of wallet network tests on attach in dedicated UI instead of relying on wallet logsdon't show "logs", show wallet events like a walletthis means that there won't be multiple log messages for the same payment. we will just show the current status of a payment, just like a wallet would do.WALLETS
vsWALLET
query)WALLET
on save or detachWALLETS
if wallets changedCUSTOM
wallet intoNWC
andLN_ADDR
walletcan key hash and wallet encryption get out of sync?Additional context
I will do
in a follow-up PR to not block the review of this PR any longer.
Screenshots
we now show wallets instead of protocols:
each wallet can now support multiple protocols for send or receive:
Checklist
Are your changes backwards compatible? Please answer below:
no
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:
8
docker/db/wallet-seed.sql
. seed contains some wallets fortest_wallet_v2
user.media fit youth secret combine live cupboard response enable loyal kitchen angle
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
yes
Did you introduce any new environment variables? If so, call them out explicitly here:
no