Wallet Logs #994
Wallet Logs #994
Conversation
WalkthroughWalkthroughThis update introduces comprehensive logging mechanisms across various components, including the addition of new logger providers for service workers and wallets. It enhances error handling and logging capabilities, particularly for wallet-related operations, and removes a specific validation check. The changes aim to improve debugging, error tracking, and operational transparency within the application. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
pages/wallet/logs.js
Outdated
| export default function WalletLogs () { | ||
| const { logs } = useWalletLogger() | ||
| // TODO add filter by wallet, add sort by timestamp | ||
| return ( | ||
| <> | ||
| <CenterLayout> | ||
| <h2 className='text-center'>wallet logs</h2> | ||
| <div> | ||
| {logs.map((log, i) => <LogMessage key={i} {...log} />)} | ||
| </div> | ||
| </CenterLayout> | ||
| </> | ||
| ) |
There was a problem hiding this comment.
The implementation of the WalletLogs page is straightforward and correctly uses the useWalletLogger hook to fetch and display wallet logs. However, as mentioned in the PR objectives, there's a TODO comment for adding filter and sort functionality. It's important to track this feature enhancement for future development.
Would you like me to help implement the filter and sort functionality or open a GitHub issue to track this task?
components/logger.js
Outdated
| const WalletLoggerProvider = ({ children }) => { | ||
| // TODO: persist logs in local storage | ||
| // limit to last 24h? | ||
| const [logs, setLogs] = useState([]) | ||
|
|
||
| const appendLog = useCallback((wallet, level, message) => { | ||
| setLogs((prevLogs) => [...prevLogs, { wallet, level, message, ts: +new Date() }]) | ||
| }, [setLogs]) | ||
|
|
||
| return ( | ||
| <WalletLoggerContext.Provider value={{ logs, appendLog }}> | ||
| {children} | ||
| </LoggerContext.Provider> | ||
| </WalletLoggerContext.Provider> |
There was a problem hiding this comment.
For WalletLoggerProvider, the comment mentions a TODO for persisting logs in local storage. It's important to address this to ensure logs are not lost between sessions. Consider implementing this feature or tracking it as a task for future development.
Would you like me to implement the log persistence feature or open a GitHub issue to track this task?
If this component is composable it should be trivial to add an accordian (or a tab) of the wallet logs to every wallet while still having a single superlog on another page. That's the route I'd take at least. |
Makes sense! I forgot about the use case of using the logs while configuring the wallet. I mostly had in mind to use the logs for payment failures etc. Current state (6368d4d): 2024-04-01.03-07-12.mp4Only thing left to do is to have logs for the autowithdrawal methods. I was also thinking about having a dropdown to filter by wallet in the superlog but maybe that's MVP feature creep. It shouldn't be difficult to add but I probably shouldn't judge features by "how difficult they would be to add", lol. Additional context In cb1f1a6, I started to simply load all logs from IndexedDB at once (instead of only the last 5m) and remove pagination that looked like this:
-- cb1f1a6, commit message Deciding against any form of navigation and "performance optimization" and simply relying on a scrollbar in all cases made it possible to delete a lot of complicated code: I am still a little bit worried what happens if one has a lot of logs (since storage is currently unbounded) but I am pretty sure it's not that big of a problem as I initially thought. |
ekzyis
force-pushed
the
wallet-logs
branch
2 times, most recently
from
7a97702
to
5d062c7
Compare
| model WalletLog { | ||
| id Int @id @default(autoincrement()) | ||
| createdAt DateTime @default(now()) @map("created_at") | ||
| userId Int | ||
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | ||
| wallet String | ||
| level LogLevel | ||
| message String | ||
|
|
||
| @@index([userId, createdAt]) | ||
| } | ||
|
|
There was a problem hiding this comment.
I had to remove a foreign key to the wallet table since on errors, we don't create wallets.
Instead of doing some mental gymnastics to have a wallet that doesn't work in the wallet table, I simply abandoned the idea to have a link between the wallet table and the logs.
This also makes it possible to persist logs about wallets that have been detached.
* <table> fixes bad format with fixed width and message overflow into start of next row * also using ---start of log--- instead of ---end of log--- now * removed time string in header nav
I realized the code for navigation was most likely premature optimization which even resulted in worse UX: Using the buttons to load logs from 5m, 1h, 6h ago sometimes meant that nothing happened at all since there were no logs from 5m, 1h, 6h ago. That's why I added a time string as "start of logs" so it's at least visible that it changed but that looked bad so I removed it. But all of this was not necessary: I can simply load all logs at once and then the user can scroll around however they like. I was worried that it would be bad for performance to load all logs at once since we might store a lot of logs but as mentioned, that's probably premature optimization. WHEN a lot of logs are stored AND this becomes a problem (What problem even? Slow page load?), THEN we can think about this. If page load ever becomes slow because of loading logs, we could probably simply not load the logs at page load but only when /wallet/logs is visited. But for now, this works fine.
| walletLogs: async (parent, args, { me, models }) => { | ||
| if (!me) { | ||
| throw new GraphQLError('you must be logged in', { extensions: { code: 'FORBIDDEN' } }) | ||
| } | ||
|
|
||
| return await models.walletLog.findMany({ | ||
| where: { | ||
| userId: me.id | ||
| }, | ||
| orderBy: { | ||
| createdAt: 'asc' | ||
| } | ||
| }) |
There was a problem hiding this comment.
We are wasting a lot of bandwidth per wallet save here since we always fetch all logs instead of passing the last id the frontend already knows.
But this is as simple as it can be and maybe it's as unnecessary as trying to optimize log performance by not loading all logs from IDB at once but have pagination but that made the code a lot more complicated (see cb1f1a6).
| model WalletLog { | ||
| id Int @id @default(autoincrement()) | ||
| createdAt DateTime @default(now()) @map("created_at") | ||
| userId Int | ||
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | ||
| wallet String | ||
| level LogLevel | ||
| message String | ||
|
|
||
| @@index([userId, createdAt]) | ||
| } |
There was a problem hiding this comment.
Consider adding an index on the level field of the WalletLog model to optimize queries filtering by log level.
model WalletLog {
id Int @id @default(autoincrement())
createdAt DateTime @default(now()) @map("created_at")
userId Int
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
wallet String
level LogLevel
message String
@@index([userId, createdAt])
+ @@index([level])
}This index can significantly improve the performance of log retrieval operations, especially when filtering logs by their severity level, which is a common use case.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| model WalletLog { | |
| id Int @id @default(autoincrement()) | |
| createdAt DateTime @default(now()) @map("created_at") | |
| userId Int | |
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | |
| wallet String | |
| level LogLevel | |
| message String | |
| @@index([userId, createdAt]) | |
| } | |
| model WalletLog { | |
| id Int @id @default(autoincrement()) | |
| createdAt DateTime @default(now()) @map("created_at") | |
| userId Int | |
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | |
| wallet String | |
| level LogLevel | |
| message String | |
| @@index([userId, createdAt]) | |
| @@index([level]) | |
| } |
| @@ -1,7 +1,7 @@ | |||
| import { authenticatedLndGrpc, createInvoice } from 'ln-service' | |||
| import { msatsToSats, satsToMsats } from '@/lib/format' | |||
| import { msatsToSats, numWithUnits, satsToMsats } from '@/lib/format' | |||
There was a problem hiding this comment.
Consider removing the unused import authenticatedLndGrpc and createInvoice if they are not being used elsewhere in this file.
components/logger.js
Outdated
| const WalletLoggerProvider = ({ children }) => { | ||
| const [logs, setLogs] = useState([]) | ||
| const idbStoreName = 'wallet_logs' | ||
| const idb = useRef() | ||
| const logQueue = useRef([]) | ||
|
|
||
| useQuery(WALLET_LOGS, { | ||
| fetchPolicy: 'network-only', | ||
| // required to trigger onCompleted on refetches | ||
| notifyOnNetworkStatusChange: true, | ||
| onCompleted: ({ walletLogs }) => { | ||
| setLogs((prevLogs) => { | ||
| const existingIds = prevLogs.map(({ id }) => id) | ||
| const logs = walletLogs | ||
| .filter(({ id }) => !existingIds.includes(id)) | ||
| .map(({ createdAt, wallet, ...log }) => ({ ts: +new Date(createdAt), wallet: renameWallet(wallet), ...log })) | ||
| return [...prevLogs, ...logs].sort((a, b) => a.ts - b.ts) | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| const saveLog = useCallback((log) => { | ||
| if (!idb.current) { | ||
| // IDB may not be ready yet | ||
| return logQueue.current.push(log) | ||
| } | ||
| const tx = idb.current.transaction(idbStoreName, 'readwrite') | ||
| const request = tx.objectStore(idbStoreName).add(log) | ||
| request.onerror = () => console.error('failed to save log:', log) | ||
| }, []) | ||
|
|
||
| useEffect(() => { | ||
| initIndexedDB(idbStoreName) | ||
| .then(db => { | ||
| idb.current = db | ||
|
|
||
| // load all logs from IDB | ||
| const tx = idb.current.transaction(idbStoreName, 'readonly') | ||
| const store = tx.objectStore(idbStoreName) | ||
| const index = store.index('ts') | ||
| const request = index.getAll() | ||
| request.onsuccess = () => { | ||
| const logs = request.result | ||
| setLogs((prevLogs) => { | ||
| // sort oldest first to keep same order as logs are appended | ||
| return [...prevLogs, ...logs].sort((a, b) => a.ts - b.ts) | ||
| }) | ||
| } | ||
|
|
||
| // flush queued logs to IDB | ||
| logQueue.current.forEach(q => { | ||
| const isLog = !!q.wallet | ||
| if (isLog) saveLog(q) | ||
| }) | ||
|
|
||
| logQueue.current = [] | ||
| }) | ||
| .catch(console.error) | ||
| return () => idb.current?.close() | ||
| }, []) | ||
|
|
||
| const appendLog = useCallback((wallet, level, message) => { | ||
| const log = { wallet, level, message, ts: +new Date() } | ||
| saveLog(log) | ||
| setLogs((prevLogs) => [...prevLogs, log]) | ||
| }, [setLogs, saveLog]) | ||
|
|
||
| return ( | ||
| <WalletLoggerContext.Provider value={{ logs, appendLog }}> | ||
| {children} | ||
| </LoggerContext.Provider> | ||
| </WalletLoggerContext.Provider> |
There was a problem hiding this comment.
The WalletLoggerProvider component's implementation, including log persistence and state management, is well-structured. However, consider addressing the TODO for sending logs to a server when diagnostics are enabled, as this could provide valuable insights for debugging and monitoring.
Would you like me to implement the server-side log collection feature or open a GitHub issue to track this task?
|
|
||
| .logTable { | ||
| width: 100%; | ||
| max-height: 60svh; |
There was a problem hiding this comment.
Fix the typo in the max-height property value.
- max-height: 60svh;
+ max-height: 60vh;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| max-height: 60svh; | |
| max-height: 60vh; |
|
|
||
| @media screen and (min-width: 768px) { | ||
| .logTable { | ||
| max-height: 70svh; |
There was a problem hiding this comment.
Fix the typo in the max-height property values within the media queries.
- max-height: 70svh;
+ max-height: 70vh;
- max-height: 30svh;
+ max-height: 30vh;Also applies to: 22-22
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| max-height: 70svh; | |
| max-height: 70vh; |
components/wallet-logs.js
Outdated
| {filtered.length === 0 && <div className='w-100 text-center'>empty</div>} | ||
| <table> | ||
| <tbody> | ||
| {filtered.map((log, i) => <LogMessage key={i} {...log} />)} |
There was a problem hiding this comment.
Consider using a more stable identifier than the array index for the key prop in the list rendering. If logs have unique identifiers, use those to help React optimize rendering and re-rendering of list items.
- {filtered.map((log, i) => <LogMessage key={i} {...log} />)}
+ {filtered.map((log) => <LogMessage key={log.id} {...log} />)}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| {filtered.map((log, i) => <LogMessage key={i} {...log} />)} | |
| {filtered.map((log) => <LogMessage key={log.id} {...log} />)} |
|
I don't see any glaring issues and I haven't QA'd it. Is there anything you'd like feedback on? If not, can I assume you are happy and confident enough in it that I can just merge it and deploy it? |
I see what you're doing 👀 I am happy with the UX, the code is simple enough and I tested everything except logs for successful autowithdrawals. But I also tested that now and had to fund my wallet for it. Then I noticed that there is a pretty serious render bug where |
🥸😆 |
|
Mhh, 2b632cb fixes it but for a proper fix which does no longer make Then I can use I am also recreating the context value on every render currently but that's the smaller issue. |
| export const LoggerProvider = ({ children }) => { | ||
| return ( | ||
| <ServiceWorkerLoggerProvider> | ||
| <WalletLoggerProvider> | ||
| {children} | ||
| </WalletLoggerProvider> | ||
| </ServiceWorkerLoggerProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Convert LoggerProvider to an arrow function for consistency with other components in the file.
- export const LoggerProvider = ({ children }) => {
+ export const LoggerProvider = ({ children }) => (
<ServiceWorkerLoggerProvider>
<WalletLoggerProvider>
{children}
</WalletLoggerProvider>
</ServiceWorkerLoggerProvider>
- )
+ );Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export const LoggerProvider = ({ children }) => { | |
| return ( | |
| <ServiceWorkerLoggerProvider> | |
| <WalletLoggerProvider> | |
| {children} | |
| </WalletLoggerProvider> | |
| </ServiceWorkerLoggerProvider> | |
| ) | |
| } | |
| export const LoggerProvider = ({ children }) => ( | |
| <ServiceWorkerLoggerProvider> | |
| <WalletLoggerProvider> | |
| {children} | |
| </WalletLoggerProvider> | |
| </ServiceWorkerLoggerProvider> | |
| ); |
| const initIndexedDB = async (storeName) => { | ||
| return new Promise((resolve, reject) => { | ||
| if (!window.indexedDB) { | ||
| return reject(new Error('IndexedDB not supported')) | ||
| } | ||
|
|
||
| // https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB | ||
| const request = window.indexedDB.open('app:storage', 1) | ||
|
|
||
| let db | ||
| request.onupgradeneeded = () => { | ||
| // this only runs if version was changed during open | ||
| db = request.result | ||
| if (!db.objectStoreNames.contains(storeName)) { | ||
| const objectStore = db.createObjectStore(storeName, { autoIncrement: true }) | ||
| objectStore.createIndex('ts', 'ts') | ||
| objectStore.createIndex('wallet_ts', ['wallet', 'ts']) | ||
| } | ||
| } | ||
|
|
||
| request.onsuccess = () => { | ||
| // this gets called after onupgradeneeded finished | ||
| db = request.result | ||
| resolve(db) | ||
| } | ||
|
|
||
| request.onerror = () => { | ||
| reject(new Error('failed to open IndexedDB')) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Add error handling for the onupgradeneeded event in initIndexedDB to ensure robustness.
request.onupgradeneeded = () => {
db = request.result
if (!db.objectStoreNames.contains(storeName)) {
const objectStore = db.createObjectStore(storeName, { autoIncrement: true })
objectStore.createIndex('ts', 'ts')
objectStore.createIndex('wallet_ts', ['wallet', 'ts'])
}
+ request.onerror = (event) => {
+ console.error('Error during onupgradeneeded:', event);
+ };
}This addition ensures that any errors that occur during the database upgrade process are properly logged, aiding in debugging and ensuring the application's resilience.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const initIndexedDB = async (storeName) => { | |
| return new Promise((resolve, reject) => { | |
| if (!window.indexedDB) { | |
| return reject(new Error('IndexedDB not supported')) | |
| } | |
| // https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB | |
| const request = window.indexedDB.open('app:storage', 1) | |
| let db | |
| request.onupgradeneeded = () => { | |
| // this only runs if version was changed during open | |
| db = request.result | |
| if (!db.objectStoreNames.contains(storeName)) { | |
| const objectStore = db.createObjectStore(storeName, { autoIncrement: true }) | |
| objectStore.createIndex('ts', 'ts') | |
| objectStore.createIndex('wallet_ts', ['wallet', 'ts']) | |
| } | |
| } | |
| request.onsuccess = () => { | |
| // this gets called after onupgradeneeded finished | |
| db = request.result | |
| resolve(db) | |
| } | |
| request.onerror = () => { | |
| reject(new Error('failed to open IndexedDB')) | |
| } | |
| }) | |
| } | |
| const initIndexedDB = async (storeName) => { | |
| return new Promise((resolve, reject) => { | |
| if (!window.indexedDB) { | |
| return reject(new Error('IndexedDB not supported')) | |
| } | |
| // https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB | |
| const request = window.indexedDB.open('app:storage', 1) | |
| let db | |
| request.onupgradeneeded = () => { | |
| // this only runs if version was changed during open | |
| db = request.result | |
| if (!db.objectStoreNames.contains(storeName)) { | |
| const objectStore = db.createObjectStore(storeName, { autoIncrement: true }) | |
| objectStore.createIndex('ts', 'ts') | |
| objectStore.createIndex('wallet_ts', ['wallet', 'ts']) | |
| } | |
| request.onerror = (event) => { | |
| console.error('Error during onupgradeneeded:', event); | |
| }; | |
| } | |
| request.onsuccess = () => { | |
| // this gets called after onupgradeneeded finished | |
| db = request.result | |
| resolve(db) | |
| } | |
| request.onerror = () => { | |
| reject(new Error('failed to open IndexedDB')) | |
| } | |
| }) | |
| } |
| const saveLog = useCallback((log) => { | ||
| if (!idb.current) { | ||
| // IDB may not be ready yet | ||
| return logQueue.current.push(log) | ||
| } | ||
| const tx = idb.current.transaction(idbStoreName, 'readwrite') | ||
| const request = tx.objectStore(idbStoreName).add(log) | ||
| request.onerror = () => console.error('failed to save log:', log) | ||
| }, []) |
There was a problem hiding this comment.
Consider batching log saves to IndexedDB in WalletLoggerProvider to improve performance.
const saveLog = useCallback((log) => {
if (!idb.current) {
return logQueue.current.push(log)
}
const tx = idb.current.transaction(idbStoreName, 'readwrite')
- const request = tx.objectStore(idbStoreName).add(log)
+ logQueue.current.push(log);
+ if (logQueue.current.length >= 10) { // Batch size of 10, adjust as necessary
+ const store = tx.objectStore(idbStoreName);
+ logQueue.current.forEach(log => store.add(log));
+ logQueue.current = [];
+ }
request.onerror = () => console.error('failed to save log:', log)
}, [])Batching log saves can significantly reduce the overhead of writing to IndexedDB, especially under heavy logging conditions. This change introduces a simple batching mechanism that accumulates logs and writes them in batches, improving the efficiency of log persistence.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const saveLog = useCallback((log) => { | |
| if (!idb.current) { | |
| // IDB may not be ready yet | |
| return logQueue.current.push(log) | |
| } | |
| const tx = idb.current.transaction(idbStoreName, 'readwrite') | |
| const request = tx.objectStore(idbStoreName).add(log) | |
| request.onerror = () => console.error('failed to save log:', log) | |
| }, []) | |
| const saveLog = useCallback((log) => { | |
| if (!idb.current) { | |
| return logQueue.current.push(log) | |
| } | |
| const tx = idb.current.transaction(idbStoreName, 'readwrite') | |
| logQueue.current.push(log); | |
| if (logQueue.current.length >= 10) { // Batch size of 10, adjust as necessary | |
| const store = tx.objectStore(idbStoreName); | |
| logQueue.current.forEach(log => store.add(log)); | |
| logQueue.current = []; | |
| } | |
| request.onerror = () => console.error('failed to save log:', log) | |
| }, []) |
| const log = useCallback(level => message => { | ||
| // TODO: | ||
| // also send this to us if diagnostics was enabled, | ||
| // very similar to how the service worker logger works. | ||
| appendLog(wallet, level, message) | ||
| console[level !== 'error' ? 'info' : 'error'](`[${wallet}]`, message) | ||
| }, [appendLog, wallet]) |
There was a problem hiding this comment.
Implement the TODO for sending logs to a server when diagnostics are enabled in useWalletLogger.
const log = useCallback(level => message => {
- // TODO:
- // also send this to us if diagnostics was enabled,
- // very similar to how the service worker logger works.
+ if (diagnosticsEnabled) { // Assume diagnosticsEnabled is a state or prop indicating if diagnostics are enabled
+ fetch('/api/diagnostics/log', {
+ method: 'post',
+ headers: {
+ 'Content-type': 'application/json'
+ },
+ body: JSON.stringify({ wallet, level, message })
+ }).catch(console.error);
+ }
appendLog(wallet, level, message)
console[level !== 'error' ? 'info' : 'error'](`[${wallet}]`, message)
}, [appendLog, wallet])This change implements the previously mentioned TODO by conditionally sending logs to a server endpoint when diagnostics are enabled. This allows for remote monitoring and debugging of wallet-related issues.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const log = useCallback(level => message => { | |
| // TODO: | |
| // also send this to us if diagnostics was enabled, | |
| // very similar to how the service worker logger works. | |
| appendLog(wallet, level, message) | |
| console[level !== 'error' ? 'info' : 'error'](`[${wallet}]`, message) | |
| }, [appendLog, wallet]) | |
| const log = useCallback(level => message => { | |
| if (diagnosticsEnabled) { // Assume diagnosticsEnabled is a state or prop indicating if diagnostics are enabled | |
| fetch('/api/diagnostics/log', { | |
| method: 'post', | |
| headers: { | |
| 'Content-type': 'application/json' | |
| }, | |
| body: JSON.stringify({ wallet, level, message }) | |
| }).catch(console.error); | |
| } | |
| appendLog(wallet, level, message) | |
| console[level !== 'error' ? 'info' : 'error'](`[${wallet}]`, message) | |
| }, [appendLog, wallet]) |
|
Ok, 2b4e634 fixes that the provider context value changes due to logging. Tested that logs still show up fine and logs are still appended fine. We can merge this now. |
Description
Close #823
This PR adds a new React context:
WalletLoggerContextThis context can be used with
useWalletLoggerto show wallet logs at /wallet/logs.Screenshots
desktop (72ca741)
mobile (72ca741)
UX video (72ca741)
2024-04-03.17-52-10.mp4
How to test
Sending via NWC
nostr-wallet-connect-lndstacker_lndcontainer:nostr-wallet-connect-lnd:nostr+walletconnect://URLReceiving via LND
Use
stacker_lnd:10009andxxd -p -c0on invoice.macaroon and tls.cert from container.Open Questions
1. Where should link to full logs be?
It's currently at /wallet below
wallet history:click to show screenshot
I thought about putting it into
attached walletssince it's logs about attached wallets but that's too many clicks imoupdate: Just saw that you mentioned it to be on the wallet attach page in #823:
However, I planned to have all logs for every wallet at the same page with an optional dropdown filter to filter for specific wallets instead of having individual pages with no option to see all logs together. I think it's useful to see all interaction with attached wallets at one place (send + receive). When we have wallet fallbacks, that will be even more useful. What do you think @huumn?
update: I think I could also link to /wallet/logs on every wallet attach page with a query param to immediately filter the logs by that wallet.
update: Added embedded logs and skipped adding a filter. You can just go to the individual page.
2. Show full payment hash in logs + more info like payment preimage?
The hash was truncated because of overflow issues and not sure how relevant the full hash is.
update: I think it's useful for debugging to be able to easily copy-paste payment hashes etc.
I now show the full hash + preimage in the logs with a horizontal scrollbar on x-overflow.
3 . Font size too small?
I am using
x-smallas the font size since it makes it easy to look at a lot of logs at the same time (good overview)4. Expose secret in logs but don't persist it?
The original wallet logs screenshots show that the first and last 6 characters of the secret are shown in the log. I originally planned to not persist the secret in the logs so for loaded logs, the secret would show
******.update: I decided against that now since it's probably confusing. Secret is now never shown in the logs.
5. Delete logs on server for privacy reasons?
I shortly considered to delete all logs about a wallet if it was unattached but that might be confusing behavior.
I think we can keep this out of this PR and do it later.
TODO:
ability to sort logs by recent: if we store a lot of logs, I don't think one is interested in the oldest logs first.autoscroll is betterArray.filterfor pagination should make the code less complexSummary by CodeRabbit
WalletLogscomponent for real-time log following, filtering, and display in wallet settings.wss://protocol in relay URLs to prevent false errors.